Skip to content

Commit

Permalink
Auto merge of #15473 - Veykril:doc-hidden-imports, r=Veykril
Browse files Browse the repository at this point in the history
fix: Fix auto-import (and completions) importing `#[doc(hidden)]` items

Fixes #9197 and #9911

Turns out while #15472 didn't give access to the import stuff to the IDE layer, these can be fixed already since they use the import map which works in the hir-def crate 🎉
  • Loading branch information
bors committed Aug 17, 2023
2 parents 49716e6 + 637f496 commit 44cf174
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 16 deletions.
49 changes: 49 additions & 0 deletions crates/hir-def/src/find_path.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
);
}
}
27 changes: 22 additions & 5 deletions crates/hir-def/src/import_map.rs
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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(_) => {
Expand All @@ -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);
}
Expand Down
11 changes: 6 additions & 5 deletions crates/hir-def/src/item_scope.rs
Expand Up @@ -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>)>,
Expand Down Expand Up @@ -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)) => {
Expand Down Expand Up @@ -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)) => {
Expand Down
12 changes: 11 additions & 1 deletion crates/hir-def/src/lib.rs
Expand Up @@ -870,7 +870,8 @@ impl_from!(
MacroId(Macro2Id, MacroRulesId, ProcMacroId),
ImplId,
GenericParamId,
ExternCrateId
ExternCrateId,
UseId
for AttrDefId
);

Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 12 additions & 4 deletions crates/hir-def/src/per_ns.rs
Expand Up @@ -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(),
)
}
}
2 changes: 1 addition & 1 deletion crates/hir/src/attrs.rs
Expand Up @@ -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),
Expand Down
54 changes: 54 additions & 0 deletions crates/ide-completion/src/tests/flyimport.rs
Expand Up @@ -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)
"#]],
);
}

0 comments on commit 44cf174

Please sign in to comment.