Skip to content

Commit c21e607

Browse files
fmeasefmease
committed
on unresolved import disambiguate suggested path if it would collide
1 parent 4578435 commit c21e607

9 files changed

+195
-20
lines changed

‎compiler/rustc_resolve/src/diagnostics.rs‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use rustc_span::hygiene::MacroKind;
2525
use rustc_span::source_map::SourceMap;
2626
use rustc_span::symbol::{kw, sym, Ident, Symbol};
2727
use rustc_span::{BytePos, Span, SyntaxContext};
28-
use thin_vec::ThinVec;
28+
use thin_vec::{thin_vec, ThinVec};
2929

3030
use crate::errors::{
3131
AddedMacroUse, ChangeImportBinding, ChangeImportBindingSuggestion, ConsiderAddingADerive,
@@ -1147,7 +1147,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
11471147
namespace: Namespace,
11481148
parent_scope: &ParentScope<'a>,
11491149
start_module: Module<'a>,
1150-
crate_name: Ident,
1150+
crate_path: ThinVec<ast::PathSegment>,
11511151
filter_fn: FilterFn,
11521152
) -> Vec<ImportSuggestion>
11531153
where
@@ -1163,8 +1163,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
11631163
Some(x) => Some(x),
11641164
} {
11651165
let in_module_is_extern = !in_module.def_id().is_local();
1166-
// We have to visit module children in deterministic order to avoid
1167-
// instabilities in reported imports (#43552).
11681166
in_module.for_each_child(self, |this, ident, ns, name_binding| {
11691167
// avoid non-importable candidates
11701168
if !name_binding.is_importable() {
@@ -1214,12 +1212,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12141212
let res = name_binding.res();
12151213
if filter_fn(res) {
12161214
// create the path
1217-
let mut segms = path_segments.clone();
1218-
if lookup_ident.span.at_least_rust_2018() {
1215+
let mut segms = if lookup_ident.span.at_least_rust_2018() {
12191216
// crate-local absolute paths start with `crate::` in edition 2018
12201217
// FIXME: may also be stabilized for Rust 2015 (Issues #45477, #44660)
1221-
segms.insert(0, ast::PathSegment::from_ident(crate_name));
1222-
}
1218+
crate_path.clone()
1219+
} else {
1220+
ThinVec::new()
1221+
};
1222+
segms.append(&mut path_segments.clone());
12231223

12241224
segms.push(ast::PathSegment::from_ident(ident));
12251225
let path = Path { span: name_binding.span, segments: segms, tokens: None };
@@ -1318,18 +1318,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13181318
where
13191319
FilterFn: Fn(Res) -> bool,
13201320
{
1321+
let crate_path = thin_vec![ast::PathSegment::from_ident(Ident::with_dummy_span(kw::Crate))];
13211322
let mut suggestions = self.lookup_import_candidates_from_module(
13221323
lookup_ident,
13231324
namespace,
13241325
parent_scope,
13251326
self.graph_root,
1326-
Ident::with_dummy_span(kw::Crate),
1327+
crate_path,
13271328
&filter_fn,
13281329
);
13291330

13301331
if lookup_ident.span.at_least_rust_2018() {
1331-
let extern_prelude_names = self.extern_prelude.clone();
1332-
for (ident, _) in extern_prelude_names.into_iter() {
1332+
for ident in self.extern_prelude.clone().into_keys() {
13331333
if ident.span.from_expansion() {
13341334
// Idents are adjusted to the root context before being
13351335
// resolved in the extern prelude, so reporting this to the
@@ -1340,13 +1340,40 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13401340
}
13411341
let crate_id = self.crate_loader(|c| c.maybe_process_path_extern(ident.name));
13421342
if let Some(crate_id) = crate_id {
1343-
let crate_root = self.expect_module(crate_id.as_def_id());
1343+
let crate_def_id = crate_id.as_def_id();
1344+
let crate_root = self.expect_module(crate_def_id);
1345+
1346+
// Check if there's already an item in scope with the same name as the crate
1347+
// If so, we have to disambiguate the potential import suggestion by making
1348+
// the path *global* (i.e., by prefixing it with `::`).
1349+
let needs_disambiguation =
1350+
self.resolutions(parent_scope.module).borrow().iter().any(
1351+
|(key, name_resolution)| {
1352+
if key.ns == TypeNS
1353+
&& key.ident == ident
1354+
&& let Some(binding) = name_resolution.borrow().binding
1355+
&& let crate::Res::Def(_, def_id) = binding.res()
1356+
{
1357+
// No disambiguation needed if the identically named item we
1358+
// found in scope actually refers to the crate in question.
1359+
def_id != crate_def_id
1360+
} else {
1361+
false
1362+
}
1363+
},
1364+
);
1365+
let mut crate_path = ThinVec::new();
1366+
if needs_disambiguation {
1367+
crate_path.push(ast::PathSegment::path_root(rustc_span::DUMMY_SP));
1368+
}
1369+
crate_path.push(ast::PathSegment::from_ident(ident));
1370+
13441371
suggestions.extend(self.lookup_import_candidates_from_module(
13451372
lookup_ident,
13461373
namespace,
13471374
parent_scope,
13481375
crate_root,
1349-
ident,
1376+
crate_path,
13501377
&filter_fn,
13511378
));
13521379
}
@@ -2541,7 +2568,7 @@ fn show_candidates(
25412568

25422569
candidates.iter().for_each(|c| {
25432570
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
2544-
.push((path_names_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
2571+
.push((pprust::path_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
25452572
});
25462573

25472574
// we want consistent results across executions, but candidates are produced

‎tests/ui/imports/issue-56125.stderr‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ LL | use empty::issue_56125;
66
|
77
help: consider importing one of these items instead
88
|
9+
LL | use ::issue_56125::issue_56125;
10+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
11+
LL | use ::issue_56125::last_segment::issue_56125;
12+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
13+
LL | use ::issue_56125::non_last_segment::non_last_segment::issue_56125;
14+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
915
LL | use crate::m3::last_segment::issue_56125;
1016
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
11-
LL | use crate::m3::non_last_segment::non_last_segment::issue_56125;
12-
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
13-
LL | use issue_56125::issue_56125;
14-
| ~~~~~~~~~~~~~~~~~~~~~~~~
15-
LL | use issue_56125::last_segment::issue_56125;
16-
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1717
and 1 other candidate
1818

1919
error[E0659]: `issue_56125` is ambiguous

‎tests/ui/unresolved/auxiliary/library.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub struct SomeUsefulType;

‎tests/ui/unresolved/unresolved-import-avoid-suggesting-global-path.fixed‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Test that we don't prepend `::` to paths referencing crates from the extern prelude
2+
// when it can be avoided[^1] since it's more idiomatic.
3+
//
4+
// [^1]: Counterexample: `unresolved-import-disambiguate-suggested-crate.rs`
5+
6+
#![feature(decl_macro)] // allows us to create items with hygienic names
7+
#![allow(unused_imports)]
8+
9+
// run-rustfix
10+
11+
// aux-crate:library=library.rs
12+
// edition: 2021
13+
14+
mod hygiene {
15+
macro make() {
16+
// This won't conflict with the suggested *non-global* path as the syntax context differs.
17+
mod library {}
18+
}
19+
20+
make!();
21+
22+
mod module {}
23+
use library::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
24+
}
25+
26+
mod glob {
27+
mod inner {
28+
mod library {}
29+
}
30+
31+
use inner::*;
32+
33+
mod module {}
34+
use library::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
35+
}
36+
37+
fn main() {}

‎tests/ui/unresolved/unresolved-import-avoid-suggesting-global-path.rs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Test that we don't prepend `::` to paths referencing crates from the extern prelude
2+
// when it can be avoided[^1] since it's more idiomatic.
3+
//
4+
// [^1]: Counterexample: `unresolved-import-disambiguate-suggested-crate.rs`
5+
6+
#![feature(decl_macro)] // allows us to create items with hygienic names
7+
#![allow(unused_imports)]
8+
9+
// run-rustfix
10+
11+
// aux-crate:library=library.rs
12+
// edition: 2021
13+
14+
mod hygiene {
15+
macro make() {
16+
// This won't conflict with the suggested *non-global* path as the syntax context differs.
17+
mod library {}
18+
}
19+
20+
make!();
21+
22+
mod module {}
23+
use module::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
24+
}
25+
26+
mod glob {
27+
mod inner {
28+
mod library {}
29+
}
30+
31+
use inner::*;
32+
33+
mod module {}
34+
use module::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
35+
}
36+
37+
fn main() {}

‎tests/ui/unresolved/unresolved-import-avoid-suggesting-global-path.stderr‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
error[E0432]: unresolved import `module::SomeUsefulType`
2+
--> $DIR/unresolved-import-avoid-suggesting-global-path.rs:23:9
3+
|
4+
LL | use module::SomeUsefulType;
5+
| ^^^^^^^^^^^^^^^^^^^^^^ no `SomeUsefulType` in `hygiene::module`
6+
|
7+
help: consider importing this struct instead
8+
|
9+
LL | use library::SomeUsefulType;
10+
| ~~~~~~~~~~~~~~~~~~~~~~~
11+
12+
error[E0432]: unresolved import `module::SomeUsefulType`
13+
--> $DIR/unresolved-import-avoid-suggesting-global-path.rs:34:9
14+
|
15+
LL | use module::SomeUsefulType;
16+
| ^^^^^^^^^^^^^^^^^^^^^^ no `SomeUsefulType` in `glob::module`
17+
|
18+
help: consider importing this struct instead
19+
|
20+
LL | use library::SomeUsefulType;
21+
| ~~~~~~~~~~~~~~~~~~~~~~~
22+
23+
error: aborting due to 2 previous errors
24+
25+
For more information about this error, try `rustc --explain E0432`.

‎tests/ui/unresolved/unresolved-import-disambiguate-suggested-crate.fixed‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Regression test for issue #116970.
2+
// When we suggest importing an item from a crate contained in the extern prelude and there
3+
// happens to exist a module or type in the current scope with the same name as the crate,
4+
// disambiguate the suggested path by making it global (i.e., prefixing it with `::`).
5+
6+
// run-rustfix
7+
8+
// compile-flags: --crate-type=lib
9+
// aux-crate:library=library.rs
10+
// edition: 2021
11+
12+
mod library {} // this module shares the same name as the external crate!
13+
14+
mod module {}
15+
pub use ::library::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`

‎tests/ui/unresolved/unresolved-import-disambiguate-suggested-crate.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Regression test for issue #116970.
2+
//
3+
// When we suggest importing an item from a crate contained in the extern prelude and there
4+
// happens to exist a module or type in the current scope with the same name as the crate,
5+
// disambiguate the suggested path by making it global (i.e., by prefixing it with `::`).
6+
//
7+
// For context, when it can be avoided we don't want to prepend `::` to paths referencing
8+
// crates from the extern prelude. See also `unresolved-import-avoid-suggesting-global-path.rs`.
9+
10+
// run-rustfix
11+
12+
// compile-flags: --crate-type=lib
13+
// aux-crate:library=library.rs
14+
// edition: 2021
15+
16+
mod library {} // this module shares the same name as the external crate!
17+
18+
mod module {}
19+
pub use module::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`

‎tests/ui/unresolved/unresolved-import-disambiguate-suggested-crate.stderr‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error[E0432]: unresolved import `module::SomeUsefulType`
2+
--> $DIR/unresolved-import-disambiguate-suggested-crate.rs:15:9
3+
|
4+
LL | pub use module::SomeUsefulType;
5+
| ^^^^^^^^^^^^^^^^^^^^^^ no `SomeUsefulType` in `module`
6+
|
7+
help: consider importing this struct instead
8+
|
9+
LL | pub use ::library::SomeUsefulType;
10+
| ~~~~~~~~~~~~~~~~~~~~~~~~~
11+
12+
error: aborting due to previous error
13+
14+
For more information about this error, try `rustc --explain E0432`.

0 commit comments

Comments
 (0)