fix incorrect suggestions in private import diagnostic rust-lang/rust#156244
rustbot has assigned @jdonszelmann.
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 compilerexpanded to 73 candidates- Random selection from 19 candidates
By the way, the following will also emit an invalid suggestion (invalid after edition 2015), because we skipped the first segment:
mod one {
pub struct One;
}
mod two {
use crate::one::One;
}
mod test {
use crate::two::One;
}
compiler/rustc_resolve/src/diagnostics.rs · outdated
Could you merge the check of single_nested into here? Or after this but before if sugg.len() <= 1? Because it could break early
Moved. Also fixed the other invalid suggestion by dropping .skip(1).
Found another diagnostic issue:
help: import `One` through the re-export
|
LL - use crate::two::One;
LL + use crate::one::One;
|
Actually, crate::one::One is not a re-export; it is the definition.
Moved. Also fixed the other invalid suggestion by dropping
.skip(1).Found another diagnostic issue:
help: import ``One`` through the re-export | LL - use crate::two::One; LL + use crate::one::One; |Actually,
crate::one::Oneis not a re-export; it is the definition.
Fixed that.
View on GitHub
The job aarch64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
PR_CI_JOB set; skipping tidy
##[group]Building bootstrap
Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
---
---- [ui] tests/ui/shadowed/shadowed-use-visibility.rs stdout ----
Saved the actual stderr to `/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/shadowed/shadowed-use-visibility/shadowed-use-visibility.stderr`
diff of stderr:
19 LL - use crate::foo::bar::f as g;
20 LL + use foo::f as g;
21 |
+ help: import `bar` directly
+ |
+ LL - use crate::foo::bar::f as g;
+ LL + use crate::bar;
+ |
22
23 error[E0603]: module import `f` is private
24 --> $DIR/shadowed-use-visibility.rs:15:10
40 |
41 LL - use bar::f::f;
42 LL + use bar::f;
+ |
+ help: import `f` directly
+ |
+ LL - use bar::f::f;
+ LL + use crate::f;
43 |
44
45 error: aborting due to 2 previous errors
Note: some mismatched output was normalized before being compared
- LL - use crate::foo::bar::f as g; //~ ERROR module import `bar` is private
- LL + use crate::bar; //~ ERROR module import `bar` is private
- LL - use bar::f::f; //~ ERROR module import `f` is private
- LL + use crate::f; //~ ERROR module import `f` is private
+ help: import `bar` directly
+ |
+ LL - use crate::foo::bar::f as g;
+ LL + use crate::bar;
+ |
+ |
+ help: import `f` directly
+ |
+ LL - use bar::f::f;
+ LL + use crate::f;
The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args shadowed/shadowed-use-visibility.rs`
error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/shadowed/shadowed-use-visibility.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/shadowed/shadowed-use-visibility" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
error[E0603]: module import `bar` is private
##[error] --> /checkout/tests/ui/shadowed/shadowed-use-visibility.rs:9:21
|
LL | use crate::foo::bar::f as g; //~ ERROR module import `bar` is private
| ^^^ private module import
|
note: the module import `bar` is defined here...
--> /checkout/tests/ui/shadowed/shadowed-use-visibility.rs:4:9
|
---
LL | mod foo {
| ^^^^^^^
help: consider importing this function instead
|
LL - use crate::foo::bar::f as g; //~ ERROR module import `bar` is private
LL + use foo::f as g; //~ ERROR module import `bar` is private
|
help: import `bar` directly
|
LL - use crate::foo::bar::f as g; //~ ERROR module import `bar` is private
LL + use crate::bar; //~ ERROR module import `bar` is private
|
error[E0603]: module import `f` is private
##[error] --> /checkout/tests/ui/shadowed/shadowed-use-visibility.rs:15:10
|
LL | use bar::f::f; //~ ERROR module import `f` is private
| ^ private module import
|
note: the module import `f` is defined here...
--> /checkout/tests/ui/shadowed/shadowed-use-visibility.rs:11:9
|
LL | use crate::foo as f;
---
LL | mod foo {
| ^^^^^^^
help: consider importing this function through its public re-export instead
|
LL - use bar::f::f; //~ ERROR module import `f` is private
LL + use bar::f; //~ ERROR module import `f` is private
|
help: import `f` directly
|
LL - use bar::f::f; //~ ERROR module import `f` is private
LL + use crate::f; //~ ERROR module import `f` is private
|
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0603`.
I'll take a look later this day
compiler/rustc_resolve/src/diagnostics.rs · resolved
| 2479 | // |
|
| 2480 | // See issue #156060. |
|
| 2481 | let can_replace_use = |
|
| 2482 | !single_nested && !outermost_res.is_some_and(|(_, outer)| outer.span != ident.span); |
Why does this new condition not need to check point_to_def anymore?
point_to_def is irrelevant here. We want a suggestion here as long as its valid, regardless of whether a public re-export was found.
Will this result in fewer suggestions being emitted? IIUC, this condition is more conservative than before. Before we only break when !show_candidates && ident.span != outer_ident.span. But now, we will break even if there are available candidates.
Yes, two kinds of broken cases are rejected as the comment states.
Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=0cdf02e84649d7dde83d274e622a4c78
compiler/rustc_resolve/src/diagnostics.rs · outdated
Could you remove this? Seems this is only used at line 2434 and 2439. And the logic is weird, because seems there is no place to clear it.
Moved to the use site.
compiler/rustc_resolve/src/diagnostics.rs · outdated
| 2429 | let mut path = Vec::new(); |
|
| 2430 | // Don't include `{{root}}` in suggestions - it's an internal symbol |
|
| 2431 | // that should never be shown to users. |
|
| 2432 | for segment in import |
|
| 2433 | .module_path |
|
| 2434 | .iter() |
|
| 2435 | .skip_while(|segment| segment.ident.name == kw::PathRoot) |
|
| 2436 | { |
|
| 2437 | path.push(segment.ident); |
|
| 2438 | 2438 | } |
| 2439 | sugg_paths.push(( |
|
| 2440 | path.iter().cloned().chain(std::iter::once(ident)).collect::<Vec<_>>(), |
|
| 2441 | true, // re-export |
|
| 2442 | )); |
|
| 2439 | path.push(ident); |
|
| 2440 | let through_reexport = !matches!(source_decl.kind, DeclKind::Def(_)); |
|
| 2441 | sugg_paths.push((path, through_reexport)); |
Suggested change
| let mut path = Vec::new(); | |
| // Don't include `{{root}}` in suggestions - it's an internal symbol | |
| // that should never be shown to users. | |
| for segment in import | |
| .module_path | |
| .iter() | |
| .skip_while(|segment| segment.ident.name == kw::PathRoot) | |
| { | |
| path.push(segment.ident); | |
| } | |
| sugg_paths.push(( | |
| path.iter().cloned().chain(std::iter::once(ident)).collect::<Vec<_>>(), | |
| true, // re-export | |
| )); | |
| path.push(ident); | |
| let through_reexport = !matches!(source_decl.kind, DeclKind::Def(_)); | |
| sugg_paths.push((path, through_reexport)); | |
| // Don't include `{{root}}` in suggestions - it's an internal symbol | |
| // that should never be shown to users. | |
| let path = import.module_path | |
| .iter().cloned() | |
| .filter(|seg| seg.ident.name != kw::PathRoot) | |
| .chain(std::iter::once(ident)) | |
| .collect::<Vec<_>>(); | |
| let through_reexport = !matches!(source_decl.kind, DeclKind::Def(_)); | |
| sugg_paths.push((path, through_reexport)); |
That doesn't compile. Rewrote as
let path = import
.module_path
.iter()
.filter(|seg| seg.ident.name != kw::PathRoot)
.map(|seg| seg.ident.clone())
.chain(std::iter::once(ident))
.collect::<Vec<_>>();
View on GitHub
The job aarch64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
PR_CI_JOB set; skipping tidy
##[group]Building bootstrap
Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
---
28 error: aborting due to 1 previous error
29
Note: some mismatched output was normalized before being compared
- LL - pub use parser::ParseOptions; //~ ERROR struct import `ParseOptions` is private
- LL + pub use options::ParseOptions; //~ ERROR struct import `ParseOptions` is private
+ help: import `ParseOptions` directly
+ |
+ LL - pub use parser::ParseOptions;
+ LL + pub use options::ParseOptions;
+ |
The actual stderr differed from the expected stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args imports/issue-55884-2.rs`
error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/imports/issue-55884-2.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/imports/issue-55884-2" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "--edition=2015"
stdout: none
--- stderr -------------------------------
error[E0603]: struct import `ParseOptions` is private
##[error] --> /checkout/tests/ui/imports/issue-55884-2.rs:14:17
|
LL | pub use parser::ParseOptions; //~ ERROR struct import `ParseOptions` is private
| ^^^^^^^^^^^^ private struct import
|
note: the struct import `ParseOptions` is defined here...
--> /checkout/tests/ui/imports/issue-55884-2.rs:11:9
|
LL | use ParseOptions;
| ^^^^^^^^^^^^
note: ...and refers to the struct import `ParseOptions` which is defined here...
--> /checkout/tests/ui/imports/issue-55884-2.rs:14:9
|
LL | pub use parser::ParseOptions; //~ ERROR struct import `ParseOptions` is private
| ^^^^^^^^^^^^^^^^^^^^ you could import this re-export
note: ...and refers to the struct import `ParseOptions` which is defined here...
--> /checkout/tests/ui/imports/issue-55884-2.rs:7:13
|
LL | pub use options::*;
| ^^^^^^^^^^ you could import this re-export
note: ...and refers to the struct `ParseOptions` which is defined here
--> /checkout/tests/ui/imports/issue-55884-2.rs:3:5
|
LL | pub struct ParseOptions {}
| ^^^^^^^^^^^^^^^^^^^^^^^ you could import this directly
help: import `ParseOptions` directly
|
LL - pub use parser::ParseOptions; //~ ERROR struct import `ParseOptions` is private
LL + pub use options::ParseOptions; //~ ERROR struct import `ParseOptions` is private
|
error: aborting due to 1 previous error
For more information about this error, try `rustc --explain E0603`.
tests/ui/imports/private-import-no-suggestion-156244.stderr · outdated
| 51 | help: consider importing this struct instead |
|
| 52 | | |
|
| 53 | LL - use crate::rename::inner::Item; |
|
| 54 | LL + use outer::actual::Item; |
wait this still looks like a bug
nothing. forgot //@ edition:2018
tests/ui/imports/private-import-nested-suggestion-156060.rs · resolved
| 1 | // Regression test for #156060. |
Could you combine other tests into this, because some are duplicate. You can use //@ revisions (revisions directive) for different editions, e.g., https://github.com/rust-lang/rust/blob/main/tests/ui/use/use-path-segment-kw.rs#L1-L3.
Combined.
r? me
View all comments
Resolves #156060.
use two::{One, ...}, the diagnostic suggested replacing theOnewith a multi-segment path of a different module, producing invalid code likeuse crate::two::{one::One, Two}. Skip it whensingle_nested == true.import.module_path, which can produce incorrect paths in edition 2018 and later.