Improve `E0603` suggestions for private grouped imports by raushan728 · Pull Request #158445 · rust-lang/rust

raushan728 · 2026-06-26T12:49:34Z

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 · 2026-06-26T12:49:39Z

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

petrochenkov · 2026-06-29T14:54:14Z

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 · 2026-06-29T14:54:19Z

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

rustbot · 2026-07-09T20:16:20Z

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

raushan728 · 2026-07-10T13:51:31Z

@rustbot ready

fee1-dead

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

raushan728 · 2026-08-01T10:57:54Z

relies heavily on index computations when that can be avoided

I removed the manual span/index computations and simplifying the data flow. Could you take a another look? thanks!

fee1-dead

This is much better, but I fear it would not be a great use of my time reviewing this. I believe this has more value to be done by someone who is genuinely learning about the codebase rather than someone using an LLM.

View changes since this review

raushan728 · 2026-08-02T03:52:34Z

Sorry for the previous revisions. I've cleaned up the code and addressed the earlier feedback where I could. I think it's in a much better state now and hopefully easier to review.

oli-obk · 2026-08-04T16:14:04Z

I invite you to join https://rust-lang.zulipchat.com/join/rlfvpemsaacs3pfi6kwqnqjb/ and start a thread asking for a mentor if you want to make progress here.

cjgillot · 2026-08-04T18:45:17Z

+                    if !inner.contains(',') && !inner.is_empty() {
+                        replacement = format!("{}{}", replacement[..open].trim_end(), inner);
+                    }
+                }


Don't try to be smart with braces. The user has rustfmt.

View changes since the review

I wasn't sure I understood this comment my intention wasn't just formatting but to produce a cleaner suggestion (use foo::Bar instead of use foo::{Bar}) when only one item remains. does rustfmt rewrite use foo::{Bar} into use foo::Bar, or is the idea that we shouldn't try to simplify the syntax in diagnostics?

we shouldn't try to simplify the syntax in diagnostics?

i dont thing the UI should change regardless of what we do in the code.

does rustfmt rewrite use foo::{Bar} into use foo::Bar, or is the idea that we shouldn't try to simplify the syntax in diagnostics?

Both.

Simplifying the syntax is ok when it makes rustc code simpler too. If you need to count BytePos for span arithmetic, it's not worth it. (Span arithmetic is known to cause ICEs when the user uses multibyte characters.)

As far i can tell report_privacy_error only has root_span and span_to_remove not the UseTree AST, so I couldn't find existing helper that would simplify {Bar} to Bar without doing manual BytePos-based span arithmetic. check_unused.rs can do this because it still has the UseTree spans available.

i wanted the diagnostic to present the import in the form users would normally write use foo::Bar rather than use foo::{Bar} especially for a single remaining item. That said, if the extra span arithmetic isn't considered worth the complexity, i'm happy to drop brace simplification but, personally i dont want we should suggest {Bar} instead of Bar for a single item.

cjgillot · 2026-08-04T18:46:34Z

+                // Replace the entire `use` instead of leaving `use foo::{}`.
+                if leaves_empty_group {
+                    let line_span = self.tcx.sess.source_map().span_extend_to_line(root_span);
+                    let suggestion_text = format!("{indentation}use {path};");
+                    err.multipart_suggestion(
+                        msg,
+                        vec![(line_span, suggestion_text)],
+                        Applicability::MachineApplicable,
+                    );
+                    break;
+                }


Why not just replace the contents of root_span with path?

View changes since the review

That's what the current version does now. In the single-item case I replace root_span with path which reuses the existing use keyword instead of replacing the whole statement.

cjgillot · 2026-08-04T18:48:23Z

+                // Insert before `root_span` to reuse the existing `use`.
+                err.multipart_suggestion(
+                    msg,
+                    vec![
+                        (root_span.shrink_to_lo(), format!("{path};\n{indentation}use ")),


Would it be simpler to insert a clean line at line_span.shrink_to_lo()?

View changes since the review

I considered that, but I chose root_span.shrink_to_lo so the suggestion only depends on the import path span and preserves formatting when the use statement isn't alone on its line (for example inside inline blocks).

rustbot · 2026-08-05T09:08:02Z

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.

Suggest direct imports for private items inside grouped imports. Split grouped imports when necessary and replace single-item groups with direct imports.

rustbot assigned jackh726 Jun 26, 2026

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 assigned petrochenkov and unassigned jackh726 Jun 26, 2026

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

raushan728 mentioned this pull request Jun 30, 2026

Improve E0277 diagnostics for conditionally implemented traits #158494

Merged

fee1-dead requested changes Jul 9, 2026

View reviewed changes

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

raushan728 force-pushed the issues/157453 branch from 59ad765 to 4706b08 Compare July 10, 2026 13:37