Skip to content

Commit 62396df

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #156244 - el-ev:issue156060, r=mu001999
fix incorrect suggestions in private import diagnostic Resolves #156060. 1. In nested imports like `use two::{One, ...}`, the diagnostic suggested replacing the `One` with a multi-segment path of a different module, producing invalid code like `use crate::two::{one::One, Two}`. Skip it when `single_nested == true`. 2. Stop unconditionally skipping the first segment of `import.module_path`, which can produce incorrect paths in edition 2018 and later. 3. Mark the suggestion as "directly" instead of "through the re-export" when the import's source is the definition itself.
2 parents dbc4f62 + 6d5fc32 commit 62396df

10 files changed

Lines changed: 294 additions & 37 deletions

‎compiler/rustc_resolve/src/diagnostics.rs‎

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,7 +2307,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
23072307

23082308
self.mention_default_field_values(source, ident, &mut err);
23092309

2310-
let mut not_publicly_reexported = false;
23112310
if let Some((this_res, outer_ident)) = outermost_res {
23122311
let mut import_suggestions = self.lookup_import_candidates(
23132312
outer_ident,
@@ -2332,7 +2331,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
23322331
);
23332332
// If we suggest importing a public re-export, don't point at the definition.
23342333
if point_to_def && ident.span != outer_ident.span {
2335-
not_publicly_reexported = true;
23362334
let label = errors::OuterIdentIsNotPubliclyReexported {
23372335
span: outer_ident.span,
23382336
outer_ident_descr: this_res.descr(),
@@ -2408,7 +2406,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
24082406
let first_binding = decl;
24092407
let mut next_binding = Some(decl);
24102408
let mut next_ident = ident;
2411-
let mut path = vec![];
24122409
while let Some(binding) = next_binding {
24132410
let name = next_ident;
24142411
next_binding = match binding.kind {
@@ -2428,18 +2425,18 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
24282425
};
24292426

24302427
match binding.kind {
2431-
DeclKind::Import { import, .. } => {
2432-
for segment in import.module_path.iter().skip(1) {
2433-
// Don't include `{{root}}` in suggestions - it's an internal symbol
2434-
// that should never be shown to users.
2435-
if segment.ident.name != kw::PathRoot {
2436-
path.push(segment.ident);
2437-
}
2438-
}
2439-
sugg_paths.push((
2440-
path.iter().cloned().chain(std::iter::once(ident)).collect::<Vec<_>>(),
2441-
true, // re-export
2442-
));
2428+
DeclKind::Import { source_decl, import, .. } => {
2429+
// Don't include `{{root}}` in suggestions - it's an internal symbol
2430+
// that should never be shown to users.
2431+
let path = import
2432+
.module_path
2433+
.iter()
2434+
.filter(|seg| seg.ident.name != kw::PathRoot)
2435+
.map(|seg| seg.ident.clone())
2436+
.chain(std::iter::once(ident))
2437+
.collect::<Vec<_>>();
2438+
let through_reexport = !matches!(source_decl.kind, DeclKind::Def(_));
2439+
sugg_paths.push((path, through_reexport));
24432440
}
24442441
DeclKind::Def(_) => {}
24452442
}
@@ -2472,25 +2469,34 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
24722469
};
24732470
err.subdiagnostic(note);
24742471
}
2475-
// We prioritize shorter paths, non-core imports and direct imports over the alternatives.
2476-
sugg_paths.sort_by_key(|(p, reexport)| (p.len(), p[0].name == sym::core, *reexport));
2477-
for (sugg, reexport) in sugg_paths {
2478-
if not_publicly_reexported {
2472+
// The suggestion replaces `dedup_span` with a path reaching the failing ident.
2473+
// That's valid only when
2474+
// 1) the failing ident is the imported leaf, otherwise `as` renames and trailing segments
2475+
// get dropped, and
2476+
// 2) the use isn't nested, otherwise `dedup_span` is one ident in `{...}`.
2477+
//
2478+
// See issue #156060.
2479+
let can_replace_use =
2480+
!single_nested && !outermost_res.is_some_and(|(_, outer)| outer.span != ident.span);
2481+
if can_replace_use {
2482+
// We prioritize shorter paths, non-core imports and direct imports over the
2483+
// alternatives.
2484+
sugg_paths.sort_by_key(|(p, reexport)| (p.len(), p[0].name == sym::core, *reexport));
2485+
for (sugg, reexport) in sugg_paths {
2486+
if sugg.len() <= 1 {
2487+
// A single path segment suggestion is wrong. This happens on circular
2488+
// imports. `tests/ui/imports/issue-55884-2.rs`
2489+
continue;
2490+
}
2491+
let path = join_path_idents(sugg);
2492+
let sugg = if reexport {
2493+
errors::ImportIdent::ThroughReExport { span: dedup_span, ident, path }
2494+
} else {
2495+
errors::ImportIdent::Directly { span: dedup_span, ident, path }
2496+
};
2497+
err.subdiagnostic(sugg);
24792498
break;
24802499
}
2481-
if sugg.len() <= 1 {
2482-
// A single path segment suggestion is wrong. This happens on circular imports.
2483-
// `tests/ui/imports/issue-55884-2.rs`
2484-
continue;
2485-
}
2486-
let path = join_path_idents(sugg);
2487-
let sugg = if reexport {
2488-
errors::ImportIdent::ThroughReExport { span: dedup_span, ident, path }
2489-
} else {
2490-
errors::ImportIdent::Directly { span: dedup_span, ident, path }
2491-
};
2492-
err.subdiagnostic(sugg);
2493-
break;
24942500
}
24952501

24962502
err.emit();

‎tests/ui/imports/ambiguous-import-visibility-globglob-priv.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ note: ...and refers to the struct `S` which is defined here
1414
|
1515
LL | pub struct S {}
1616
| ^^^^^^^^^^^^ you could import this directly
17-
help: import `S` through the re-export
17+
help: import `S` directly
1818
|
1919
LL - use crate::both::private::S;
20-
LL + use m::S;
20+
LL + use crate::m::S;
2121
|
2222

2323
error[E0603]: struct import `S` is private
@@ -36,10 +36,10 @@ note: ...and refers to the struct `S` which is defined here
3636
|
3737
LL | pub struct S {}
3838
| ^^^^^^^^^^^^ you could import this directly
39-
help: import `S` through the re-export
39+
help: import `S` directly
4040
|
4141
LL - use crate::both::private::S;
42-
LL + use m::S;
42+
LL + use crate::m::S;
4343
|
4444

4545
error: aborting due to 2 previous errors

‎tests/ui/imports/issue-55884-2.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ note: ...and refers to the struct `ParseOptions` which is defined here
2424
|
2525
LL | pub struct ParseOptions {}
2626
| ^^^^^^^^^^^^^^^^^^^^^^^ you could import this directly
27+
help: import `ParseOptions` directly
28+
|
29+
LL - pub use parser::ParseOptions;
30+
LL + pub use options::ParseOptions;
31+
|
2732

2833
error: aborting due to 1 previous error
2934

‎tests/ui/imports/private-import-nested-suggestion-156060.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Regression test for #156060.
2+
3+
mod one {
4+
pub struct One();
5+
}
6+
7+
mod two {
8+
use crate::one::One;
9+
pub struct Two();
10+
}
11+
12+
mod test {
13+
use crate::two::{One, Two};
14+
//~^ ERROR struct import `One` is private [E0603]
15+
}
16+
17+
fn main() {}

‎tests/ui/imports/private-import-nested-suggestion-156060.stderr‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error[E0603]: struct import `One` is private
2+
--> $DIR/private-import-nested-suggestion-156060.rs:13:22
3+
|
4+
LL | use crate::two::{One, Two};
5+
| ^^^ private struct import
6+
|
7+
note: the struct import `One` is defined here...
8+
--> $DIR/private-import-nested-suggestion-156060.rs:8:9
9+
|
10+
LL | use crate::one::One;
11+
| ^^^^^^^^^^^^^^^
12+
note: ...and refers to the struct `One` which is defined here
13+
--> $DIR/private-import-nested-suggestion-156060.rs:4:5
14+
|
15+
LL | pub struct One();
16+
| ^^^^^^^^^^^^^^^^^ you could import this directly
17+
18+
error: aborting due to 1 previous error
19+
20+
For more information about this error, try `rustc --explain E0603`.

‎tests/ui/imports/private-import-suggestion-path-156244.edition_2015.stderr‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
error[E0603]: struct import `One` is private
2+
--> $DIR/private-import-suggestion-path-156244.rs:17:12
3+
|
4+
LL | use b::One;
5+
| ^^^ private struct import
6+
|
7+
note: the struct import `One` is defined here...
8+
--> $DIR/private-import-suggestion-path-156244.rs:12:20
9+
|
10+
LL | use crate::a::{One, Two};
11+
| ^^^
12+
note: ...and refers to the struct `One` which is defined here
13+
--> $DIR/private-import-suggestion-path-156244.rs:7:5
14+
|
15+
LL | pub struct One;
16+
| ^^^^^^^^^^^^^^^ you could import this directly
17+
help: import `One` directly
18+
|
19+
LL - use b::One;
20+
LL + use crate::a::One;
21+
|
22+
23+
error[E0603]: struct import `One` is private
24+
--> $DIR/private-import-suggestion-path-156244.rs:35:20
25+
|
26+
LL | use crate::b::{One, Two};
27+
| ^^^ private struct import
28+
|
29+
note: the struct import `One` is defined here...
30+
--> $DIR/private-import-suggestion-path-156244.rs:12:20
31+
|
32+
LL | use crate::a::{One, Two};
33+
| ^^^
34+
note: ...and refers to the struct `One` which is defined here
35+
--> $DIR/private-import-suggestion-path-156244.rs:7:5
36+
|
37+
LL | pub struct One;
38+
| ^^^^^^^^^^^^^^^ you could import this directly
39+
40+
error[E0603]: struct import `Two` is private
41+
--> $DIR/private-import-suggestion-path-156244.rs:35:25
42+
|
43+
LL | use crate::b::{One, Two};
44+
| ^^^ private struct import
45+
|
46+
note: the struct import `Two` is defined here...
47+
--> $DIR/private-import-suggestion-path-156244.rs:12:25
48+
|
49+
LL | use crate::a::{One, Two};
50+
| ^^^
51+
note: ...and refers to the struct `Two` which is defined here
52+
--> $DIR/private-import-suggestion-path-156244.rs:8:5
53+
|
54+
LL | pub struct Two;
55+
| ^^^^^^^^^^^^^^^ you could import this directly
56+
57+
error[E0603]: module import `inner` is private
58+
--> $DIR/private-import-suggestion-path-156244.rs:38:24
59+
|
60+
LL | use crate::rename::inner::Item as Item1;
61+
| ^^^^^ private module import
62+
|
63+
note: the module import `inner` is defined here...
64+
--> $DIR/private-import-suggestion-path-156244.rs:31:9
65+
|
66+
LL | use crate::outer::actual as inner;
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
68+
note: ...and refers to the module `actual` which is defined here
69+
--> $DIR/private-import-suggestion-path-156244.rs:25:5
70+
|
71+
LL | pub mod actual {
72+
| ^^^^^^^^^^^^^^ you could import this directly
73+
help: consider importing this struct instead
74+
|
75+
LL - use crate::rename::inner::Item as Item1;
76+
LL + use outer::actual::Item as Item1;
77+
|
78+
79+
error: aborting due to 4 previous errors
80+
81+
For more information about this error, try `rustc --explain E0603`.

‎tests/ui/imports/private-import-suggestion-path-156244.edition_2018.stderr‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
error[E0603]: struct import `One` is private
2+
--> $DIR/private-import-suggestion-path-156244.rs:20:19
3+
|
4+
LL | use crate::b::One;
5+
| ^^^ private struct import
6+
|
7+
note: the struct import `One` is defined here...
8+
--> $DIR/private-import-suggestion-path-156244.rs:12:20
9+
|
10+
LL | use crate::a::{One, Two};
11+
| ^^^
12+
note: ...and refers to the struct `One` which is defined here
13+
--> $DIR/private-import-suggestion-path-156244.rs:7:5
14+
|
15+
LL | pub struct One;
16+
| ^^^^^^^^^^^^^^^ you could import this directly
17+
help: import `One` directly
18+
|
19+
LL - use crate::b::One;
20+
LL + use crate::a::One;
21+
|
22+
23+
error[E0603]: struct import `One` is private
24+
--> $DIR/private-import-suggestion-path-156244.rs:35:20
25+
|
26+
LL | use crate::b::{One, Two};
27+
| ^^^ private struct import
28+
|
29+
note: the struct import `One` is defined here...
30+
--> $DIR/private-import-suggestion-path-156244.rs:12:20
31+
|
32+
LL | use crate::a::{One, Two};
33+
| ^^^
34+
note: ...and refers to the struct `One` which is defined here
35+
--> $DIR/private-import-suggestion-path-156244.rs:7:5
36+
|
37+
LL | pub struct One;
38+
| ^^^^^^^^^^^^^^^ you could import this directly
39+
40+
error[E0603]: struct import `Two` is private
41+
--> $DIR/private-import-suggestion-path-156244.rs:35:25
42+
|
43+
LL | use crate::b::{One, Two};
44+
| ^^^ private struct import
45+
|
46+
note: the struct import `Two` is defined here...
47+
--> $DIR/private-import-suggestion-path-156244.rs:12:25
48+
|
49+
LL | use crate::a::{One, Two};
50+
| ^^^
51+
note: ...and refers to the struct `Two` which is defined here
52+
--> $DIR/private-import-suggestion-path-156244.rs:8:5
53+
|
54+
LL | pub struct Two;
55+
| ^^^^^^^^^^^^^^^ you could import this directly
56+
57+
error[E0603]: module import `inner` is private
58+
--> $DIR/private-import-suggestion-path-156244.rs:38:24
59+
|
60+
LL | use crate::rename::inner::Item as Item1;
61+
| ^^^^^ private module import
62+
|
63+
note: the module import `inner` is defined here...
64+
--> $DIR/private-import-suggestion-path-156244.rs:31:9
65+
|
66+
LL | use crate::outer::actual as inner;
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
68+
note: ...and refers to the module `actual` which is defined here
69+
--> $DIR/private-import-suggestion-path-156244.rs:25:5
70+
|
71+
LL | pub mod actual {
72+
| ^^^^^^^^^^^^^^ you could import this directly
73+
help: consider importing this struct instead
74+
|
75+
LL - use crate::rename::inner::Item as Item1;
76+
LL + use crate::outer::actual::Item as Item1;
77+
|
78+
79+
error: aborting due to 4 previous errors
80+
81+
For more information about this error, try `rustc --explain E0603`.

‎tests/ui/imports/private-import-suggestion-path-156244.rs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// PR #156244 comment
2+
//@ revisions: edition_2015 edition_2018
3+
//@[edition_2015] edition: 2015
4+
//@[edition_2018] edition: 2018
5+
6+
mod a {
7+
pub struct One;
8+
pub struct Two;
9+
}
10+
11+
mod b {
12+
use crate::a::{One, Two};
13+
}
14+
15+
mod test {
16+
#[cfg(edition_2015)]
17+
use b::One;
18+
//[edition_2015]~^ ERROR struct import `One` is private [E0603]
19+
#[cfg(edition_2018)]
20+
use crate::b::One;
21+
//[edition_2018]~^ ERROR struct import `One` is private [E0603]
22+
}
23+
24+
mod outer {
25+
pub mod actual {
26+
pub struct Item;
27+
}
28+
}
29+
30+
mod rename {
31+
use crate::outer::actual as inner;
32+
}
33+
34+
mod bad {
35+
use crate::b::{One, Two};
36+
//~^ ERROR struct import `One` is private [E0603]
37+
//~| ERROR struct import `Two` is private [E0603]
38+
use crate::rename::inner::Item as Item1;
39+
//~^ ERROR module import `inner` is private [E0603]
40+
}
41+
42+
fn main() {}

‎tests/ui/imports/private-std-reexport-suggest-public.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ note: ...and refers to the module `mem` which is defined here
1313
--> $SRC_DIR/std/src/lib.rs:LL:COL
1414
|
1515
= note: you could import this directly
16-
help: import `mem` through the re-export
16+
help: import `mem` directly
1717
|
1818
LL - use foo::mem;
1919
LL + use std::mem;

0 commit comments

Comments
 (0)