Rollup merge of #119151 - Jules-Bertholet:no-foreign-doc-hidden-suggest, r=davidtwco

Hide foreign `#[doc(hidden)]` paths in import suggestions

Stops the compiler from suggesting to import foreign `#[doc(hidden)]` paths.

```@rustbot``` label A-suggestion-diagnostics

compiler/rustc_middle/src/ty/util.rs

@@ -1483,7 +1483,7 @@ pub fn reveal_opaque_types_in_bounds<'tcx>(
     val.fold_with(&mut visitor)
 }
 
-/// Determines whether an item is annotated with `doc(hidden)`.
+/// Determines whether an item is directly annotated with `doc(hidden)`.
 fn is_doc_hidden(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
     tcx.get_attrs(def_id, sym::doc)
         .filter_map(|attr| attr.meta_item_list())

compiler/rustc_resolve/src/diagnostics.rs

@@ -100,6 +100,8 @@ pub(crate) struct ImportSuggestion {
     pub descr: &'static str,
     pub path: Path,
     pub accessible: bool,
+    // false if the path traverses a foreign `#[doc(hidden)]` item.
+    pub doc_visible: bool,
     pub via_import: bool,
     /// An extra note that should be issued if this item is suggested
     pub note: Option<String>,
@@ -1146,10 +1148,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     {
         let mut candidates = Vec::new();
         let mut seen_modules = FxHashSet::default();
-        let mut worklist = vec![(start_module, ThinVec::<ast::PathSegment>::new(), true)];
+        let start_did = start_module.def_id();
+        let mut worklist = vec![(
+            start_module,
+            ThinVec::<ast::PathSegment>::new(),
+            true,
+            start_did.is_local() || !self.tcx.is_doc_hidden(start_did),
+        )];
         let mut worklist_via_import = vec![];
 
-        while let Some((in_module, path_segments, accessible)) = match worklist.pop() {
+        while let Some((in_module, path_segments, accessible, doc_visible)) = match worklist.pop() {
             None => worklist_via_import.pop(),
             Some(x) => Some(x),
         } {
@@ -1192,6 +1200,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     }
                 }
 
+                let res = name_binding.res();
+                let did = match res {
+                    Res::Def(DefKind::Ctor(..), did) => this.tcx.opt_parent(did),
+                    _ => res.opt_def_id(),
+                };
+                let child_doc_visible = doc_visible
+                    && (did.map_or(true, |did| did.is_local() || !this.tcx.is_doc_hidden(did)));
+
                 // collect results based on the filter function
                 // avoid suggesting anything from the same module in which we are resolving
                 // avoid suggesting anything with a hygienic name
@@ -1200,7 +1216,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     && in_module != parent_scope.module
                     && !ident.span.normalize_to_macros_2_0().from_expansion()
                 {
-                    let res = name_binding.res();
                     if filter_fn(res) {
                         // create the path
                         let mut segms = if lookup_ident.span.at_least_rust_2018() {
@@ -1214,10 +1229,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
                         segms.push(ast::PathSegment::from_ident(ident));
                         let path = Path { span: name_binding.span, segments: segms, tokens: None };
-                        let did = match res {
-                            Res::Def(DefKind::Ctor(..), did) => this.tcx.opt_parent(did),
-                            _ => res.opt_def_id(),
-                        };
 
                         if child_accessible {
                             // Remove invisible match if exists
@@ -1257,6 +1268,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                                 descr: res.descr(),
                                 path,
                                 accessible: child_accessible,
+                                doc_visible: child_doc_visible,
                                 note,
                                 via_import,
                             });
@@ -1277,7 +1289,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                         // add the module to the lookup
                         if seen_modules.insert(module.def_id()) {
                             if via_import { &mut worklist_via_import } else { &mut worklist }
-                                .push((module, path_segments, child_accessible));
+                                .push((module, path_segments, child_accessible, child_doc_visible));
                         }
                     }
                 }
@@ -2687,8 +2699,26 @@ fn show_candidates(
         Vec::new();
 
     candidates.iter().for_each(|c| {
-        (if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
+        if c.accessible {
-            .push((pprust::path_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
+            // Don't suggest `#[doc(hidden)]` items from other crates
+            if c.doc_visible {
+                accessible_path_strings.push((
+                    pprust::path_to_string(&c.path),
+                    c.descr,
+                    c.did,
+                    &c.note,
+                    c.via_import,
+                ))
+            }
+        } else {
+            inaccessible_path_strings.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
@@ -2787,9 +2817,7 @@ fn show_candidates(
             err.help(msg);
         }
         true
-    } else if !matches!(mode, DiagnosticMode::Import) {
+    } else if !(inaccessible_path_strings.is_empty() || matches!(mode, DiagnosticMode::Import)) {
-        assert!(!inaccessible_path_strings.is_empty());
-
         let prefix = if let DiagnosticMode::Pattern = mode {
             "you might have meant to match on "
         } else {

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -2191,15 +2191,20 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
     fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> {
         let mut result = None;
         let mut seen_modules = FxHashSet::default();
-        let mut worklist = vec![(self.r.graph_root, ThinVec::new())];
+        let root_did = self.r.graph_root.def_id();
+        let mut worklist = vec![(
+            self.r.graph_root,
+            ThinVec::new(),
+            root_did.is_local() || !self.r.tcx.is_doc_hidden(root_did),
+        )];
 
-        while let Some((in_module, path_segments)) = worklist.pop() {
+        while let Some((in_module, path_segments, doc_visible)) = worklist.pop() {
             // abort if the module is already found
             if result.is_some() {
                 break;
             }
 
-            in_module.for_each_child(self.r, |_, ident, _, name_binding| {
+            in_module.for_each_child(self.r, |r, ident, _, name_binding| {
                 // abort if the module is already found or if name_binding is private external
                 if result.is_some() || !name_binding.vis.is_visible_locally() {
                     return;
@@ -2209,6 +2214,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                     let mut path_segments = path_segments.clone();
                     path_segments.push(ast::PathSegment::from_ident(ident));
                     let module_def_id = module.def_id();
+                    let doc_visible = doc_visible
+                        && (module_def_id.is_local() || !r.tcx.is_doc_hidden(module_def_id));
                     if module_def_id == def_id {
                         let path =
                             Path { span: name_binding.span, segments: path_segments, tokens: None };
@@ -2219,6 +2226,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                                 descr: "module",
                                 path,
                                 accessible: true,
+                                doc_visible,
                                 note: None,
                                 via_import: false,
                             },
@@ -2226,7 +2234,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
                     } else {
                         // add the module to the lookup
                         if seen_modules.insert(module_def_id) {
-                            worklist.push((module, path_segments));
+                            worklist.push((module, path_segments, doc_visible));
                         }
                     }
                 }

src/tools/clippy/tests/ui/crashes/ice-6252.stderr

@@ -8,8 +8,6 @@ help: consider importing one of these items
    |
 LL + use core::marker::PhantomData;
    |
-LL + use serde::__private::PhantomData;
-   |
 LL + use std::marker::PhantomData;
    |
 

tests/ui/suggestions/auxiliary/hidden-struct.rs

@@ -0,0 +1,17 @@
+#[doc(hidden)]
+pub mod hidden {
+    pub struct Foo;
+}
+
+pub mod hidden1 {
+    #[doc(hidden)]
+    pub struct Foo;
+}
+
+
+#[doc(hidden)]
+pub(crate) mod hidden2 {
+    pub struct Bar;
+}
+
+pub use hidden2::Bar;

tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs

@@ -0,0 +1,15 @@
+// aux-build:hidden-struct.rs
+// compile-flags: --crate-type lib
+
+extern crate hidden_struct;
+
+#[doc(hidden)]
+mod local {
+    pub struct Foo;
+}
+
+pub fn test(_: Foo) {}
+//~^ ERROR cannot find type `Foo` in this scope
+
+pub fn test2(_: Bar) {}
+//~^ ERROR cannot find type `Bar` in this scope

tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr

@@ -0,0 +1,25 @@
+error[E0412]: cannot find type `Foo` in this scope
+  --> $DIR/dont-suggest-foreign-doc-hidden.rs:11:16
+   |
+LL | pub fn test(_: Foo) {}
+   |                ^^^ not found in this scope
+   |
+help: consider importing this struct
+   |
+LL + use local::Foo;
+   |
+
+error[E0412]: cannot find type `Bar` in this scope
+  --> $DIR/dont-suggest-foreign-doc-hidden.rs:14:17
+   |
+LL | pub fn test2(_: Bar) {}
+   |                 ^^^ not found in this scope
+   |
+help: consider importing this struct
+   |
+LL + use hidden_struct::Bar;
+   |
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0412`.

tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs

@@ -1,8 +1,8 @@
 #![feature(type_alias_impl_trait)]
 
-pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
+pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
 //~^ ERROR use of undeclared lifetime name `'db`
-//~| ERROR cannot find type `Key` in this scope
+//~| ERROR cannot find type `LocalKey` in this scope
 //~| ERROR unconstrained opaque type
 //~| ERROR unconstrained opaque type
 

tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr

@@ -1,43 +1,43 @@
 error[E0261]: use of undeclared lifetime name `'db`
   --> $DIR/nested-impl-trait-in-tait.rs:3:40
    |
-LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
+LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
    |                                        ^^^ undeclared lifetime
    |
    = note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
 help: consider making the bound lifetime-generic with a new `'db` lifetime
    |
-LL | pub type Tait = impl for<'db> Iterator<Item = (&'db Key, impl Iterator)>;
+LL | pub type Tait = impl for<'db> Iterator<Item = (&'db LocalKey, impl Iterator)>;
    |                      ++++++++
 help: consider introducing lifetime `'db` here
    |
-LL | pub type Tait<'db> = impl Iterator<Item = (&'db Key, impl Iterator)>;
+LL | pub type Tait<'db> = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
    |              +++++
 
-error[E0412]: cannot find type `Key` in this scope
+error[E0412]: cannot find type `LocalKey` in this scope
   --> $DIR/nested-impl-trait-in-tait.rs:3:44
    |
-LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
+LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
-   |                                            ^^^ not found in this scope
+   |                                            ^^^^^^^^ not found in this scope
    |
 help: consider importing this struct
    |
-LL + use std::thread::local_impl::Key;
+LL + use std::thread::LocalKey;
    |
 
 error: unconstrained opaque type
   --> $DIR/nested-impl-trait-in-tait.rs:3:17
    |
-LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
+LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `Tait` must be used in combination with a concrete type within the same module
 
 error: unconstrained opaque type
-  --> $DIR/nested-impl-trait-in-tait.rs:3:49
+  --> $DIR/nested-impl-trait-in-tait.rs:3:54
    |
-LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
+LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
-   |                                                 ^^^^^^^^^^^^^
+   |                                                      ^^^^^^^^^^^^^
    |
    = note: `Tait` must be used in combination with a concrete type within the same module