Skip to content

Improve E0603 suggestions for private grouped imports - #158445

Open
raushan728 wants to merge 2 commits into
rust-lang:mainfrom
raushan728:issues/157453
Open

Improve E0603 suggestions for private grouped imports#158445
raushan728 wants to merge 2 commits into
rust-lang:mainfrom
raushan728:issues/157453

Conversation

@raushan728

@raushan728 raushan728 commented Jun 26, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

View all comments

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 private One:

  + use crate::one::One;
  ~ use crate::two::Two;

Single item groups get a direct replacement. Braces are removed when one item remains. Re-export chains are handled.

Fixes #157453.

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 26, 2026
@rustbot

rustbot commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 18 candidates

@rustbot rustbot assigned petrochenkov and unassigned jackh726 Jun 26, 2026
@petrochenkov

petrochenkov commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

I reviewed everything except the compiler/rustc_resolve/src/error_helper.rs part, which is full of byte pos arithmetics and other wonders.

r? @fee1-dead please review, since you wanted this.

@rustbot rustbot assigned fee1-dead and unassigned petrochenkov Jun 29, 2026
@rustbot

rustbot commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

fee1-dead is not on the review rotation at the moment.
They may take a while to respond.

@raushan728 raushan728 mentioned this pull request Jun 30, 2026
Merged
fee1-dead
fee1-dead requested changes Jul 9, 2026
View reviewed changes
Comment thread compiler/rustc_resolve/src/error_helper.rs Outdated
Comment thread compiler/rustc_resolve/src/error_helper.rs Outdated
Comment thread compiler/rustc_resolve/src/error_helper.rs Outdated
Comment thread compiler/rustc_resolve/src/error_helper.rs Outdated
Comment thread compiler/rustc_resolve/src/ident.rs Outdated
Comment on lines +1358 to +1364
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
},

@fee1-dead fee1-dead Jul 9, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the significance of using an Option here?

View changes since the review

@raushan728 raushan728 Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@fee1-dead fee1-dead Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not what I meant.

@raushan728 raushan728 Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusion. I've reworked this by storing root_span directly in PrivacyError and deriving the nested import check where it's used instead of carrying the extra state. I think this better matches what you were suggesting.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2026
@rustbot

rustbot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot

This comment has been minimized.

Sign in to view
@raushan728

raushan728 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2026
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@raushan728

This comment has been minimized.

Sign in to view
@raushan728
raushan728 requested a review from fee1-dead July 15, 2026 11:04
fee1-dead
fee1-dead requested changes Jul 22, 2026
View reviewed changes

@fee1-dead fee1-dead left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of these string fiddling is still not sitting well with me. It shouldn't be too hard to use the string search functions Rust already have to do some of these. Please use them, as we want compiler code to be readable.

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 22, 2026
@raushan728
test(imports): add baseline test for E0603 grouped import suggestion
f61b3b6
@raushan728
fix(resolve): improve E0603 suggestions for grouped imports
567fb3a 
Suggest direct imports for private items inside grouped imports. Split grouped imports when necessary and replace single-item groups with direct imports.
@rustbot

rustbot commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

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.

@raushan728 raushan728 changed the title fix(E0603): suggest splitting grouped imports with private items Improve E0603 suggestions for private grouped imports Jul 25, 2026
@raushan728

raushan728 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

A lot of these string fiddling is still not sitting well with me. It shouldn't be too hard to use the string search functions Rust already have to do some of these. Please use them, as we want compiler code to be readable.

I've cleaned this up to rely on the standard string search APIs (find, starts_with, trim_*, etc.). I think its better If anything you'd like to change?

@raushan728
raushan728 requested a review from fee1-dead July 25, 2026 13:32
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fee1-dead fee1-dead Awaiting requested review from fee1-dead

Requested changes must be addressed to merge this pull request.

Assignees

@fee1-dead fee1-dead

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

E0603: import directly/through re-export should attach good suggestions for grouped imports

5 participants

@raushan728 @rustbot @petrochenkov @fee1-dead @jackh726