From 637f496a814ec850e3a18064c648e0835f56c90c Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 17 Aug 2023 12:17:41 +0200 Subject: [PATCH] fix: Fix auto-import (and completions) importing `#[doc(hidden)]` items --- crates/hir-def/src/find_path.rs | 49 ++++++++++++++++++ crates/hir-def/src/import_map.rs | 27 ++++++++-- crates/hir-def/src/item_scope.rs | 11 ++-- crates/hir-def/src/lib.rs | 12 ++++- crates/hir-def/src/per_ns.rs | 16 ++++-- crates/hir/src/attrs.rs | 2 +- crates/ide-completion/src/tests/flyimport.rs | 54 ++++++++++++++++++++ 7 files changed, 155 insertions(+), 16 deletions(-) diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index 234d3eaed56..b60e7909105 100644 --- a/crates/hir-def/src/find_path.rs +++ b/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 @@ fn different_crate_renamed_through_dep() { "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", + ); + } } diff --git a/crates/hir-def/src/import_map.rs b/crates/hir-def/src/import_map.rs index 5d7bb0b49d7..c961d7d86d8 100644 --- a/crates/hir-def/src/import_map.rs +++ b/crates/hir-def/src/import_map.rs @@ -11,6 +11,7 @@ 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 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 `::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); } diff --git a/crates/hir-def/src/item_scope.rs b/crates/hir-def/src/item_scope.rs index 0baf3fa7f44..699231fd378 100644 --- a/crates/hir-def/src/item_scope.rs +++ b/crates/hir-def/src/item_scope.rs @@ -66,7 +66,8 @@ pub struct ItemScope { _c: Count, /// 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)>, values: FxHashMap)>, macros: FxHashMap)>, @@ -375,8 +376,8 @@ pub(crate) fn push_res_with_import( 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 @@ pub(crate) fn push_res_with_import( 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)) => { diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 3c0ed8c2e51..3f87fe62b83 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -870,7 +870,8 @@ pub enum AttrDefId { MacroId(Macro2Id, MacroRulesId, ProcMacroId), ImplId, GenericParamId, - ExternCrateId + ExternCrateId, + UseId for AttrDefId ); @@ -904,6 +905,15 @@ fn from(acid: ItemContainerId) -> Self { } } } +impl From 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 { diff --git a/crates/hir-def/src/per_ns.rs b/crates/hir-def/src/per_ns.rs index 3f3f9f42491..da9c13740b3 100644 --- a/crates/hir-def/src/per_ns.rs +++ b/crates/hir-def/src/per_ns.rs @@ -117,12 +117,20 @@ pub fn or_else(self, f: impl FnOnce() -> PerNs) -> PerNs { } } - pub fn iter_items(self) -> impl Iterator { + pub fn iter_items(self) -> impl Iterator)> { 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(), + ) } } diff --git a/crates/hir/src/attrs.rs b/crates/hir/src/attrs.rs index cd0410168c2..364405f56a8 100644 --- a/crates/hir/src/attrs.rs +++ b/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), diff --git a/crates/ide-completion/src/tests/flyimport.rs b/crates/ide-completion/src/tests/flyimport.rs index 8c038c0fbaa..4cdfd546f6a 100644 --- a/crates/ide-completion/src/tests/flyimport.rs +++ b/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) + "#]], + ); +}