From 48b0c9da69965cd40d037cee42cf3093ed09c6ee Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 11 Feb 2019 19:29:10 +0100 Subject: [PATCH] Only suggest imports if not imported. This commit modifies name resolution error reporting so that if a name is in scope and has been imported then we do not suggest importing it. This can occur when we add a label about constructors not being visible due to private fields. In these cases, we know that the struct/variant has been imported and we should silence any suggestions to import the struct/variant. --- src/librustc_resolve/error_reporting.rs | 10 +++++++++- src/librustc_resolve/lib.rs | 16 +++++++++++---- src/test/ui/issue-42944.rs | 19 ++++++++++++++++++ src/test/ui/issue-42944.stderr | 20 +++++++++++++++++++ .../ui/resolve/privacy-struct-ctor.stderr | 15 +++----------- 5 files changed, 63 insertions(+), 17 deletions(-) create mode 100644 src/test/ui/issue-42944.rs create mode 100644 src/test/ui/issue-42944.stderr diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 8300e691bbe..a6e27c0a1ae 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -106,7 +106,15 @@ impl<'a> Resolver<'a> { // Try to lookup name in more relaxed fashion for better error reporting. let ident = path.last().unwrap().ident; - let candidates = self.lookup_import_candidates(ident, ns, is_expected); + let candidates = self.lookup_import_candidates(ident, ns, is_expected) + .drain(..) + .filter(|ImportSuggestion { did, .. }| { + match (did, def.and_then(|def| def.opt_def_id())) { + (Some(suggestion_did), Some(actual_did)) => *suggestion_did != actual_did, + _ => true, + } + }) + .collect::>(); if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { let enum_candidates = self.lookup_import_candidates(ident, ns, is_enum_variant); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index ecbfcec3c5e..428d0bd4c2b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -25,7 +25,7 @@ use rustc::hir::def::*; use rustc::hir::def::Namespace::*; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId}; use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap}; -use rustc::ty; +use rustc::ty::{self, DefIdTree}; use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap}; use rustc::{bug, span_bug}; @@ -93,6 +93,7 @@ enum ScopeSet { /// A free importable items suggested in case of resolution failure. struct ImportSuggestion { + did: Option, path: Path, } @@ -4392,7 +4393,8 @@ impl<'a> Resolver<'a> { // collect results based on the filter function if ident.name == lookup_ident.name && ns == namespace { - if filter_fn(name_binding.def()) { + let def = name_binding.def(); + if filter_fn(def) { // create the path let mut segms = path_segments.clone(); if lookup_ident.span.rust_2018() { @@ -4416,7 +4418,12 @@ impl<'a> Resolver<'a> { // declared as public (due to pruning, we don't explore // outside crate private modules => no need to check this) if !in_module_is_extern || name_binding.vis == ty::Visibility::Public { - candidates.push(ImportSuggestion { path }); + let did = match def { + Def::StructCtor(did, _) | Def::VariantCtor(did, _) => + self.parent(did), + _ => def.opt_def_id(), + }; + candidates.push(ImportSuggestion { did, path }); } } } @@ -4513,7 +4520,8 @@ impl<'a> Resolver<'a> { span: name_binding.span, segments: path_segments, }; - result = Some((module, ImportSuggestion { path })); + let did = module.def().and_then(|def| def.opt_def_id()); + result = Some((module, ImportSuggestion { did, path })); } else { // add the module to the lookup if seen_modules.insert(module.def_id().unwrap()) { diff --git a/src/test/ui/issue-42944.rs b/src/test/ui/issue-42944.rs new file mode 100644 index 00000000000..9d746673f4d --- /dev/null +++ b/src/test/ui/issue-42944.rs @@ -0,0 +1,19 @@ +mod foo { + pub struct B(()); +} + +mod bar { + use foo::B; + + fn foo() { + B(()); //~ ERROR expected function, found struct `B` [E0423] + } +} + +mod baz { + fn foo() { + B(()); //~ ERROR cannot find function `B` in this scope [E0425] + } +} + +fn main() {} diff --git a/src/test/ui/issue-42944.stderr b/src/test/ui/issue-42944.stderr new file mode 100644 index 00000000000..43fd0ffb724 --- /dev/null +++ b/src/test/ui/issue-42944.stderr @@ -0,0 +1,20 @@ +error[E0423]: expected function, found struct `B` + --> $DIR/issue-42944.rs:9:9 + | +LL | B(()); //~ ERROR expected function, found struct `B` [E0423] + | ^ constructor is not visible here due to private fields + +error[E0425]: cannot find function `B` in this scope + --> $DIR/issue-42944.rs:15:9 + | +LL | B(()); //~ ERROR cannot find function `B` in this scope [E0425] + | ^ not found in this scope +help: possible candidate is found in another module, you can import it into scope + | +LL | use foo::B; + | + +error: aborting due to 2 previous errors + +Some errors occurred: E0423, E0425. +For more information about an error, try `rustc --explain E0423`. diff --git a/src/test/ui/resolve/privacy-struct-ctor.stderr b/src/test/ui/resolve/privacy-struct-ctor.stderr index 44ecf6b97bf..519e74d9f63 100644 --- a/src/test/ui/resolve/privacy-struct-ctor.stderr +++ b/src/test/ui/resolve/privacy-struct-ctor.stderr @@ -2,25 +2,16 @@ error[E0423]: expected value, found struct `Z` --> $DIR/privacy-struct-ctor.rs:20:9 | LL | Z; - | ^ constructor is not visible here due to private fields -help: a tuple struct with a similar name exists - | -LL | S; | ^ -help: possible better candidate is found in another module, you can import it into scope - | -LL | use m::n::Z; - | + | | + | constructor is not visible here due to private fields + | help: a tuple struct with a similar name exists: `S` error[E0423]: expected value, found struct `S` --> $DIR/privacy-struct-ctor.rs:33:5 | LL | S; | ^ constructor is not visible here due to private fields -help: possible better candidate is found in another module, you can import it into scope - | -LL | use m::S; - | error[E0423]: expected value, found struct `S2` --> $DIR/privacy-struct-ctor.rs:38:5