From d7eebd9706ebc48fc4c6e09249dc45baa9e1dc61 Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Tue, 14 Jun 2022 21:41:09 +0800 Subject: [PATCH 1/7] add test cases to complete fn generated by macro in pub trait --- crates/ide-completion/src/tests/special.rs | 111 ++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/crates/ide-completion/src/tests/special.rs b/crates/ide-completion/src/tests/special.rs index 6195537a18efb..8231336237ebd 100644 --- a/crates/ide-completion/src/tests/special.rs +++ b/crates/ide-completion/src/tests/special.rs @@ -2,13 +2,21 @@ use expect_test::{expect, Expect}; -use crate::tests::{check_edit, completion_list_no_kw}; +use crate::{ + tests::{check_edit, completion_list_no_kw, completion_list_with_config, TEST_CONFIG}, + CompletionConfig, +}; fn check(ra_fixture: &str, expect: Expect) { let actual = completion_list_no_kw(ra_fixture); expect.assert_eq(&actual) } +fn check_with_config(config: CompletionConfig, ra_fixture: &str, expect: Expect) { + let actual = completion_list_with_config(config, ra_fixture, true, None); + expect.assert_eq(&actual) +} + #[test] fn completes_if_prefix_is_keyword() { check_edit( @@ -636,3 +644,104 @@ fn bar() -> Bar { "#]], ) } + +#[test] +fn completes_fn_in_pub_trait_generated_by_macro() { + let mut config = TEST_CONFIG.clone(); + config.enable_private_editable = false; + + check_with_config( + config, + r#" +mod other_mod { + macro_rules! make_method { + ($name:ident) => { + fn $name(&self) {} + }; + } + + pub trait MyTrait { + make_method! { by_macro } + fn not_by_macro(&self) {} + } + + pub struct Foo {} + + impl MyTrait for Foo {} +} + +fn main() { + use other_mod::{Foo, MyTrait}; + let f = Foo {}; + f.$0 +} +"#, + expect![[r#" + me by_macro() (as MyTrait) fn(&self) + me not_by_macro() (as MyTrait) fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn let let + sn letm let mut + sn match match expr {} + sn ref &expr + sn refm &mut expr + "#]], + ) +} + + +#[test] +fn completes_fn_in_pub_trait_generated_by_recursive_macro() { + let mut config = TEST_CONFIG.clone(); + config.enable_private_editable = false; + + check_with_config( + config, + r#" +mod other_mod { + macro_rules! make_method { + ($name:ident) => { + fn $name(&self) {} + }; + } + + macro_rules! make_trait { + () => { + pub trait MyTrait { + make_method! { by_macro } + fn not_by_macro(&self) {} + } + } + } + + make_trait!(); + + pub struct Foo {} + + impl MyTrait for Foo {} +} + +fn main() { + use other_mod::{Foo, MyTrait}; + let f = Foo {}; + f.$0 +} +"#, + expect![[r#" + me by_macro() (as MyTrait) fn(&self) + me not_by_macro() (as MyTrait) fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn let let + sn letm let mut + sn match match expr {} + sn ref &expr + sn refm &mut expr + "#]], + ) +} \ No newline at end of file From 3f60e71a12eea313ffab4e4140331ee9e13c262d Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Tue, 14 Jun 2022 21:44:07 +0800 Subject: [PATCH 2/7] remove inherited_visibility in lower.rs --- crates/hir-def/src/item_tree/lower.rs | 50 +++++++-------------------- 1 file changed, 13 insertions(+), 37 deletions(-) diff --git a/crates/hir-def/src/item_tree/lower.rs b/crates/hir-def/src/item_tree/lower.rs index cdae0d0803bec..7ba983fa716da 100644 --- a/crates/hir-def/src/item_tree/lower.rs +++ b/crates/hir-def/src/item_tree/lower.rs @@ -21,7 +21,6 @@ pub(super) struct Ctx<'a> { tree: ItemTree, source_ast_id_map: Arc, body_ctx: crate::body::LowerCtx<'a>, - forced_visibility: Option, } impl<'a> Ctx<'a> { @@ -31,7 +30,6 @@ impl<'a> Ctx<'a> { tree: ItemTree::default(), source_ast_id_map: db.ast_id_map(file), body_ctx: crate::body::LowerCtx::new(db, file), - forced_visibility: None, } } @@ -225,11 +223,10 @@ impl<'a> Ctx<'a> { let visibility = self.lower_visibility(enum_); let name = enum_.name()?.as_name(); let generic_params = self.lower_generic_params(GenericsOwner::Enum, enum_); - let variants = - self.with_inherited_visibility(visibility, |this| match &enum_.variant_list() { - Some(variant_list) => this.lower_variants(variant_list), - None => IdxRange::new(this.next_variant_idx()..this.next_variant_idx()), - }); + let variants = match &enum_.variant_list() { + Some(variant_list) => self.lower_variants(variant_list), + None => IdxRange::new(self.next_variant_idx()..self.next_variant_idx()), + }; let ast_id = self.source_ast_id_map.ast_id(enum_); let res = Enum { name, visibility, generic_params, variants, ast_id }; Some(id(self.data().enums.alloc(res))) @@ -440,18 +437,15 @@ impl<'a> Ctx<'a> { let is_auto = trait_def.auto_token().is_some(); let is_unsafe = trait_def.unsafe_token().is_some(); let items = trait_def.assoc_item_list().map(|list| { - let db = self.db; - self.with_inherited_visibility(visibility, |this| { - list.assoc_items() - .filter_map(|item| { - let attrs = RawAttrs::new(db, &item, this.hygiene()); - this.lower_assoc_item(&item).map(|item| { - this.add_attrs(ModItem::from(item).into(), attrs); - item - }) + list.assoc_items() + .filter_map(|item| { + let attrs = RawAttrs::new(self.db, &item, self.hygiene()); + self.lower_assoc_item(&item).map(|item| { + self.add_attrs(ModItem::from(item).into(), attrs); + item }) - .collect() - }) + }) + .collect() }); let ast_id = self.source_ast_id_map.ast_id(trait_def); let res = Trait { @@ -622,13 +616,7 @@ impl<'a> Ctx<'a> { } fn lower_visibility(&mut self, item: &dyn ast::HasVisibility) -> RawVisibilityId { - let vis = match self.forced_visibility { - Some(vis) => return vis, - None => { - RawVisibility::from_ast_with_hygiene(self.db, item.visibility(), self.hygiene()) - } - }; - + let vis = RawVisibility::from_ast_with_hygiene(self.db, item.visibility(), self.hygiene()); self.data().vis.alloc(vis) } @@ -649,18 +637,6 @@ impl<'a> Ctx<'a> { } } - /// Forces the visibility `vis` to be used for all items lowered during execution of `f`. - fn with_inherited_visibility( - &mut self, - vis: RawVisibilityId, - f: impl FnOnce(&mut Self) -> R, - ) -> R { - let old = mem::replace(&mut self.forced_visibility, Some(vis)); - let res = f(self); - self.forced_visibility = old; - res - } - fn next_field_idx(&self) -> Idx { Idx::from_raw(RawIdx::from( self.tree.data.as_ref().map_or(0, |data| data.fields.len() as u32), From ded412d56b7d2a9552db7626df6f52133d030858 Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Tue, 14 Jun 2022 23:23:15 +0800 Subject: [PATCH 3/7] implement inherited_visibility in collector --- crates/hir-def/src/data.rs | 52 ++++++++++++---- crates/hir-def/src/item_tree/lower.rs | 2 +- crates/hir-def/src/item_tree/tests.rs | 72 +++++++++++----------- crates/hir-def/src/lib.rs | 23 ++++--- crates/hir-def/src/nameres/collector.rs | 36 ++++++++--- crates/ide-completion/src/tests/special.rs | 59 +++++++++++------- 6 files changed, 156 insertions(+), 88 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index f2d8318f7d702..bb2e9a5b29f89 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -15,8 +15,8 @@ use crate::{ type_ref::{TraitRef, TypeBound, TypeRef}, visibility::RawVisibility, AssocItemId, AstIdWithPath, ConstId, ConstLoc, FunctionId, FunctionLoc, HasModule, ImplId, - Intern, ItemContainerId, Lookup, Macro2Id, MacroRulesId, ModuleId, ProcMacroId, StaticId, - TraitId, TypeAliasId, TypeAliasLoc, + InheritedVisibilityLoc, Intern, ItemContainerId, Lookup, Macro2Id, MacroRulesId, ModuleId, + ProcMacroId, StaticId, TraitId, TypeAliasId, TypeAliasLoc, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -41,6 +41,12 @@ impl FunctionData { let item_tree = loc.id.item_tree(db); let func = &item_tree[loc.id.value]; + let visibility = if let Some(inherited_vis) = loc.inherited_visibility { + inherited_vis.tree_id.item_tree(db)[inherited_vis.raw_visibility_id].clone() + } else { + item_tree[func.visibility].clone() + }; + let enabled_params = func .params .clone() @@ -93,7 +99,7 @@ impl FunctionData { ret_type: func.ret_type.clone(), async_ret_type: func.async_ret_type.clone(), attrs: item_tree.attrs(db, krate, ModItem::from(loc.id.value).into()), - visibility: item_tree[func.visibility].clone(), + visibility, abi: func.abi.clone(), legacy_const_generics_indices, flags, @@ -171,11 +177,16 @@ impl TypeAliasData { let loc = typ.lookup(db); let item_tree = loc.id.item_tree(db); let typ = &item_tree[loc.id.value]; + let visibility = if let Some(inherited_vis) = loc.inherited_visibility { + inherited_vis.tree_id.item_tree(db)[inherited_vis.raw_visibility_id].clone() + } else { + item_tree[typ.visibility].clone() + }; Arc::new(TypeAliasData { name: typ.name.clone(), type_ref: typ.type_ref.clone(), - visibility: item_tree[typ.visibility].clone(), + visibility, is_extern: matches!(loc.container, ItemContainerId::ExternBlockId(_)), bounds: typ.bounds.to_vec(), }) @@ -221,6 +232,7 @@ impl TraitData { module_id, tr_loc.id.file_id(), ItemContainerId::TraitId(tr), + Some(InheritedVisibilityLoc::new(tr_def.visibility, tr_loc.id.tree_id())), ); collector.collect(tr_loc.id.tree_id(), &tr_def.items); @@ -288,6 +300,7 @@ impl ImplData { module_id, impl_loc.id.file_id(), ItemContainerId::ImplId(id), + None, ); collector.collect(impl_loc.id.tree_id(), &impl_def.items); @@ -385,11 +398,16 @@ impl ConstData { let loc = konst.lookup(db); let item_tree = loc.id.item_tree(db); let konst = &item_tree[loc.id.value]; + let visibility = if let Some(inherited_vis) = loc.inherited_visibility { + inherited_vis.tree_id.item_tree(db)[inherited_vis.raw_visibility_id].clone() + } else { + item_tree[konst.visibility].clone() + }; Arc::new(ConstData { name: konst.name.clone(), type_ref: konst.type_ref.clone(), - visibility: item_tree[konst.visibility].clone(), + visibility, }) } } @@ -428,6 +446,8 @@ struct AssocItemCollector<'a> { items: Vec<(Name, AssocItemId)>, attr_calls: Vec<(AstId, MacroCallId)>, + + inherited_visibility: Option, } impl<'a> AssocItemCollector<'a> { @@ -436,6 +456,7 @@ impl<'a> AssocItemCollector<'a> { module_id: ModuleId, file_id: HirFileId, container: ItemContainerId, + inherited_visibility: Option, ) -> Self { Self { db, @@ -446,6 +467,8 @@ impl<'a> AssocItemCollector<'a> { items: Vec::new(), attr_calls: Vec::new(), + + inherited_visibility, } } @@ -488,9 +511,12 @@ impl<'a> AssocItemCollector<'a> { match item { AssocItem::Function(id) => { let item = &item_tree[id]; - let def = - FunctionLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } - .intern(self.db); + let def = FunctionLoc { + container: self.container, + id: ItemTreeId::new(tree_id, id), + inherited_visibility: self.inherited_visibility, + } + .intern(self.db); self.items.push((item.name.clone(), def.into())); } AssocItem::Const(id) => { @@ -499,9 +525,12 @@ impl<'a> AssocItemCollector<'a> { Some(name) => name, None => continue, }; - let def = - ConstLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } - .intern(self.db); + let def = ConstLoc { + container: self.container, + id: ItemTreeId::new(tree_id, id), + inherited_visibility: self.inherited_visibility, + } + .intern(self.db); self.items.push((name, def.into())); } AssocItem::TypeAlias(id) => { @@ -509,6 +538,7 @@ impl<'a> AssocItemCollector<'a> { let def = TypeAliasLoc { container: self.container, id: ItemTreeId::new(tree_id, id), + inherited_visibility: self.inherited_visibility, } .intern(self.db); self.items.push((item.name.clone(), def.into())); diff --git a/crates/hir-def/src/item_tree/lower.rs b/crates/hir-def/src/item_tree/lower.rs index 7ba983fa716da..7f2551e941871 100644 --- a/crates/hir-def/src/item_tree/lower.rs +++ b/crates/hir-def/src/item_tree/lower.rs @@ -1,6 +1,6 @@ //! AST -> `ItemTree` lowering code. -use std::{collections::hash_map::Entry, mem, sync::Arc}; +use std::{collections::hash_map::Entry, sync::Arc}; use hir_expand::{ast_id_map::AstIdMap, hygiene::Hygiene, HirFileId}; use syntax::ast::{self, HasModuleItem}; diff --git a/crates/hir-def/src/item_tree/tests.rs b/crates/hir-def/src/item_tree/tests.rs index fb3811dbd56b6..bd7c9588b6014 100644 --- a/crates/hir-def/src/item_tree/tests.rs +++ b/crates/hir-def/src/item_tree/tests.rs @@ -359,39 +359,39 @@ trait Tr<'a, T: 'a>: Super where Self: for<'a> Tr<'a, T> {} ) } -#[test] -fn inherit_visibility() { - check( - r#" -pub(crate) enum En { - Var1(u8), - Var2 { - fld: u8, - }, -} - -pub(crate) trait Tr { - fn f(); - fn method(&self) {} -} - "#, - expect![[r#" - pub(crate) enum En { - Var1( - pub(crate) 0: u8, - ), - Var2 { - pub(crate) fld: u8, - }, - } - - pub(crate) trait Tr { - pub(crate) fn f() -> (); - - pub(crate) fn method( - _: &Self, // self - ) -> () { ... } - } - "#]], - ) -} +// #[test] +// fn inherit_visibility() { +// check( +// r#" +// pub(crate) enum En { +// Var1(u8), +// Var2 { +// fld: u8, +// }, +// } + +// pub(crate) trait Tr { +// fn f(); +// fn method(&self) {} +// } +// "#, +// expect![[r#" +// pub(crate) enum En { +// Var1( +// pub(crate) 0: u8, +// ), +// Var2 { +// pub(crate) fld: u8, +// }, +// } + +// pub(crate) trait Tr { +// pub(crate) fn f() -> (); + +// pub(crate) fn method( +// _: &Self, // self +// ) -> () { ... } +// } +// "#]], +// ) +// } diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 4431d1b3c817e..e3ac7faa6cc17 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -70,7 +70,7 @@ use hir_expand::{ AstId, ExpandError, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind, UnresolvedMacro, }; -use item_tree::ExternBlock; +use item_tree::{ExternBlock, RawVisibilityId, TreeId}; use la_arena::Idx; use nameres::DefMap; use stdx::impl_from; @@ -156,19 +156,24 @@ impl Hash for ItemLoc { } } -#[derive(Debug)] -pub struct AssocItemLoc { - pub container: ItemContainerId, - pub id: ItemTreeId, +#[derive(Debug, Clone, Copy)] +pub struct InheritedVisibilityLoc { + pub raw_visibility_id: RawVisibilityId, + pub tree_id: TreeId, } -impl Clone for AssocItemLoc { - fn clone(&self) -> Self { - Self { container: self.container, id: self.id } +impl InheritedVisibilityLoc { + pub fn new(visibility_id: RawVisibilityId, tree_id: TreeId) -> Self { + Self { raw_visibility_id: visibility_id, tree_id } } } -impl Copy for AssocItemLoc {} +#[derive(Debug, Clone, Copy)] +pub struct AssocItemLoc { + pub container: ItemContainerId, + pub id: ItemTreeId, + pub inherited_visibility: Option, +} impl PartialEq for AssocItemLoc { fn eq(&self, other: &Self) -> bool { diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 7fea46bee3ca8..5e3e143bef42b 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1549,8 +1549,12 @@ impl ModCollector<'_, '_> { } ModItem::Function(id) => { let it = &self.item_tree[id]; - let fn_id = - FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db); + let fn_id = FunctionLoc { + container, + id: ItemTreeId::new(self.tree_id, id), + inherited_visibility: None, + } + .intern(db); let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); if self.def_collector.is_proc_macro { @@ -1613,8 +1617,12 @@ impl ModCollector<'_, '_> { } ModItem::Const(id) => { let it = &self.item_tree[id]; - let const_id = - ConstLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db); + let const_id = ConstLoc { + container, + id: ItemTreeId::new(self.tree_id, id), + inherited_visibility: None, + } + .intern(db); match &it.name { Some(name) => { @@ -1635,9 +1643,13 @@ impl ModCollector<'_, '_> { let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); update_def( self.def_collector, - StaticLoc { container, id: ItemTreeId::new(self.tree_id, id) } - .intern(db) - .into(), + StaticLoc { + container, + id: ItemTreeId::new(self.tree_id, id), + inherited_visibility: None, + } + .intern(db) + .into(), &it.name, vis, false, @@ -1663,9 +1675,13 @@ impl ModCollector<'_, '_> { let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); update_def( self.def_collector, - TypeAliasLoc { container, id: ItemTreeId::new(self.tree_id, id) } - .intern(db) - .into(), + TypeAliasLoc { + container, + id: ItemTreeId::new(self.tree_id, id), + inherited_visibility: None, + } + .intern(db) + .into(), &it.name, vis, false, diff --git a/crates/ide-completion/src/tests/special.rs b/crates/ide-completion/src/tests/special.rs index 8231336237ebd..2f2351e27bf9d 100644 --- a/crates/ide-completion/src/tests/special.rs +++ b/crates/ide-completion/src/tests/special.rs @@ -13,7 +13,7 @@ fn check(ra_fixture: &str, expect: Expect) { } fn check_with_config(config: CompletionConfig, ra_fixture: &str, expect: Expect) { - let actual = completion_list_with_config(config, ra_fixture, true, None); + let actual = completion_list_with_config(config, ra_fixture, false, None); expect.assert_eq(&actual) } @@ -679,20 +679,10 @@ fn main() { expect![[r#" me by_macro() (as MyTrait) fn(&self) me not_by_macro() (as MyTrait) fn(&self) - sn box Box::new(expr) - sn call function(expr) - sn dbg dbg!(expr) - sn dbgr dbg!(&expr) - sn let let - sn letm let mut - sn match match expr {} - sn ref &expr - sn refm &mut expr "#]], ) } - #[test] fn completes_fn_in_pub_trait_generated_by_recursive_macro() { let mut config = TEST_CONFIG.clone(); @@ -733,15 +723,42 @@ fn main() { expect![[r#" me by_macro() (as MyTrait) fn(&self) me not_by_macro() (as MyTrait) fn(&self) - sn box Box::new(expr) - sn call function(expr) - sn dbg dbg!(expr) - sn dbgr dbg!(&expr) - sn let let - sn letm let mut - sn match match expr {} - sn ref &expr - sn refm &mut expr "#]], ) -} \ No newline at end of file +} + +#[test] +fn completes_const_in_pub_trait_generated_by_macro() { + let mut config = TEST_CONFIG.clone(); + config.enable_private_editable = false; + + check_with_config( + config, + r#" +mod other_mod { + macro_rules! make_const { + ($name:ident) => { + const $name: u8 = 1; + }; + } + + pub trait MyTrait { + make_const! { by_macro } + } + + pub struct Foo {} + + impl MyTrait for Foo {} +} + +fn main() { + use other_mod::{Foo, MyTrait}; + let f = Foo {}; + Foo::$0 +} +"#, + expect![[r#" + ct by_macro (as MyTrait) pub const by_macro: u8 + "#]], + ) +} From 070456838da319ec73768edeff5df2a828abb869 Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Tue, 14 Jun 2022 23:24:48 +0800 Subject: [PATCH 4/7] remove inherit_visibility test case in item_tree --- crates/hir-def/src/item_tree/tests.rs | 37 --------------------------- 1 file changed, 37 deletions(-) diff --git a/crates/hir-def/src/item_tree/tests.rs b/crates/hir-def/src/item_tree/tests.rs index bd7c9588b6014..5cdf36cc61b83 100644 --- a/crates/hir-def/src/item_tree/tests.rs +++ b/crates/hir-def/src/item_tree/tests.rs @@ -358,40 +358,3 @@ trait Tr<'a, T: 'a>: Super where Self: for<'a> Tr<'a, T> {} "#]], ) } - -// #[test] -// fn inherit_visibility() { -// check( -// r#" -// pub(crate) enum En { -// Var1(u8), -// Var2 { -// fld: u8, -// }, -// } - -// pub(crate) trait Tr { -// fn f(); -// fn method(&self) {} -// } -// "#, -// expect![[r#" -// pub(crate) enum En { -// Var1( -// pub(crate) 0: u8, -// ), -// Var2 { -// pub(crate) fld: u8, -// }, -// } - -// pub(crate) trait Tr { -// pub(crate) fn f() -> (); - -// pub(crate) fn method( -// _: &Self, // self -// ) -> () { ... } -// } -// "#]], -// ) -// } From 8805a768d442b07d7f0152a5f6a8dd1ce2b21350 Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Wed, 15 Jun 2022 07:47:06 +0800 Subject: [PATCH 5/7] check if the container is trait and inherit the visibility --- crates/hir-def/src/data.rs | 43 ++++++++----------------- crates/hir-def/src/lib.rs | 24 ++++++-------- crates/hir-def/src/nameres/collector.rs | 36 ++++++--------------- 3 files changed, 33 insertions(+), 70 deletions(-) diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index bb2e9a5b29f89..7436f0100bddd 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -15,8 +15,8 @@ use crate::{ type_ref::{TraitRef, TypeBound, TypeRef}, visibility::RawVisibility, AssocItemId, AstIdWithPath, ConstId, ConstLoc, FunctionId, FunctionLoc, HasModule, ImplId, - InheritedVisibilityLoc, Intern, ItemContainerId, Lookup, Macro2Id, MacroRulesId, ModuleId, - ProcMacroId, StaticId, TraitId, TypeAliasId, TypeAliasLoc, + Intern, ItemContainerId, Lookup, Macro2Id, MacroRulesId, ModuleId, ProcMacroId, StaticId, + TraitId, TypeAliasId, TypeAliasLoc, }; #[derive(Debug, Clone, PartialEq, Eq)] @@ -40,9 +40,8 @@ impl FunctionData { let cfg_options = &crate_graph[krate].cfg_options; let item_tree = loc.id.item_tree(db); let func = &item_tree[loc.id.value]; - - let visibility = if let Some(inherited_vis) = loc.inherited_visibility { - inherited_vis.tree_id.item_tree(db)[inherited_vis.raw_visibility_id].clone() + let visibility = if let ItemContainerId::TraitId(trait_id) = loc.container { + db.trait_data(trait_id).visibility.clone() } else { item_tree[func.visibility].clone() }; @@ -177,8 +176,8 @@ impl TypeAliasData { let loc = typ.lookup(db); let item_tree = loc.id.item_tree(db); let typ = &item_tree[loc.id.value]; - let visibility = if let Some(inherited_vis) = loc.inherited_visibility { - inherited_vis.tree_id.item_tree(db)[inherited_vis.raw_visibility_id].clone() + let visibility = if let ItemContainerId::TraitId(trait_id) = loc.container { + db.trait_data(trait_id).visibility.clone() } else { item_tree[typ.visibility].clone() }; @@ -232,7 +231,6 @@ impl TraitData { module_id, tr_loc.id.file_id(), ItemContainerId::TraitId(tr), - Some(InheritedVisibilityLoc::new(tr_def.visibility, tr_loc.id.tree_id())), ); collector.collect(tr_loc.id.tree_id(), &tr_def.items); @@ -300,7 +298,6 @@ impl ImplData { module_id, impl_loc.id.file_id(), ItemContainerId::ImplId(id), - None, ); collector.collect(impl_loc.id.tree_id(), &impl_def.items); @@ -398,8 +395,8 @@ impl ConstData { let loc = konst.lookup(db); let item_tree = loc.id.item_tree(db); let konst = &item_tree[loc.id.value]; - let visibility = if let Some(inherited_vis) = loc.inherited_visibility { - inherited_vis.tree_id.item_tree(db)[inherited_vis.raw_visibility_id].clone() + let visibility = if let ItemContainerId::TraitId(trait_id) = loc.container { + db.trait_data(trait_id).visibility.clone() } else { item_tree[konst.visibility].clone() }; @@ -446,8 +443,6 @@ struct AssocItemCollector<'a> { items: Vec<(Name, AssocItemId)>, attr_calls: Vec<(AstId, MacroCallId)>, - - inherited_visibility: Option, } impl<'a> AssocItemCollector<'a> { @@ -456,7 +451,6 @@ impl<'a> AssocItemCollector<'a> { module_id: ModuleId, file_id: HirFileId, container: ItemContainerId, - inherited_visibility: Option, ) -> Self { Self { db, @@ -467,8 +461,6 @@ impl<'a> AssocItemCollector<'a> { items: Vec::new(), attr_calls: Vec::new(), - - inherited_visibility, } } @@ -511,12 +503,9 @@ impl<'a> AssocItemCollector<'a> { match item { AssocItem::Function(id) => { let item = &item_tree[id]; - let def = FunctionLoc { - container: self.container, - id: ItemTreeId::new(tree_id, id), - inherited_visibility: self.inherited_visibility, - } - .intern(self.db); + let def = + FunctionLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } + .intern(self.db); self.items.push((item.name.clone(), def.into())); } AssocItem::Const(id) => { @@ -525,12 +514,9 @@ impl<'a> AssocItemCollector<'a> { Some(name) => name, None => continue, }; - let def = ConstLoc { - container: self.container, - id: ItemTreeId::new(tree_id, id), - inherited_visibility: self.inherited_visibility, - } - .intern(self.db); + let def = + ConstLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } + .intern(self.db); self.items.push((name, def.into())); } AssocItem::TypeAlias(id) => { @@ -538,7 +524,6 @@ impl<'a> AssocItemCollector<'a> { let def = TypeAliasLoc { container: self.container, id: ItemTreeId::new(tree_id, id), - inherited_visibility: self.inherited_visibility, } .intern(self.db); self.items.push((item.name.clone(), def.into())); diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index e3ac7faa6cc17..071cf7be6051d 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -70,7 +70,7 @@ use hir_expand::{ AstId, ExpandError, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind, UnresolvedMacro, }; -use item_tree::{ExternBlock, RawVisibilityId, TreeId}; +use item_tree::ExternBlock; use la_arena::Idx; use nameres::DefMap; use stdx::impl_from; @@ -156,25 +156,19 @@ impl Hash for ItemLoc { } } -#[derive(Debug, Clone, Copy)] -pub struct InheritedVisibilityLoc { - pub raw_visibility_id: RawVisibilityId, - pub tree_id: TreeId, -} - -impl InheritedVisibilityLoc { - pub fn new(visibility_id: RawVisibilityId, tree_id: TreeId) -> Self { - Self { raw_visibility_id: visibility_id, tree_id } - } -} - -#[derive(Debug, Clone, Copy)] +#[derive(Debug)] pub struct AssocItemLoc { pub container: ItemContainerId, pub id: ItemTreeId, - pub inherited_visibility: Option, } +impl Clone for AssocItemLoc { + fn clone(&self) -> Self { + Self { container: self.container, id: self.id } + } +} + +impl Copy for AssocItemLoc {} impl PartialEq for AssocItemLoc { fn eq(&self, other: &Self) -> bool { self.container == other.container && self.id == other.id diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 5e3e143bef42b..7fea46bee3ca8 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -1549,12 +1549,8 @@ impl ModCollector<'_, '_> { } ModItem::Function(id) => { let it = &self.item_tree[id]; - let fn_id = FunctionLoc { - container, - id: ItemTreeId::new(self.tree_id, id), - inherited_visibility: None, - } - .intern(db); + let fn_id = + FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db); let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); if self.def_collector.is_proc_macro { @@ -1617,12 +1613,8 @@ impl ModCollector<'_, '_> { } ModItem::Const(id) => { let it = &self.item_tree[id]; - let const_id = ConstLoc { - container, - id: ItemTreeId::new(self.tree_id, id), - inherited_visibility: None, - } - .intern(db); + let const_id = + ConstLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db); match &it.name { Some(name) => { @@ -1643,13 +1635,9 @@ impl ModCollector<'_, '_> { let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); update_def( self.def_collector, - StaticLoc { - container, - id: ItemTreeId::new(self.tree_id, id), - inherited_visibility: None, - } - .intern(db) - .into(), + StaticLoc { container, id: ItemTreeId::new(self.tree_id, id) } + .intern(db) + .into(), &it.name, vis, false, @@ -1675,13 +1663,9 @@ impl ModCollector<'_, '_> { let vis = resolve_vis(def_map, &self.item_tree[it.visibility]); update_def( self.def_collector, - TypeAliasLoc { - container, - id: ItemTreeId::new(self.tree_id, id), - inherited_visibility: None, - } - .intern(db) - .into(), + TypeAliasLoc { container, id: ItemTreeId::new(self.tree_id, id) } + .intern(db) + .into(), &it.name, vis, false, From 549c810436669378c05fa1847be2471cc476d9e8 Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Wed, 15 Jun 2022 07:48:34 +0800 Subject: [PATCH 6/7] revert hir-def lib.rs --- crates/hir-def/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 071cf7be6051d..4431d1b3c817e 100644 --- a/crates/hir-def/src/lib.rs +++ b/crates/hir-def/src/lib.rs @@ -169,6 +169,7 @@ impl Clone for AssocItemLoc { } impl Copy for AssocItemLoc {} + impl PartialEq for AssocItemLoc { fn eq(&self, other: &Self) -> bool { self.container == other.container && self.id == other.id From 534d71a852264cced80290afb2f994b179ddb40a Mon Sep 17 00:00:00 2001 From: Hongxu Xu Date: Thu, 16 Jun 2022 08:52:57 +0800 Subject: [PATCH 7/7] disable private editable in TEST_CONFIG by default adjust test_visibility_filter test case --- crates/ide-completion/src/completions/dot.rs | 112 ++++++++++++++++++- crates/ide-completion/src/tests.rs | 8 +- crates/ide-completion/src/tests/special.rs | 28 +---- 3 files changed, 118 insertions(+), 30 deletions(-) diff --git a/crates/ide-completion/src/completions/dot.rs b/crates/ide-completion/src/completions/dot.rs index a11652ca302fb..4eb1fccd7d327 100644 --- a/crates/ide-completion/src/completions/dot.rs +++ b/crates/ide-completion/src/completions/dot.rs @@ -117,13 +117,20 @@ fn complete_methods( mod tests { use expect_test::{expect, Expect}; - use crate::tests::{check_edit, completion_list_no_kw}; + use crate::tests::{ + check_edit, completion_list_no_kw, completion_list_no_kw_with_private_editable, + }; fn check(ra_fixture: &str, expect: Expect) { let actual = completion_list_no_kw(ra_fixture); expect.assert_eq(&actual); } + fn check_with_private_editable(ra_fixture: &str, expect: Expect) { + let actual = completion_list_no_kw_with_private_editable(ra_fixture); + expect.assert_eq(&actual); + } + #[test] fn test_struct_field_and_method_completion() { check( @@ -200,6 +207,101 @@ pub mod m { } //- /main.rs crate:main deps:lib new_source_root:local fn foo(a: lib::m::A) { a.$0 } +"#, + expect![[r#" + fd pub_field u32 + "#]], + ); + + check( + r#" +//- /lib.rs crate:lib new_source_root:library +pub mod m { + pub struct A { + private_field: u32, + pub pub_field: u32, + pub(crate) crate_field: u32, + pub(super) super_field: u32, + } +} +//- /main.rs crate:main deps:lib new_source_root:local +fn foo(a: lib::m::A) { a.$0 } +"#, + expect![[r#" + fd pub_field u32 + "#]], + ); + + check( + r#" +//- /lib.rs crate:lib new_source_root:library +pub mod m { + pub struct A( + i32, + pub f64, + ); +} +//- /main.rs crate:main deps:lib new_source_root:local +fn foo(a: lib::m::A) { a.$0 } +"#, + expect![[r#" + fd 1 f64 + "#]], + ); + + check( + r#" +//- /lib.rs crate:lib new_source_root:local +pub struct A {} +mod m { + impl super::A { + fn private_method(&self) {} + pub(crate) fn crate_method(&self) {} + pub fn pub_method(&self) {} + } +} +//- /main.rs crate:main deps:lib new_source_root:local +fn foo(a: lib::A) { a.$0 } +"#, + expect![[r#" + me pub_method() fn(&self) + "#]], + ); + check( + r#" +//- /lib.rs crate:lib new_source_root:library +pub struct A {} +mod m { + impl super::A { + fn private_method(&self) {} + pub(crate) fn crate_method(&self) {} + pub fn pub_method(&self) {} + } +} +//- /main.rs crate:main deps:lib new_source_root:local +fn foo(a: lib::A) { a.$0 } +"#, + expect![[r#" + me pub_method() fn(&self) + "#]], + ); + } + + #[test] + fn test_visibility_filtering_with_private_editable_enabled() { + check_with_private_editable( + r#" +//- /lib.rs crate:lib new_source_root:local +pub mod m { + pub struct A { + private_field: u32, + pub pub_field: u32, + pub(crate) crate_field: u32, + pub(super) super_field: u32, + } +} +//- /main.rs crate:main deps:lib new_source_root:local +fn foo(a: lib::m::A) { a.$0 } "#, expect![[r#" fd crate_field u32 @@ -209,7 +311,7 @@ fn foo(a: lib::m::A) { a.$0 } "#]], ); - check( + check_with_private_editable( r#" //- /lib.rs crate:lib new_source_root:library pub mod m { @@ -228,7 +330,7 @@ fn foo(a: lib::m::A) { a.$0 } "#]], ); - check( + check_with_private_editable( r#" //- /lib.rs crate:lib new_source_root:library pub mod m { @@ -245,7 +347,7 @@ fn foo(a: lib::m::A) { a.$0 } "#]], ); - check( + check_with_private_editable( r#" //- /lib.rs crate:lib new_source_root:local pub struct A {} @@ -265,7 +367,7 @@ fn foo(a: lib::A) { a.$0 } me pub_method() fn(&self) "#]], ); - check( + check_with_private_editable( r#" //- /lib.rs crate:lib new_source_root:library pub struct A {} diff --git a/crates/ide-completion/src/tests.rs b/crates/ide-completion/src/tests.rs index d30ff77bab622..4be6acbe8461e 100644 --- a/crates/ide-completion/src/tests.rs +++ b/crates/ide-completion/src/tests.rs @@ -65,7 +65,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig { enable_postfix_completions: true, enable_imports_on_the_fly: true, enable_self_on_the_fly: true, - enable_private_editable: true, + enable_private_editable: false, callable: Some(CallableSnippets::FillArguments), snippet_cap: SnippetCap::new(true), insert_use: InsertUseConfig { @@ -86,6 +86,12 @@ pub(crate) fn completion_list_no_kw(ra_fixture: &str) -> String { completion_list_with_config(TEST_CONFIG, ra_fixture, false, None) } +pub(crate) fn completion_list_no_kw_with_private_editable(ra_fixture: &str) -> String { + let mut config = TEST_CONFIG.clone(); + config.enable_private_editable = true; + completion_list_with_config(config, ra_fixture, false, None) +} + pub(crate) fn completion_list_with_trigger_character( ra_fixture: &str, trigger_character: Option, diff --git a/crates/ide-completion/src/tests/special.rs b/crates/ide-completion/src/tests/special.rs index 2f2351e27bf9d..4535923b28b5a 100644 --- a/crates/ide-completion/src/tests/special.rs +++ b/crates/ide-completion/src/tests/special.rs @@ -2,21 +2,13 @@ use expect_test::{expect, Expect}; -use crate::{ - tests::{check_edit, completion_list_no_kw, completion_list_with_config, TEST_CONFIG}, - CompletionConfig, -}; +use crate::tests::{check_edit, completion_list_no_kw}; fn check(ra_fixture: &str, expect: Expect) { let actual = completion_list_no_kw(ra_fixture); expect.assert_eq(&actual) } -fn check_with_config(config: CompletionConfig, ra_fixture: &str, expect: Expect) { - let actual = completion_list_with_config(config, ra_fixture, false, None); - expect.assert_eq(&actual) -} - #[test] fn completes_if_prefix_is_keyword() { check_edit( @@ -647,11 +639,7 @@ fn bar() -> Bar { #[test] fn completes_fn_in_pub_trait_generated_by_macro() { - let mut config = TEST_CONFIG.clone(); - config.enable_private_editable = false; - - check_with_config( - config, + check( r#" mod other_mod { macro_rules! make_method { @@ -685,11 +673,7 @@ fn main() { #[test] fn completes_fn_in_pub_trait_generated_by_recursive_macro() { - let mut config = TEST_CONFIG.clone(); - config.enable_private_editable = false; - - check_with_config( - config, + check( r#" mod other_mod { macro_rules! make_method { @@ -729,11 +713,7 @@ fn main() { #[test] fn completes_const_in_pub_trait_generated_by_macro() { - let mut config = TEST_CONFIG.clone(); - config.enable_private_editable = false; - - check_with_config( - config, + check( r#" mod other_mod { macro_rules! make_const {