Auto merge of rust-lang#12517 - xuhongxu96:master, r=Veykril

fix methods in pub trait generated by macro cannot be completed

Fix rust-lang#12483

Check if the container is trait and inherit the visibility to associate items during collection.

crates/hir-def/src/data.rs

@@ -40,6 +40,11 @@ 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 ItemContainerId::TraitId(trait_id) = loc.container {
+            db.trait_data(trait_id).visibility.clone()
+        } else {
+            item_tree[func.visibility].clone()
+        };
 
         let enabled_params = func
             .params
@@ -93,7 +98,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 +176,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 ItemContainerId::TraitId(trait_id) = loc.container {
+            db.trait_data(trait_id).visibility.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(),
         })
@@ -385,11 +395,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 ItemContainerId::TraitId(trait_id) = loc.container {
+            db.trait_data(trait_id).visibility.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,
         })
     }
 }

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};
@@ -21,7 +21,6 @@ pub(super) struct Ctx<'a> {
     tree: ItemTree,
     source_ast_id_map: Arc<AstIdMap>,
     body_ctx: crate::body::LowerCtx<'a>,
-    forced_visibility: Option<RawVisibilityId>,
 }
 
 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<R>(
-        &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<Field> {
         Idx::from_raw(RawIdx::from(
             self.tree.data.as_ref().map_or(0, |data| data.fields.len() as u32),

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<Self> {
-                pub(crate) fn f() -> ();
-
-                pub(crate) fn method(
-                    _: &Self,  // self
-                ) -> () { ... }
-            }
-        "#]],
-    )
-}

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 {}

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<char>,