-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009
On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items #117009
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -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 | ||
|
@@ -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() { | ||
|
@@ -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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be able to move this here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean the whole E.g., it would get executed twice if pub struct SomeUsefulType;
pub mod inner {
pub enum SomeUsefulType {}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤦 sorry, should have looked more carefully. |
||
} 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 }; | ||
|
@@ -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 | ||
|
@@ -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) => def_id != crate_def_id, | ||
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, | ||
)); | ||
} | ||
|
@@ -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 | ||
|
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,31 @@ | ||
// 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 | ||
|
||
// 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:18: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:28: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`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment was added in #43552 but has been outdated since #65043.