fix: Fix auto-import (and completions) importing #[doc(hidden)] items

crates/hir-def/src/find_path.rs

@@ -224,6 +224,7 @@ fn find_path_for_module(
     )
 }
 
+// FIXME: Do we still need this now that we record import origins, and hence aliases?
 fn find_in_scope(
     db: &dyn DefDatabase,
     def_map: &DefMap,
@@ -346,6 +347,11 @@ fn calculate_best_path(
         let extern_paths = crate_graph[from.krate].dependencies.iter().filter_map(|dep| {
             let import_map = db.import_map(dep.crate_id);
             import_map.import_info_for(item).and_then(|info| {
+                if info.is_doc_hidden {
+                    // the item or import is `#[doc(hidden)]`, so skip it as it is in an external crate
+                    return None;
+                }
+
                 // Determine best path for containing module and append last segment from `info`.
                 // FIXME: we should guide this to look up the path locally, or from the same crate again?
                 let mut path = find_path_for_module(
@@ -1311,4 +1317,47 @@ pub struct S;
             "intermediate::std_renamed::S",
         );
     }
+
+    #[test]
+    fn different_crate_doc_hidden() {
+        check_found_path(
+            r#"
+//- /main.rs crate:main deps:intermediate
+$0
+//- /intermediate.rs crate:intermediate deps:std
+#[doc(hidden)]
+pub extern crate std;
+pub extern crate std as longer;
+//- /std.rs crate:std
+pub struct S;
+    "#,
+            "intermediate::longer::S",
+            "intermediate::longer::S",
+            "intermediate::longer::S",
+            "intermediate::longer::S",
+        );
+    }
+
+    #[test]
+    fn respect_doc_hidden() {
+        check_found_path(
+            r#"
+//- /main.rs crate:main deps:std,lazy_static
+$0
+//- /lazy_static.rs crate:lazy_static deps:core
+#[doc(hidden)]
+pub use core::ops::Deref as __Deref;
+//- /std.rs crate:std deps:core
+pub use core::ops;
+//- /core.rs crate:core
+pub mod ops {
+    pub trait Deref {}
+}
+    "#,
+            "std::ops::Deref",
+            "std::ops::Deref",
+            "std::ops::Deref",
+            "std::ops::Deref",
+        );
+    }
 }

crates/hir-def/src/import_map.rs

@@ -11,6 +11,7 @@ use itertools::Itertools;
 use rustc_hash::{FxHashMap, FxHashSet, FxHasher};
 use triomphe::Arc;
 
+use crate::item_scope::ImportOrExternCrate;
 use crate::{
     db::DefDatabase, item_scope::ItemInNs, nameres::DefMap, visibility::Visibility, AssocItemId,
     ModuleDefId, ModuleId, TraitId,
@@ -29,6 +30,8 @@ pub struct ImportInfo {
     pub container: ModuleId,
     /// Whether the import is a trait associated item or not.
     pub is_trait_assoc_item: bool,
+    /// Whether this item is annotated with `#[doc(hidden)]`.
+    pub is_doc_hidden: bool,
 }
 
 /// A map from publicly exported items to its name.
@@ -113,14 +116,27 @@ fn collect_import_map(db: &dyn DefDatabase, krate: CrateId) -> FxIndexMap<ItemIn
         });
 
         for (name, per_ns) in visible_items {
-            for item in per_ns.iter_items() {
+            for (item, import) in per_ns.iter_items() {
                 // FIXME: Not yet used, but will be once we handle doc(hidden) import sources
-                let is_doc_hidden = false;
+                let attr_id = if let Some(import) = import {
+                    match import {
+                        ImportOrExternCrate::ExternCrate(id) => Some(id.into()),
+                        ImportOrExternCrate::Import(id) => Some(id.import.into()),
+                    }
+                } else {
+                    match item {
+                        ItemInNs::Types(id) | ItemInNs::Values(id) => id.try_into().ok(),
+                        ItemInNs::Macros(id) => Some(id.into()),
+                    }
+                };
+                let is_doc_hidden =
+                    attr_id.map_or(false, |attr_id| db.attrs(attr_id).has_doc_hidden());
 
                 let import_info = ImportInfo {
                     name: name.clone(),
                     container: module,
                     is_trait_assoc_item: false,
+                    is_doc_hidden,
                 };
 
                 match depth_map.entry(item) {
@@ -171,10 +187,10 @@ fn collect_trait_assoc_items(
     trait_import_info: &ImportInfo,
 ) {
     let _p = profile::span("collect_trait_assoc_items");
-    for (assoc_item_name, item) in &db.trait_data(tr).items {
+    for &(ref assoc_item_name, item) in &db.trait_data(tr).items {
         let module_def_id = match item {
-            AssocItemId::FunctionId(f) => ModuleDefId::from(*f),
-            AssocItemId::ConstId(c) => ModuleDefId::from(*c),
+            AssocItemId::FunctionId(f) => ModuleDefId::from(f),
+            AssocItemId::ConstId(c) => ModuleDefId::from(c),
             // cannot use associated type aliases directly: need a `<Struct as Trait>::TypeAlias`
             // qualifier, ergo no need to store it for imports in import_map
             AssocItemId::TypeAliasId(_) => {
@@ -192,6 +208,7 @@ fn collect_trait_assoc_items(
             container: trait_import_info.container,
             name: assoc_item_name.clone(),
             is_trait_assoc_item: true,
+            is_doc_hidden: db.attrs(item.into()).has_doc_hidden(),
         };
         map.insert(assoc_item, assoc_item_info);
     }

crates/hir-def/src/item_scope.rs

@@ -66,7 +66,8 @@ pub struct ItemScope {
     _c: Count<Self>,
 
     /// Defs visible in this scope. This includes `declarations`, but also
-    /// imports.
+    /// imports. The imports belong to this module and can be resolved by using them on
+    /// the `use_imports_*` fields.
     types: FxHashMap<Name, (ModuleDefId, Visibility, Option<ImportOrExternCrate>)>,
     values: FxHashMap<Name, (ModuleDefId, Visibility, Option<ImportId>)>,
     macros: FxHashMap<Name, (MacroId, Visibility, Option<ImportId>)>,
@@ -375,8 +376,8 @@ impl ItemScope {
                         None | Some(ImportType::Glob(_)) => None,
                     };
                     let prev = std::mem::replace(&mut fld.2, import);
-                    if let Some(ImportOrExternCrate::Import(import)) = import {
-                        self.use_imports_values.insert(
+                    if let Some(import) = import {
+                        self.use_imports_types.insert(
                             import,
                             match prev {
                                 Some(ImportOrExternCrate::Import(import)) => {
@@ -404,8 +405,8 @@ impl ItemScope {
                             None | Some(ImportType::Glob(_)) => None,
                         };
                         let prev = std::mem::replace(&mut fld.2, import);
-                        if let Some(ImportOrExternCrate::Import(import)) = import {
-                            self.use_imports_values.insert(
+                        if let Some(import) = import {
+                            self.use_imports_types.insert(
                                 import,
                                 match prev {
                                     Some(ImportOrExternCrate::Import(import)) => {

crates/hir-def/src/lib.rs

@@ -870,7 +870,8 @@ impl_from!(
     MacroId(Macro2Id, MacroRulesId, ProcMacroId),
     ImplId,
     GenericParamId,
-    ExternCrateId
+    ExternCrateId,
+    UseId
     for AttrDefId
 );
 
@@ -904,6 +905,15 @@ impl From<ItemContainerId> for AttrDefId {
         }
     }
 }
+impl From<AssocItemId> for AttrDefId {
+    fn from(assoc: AssocItemId) -> Self {
+        match assoc {
+            AssocItemId::FunctionId(it) => AttrDefId::FunctionId(it),
+            AssocItemId::ConstId(it) => AttrDefId::ConstId(it),
+            AssocItemId::TypeAliasId(it) => AttrDefId::TypeAliasId(it),
+        }
+    }
+}
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum VariantId {

crates/hir-def/src/per_ns.rs

@@ -117,12 +117,20 @@ impl PerNs {
         }
     }
 
-    pub fn iter_items(self) -> impl Iterator<Item = ItemInNs> {
+    pub fn iter_items(self) -> impl Iterator<Item = (ItemInNs, Option<ImportOrExternCrate>)> {
         let _p = profile::span("PerNs::iter_items");
         self.types
-            .map(|it| ItemInNs::Types(it.0))
+            .map(|it| (ItemInNs::Types(it.0), it.2))
             .into_iter()
-            .chain(self.values.map(|it| ItemInNs::Values(it.0)).into_iter())
-            .chain(self.macros.map(|it| ItemInNs::Macros(it.0)).into_iter())
+            .chain(
+                self.values
+                    .map(|it| (ItemInNs::Values(it.0), it.2.map(ImportOrExternCrate::Import)))
+                    .into_iter(),
+            )
+            .chain(
+                self.macros
+                    .map(|it| (ItemInNs::Macros(it.0), it.2.map(ImportOrExternCrate::Import)))
+                    .into_iter(),
+            )
     }
 }

crates/hir/src/attrs.rs

@@ -212,7 +212,7 @@ fn resolve_doc_path(
             Some(Namespace::Types) => resolved.take_types(),
             Some(Namespace::Values) => resolved.take_values(),
             Some(Namespace::Macros) => resolved.take_macros().map(ModuleDefId::MacroId),
-            None => resolved.iter_items().next().map(|it| match it {
+            None => resolved.iter_items().next().map(|(it, _)| match it {
                 ItemInNs::Types(it) => it,
                 ItemInNs::Values(it) => it,
                 ItemInNs::Macros(it) => ModuleDefId::MacroId(it),

crates/ide-completion/src/tests/flyimport.rs

@@ -1286,3 +1286,57 @@ macro_rules! println {
         expect![""],
     )
 }
+
+#[test]
+fn no_completions_for_external_doc_hidden_in_path() {
+    check(
+        r#"
+//- /main.rs crate:main deps:dep
+fn main() {
+    Span$0
+}
+//- /lib.rs crate:dep
+#[doc(hidden)]
+pub mod bridge {
+    pub mod server {
+        pub trait Span
+    }
+}
+pub mod bridge2 {
+    #[doc(hidden)]
+    pub mod server2 {
+        pub trait Span
+    }
+}
+"#,
+        expect![""],
+    );
+    // unless re-exported
+    check(
+        r#"
+//- /main.rs crate:main deps:dep
+fn main() {
+    Span$0
+}
+//- /lib.rs crate:dep
+#[doc(hidden)]
+pub mod bridge {
+    pub mod server {
+        pub trait Span
+    }
+}
+pub use bridge::server::Span;
+pub mod bridge2 {
+    #[doc(hidden)]
+    pub mod server2 {
+        pub trait Span2
+    }
+}
+pub use bridge2::server2::Span2;
+"#,
+        expect![[r#"
+            tt Span (use dep::Span)
+            tt Span2 (use dep::Span2)
+        "#]],
+    );
+}