Rollup merge of rust-lang#58381 - davidtwco:issue-42944, r=estebank

Only suggest imports if not imported.

Fixes rust-lang#42944 and fixes rust-lang#53430.

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.

r? @estebank

Author: Centril

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::<Vec<_>>();
         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);

src/librustc_resolve/lib.rs

@@ -24,7 +24,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};
 
@@ -92,6 +92,7 @@ enum ScopeSet {
 
 /// A free importable items suggested in case of resolution failure.
 struct ImportSuggestion {
+    did: Option<DefId>,
     path: Path,
 }
 
@@ -4391,7 +4392,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() {
@@ -4415,7 +4417,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 });
                         }
                     }
                 }
@@ -4512,7 +4519,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()) {

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() {}

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`.

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