fix(E0603): suggest splitting grouped imports with private items#158445
fix(E0603): suggest splitting grouped imports with private items#158445raushan728 wants to merge 2 commits into
Conversation
rustbot
added
S-waiting-on-review
|
r? @jackh726 rustbot has assigned @jackh726. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I reviewed everything except the r? @fee1-dead please review, since you wanted this. |
|
|
| while temp_start > 0 { | ||
| let ch = source_text[..temp_start].chars().next_back().unwrap(); | ||
| if ch.is_whitespace() { | ||
| temp_start -= ch.len_utf8(); | ||
| } else if ch == ',' { | ||
| temp_start -= ch.len_utf8(); | ||
| break; | ||
| } else { | ||
| break; | ||
| } |
There was a problem hiding this comment.
For all of these while loops can we not use char_indices and just iterate over?
There was a problem hiding this comment.
Extracted the identifier removal logic into expand_span_to_surrounding_comma, which uses char_indices iteration and avoids manual byte pushes. The function handles both trailing and leading comma cases.
| let indentation = { | ||
| if let Ok(line) = self.tcx.sess.source_map().span_to_snippet(line_span) { | ||
| let indent_len = line.chars().take_while(|c| c.is_whitespace()).count(); | ||
| " ".repeat(indent_len) | ||
| } else { | ||
| let pos = self.tcx.sess.source_map().lookup_char_pos(stmt_span.lo()); | ||
| " ".repeat(pos.col.0.saturating_sub(4) as usize) | ||
| } | ||
| }; |
There was a problem hiding this comment.
can't we just scope the span so that it only starts at the use, specifically when we don't change it into multiple lines?
There was a problem hiding this comment.
In the branch where the group becomes empty, I now use the full line span line_span with a multipart suggestion and the get_indentation helper to reconstruct the line. This preserves the original indentation without manual column calculations. The else fallback is removed.
| use_stmt_span: if path_span != root_span { | ||
| // `root_span` spans the entire `use` tree, which is needed | ||
| // to correctly generate a multipart suggestion for a grouped import. | ||
| Some(root_span) | ||
| } else { | ||
| None | ||
| }, |
There was a problem hiding this comment.
what is the significance of using an Option here?
There was a problem hiding this comment.
Added a doc comment explaining that the span is only needed (and set) when single_nested is true, so Option avoids carrying meaningless data for non grouped imports.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
59ad765 to
4706b08
Compare
This comment has been minimized.
This comment has been minimized.
|
@rustbot ready |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
Add suggestion for E0603 in grouped use statements. When a private item is inside , suggest a new import line with the correct path and remove the item from the group. Single-item groups get a full-line replacement instead.
4706b08 to
12d8c86
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
But, there is no conflicts. |
5 participants
After PR #156244 removed broken suggestions for nested imports, E0603 was left without any suggestion. This restores helpful suggestions for grouped use statements.
For
use crate::two::{One, Two}with a privateOne:Single item groups get a direct replacement. Braces are removed when one item remains. Re-export chains are handled.
Fixes #157453.
r? @petrochenkov