Skip to content

fix: Don't show qualified path completions for private items #12766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 14 additions & 18 deletions crates/hir-def/src/nameres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 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 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, root_module_origin: ModuleOrigin) -> DefMap {
fn empty(krate: CrateId, edition: Edition, module_data: ModuleData) -> DefMap {
let mut modules: Arena<ModuleData> = Arena::default();

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 root = modules.alloc(module_data);

DefMap {
_c: Count::new(),
Expand Down
3 changes: 2 additions & 1 deletion crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/hir/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"),
},
Expand Down
24 changes: 24 additions & 0 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
36 changes: 24 additions & 12 deletions crates/ide-completion/src/completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
}

Expand All @@ -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(),
);
}

Expand Down
44 changes: 35 additions & 9 deletions crates/ide-completion/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub(crate) enum PatternRefutability {
Irrefutable,
}

#[derive(Debug)]
pub(crate) enum Visible {
Yes,
Editable,
Expand Down Expand Up @@ -355,10 +356,11 @@ pub(crate) struct CompletionContext<'a> {

pub(super) locals: FxHashMap<Name, Local>,

/// 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,
}

Expand All @@ -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<I>(&self, item: &I) -> Visible
where
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 9 additions & 10 deletions crates/ide-completion/src/tests/special.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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#"
Expand All @@ -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#"
Expand All @@ -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 {};
}
"#,
);
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions crates/ide/src/syntax_highlighting/highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,8 @@ fn highlight_def(sema: &Semantics<RootDatabase>, 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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,18 @@

<span class="keyword">use</span> <span class="module crate_root library">foo</span> <span class="keyword">as</span> <span class="module crate_root declaration library">foooo</span><span class="semicolon">;</span>

<span class="keyword">pub</span><span class="parenthesis">(</span><span class="keyword crate_root">crate</span><span class="parenthesis">)</span> <span class="keyword">fn</span> <span class="function declaration">main</span><span class="parenthesis">(</span><span class="parenthesis">)</span> <span class="brace">{</span>
<span class="keyword">pub</span><span class="parenthesis">(</span><span class="keyword crate_root public">crate</span><span class="parenthesis">)</span> <span class="keyword">fn</span> <span class="function declaration">main</span><span class="parenthesis">(</span><span class="parenthesis">)</span> <span class="brace">{</span>
<span class="keyword">let</span> <span class="variable declaration">baz</span> <span class="operator">=</span> <span class="module default_library library">iter</span><span class="operator">::</span><span class="function default_library library">repeat</span><span class="parenthesis">(</span><span class="numeric_literal">92</span><span class="parenthesis">)</span><span class="semicolon">;</span>
<span class="brace">}</span>

<span class="keyword">mod</span> <span class="module declaration">bar</span> <span class="brace">{</span>
<span class="keyword">pub</span><span class="parenthesis">(</span><span class="keyword control">in</span> <span class="keyword crate_root">super</span><span class="parenthesis">)</span> <span class="keyword">const</span> <span class="constant declaration">FORTY_TWO</span><span class="colon">:</span> <span class="builtin_type">u8</span> <span class="operator">=</span> <span class="numeric_literal">42</span><span class="semicolon">;</span>
<span class="keyword">pub</span><span class="parenthesis">(</span><span class="keyword control">in</span> <span class="keyword crate_root public">super</span><span class="parenthesis">)</span> <span class="keyword">const</span> <span class="constant declaration">FORTY_TWO</span><span class="colon">:</span> <span class="builtin_type">u8</span> <span class="operator">=</span> <span class="numeric_literal">42</span><span class="semicolon">;</span>

<span class="keyword">mod</span> <span class="module declaration">baz</span> <span class="brace">{</span>
<span class="keyword">use</span> <span class="keyword">super</span><span class="operator">::</span><span class="keyword crate_root">super</span><span class="operator">::</span><span class="constant public">NINETY_TWO</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="keyword crate_root">crate</span><span class="operator">::</span><span class="module crate_root library">foooo</span><span class="operator">::</span><span class="struct library">Point</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="keyword">super</span><span class="operator">::</span><span class="keyword crate_root public">super</span><span class="operator">::</span><span class="constant public">NINETY_TWO</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="keyword crate_root public">crate</span><span class="operator">::</span><span class="module crate_root library">foooo</span><span class="operator">::</span><span class="struct library">Point</span><span class="semicolon">;</span>

<span class="keyword">pub</span><span class="parenthesis">(</span><span class="keyword control">in</span> <span class="keyword">super</span><span class="operator">::</span><span class="keyword crate_root">super</span><span class="parenthesis">)</span> <span class="keyword">const</span> <span class="constant declaration">TWENTY_NINE</span><span class="colon">:</span> <span class="builtin_type">u8</span> <span class="operator">=</span> <span class="numeric_literal">29</span><span class="semicolon">;</span>
<span class="keyword">pub</span><span class="parenthesis">(</span><span class="keyword control">in</span> <span class="keyword">super</span><span class="operator">::</span><span class="keyword crate_root public">super</span><span class="parenthesis">)</span> <span class="keyword">const</span> <span class="constant declaration">TWENTY_NINE</span><span class="colon">:</span> <span class="builtin_type">u8</span> <span class="operator">=</span> <span class="numeric_literal">29</span><span class="semicolon">;</span>
<span class="brace">}</span>
<span class="brace">}</span>
</code></pre>
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
<span class="brace">}</span>
<span class="brace">}</span>

<span class="keyword">use</span> <span class="self_keyword crate_root">self</span><span class="operator">::</span><span class="struct">FooCopy</span><span class="operator">::</span><span class="brace">{</span><span class="self_keyword">self</span> <span class="keyword">as</span> <span class="struct declaration">BarCopy</span><span class="brace">}</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="self_keyword crate_root public">self</span><span class="operator">::</span><span class="struct">FooCopy</span><span class="operator">::</span><span class="brace">{</span><span class="self_keyword">self</span> <span class="keyword">as</span> <span class="struct declaration">BarCopy</span><span class="brace">}</span><span class="semicolon">;</span>

<span class="attribute_bracket attribute">#</span><span class="attribute_bracket attribute">[</span><span class="attribute attribute default_library library">derive</span><span class="parenthesis attribute">(</span><span class="derive attribute default_library library">Copy</span><span class="parenthesis attribute">)</span><span class="attribute_bracket attribute">]</span>
<span class="keyword">struct</span> <span class="struct declaration">FooCopy</span> <span class="brace">{</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@

.unresolved_reference { color: #FC5555; text-decoration: wavy underline; }
</style>
<pre><code><span class="keyword">extern</span> <span class="keyword">crate</span> <span class="self_keyword crate_root">self</span><span class="semicolon">;</span>
<pre><code><span class="keyword">extern</span> <span class="keyword">crate</span> <span class="self_keyword crate_root public">self</span><span class="semicolon">;</span>

<span class="keyword">use</span> <span class="keyword crate_root">crate</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="self_keyword crate_root">self</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="keyword crate_root public">crate</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="self_keyword crate_root public">self</span><span class="semicolon">;</span>
<span class="keyword">mod</span> <span class="module declaration">__</span> <span class="brace">{</span>
<span class="keyword">use</span> <span class="keyword crate_root">super</span><span class="operator">::</span><span class="punctuation">*</span><span class="semicolon">;</span>
<span class="keyword">use</span> <span class="keyword crate_root public">super</span><span class="operator">::</span><span class="punctuation">*</span><span class="semicolon">;</span>
<span class="brace">}</span>

<span class="keyword">macro_rules</span><span class="macro_bang">!</span> <span class="macro declaration">void</span> <span class="brace">{</span>
Expand Down