Skip to content

Commit

Permalink
on unresolved import disambiguate suggested path if it would collide
Browse files Browse the repository at this point in the history
  • Loading branch information
fmease committed Oct 21, 2023
1 parent 4578435 commit a137e46
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 20 deletions.
58 changes: 44 additions & 14 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, Span, SyntaxContext};
use thin_vec::ThinVec;
use thin_vec::{thin_vec, ThinVec};

use crate::errors::{
AddedMacroUse, ChangeImportBinding, ChangeImportBindingSuggestion, ConsiderAddingADerive,
Expand Down Expand Up @@ -1147,7 +1147,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
namespace: Namespace,
parent_scope: &ParentScope<'a>,
start_module: Module<'a>,
crate_name: Ident,
crate_path: ThinVec<ast::PathSegment>,
filter_fn: FilterFn,
) -> Vec<ImportSuggestion>
where
Expand All @@ -1163,8 +1163,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(x) => Some(x),
} {
let in_module_is_extern = !in_module.def_id().is_local();
// We have to visit module children in deterministic order to avoid
// instabilities in reported imports (#43552).
in_module.for_each_child(self, |this, ident, ns, name_binding| {
// avoid non-importable candidates
if !name_binding.is_importable() {
Expand Down Expand Up @@ -1214,12 +1212,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let res = name_binding.res();
if filter_fn(res) {
// create the path
let mut segms = path_segments.clone();
if lookup_ident.span.at_least_rust_2018() {
let mut segms = if lookup_ident.span.at_least_rust_2018() {
// crate-local absolute paths start with `crate::` in edition 2018
// FIXME: may also be stabilized for Rust 2015 (Issues #45477, #44660)
segms.insert(0, ast::PathSegment::from_ident(crate_name));
}
crate_path.clone()
} else {
ThinVec::new()
};
segms.append(&mut path_segments.clone());

segms.push(ast::PathSegment::from_ident(ident));
let path = Path { span: name_binding.span, segments: segms, tokens: None };
Expand Down Expand Up @@ -1318,18 +1318,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
where
FilterFn: Fn(Res) -> bool,
{
let crate_path = thin_vec![ast::PathSegment::from_ident(Ident::with_dummy_span(kw::Crate))];
let mut suggestions = self.lookup_import_candidates_from_module(
lookup_ident,
namespace,
parent_scope,
self.graph_root,
Ident::with_dummy_span(kw::Crate),
crate_path,
&filter_fn,
);

if lookup_ident.span.at_least_rust_2018() {
let extern_prelude_names = self.extern_prelude.clone();
for (ident, _) in extern_prelude_names.into_iter() {
for ident in self.extern_prelude.clone().into_keys() {
if ident.span.from_expansion() {
// Idents are adjusted to the root context before being
// resolved in the extern prelude, so reporting this to the
Expand All @@ -1340,13 +1340,43 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
let crate_id = self.crate_loader(|c| c.maybe_process_path_extern(ident.name));
if let Some(crate_id) = crate_id {
let crate_root = self.expect_module(crate_id.as_def_id());
let crate_def_id = crate_id.as_def_id();
let crate_root = self.expect_module(crate_def_id);

// Check if there's already an item in scope with the same name as the crate.
// If so, we have to disambiguate the potential import suggestions by making
// the paths *global* (i.e., by prefixing them with `::`).
let needs_disambiguation =
self.resolutions(parent_scope.module).borrow().iter().any(
|(key, name_resolution)| {
if key.ns == TypeNS
&& key.ident == ident
&& let Some(binding) = name_resolution.borrow().binding
{
match binding.res() {
// No disambiguation needed if the identically named item we
// found in scope actually refers to the crate in question.
Res::Def(_, def_id) if def_id != crate_def_id => true,
Res::PrimTy(_) => true,
_ => false,
}
} else {
false
}
},
);
let mut crate_path = ThinVec::new();
if needs_disambiguation {
crate_path.push(ast::PathSegment::path_root(rustc_span::DUMMY_SP));
}
crate_path.push(ast::PathSegment::from_ident(ident));

suggestions.extend(self.lookup_import_candidates_from_module(
lookup_ident,
namespace,
parent_scope,
crate_root,
ident,
crate_path,
&filter_fn,
));
}
Expand Down Expand Up @@ -2541,7 +2571,7 @@ fn show_candidates(

candidates.iter().for_each(|c| {
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
.push((path_names_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
.push((pprust::path_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
});

// we want consistent results across executions, but candidates are produced
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/imports/issue-56125.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ LL | use empty::issue_56125;
|
help: consider importing one of these items instead
|
LL | use ::issue_56125::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use ::issue_56125::last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use ::issue_56125::non_last_segment::non_last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use crate::m3::last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use crate::m3::non_last_segment::non_last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LL | use issue_56125::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~
LL | use issue_56125::last_segment::issue_56125;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
and 1 other candidate

error[E0659]: `issue_56125` is ambiguous
Expand Down
1 change: 1 addition & 0 deletions tests/ui/unresolved/auxiliary/library.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct SomeUsefulType;
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Test that we don't prepend `::` to paths referencing crates from the extern prelude
// when it can be avoided[^1] since it's more idiomatic to do so.
//
// [^1]: Counterexample: `unresolved-import-suggest-disambiguated-crate-name.rs`
#![feature(decl_macro)] // allows us to create items with hygienic names
#![allow(unused_imports)]

// aux-crate:library=library.rs
// edition: 2021

mod hygiene {
make!();
macro make() {
// This won't conflict with the suggested *non-global* path as the syntax context differs.
mod library {}
}

mod module {}
use module::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
}

mod glob {
use inner::*;
mod inner {
mod library {}
}

mod module {}
use module::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0432]: unresolved import `module::SomeUsefulType`
--> $DIR/unresolved-import-avoid-suggesting-global-path.rs:19:9
|
LL | use module::SomeUsefulType;
| ^^^^^^^^^^^^^^^^^^^^^^ no `SomeUsefulType` in `hygiene::module`
|
help: consider importing this struct instead
|
LL | use library::SomeUsefulType;
| ~~~~~~~~~~~~~~~~~~~~~~~

error[E0432]: unresolved import `module::SomeUsefulType`
--> $DIR/unresolved-import-avoid-suggesting-global-path.rs:29:9
|
LL | use module::SomeUsefulType;
| ^^^^^^^^^^^^^^^^^^^^^^ no `SomeUsefulType` in `glob::module`
|
help: consider importing this struct instead
|
LL | use library::SomeUsefulType;
| ~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0432`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test for issue #116970.
//
// When we suggest importing an item from a crate found in the extern prelude and there
// happens to exist a module or type in the current scope with the same name as the crate,
// disambiguate the suggested path by making it global (i.e., by prefixing it with `::`).
//
// For context, when it can be avoided we don't prepend `::` to paths referencing crates
// from the extern prelude. See also `unresolved-import-avoid-suggesting-global-path.rs`.

// run-rustfix

// compile-flags: --crate-type=lib
// aux-crate:library=library.rs
// edition: 2021

mod library {} // this module shares the same name as the external crate!

mod module {}
pub use ::library::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test for issue #116970.
//
// When we suggest importing an item from a crate found in the extern prelude and there
// happens to exist a module or type in the current scope with the same name as the crate,
// disambiguate the suggested path by making it global (i.e., by prefixing it with `::`).
//
// For context, when it can be avoided we don't prepend `::` to paths referencing crates
// from the extern prelude. See also `unresolved-import-avoid-suggesting-global-path.rs`.

// run-rustfix

// compile-flags: --crate-type=lib
// aux-crate:library=library.rs
// edition: 2021

mod library {} // this module shares the same name as the external crate!

mod module {}
pub use module::SomeUsefulType; //~ ERROR unresolved import `module::SomeUsefulType`
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0432]: unresolved import `module::SomeUsefulType`
--> $DIR/unresolved-import-suggest-disambiguated-crate-name.rs:19:9
|
LL | pub use module::SomeUsefulType;
| ^^^^^^^^^^^^^^^^^^^^^^ no `SomeUsefulType` in `module`
|
help: consider importing this struct instead
|
LL | pub use ::library::SomeUsefulType;
| ~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.

0 comments on commit a137e46

Please sign in to comment.