From 404df1cbbfac0126d256f99a12a175d9ce96fd0f Mon Sep 17 00:00:00 2001 From: Dan Aloni Date: Sun, 21 Jun 2020 19:31:49 +0300 Subject: [PATCH 1/2] Add re-exports to use suggestions In the following example, an inaccessible path is suggested via `use foo::bar::X;` whereas an accessible public exported path can be suggested instead. ``` mod foo { mod bar { pub struct X; } pub use self::bar::X; } fn main() { X; } ``` This fixes the issue. --- src/librustc_resolve/diagnostics.rs | 23 +++++++++++++------- src/librustc_resolve/lib.rs | 7 ++++++ src/test/ui/glob-resolve1.rs | 4 ++++ src/test/ui/glob-resolve1.stderr | 5 +++++ src/test/ui/namespace/namespace-mix.stderr | 8 +++---- src/test/ui/resolve/issue-21221-2.stderr | 4 +++- src/test/ui/resolve/privacy-enum-ctor.stderr | 8 +++---- 7 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index bb88b8191f1ad..ea4a84a9c3bd4 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -643,22 +643,24 @@ impl<'a> Resolver<'a> { let not_local_module = crate_name.name != kw::Crate; let mut worklist = vec![(start_module, Vec::::new(), true, not_local_module)]; + let mut worklist_via_import = vec![]; - while let Some((in_module, path_segments, accessible, in_module_is_extern)) = worklist.pop() + while let Some((in_module, path_segments, accessible, in_module_is_extern)) = + match worklist.pop() { + None => worklist_via_import.pop(), + Some(x) => Some(x), + } { // 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 imports entirely - if name_binding.is_import() && !name_binding.is_extern_crate() { - return; - } - - // avoid non-importable candidates as well + // avoid non-importable candidates if !name_binding.is_importable() { return; } + let via_import = name_binding.is_import() && !name_binding.is_extern_crate(); + let child_accessible = accessible && this.is_accessible_from(name_binding.vis, parent_scope.module); @@ -667,6 +669,10 @@ impl<'a> Resolver<'a> { return; } + if via_import && name_binding.is_possibly_imported_variant() { + return; + } + // collect results based on the filter function // avoid suggesting anything from the same module in which we are resolving if ident.name == lookup_ident.name @@ -724,7 +730,8 @@ impl<'a> Resolver<'a> { let is_extern = in_module_is_extern || name_binding.is_extern_crate(); // add the module to the lookup if seen_modules.insert(module.def_id().unwrap()) { - worklist.push((module, path_segments, child_accessible, is_extern)); + if via_import { &mut worklist_via_import } else { &mut worklist } + .push((module, path_segments, child_accessible, is_extern)); } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 91bd155614178..9aa0be0a71e1f 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -711,6 +711,13 @@ impl<'a> NameBinding<'a> { } } + fn is_possibly_imported_variant(&self) -> bool { + match self.kind { + NameBindingKind::Import { binding, .. } => binding.is_possibly_imported_variant(), + _ => self.is_variant(), + } + } + // We sometimes need to treat variants as `pub` for backwards compatibility. fn pseudo_vis(&self) -> ty::Visibility { if self.is_variant() && self.res().def_id().is_local() { diff --git a/src/test/ui/glob-resolve1.rs b/src/test/ui/glob-resolve1.rs index 63c435cc20641..32660fdb41876 100644 --- a/src/test/ui/glob-resolve1.rs +++ b/src/test/ui/glob-resolve1.rs @@ -29,3 +29,7 @@ fn main() { foo::(); //~ ERROR: cannot find type `C` in this scope foo::(); //~ ERROR: cannot find type `D` in this scope } + +mod other { + pub fn import() {} +} diff --git a/src/test/ui/glob-resolve1.stderr b/src/test/ui/glob-resolve1.stderr index 995da6cc1f975..3c818f3ae48ea 100644 --- a/src/test/ui/glob-resolve1.stderr +++ b/src/test/ui/glob-resolve1.stderr @@ -42,6 +42,11 @@ error[E0425]: cannot find function `import` in this scope | LL | import(); | ^^^^^^ not found in this scope + | +help: consider importing this function + | +LL | use other::import; + | error[E0412]: cannot find type `A` in this scope --> $DIR/glob-resolve1.rs:28:11 diff --git a/src/test/ui/namespace/namespace-mix.stderr b/src/test/ui/namespace/namespace-mix.stderr index c80055f00d7d9..ee730910ee441 100644 --- a/src/test/ui/namespace/namespace-mix.stderr +++ b/src/test/ui/namespace/namespace-mix.stderr @@ -16,7 +16,7 @@ help: consider importing one of these items instead | LL | use m2::S; | -LL | use namespace_mix::xm2::S; +LL | use xm2::S; | error[E0423]: expected value, found type alias `xm1::S` @@ -39,7 +39,7 @@ help: consider importing one of these items instead | LL | use m2::S; | -LL | use namespace_mix::xm2::S; +LL | use xm2::S; | error[E0423]: expected value, found struct variant `m7::V` @@ -61,7 +61,7 @@ help: consider importing one of these items instead | LL | use m8::V; | -LL | use namespace_mix::xm8::V; +LL | use xm8::V; | error[E0423]: expected value, found struct variant `xm7::V` @@ -83,7 +83,7 @@ help: consider importing one of these items instead | LL | use m8::V; | -LL | use namespace_mix::xm8::V; +LL | use xm8::V; | error[E0277]: the trait bound `c::Item: Impossible` is not satisfied diff --git a/src/test/ui/resolve/issue-21221-2.stderr b/src/test/ui/resolve/issue-21221-2.stderr index f9263d2af5026..d4fd7cb1257e0 100644 --- a/src/test/ui/resolve/issue-21221-2.stderr +++ b/src/test/ui/resolve/issue-21221-2.stderr @@ -4,7 +4,9 @@ error[E0405]: cannot find trait `T` in this scope LL | impl T for Foo { } | ^ not found in this scope | -help: consider importing this trait +help: consider importing one of these items + | +LL | use baz::T; | LL | use foo::bar::T; | diff --git a/src/test/ui/resolve/privacy-enum-ctor.stderr b/src/test/ui/resolve/privacy-enum-ctor.stderr index d9b1b9c59558a..16baa6c9b6233 100644 --- a/src/test/ui/resolve/privacy-enum-ctor.stderr +++ b/src/test/ui/resolve/privacy-enum-ctor.stderr @@ -132,7 +132,7 @@ LL | let _: E = m::n::Z; | ^ help: consider importing this enum | -LL | use m::n::Z; +LL | use m::Z; | error[E0423]: expected value, found enum `m::n::Z` @@ -165,7 +165,7 @@ LL | let _: E = m::n::Z::Fn; | ^ help: consider importing this enum | -LL | use m::n::Z; +LL | use m::Z; | error[E0412]: cannot find type `Z` in this scope @@ -183,7 +183,7 @@ LL | let _: E = m::n::Z::Struct; | ^ help: consider importing this enum | -LL | use m::n::Z; +LL | use m::Z; | error[E0423]: expected value, found struct variant `m::n::Z::Struct` @@ -212,7 +212,7 @@ LL | let _: E = m::n::Z::Unit {}; | ^ help: consider importing this enum | -LL | use m::n::Z; +LL | use m::Z; | error[E0603]: enum `Z` is private From 037e930df7983577a2093732de0bd2c1252a4e37 Mon Sep 17 00:00:00 2001 From: Dan Aloni Date: Tue, 23 Jun 2020 22:25:23 +0300 Subject: [PATCH 2/2] Review fixes --- src/librustc_resolve/diagnostics.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index ea4a84a9c3bd4..268974384a8d3 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -659,8 +659,6 @@ impl<'a> Resolver<'a> { return; } - let via_import = name_binding.is_import() && !name_binding.is_extern_crate(); - let child_accessible = accessible && this.is_accessible_from(name_binding.vis, parent_scope.module); @@ -669,6 +667,13 @@ impl<'a> Resolver<'a> { return; } + let via_import = name_binding.is_import() && !name_binding.is_extern_crate(); + + // There is an assumption elsewhere that paths of variants are in the enum's + // declaration and not imported. With this assumption, the variant component is + // chopped and the rest of the path is assumed to be the enum's own path. For + // errors where a variant is used as the type instead of the enum, this causes + // funny looking invalid suggestions, i.e `foo` instead of `foo::MyEnum`. if via_import && name_binding.is_possibly_imported_variant() { return; }