Prefer accessible paths in 'use' suggestions

This fixes an issue with the following sample:

    mod foo {
	mod inaccessible {
	    pub struct X;
	}
	pub mod avail {
	    pub struct X;
	}
    }

    fn main() { X; }

Instead of suggesting both `use crate::foo::inaccessible::X;` and `use
crate::foo::avail::X;`, it should only suggest the latter.

It is done by trimming the list of suggestions from inaccessible paths
if accessible paths are present.

Visibility is checked with `is_accessible_from` now instead of being
hard-coded.

-

Some tests fixes are trivial, and others require a bit more explaining,
here are my comments:

src/test/ui/issues/issue-35675.stderr: Only needs to make the enum
public to have the suggestion make sense.

src/test/ui/issues/issue-42944.stderr: Importing the tuple struct won't
help because its constructor is not visible, so the attempted
constructor does not work. In that case, it's better not to suggest it.
The case where the constructor is public is covered in `issue-26545.rs`.

13 files changed, 99 insertions, 82 deletions

diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs
index bd2ce5a72e8..bb88b8191f1 100644
--- a/src/librustc_resolve/diagnostics.rs
+++ b/src/librustc_resolve/diagnostics.rs
@@ -49,6 +49,7 @@ crate struct ImportSuggestion {
     pub did: Option<DefId>,
     pub descr: &'static str,
     pub path: Path,
+    pub accessible: bool,
 }
 
 /// Adjust the impl span so that just the `impl` keyword is taken by removing
@@ -640,9 +641,11 @@ impl<'a> Resolver<'a> {
         let mut candidates = Vec::new();
         let mut seen_modules = FxHashSet::default();
         let not_local_module = crate_name.name != kw::Crate;
-        let mut worklist = vec![(start_module, Vec::<ast::PathSegment>::new(), not_local_module)];
+        let mut worklist =
+            vec![(start_module, Vec::<ast::PathSegment>::new(), true, not_local_module)];
 
-        while let Some((in_module, path_segments, in_module_is_extern)) = worklist.pop() {
+        while let Some((in_module, path_segments, accessible, in_module_is_extern)) = worklist.pop()
+        {
             // 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| {
@@ -650,11 +653,20 @@ impl<'a> Resolver<'a> {
                 if name_binding.is_import() && !name_binding.is_extern_crate() {
                     return;
                 }
+
                 // avoid non-importable candidates as well
                 if !name_binding.is_importable() {
                     return;
                 }
 
+                let child_accessible =
+                    accessible && this.is_accessible_from(name_binding.vis, parent_scope.module);
+
+                // do not venture inside inaccessible items of other crates
+                if in_module_is_extern && !child_accessible {
+                    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
@@ -673,22 +685,29 @@ impl<'a> Resolver<'a> {
 
                         segms.push(ast::PathSegment::from_ident(ident));
                         let path = Path { span: name_binding.span, segments: segms };
-                        // the entity is accessible in the following cases:
-                        // 1. if it's defined in the same crate, it's always
-                        // accessible (since private entities can be made public)
-                        // 2. if it's defined in another crate, it's accessible
-                        // only if both the module is public and the entity is
-                        // 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 {
-                            let did = match res {
-                                Res::Def(DefKind::Ctor(..), did) => this.parent(did),
-                                _ => res.opt_def_id(),
-                            };
-                            if candidates.iter().all(|v: &ImportSuggestion| v.did != did) {
-                                candidates.push(ImportSuggestion { did, descr: res.descr(), path });
+                        let did = match res {
+                            Res::Def(DefKind::Ctor(..), did) => this.parent(did),
+                            _ => res.opt_def_id(),
+                        };
+
+                        if child_accessible {
+                            // Remove invisible match if exists
+                            if let Some(idx) = candidates
+                                .iter()
+                                .position(|v: &ImportSuggestion| v.did == did && !v.accessible)
+                            {
+                                candidates.remove(idx);
                             }
                         }
+
+                        if candidates.iter().all(|v: &ImportSuggestion| v.did != did) {
+                            candidates.push(ImportSuggestion {
+                                did,
+                                descr: res.descr(),
+                                path,
+                                accessible: child_accessible,
+                            });
+                        }
                     }
                 }
 
@@ -701,20 +720,22 @@ impl<'a> Resolver<'a> {
                     let is_extern_crate_that_also_appears_in_prelude =
                         name_binding.is_extern_crate() && lookup_ident.span.rust_2018();
 
-                    let is_visible_to_user =
-                        !in_module_is_extern || name_binding.vis == ty::Visibility::Public;
-
-                    if !is_extern_crate_that_also_appears_in_prelude && is_visible_to_user {
-                        // add the module to the lookup
+                    if !is_extern_crate_that_also_appears_in_prelude {
                         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, is_extern));
+                            worklist.push((module, path_segments, child_accessible, is_extern));
                         }
                     }
                 }
             })
         }
 
+        // If only some candidates are accessible, take just them
+        if !candidates.iter().all(|v: &ImportSuggestion| !v.accessible) {
+            candidates = candidates.into_iter().filter(|x| x.accessible).collect();
+        }
+
         candidates
     }
 
diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
index 05ef0aa0bb6..478698ba20c 100644
--- a/src/librustc_resolve/late/diagnostics.rs
+++ b/src/librustc_resolve/late/diagnostics.rs
@@ -887,7 +887,12 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                         let path = Path { span: name_binding.span, segments: path_segments };
                         result = Some((
                             module,
-                            ImportSuggestion { did: Some(def_id), descr: "module", path },
+                            ImportSuggestion {
+                                did: Some(def_id),
+                                descr: "module",
+                                path,
+                                accessible: true,
+                            },
                         ));
                     } else {
                         // add the module to the lookup
diff --git a/src/test/ui/hygiene/globs.stderr b/src/test/ui/hygiene/globs.stderr
index 8f6b7aca8fd..6dcbf055a8b 100644
--- a/src/test/ui/hygiene/globs.stderr
+++ b/src/test/ui/hygiene/globs.stderr
@@ -23,14 +23,10 @@ LL | |     }
    | |_____- in this macro invocation
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
-help: consider importing one of these items
+help: consider importing this function
    |
 LL | use bar::g;
    |
-LL | use foo::test2::test::g;
-   |
-LL | use foo::test::g;
-   |
 
 error[E0425]: cannot find function `f` in this scope
   --> $DIR/globs.rs:61:12
diff --git a/src/test/ui/issues/issue-26545.rs b/src/test/ui/issues/issue-26545.rs
new file mode 100644
index 00000000000..5652ee74706
--- /dev/null
+++ b/src/test/ui/issues/issue-26545.rs
@@ -0,0 +1,12 @@
+mod foo {
+    pub struct B(pub ());
+}
+
+mod baz {
+    fn foo() {
+        B(());
+        //~^ ERROR cannot find function, tuple struct or tuple variant `B` in this scope [E0425]
+    }
+}
+
+fn main() {}
diff --git a/src/test/ui/issues/issue-26545.stderr b/src/test/ui/issues/issue-26545.stderr
new file mode 100644
index 00000000000..d3c86692501
--- /dev/null
+++ b/src/test/ui/issues/issue-26545.stderr
@@ -0,0 +1,14 @@
+error[E0425]: cannot find function, tuple struct or tuple variant `B` in this scope
+  --> $DIR/issue-26545.rs:7:9
+   |
+LL |         B(());
+   |         ^ not found in this scope
+   |
+help: consider importing this tuple struct
+   |
+LL |     use foo::B;
+   |
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0425`.
diff --git a/src/test/ui/issues/issue-35675.rs b/src/test/ui/issues/issue-35675.rs
index 7876811a9ac..683761667d4 100644
--- a/src/test/ui/issues/issue-35675.rs
+++ b/src/test/ui/issues/issue-35675.rs
@@ -33,7 +33,7 @@ fn qux() -> Some {
 fn main() {}
 
 mod x {
-    enum Enum {
+    pub enum Enum {
         Variant1,
         Variant2(),
         Variant3(usize),
diff --git a/src/test/ui/issues/issue-42944.rs b/src/test/ui/issues/issue-42944.rs
index cc365dc4c93..a088f91554d 100644
--- a/src/test/ui/issues/issue-42944.rs
+++ b/src/test/ui/issues/issue-42944.rs
@@ -1,20 +1,20 @@
 mod foo {
-    pub struct B(());
+    pub struct Bx(());
 }
 
 mod bar {
-    use foo::B;
+    use foo::Bx;
 
     fn foo() {
-        B(());
-        //~^ ERROR expected function, tuple struct or tuple variant, found struct `B` [E0423]
+        Bx(());
+        //~^ ERROR expected function, tuple struct or tuple variant, found struct `Bx` [E0423]
     }
 }
 
 mod baz {
     fn foo() {
-        B(());
-        //~^ ERROR cannot find function, tuple struct or tuple variant `B` in this scope [E0425]
+        Bx(());
+        //~^ ERROR cannot find function, tuple struct or tuple variant `Bx` in this scope [E0425]
     }
 }
 
diff --git a/src/test/ui/issues/issue-42944.stderr b/src/test/ui/issues/issue-42944.stderr
index e7e251e39c0..9fad43757ba 100644
--- a/src/test/ui/issues/issue-42944.stderr
+++ b/src/test/ui/issues/issue-42944.stderr
@@ -1,18 +1,18 @@
-error[E0423]: expected function, tuple struct or tuple variant, found struct `B`
+error[E0423]: expected function, tuple struct or tuple variant, found struct `Bx`
   --> $DIR/issue-42944.rs:9:9
    |
-LL |         B(());
-   |         ^ constructor is not visible here due to private fields
+LL |         Bx(());
+   |         ^^ constructor is not visible here due to private fields
 
-error[E0425]: cannot find function, tuple struct or tuple variant `B` in this scope
+error[E0425]: cannot find function, tuple struct or tuple variant `Bx` in this scope
   --> $DIR/issue-42944.rs:16:9
    |
-LL |         B(());
-   |         ^ not found in this scope
+LL |         Bx(());
+   |         ^^ not found in this scope
    |
 help: consider importing this tuple struct
    |
-LL |     use foo::B;
+LL |     use foo::Bx;
    |
 
 error: aborting due to 2 previous errors
diff --git a/src/test/ui/issues/issue-4366-2.stderr b/src/test/ui/issues/issue-4366-2.stderr
index ecee595d4ab..a86ec7fabea 100644
--- a/src/test/ui/issues/issue-4366-2.stderr
+++ b/src/test/ui/issues/issue-4366-2.stderr
@@ -15,12 +15,10 @@ error[E0423]: expected function, found module `foo`
 LL |     foo();
    |     ^^^ not a function
    |
-help: consider importing one of these items instead
+help: consider importing this function instead
    |
 LL | use foo::foo;
    |
-LL | use m1::foo;
-   |
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/issues/issue-4366.stderr b/src/test/ui/issues/issue-4366.stderr
index a094180572d..469ea93e904 100644
--- a/src/test/ui/issues/issue-4366.stderr
+++ b/src/test/ui/issues/issue-4366.stderr
@@ -4,12 +4,10 @@ error[E0425]: cannot find function `foo` in this scope
 LL |         fn sub() -> isize { foo(); 1 }
    |                             ^^^ not found in this scope
    |
-help: consider importing one of these items
+help: consider importing this function
    |
 LL |         use foo::foo;
    |
-LL |         use m1::foo;
-   |
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/privacy/privacy-ns1.stderr b/src/test/ui/privacy/privacy-ns1.stderr
index 4d2af735fa6..eda9d4c128d 100644
--- a/src/test/ui/privacy/privacy-ns1.stderr
+++ b/src/test/ui/privacy/privacy-ns1.stderr
@@ -11,14 +11,10 @@ help: a unit struct with a similar name exists
    |
 LL |     Baz();
    |     ^^^
-help: consider importing one of these items instead
-   |
-LL | use foo1::Bar;
+help: consider importing this function instead
    |
 LL | use foo2::Bar;
    |
-LL | use foo3::Bar;
-   |
 
 error[E0425]: cannot find function, tuple struct or tuple variant `Bar` in this scope
   --> $DIR/privacy-ns1.rs:51:5
@@ -33,14 +29,10 @@ help: a unit struct with a similar name exists
    |
 LL |     Baz();
    |     ^^^
-help: consider importing one of these items
-   |
-LL | use foo1::Bar;
+help: consider importing this function
    |
 LL | use foo2::Bar;
    |
-LL | use foo3::Bar;
-   |
 
 error[E0412]: cannot find type `Bar` in this scope
   --> $DIR/privacy-ns1.rs:52:17
@@ -55,14 +47,10 @@ help: a struct with a similar name exists
    |
 LL |     let _x: Box<Baz>;
    |                 ^^^
-help: consider importing one of these items
+help: consider importing this trait
    |
 LL | use foo1::Bar;
    |
-LL | use foo2::Bar;
-   |
-LL | use foo3::Bar;
-   |
 
 error[E0107]: wrong number of const arguments: expected 0, found 1
   --> $DIR/privacy-ns1.rs:35:17
diff --git a/src/test/ui/privacy/privacy-ns2.stderr b/src/test/ui/privacy/privacy-ns2.stderr
index f1aa523742a..d7d9b835275 100644
--- a/src/test/ui/privacy/privacy-ns2.stderr
+++ b/src/test/ui/privacy/privacy-ns2.stderr
@@ -4,14 +4,10 @@ error[E0423]: expected function, tuple struct or tuple variant, found trait `Bar
 LL |     Bar();
    |     ^^^ not a function, tuple struct or tuple variant
    |
-help: consider importing one of these items instead
-   |
-LL | use foo1::Bar;
+help: consider importing this function instead
    |
 LL | use foo2::Bar;
    |
-LL | use foo3::Bar;
-   |
 
 error[E0423]: expected function, tuple struct or tuple variant, found trait `Bar`
   --> $DIR/privacy-ns2.rs:26:5
@@ -26,14 +22,10 @@ help: a unit struct with a similar name exists
    |
 LL |     Baz();
    |     ^^^
-help: consider importing one of these items instead
-   |
-LL | use foo1::Bar;
+help: consider importing this function instead
    |
 LL | use foo2::Bar;
    |
-LL | use foo3::Bar;
-   |
 
 error[E0573]: expected type, found function `Bar`
   --> $DIR/privacy-ns2.rs:43:14
@@ -45,14 +37,10 @@ help: use `=` if you meant to assign
    |
 LL |     let _x = Bar();
    |            ^
-help: consider importing one of these items instead
+help: consider importing this trait instead
    |
 LL | use foo1::Bar;
    |
-LL | use foo2::Bar;
-   |
-LL | use foo3::Bar;
-   |
 
 error[E0603]: trait `Bar` is private
   --> $DIR/privacy-ns2.rs:63:15
diff --git a/src/test/ui/resolve/issue-21221-1.stderr b/src/test/ui/resolve/issue-21221-1.stderr
index d3e19534353..538eeead9fc 100644
--- a/src/test/ui/resolve/issue-21221-1.stderr
+++ b/src/test/ui/resolve/issue-21221-1.stderr
@@ -25,11 +25,8 @@ LL | use mul1::Mul;
    |
 LL | use mul2::Mul;
    |
-LL | use mul3::Mul;
-   |
-LL | use mul4::Mul;
+LL | use std::ops::Mul;
    |
-     and 2 other candidates
 
 error[E0405]: cannot find trait `ThisTraitReallyDoesntExistInAnyModuleReally` in this scope
   --> $DIR/issue-21221-1.rs:63:6