From 7ff6c367169127a8914f8610cbbd9e217d8e7158 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 14 Jul 2022 23:22:54 +0200 Subject: [PATCH] fix: Don't show qualified path completions for private items --- crates/hir-def/src/nameres.rs | 34 +++++++------- crates/hir-def/src/nameres/collector.rs | 3 +- crates/hir/src/display.rs | 2 +- crates/hir/src/lib.rs | 24 ++++++++++ crates/ide-completion/src/completions.rs | 36 ++++++++++----- crates/ide-completion/src/context.rs | 44 +++++++++++++++---- crates/ide-completion/src/tests/special.rs | 19 ++++---- .../ide/src/syntax_highlighting/highlight.rs | 4 +- .../test_data/highlight_crate_root.html | 10 ++--- .../test_data/highlight_general.html | 2 +- .../test_data/highlight_keywords.html | 8 ++-- 11 files changed, 122 insertions(+), 64 deletions(-) diff --git a/crates/hir-def/src/nameres.rs b/crates/hir-def/src/nameres.rs index 536eecf020..a32513cf65 100644 --- a/crates/hir-def/src/nameres.rs +++ b/crates/hir-def/src/nameres.rs @@ -219,7 +219,7 @@ impl DefMap { let edition = crate_graph[krate].edition; let origin = ModuleOrigin::CrateRoot { definition: crate_graph[krate].root_file_id }; - let def_map = DefMap::empty(krate, edition, origin); + let def_map = DefMap::empty(krate, edition, ModuleData::new(origin, Visibility::Public)); let def_map = collector::collect_defs( db, def_map, @@ -241,30 +241,26 @@ impl DefMap { return None; } - let block_info = BlockInfo { block: block_id, parent: block.module }; - let parent_map = block.module.def_map(db); - let mut def_map = DefMap::empty( - block.module.krate, - parent_map.edition, - ModuleOrigin::BlockExpr { block: block.ast_id }, - ); - def_map.block = Some(block_info); - - let def_map = collector::collect_defs(db, def_map, tree_id); - Some(Arc::new(def_map)) - } - - fn empty(krate: CrateId, edition: Edition, root_module_origin: ModuleOrigin) -> DefMap { - let mut modules: Arena = Arena::default(); - + let krate = block.module.krate; let local_id = LocalModuleId::from_raw(la_arena::RawIdx::from(0)); // NB: we use `None` as block here, which would be wrong for implicit // modules declared by blocks with items. At the moment, we don't use // this visibility for anything outside IDE, so that's probably OK. let visibility = Visibility::Module(ModuleId { krate, local_id, block: None }); - let root = modules.alloc(ModuleData::new(root_module_origin, visibility)); - assert_eq!(local_id, root); + let module_data = + ModuleData::new(ModuleOrigin::BlockExpr { block: block.ast_id }, visibility); + + let mut def_map = DefMap::empty(krate, parent_map.edition, module_data); + def_map.block = Some(BlockInfo { block: block_id, parent: block.module }); + + let def_map = collector::collect_defs(db, def_map, tree_id); + Some(Arc::new(def_map)) + } + + fn empty(krate: CrateId, edition: Edition, module_data: ModuleData) -> DefMap { + let mut modules: Arena = Arena::default(); + let root = modules.alloc(module_data); DefMap { _c: Count::new(), diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index ca0cb99b37..973a0b8720 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -2141,7 +2141,8 @@ mod tests { let edition = db.crate_graph()[krate].edition; let module_origin = ModuleOrigin::CrateRoot { definition: file_id }; - let def_map = DefMap::empty(krate, edition, module_origin); + let def_map = + DefMap::empty(krate, edition, ModuleData::new(module_origin, Visibility::Public)); do_collect_defs(&db, def_map) } diff --git a/crates/hir/src/display.rs b/crates/hir/src/display.rs index e26c8440d9..b4fd70d082 100644 --- a/crates/hir/src/display.rs +++ b/crates/hir/src/display.rs @@ -505,7 +505,7 @@ impl HirDisplay for Module { // FIXME: Module doesn't have visibility saved in data. match self.name(f.db) { Some(name) => write!(f, "mod {}", name), - None if self.is_crate_root(f.db) => match self.krate().display_name(f.db) { + None if self.is_crate_root(f.db) => match self.krate(f.db).display_name(f.db) { Some(name) => write!(f, "extern crate {}", name), None => f.write_str("extern crate {unknown}"), }, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 96424d087e..4696f48f2b 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -3573,3 +3573,27 @@ impl HasCrate for Macro { self.module(db).krate() } } + +impl HasCrate for Trait { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db).krate() + } +} + +impl HasCrate for Static { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db).krate() + } +} + +impl HasCrate for Adt { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db).krate() + } +} + +impl HasCrate for Module { + fn krate(&self, _: &dyn HirDatabase) -> Crate { + Module::krate(*self) + } +} diff --git a/crates/ide-completion/src/completions.rs b/crates/ide-completion/src/completions.rs index b254e458eb..7b90307988 100644 --- a/crates/ide-completion/src/completions.rs +++ b/crates/ide-completion/src/completions.rs @@ -174,13 +174,19 @@ impl Completions { local_name: hir::Name, resolution: hir::ScopeDef, ) { - if ctx.is_scope_def_hidden(resolution) { - cov_mark::hit!(qualified_path_doc_hidden); - return; - } + let is_private_editable = match ctx.def_is_visible(&resolution) { + Visible::Yes => false, + Visible::Editable => true, + Visible::No => return, + }; self.add( - render_path_resolution(RenderContext::new(ctx), path_ctx, local_name, resolution) - .build(), + render_path_resolution( + RenderContext::new(ctx).private_editable(is_private_editable), + path_ctx, + local_name, + resolution, + ) + .build(), ); } @@ -191,13 +197,19 @@ impl Completions { local_name: hir::Name, resolution: hir::ScopeDef, ) { - if ctx.is_scope_def_hidden(resolution) { - cov_mark::hit!(qualified_path_doc_hidden); - return; - } + let is_private_editable = match ctx.def_is_visible(&resolution) { + Visible::Yes => false, + Visible::Editable => true, + Visible::No => return, + }; self.add( - render_pattern_resolution(RenderContext::new(ctx), pattern_ctx, local_name, resolution) - .build(), + render_pattern_resolution( + RenderContext::new(ctx).private_editable(is_private_editable), + pattern_ctx, + local_name, + resolution, + ) + .build(), ); } diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index c93fd0d7bb..8fd059f87c 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -33,6 +33,7 @@ pub(crate) enum PatternRefutability { Irrefutable, } +#[derive(Debug)] pub(crate) enum Visible { Yes, Editable, @@ -355,10 +356,11 @@ pub(crate) struct CompletionContext<'a> { pub(super) locals: FxHashMap, + /// The module depth of the current module of the cursor position. /// - crate-root /// - mod foo /// - mod bar - /// Here depth will be 2: {[bar<->foo], [foo<->crate-root]} + /// Here depth will be 2 pub(super) depth_from_crate_root: usize, } @@ -383,6 +385,30 @@ impl<'a> CompletionContext<'a> { FamousDefs(&self.sema, self.krate) } + /// Checks if an item is visible and not `doc(hidden)` at the completion site. + pub(crate) fn def_is_visible(&self, item: &ScopeDef) -> Visible { + match item { + ScopeDef::ModuleDef(def) => match def { + hir::ModuleDef::Module(it) => self.is_visible(it), + hir::ModuleDef::Function(it) => self.is_visible(it), + hir::ModuleDef::Adt(it) => self.is_visible(it), + hir::ModuleDef::Variant(it) => self.is_visible(it), + hir::ModuleDef::Const(it) => self.is_visible(it), + hir::ModuleDef::Static(it) => self.is_visible(it), + hir::ModuleDef::Trait(it) => self.is_visible(it), + hir::ModuleDef::TypeAlias(it) => self.is_visible(it), + hir::ModuleDef::Macro(it) => self.is_visible(it), + hir::ModuleDef::BuiltinType(_) => Visible::Yes, + }, + ScopeDef::GenericParam(_) + | ScopeDef::ImplSelfType(_) + | ScopeDef::AdtSelfType(_) + | ScopeDef::Local(_) + | ScopeDef::Label(_) + | ScopeDef::Unknown => Visible::Yes, + } + } + /// Checks if an item is visible and not `doc(hidden)` at the completion site. pub(crate) fn is_visible(&self, item: &I) -> Visible where @@ -393,14 +419,6 @@ impl<'a> CompletionContext<'a> { self.is_visible_impl(&vis, &attrs, item.krate(self.db)) } - pub(crate) fn is_scope_def_hidden(&self, scope_def: ScopeDef) -> bool { - if let (Some(attrs), Some(krate)) = (scope_def.attrs(self.db), scope_def.krate(self.db)) { - return self.is_doc_hidden(&attrs, krate); - } - - false - } - /// Check if an item is `#[doc(hidden)]`. pub(crate) fn is_item_hidden(&self, item: &hir::ItemInNs) -> bool { let attrs = item.attrs(self.db); @@ -468,6 +486,14 @@ impl<'a> CompletionContext<'a> { self.scope.process_all_names(&mut |name, def| f(name, def)); } + fn is_scope_def_hidden(&self, scope_def: ScopeDef) -> bool { + if let (Some(attrs), Some(krate)) = (scope_def.attrs(self.db), scope_def.krate(self.db)) { + return self.is_doc_hidden(&attrs, krate); + } + + false + } + fn is_visible_impl( &self, vis: &hir::Visibility, diff --git a/crates/ide-completion/src/tests/special.rs b/crates/ide-completion/src/tests/special.rs index fc59f781dd..ca779c2fc7 100644 --- a/crates/ide-completion/src/tests/special.rs +++ b/crates/ide-completion/src/tests/special.rs @@ -419,9 +419,9 @@ mod m { pub use super::p::WrongType as RightType; } mod p { - fn wrong_fn() {} - const WRONG_CONST: u32 = 1; - struct WrongType {}; + pub fn wrong_fn() {} + pub const WRONG_CONST: u32 = 1; + pub struct WrongType {}; } "#, expect![[r#" @@ -442,9 +442,9 @@ mod m { pub use super::p::WrongType as RightType; } mod p { - fn wrong_fn() {} - const WRONG_CONST: u32 = 1; - struct WrongType {}; + pub fn wrong_fn() {} + pub const WRONG_CONST: u32 = 1; + pub struct WrongType {}; } "#, r#" @@ -456,9 +456,9 @@ mod m { pub use super::p::WrongType as RightType; } mod p { - fn wrong_fn() {} - const WRONG_CONST: u32 = 1; - struct WrongType {}; + pub fn wrong_fn() {} + pub const WRONG_CONST: u32 = 1; + pub struct WrongType {}; } "#, ); @@ -627,7 +627,6 @@ fn main() { #[test] fn respects_doc_hidden2() { - cov_mark::check!(qualified_path_doc_hidden); check( r#" //- /lib.rs crate:lib deps:dep diff --git a/crates/ide/src/syntax_highlighting/highlight.rs b/crates/ide/src/syntax_highlighting/highlight.rs index 4c5bdfd967..b91d54dd92 100644 --- a/crates/ide/src/syntax_highlighting/highlight.rs +++ b/crates/ide/src/syntax_highlighting/highlight.rs @@ -328,8 +328,8 @@ fn highlight_def(sema: &Semantics, krate: hir::Crate, def: Definit Definition::Field(_) => Highlight::new(HlTag::Symbol(SymbolKind::Field)), Definition::Module(module) => { let mut h = Highlight::new(HlTag::Symbol(SymbolKind::Module)); - if module.parent(db).is_none() { - h |= HlMod::CrateRoot + if module.is_crate_root(db) { + h |= HlMod::CrateRoot; } h } diff --git a/crates/ide/src/syntax_highlighting/test_data/highlight_crate_root.html b/crates/ide/src/syntax_highlighting/test_data/highlight_crate_root.html index 6704e5e8d5..1e4c06df7e 100644 --- a/crates/ide/src/syntax_highlighting/test_data/highlight_crate_root.html +++ b/crates/ide/src/syntax_highlighting/test_data/highlight_crate_root.html @@ -49,18 +49,18 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd use foo as foooo; -pub(crate) fn main() { +pub(crate) fn main() { let baz = iter::repeat(92); } mod bar { - pub(in super) const FORTY_TWO: u8 = 42; + pub(in super) const FORTY_TWO: u8 = 42; mod baz { - use super::super::NINETY_TWO; - use crate::foooo::Point; + use super::super::NINETY_TWO; + use crate::foooo::Point; - pub(in super::super) const TWENTY_NINE: u8 = 29; + pub(in super::super) const TWENTY_NINE: u8 = 29; } } \ No newline at end of file diff --git a/crates/ide/src/syntax_highlighting/test_data/highlight_general.html b/crates/ide/src/syntax_highlighting/test_data/highlight_general.html index 7bb7ddd280..a97802cbbd 100644 --- a/crates/ide/src/syntax_highlighting/test_data/highlight_general.html +++ b/crates/ide/src/syntax_highlighting/test_data/highlight_general.html @@ -84,7 +84,7 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd } } -use self::FooCopy::{self as BarCopy}; +use self::FooCopy::{self as BarCopy}; #[derive(Copy)] struct FooCopy { diff --git a/crates/ide/src/syntax_highlighting/test_data/highlight_keywords.html b/crates/ide/src/syntax_highlighting/test_data/highlight_keywords.html index ba62b68e9d..66f9ede962 100644 --- a/crates/ide/src/syntax_highlighting/test_data/highlight_keywords.html +++ b/crates/ide/src/syntax_highlighting/test_data/highlight_keywords.html @@ -42,12 +42,12 @@ pre { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd .unresolved_reference { color: #FC5555; text-decoration: wavy underline; } -
extern crate self;
+
extern crate self;
 
-use crate;
-use self;
+use crate;
+use self;
 mod __ {
-    use super::*;
+    use super::*;
 }
 
 macro_rules! void {