Merge #7907

7907: Autoderef with visibility r=cynecx a=cynecx

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/7841.

I am not sure about the general approach here. Right now this simply tries to check whether the autoderef candidate is reachable from the current module. ~~However this doesn't exactly work with traits (see the `tests::macros::infer_derive_clone_in_core` test, which fails right now).~~ see comment below

Refs:

- `rustc_typeck` checking fields: https://github.com/rust-lang/rust/blob/66ec64ccf31883cd2c28d045912a76179c0c6ed2/compiler/rustc_typeck/src/check/expr.rs#L1610 


r? @flodiebold

Co-authored-by: cynecx <[email protected]>

10 files changed, 250 insertions, 37 deletions

diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index eb7865c84..6fa676c4d 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -1999,6 +1999,7 @@ impl Type {
             env,
             krate,
             traits_in_scope,
+            None,
             name,
             method_resolution::LookupMode::MethodCall,
             |ty, it| match it {
@@ -2031,6 +2032,7 @@ impl Type {
             env,
             krate,
             traits_in_scope,
+            None,
             name,
             method_resolution::LookupMode::Path,
             |ty, it| callback(ty, it.into()),
diff --git a/crates/hir_def/src/db.rs b/crates/hir_def/src/db.rs
index 53df85089..068b2ee38 100644
--- a/crates/hir_def/src/db.rs
+++ b/crates/hir_def/src/db.rs
@@ -16,6 +16,7 @@ use crate::{
     item_tree::ItemTree,
     lang_item::{LangItemTarget, LangItems},
     nameres::DefMap,
+    visibility::{self, Visibility},
     AttrDefId, BlockId, BlockLoc, ConstId, ConstLoc, DefWithBodyId, EnumId, EnumLoc, FunctionId,
     FunctionLoc, GenericDefId, ImplId, ImplLoc, LocalEnumVariantId, LocalFieldId, StaticId,
     StaticLoc, StructId, StructLoc, TraitId, TraitLoc, TypeAliasId, TypeAliasLoc, UnionId,
@@ -131,6 +132,12 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast<dyn AstDatabase> {
 
     #[salsa::invoke(ImportMap::import_map_query)]
     fn import_map(&self, krate: CrateId) -> Arc<ImportMap>;
+
+    #[salsa::invoke(visibility::field_visibilities_query)]
+    fn field_visibilities(&self, var: VariantId) -> Arc<ArenaMap<LocalFieldId, Visibility>>;
+
+    #[salsa::invoke(visibility::function_visibility_query)]
+    fn function_visibility(&self, def: FunctionId) -> Visibility;
 }
 
 fn crate_def_map_wait(db: &dyn DefDatabase, krate: CrateId) -> Arc<DefMap> {
diff --git a/crates/hir_def/src/visibility.rs b/crates/hir_def/src/visibility.rs
index 0e3951910..7d00a37c4 100644
--- a/crates/hir_def/src/visibility.rs
+++ b/crates/hir_def/src/visibility.rs
@@ -1,13 +1,17 @@
 //! Defines hir-level representation of visibility (e.g. `pub` and `pub(crate)`).
 
+use std::sync::Arc;
+
 use hir_expand::{hygiene::Hygiene, InFile};
+use la_arena::ArenaMap;
 use syntax::ast;
 
 use crate::{
     db::DefDatabase,
     nameres::DefMap,
     path::{ModPath, PathKind},
-    ModuleId,
+    resolver::HasResolver,
+    FunctionId, HasModule, LocalFieldId, ModuleDefId, ModuleId, VariantId,
 };
 
 /// Visibility of an item, not yet resolved.
@@ -190,3 +194,29 @@ impl Visibility {
         }
     }
 }
+
+/// Resolve visibility of all specific fields of a struct or union variant.
+pub(crate) fn field_visibilities_query(
+    db: &dyn DefDatabase,
+    variant_id: VariantId,
+) -> Arc<ArenaMap<LocalFieldId, Visibility>> {
+    let var_data = match variant_id {
+        VariantId::StructId(it) => db.struct_data(it).variant_data.clone(),
+        VariantId::UnionId(it) => db.union_data(it).variant_data.clone(),
+        VariantId::EnumVariantId(it) => {
+            db.enum_data(it.parent).variants[it.local_id].variant_data.clone()
+        }
+    };
+    let resolver = variant_id.module(db).resolver(db);
+    let mut res = ArenaMap::default();
+    for (field_id, field_data) in var_data.fields().iter() {
+        res.insert(field_id, field_data.visibility.resolve(db, &resolver))
+    }
+    Arc::new(res)
+}
+
+/// Resolve visibility of a function.
+pub(crate) fn function_visibility_query(db: &dyn DefDatabase, def: FunctionId) -> Visibility {
+    let resolver = ModuleDefId::from(def).module(db).unwrap().resolver(db);
+    db.function_data(def).visibility.resolve(db, &resolver)
+}
diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs
index 19249973c..3f3187ea2 100644
--- a/crates/hir_ty/src/infer/expr.rs
+++ b/crates/hir_ty/src/infer/expr.rs
@@ -443,27 +443,47 @@ impl<'a> InferenceContext<'a> {
                     },
                 )
                 .find_map(|derefed_ty| {
+                    let def_db = self.db.upcast();
+                    let module = self.resolver.module();
+                    let is_visible = |field_id: &FieldId| {
+                        module
+                            .map(|mod_id| {
+                                self.db.field_visibilities(field_id.parent)[field_id.local_id]
+                                    .is_visible_from(def_db, mod_id)
+                            })
+                            .unwrap_or(true)
+                    };
                     match canonicalized.decanonicalize_ty(derefed_ty.value).interned(&Interner) {
                         TyKind::Tuple(_, substs) => {
                             name.as_tuple_index().and_then(|idx| substs.0.get(idx).cloned())
                         }
                         TyKind::Adt(AdtId(hir_def::AdtId::StructId(s)), parameters) => {
-                            self.db.struct_data(*s).variant_data.field(name).map(|local_id| {
-                                let field = FieldId { parent: (*s).into(), local_id };
+                            let local_id = self.db.struct_data(*s).variant_data.field(name)?;
+                            let field = FieldId { parent: (*s).into(), local_id };
+                            if is_visible(&field) {
                                 self.write_field_resolution(tgt_expr, field);
-                                self.db.field_types((*s).into())[field.local_id]
-                                    .clone()
-                                    .subst(&parameters)
-                            })
+                                Some(
+                                    self.db.field_types((*s).into())[field.local_id]
+                                        .clone()
+                                        .subst(&parameters),
+                                )
+                            } else {
+                                None
+                            }
                         }
                         TyKind::Adt(AdtId(hir_def::AdtId::UnionId(u)), parameters) => {
-                            self.db.union_data(*u).variant_data.field(name).map(|local_id| {
-                                let field = FieldId { parent: (*u).into(), local_id };
+                            let local_id = self.db.union_data(*u).variant_data.field(name)?;
+                            let field = FieldId { parent: (*u).into(), local_id };
+                            if is_visible(&field) {
                                 self.write_field_resolution(tgt_expr, field);
-                                self.db.field_types((*u).into())[field.local_id]
-                                    .clone()
-                                    .subst(&parameters)
-                            })
+                                Some(
+                                    self.db.field_types((*u).into())[field.local_id]
+                                        .clone()
+                                        .subst(&parameters),
+                                )
+                            } else {
+                                None
+                            }
                         }
                         _ => None,
                     }
@@ -828,6 +848,7 @@ impl<'a> InferenceContext<'a> {
                 self.trait_env.clone(),
                 krate,
                 &traits_in_scope,
+                self.resolver.module(),
                 method_name,
             )
         });
diff --git a/crates/hir_ty/src/infer/path.rs b/crates/hir_ty/src/infer/path.rs
index 58cce56ab..cefa38509 100644
--- a/crates/hir_ty/src/infer/path.rs
+++ b/crates/hir_ty/src/infer/path.rs
@@ -230,6 +230,7 @@ impl<'a> InferenceContext<'a> {
             self.trait_env.clone(),
             krate,
             &traits_in_scope,
+            None,
             Some(name),
             method_resolution::LookupMode::Path,
             move |_ty, item| {
diff --git a/crates/hir_ty/src/method_resolution.rs b/crates/hir_ty/src/method_resolution.rs
index 8e986ddde..84d9a1e18 100644
--- a/crates/hir_ty/src/method_resolution.rs
+++ b/crates/hir_ty/src/method_resolution.rs
@@ -296,6 +296,7 @@ pub(crate) fn lookup_method(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: &Name,
 ) -> Option<(Ty, FunctionId)> {
     iterate_method_candidates(
@@ -304,6 +305,7 @@ pub(crate) fn lookup_method(
         env,
         krate,
         &traits_in_scope,
+        visible_from_module,
         Some(name),
         LookupMode::MethodCall,
         |ty, f| match f {
@@ -334,6 +336,7 @@ pub fn iterate_method_candidates<T>(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mode: LookupMode,
     mut callback: impl FnMut(&Ty, AssocItemId) -> Option<T>,
@@ -345,6 +348,7 @@ pub fn iterate_method_candidates<T>(
         env,
         krate,
         traits_in_scope,
+        visible_from_module,
         name,
         mode,
         &mut |ty, item| {
@@ -362,6 +366,7 @@ fn iterate_method_candidates_impl(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mode: LookupMode,
     callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
@@ -399,6 +404,7 @@ fn iterate_method_candidates_impl(
                     env.clone(),
                     krate,
                     traits_in_scope,
+                    visible_from_module,
                     name,
                     callback,
                 ) {
@@ -415,6 +421,7 @@ fn iterate_method_candidates_impl(
                 env,
                 krate,
                 traits_in_scope,
+                visible_from_module,
                 name,
                 callback,
             )
@@ -428,6 +435,7 @@ fn iterate_method_candidates_with_autoref(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
@@ -438,6 +446,7 @@ fn iterate_method_candidates_with_autoref(
         env.clone(),
         krate,
         &traits_in_scope,
+        visible_from_module,
         name,
         &mut callback,
     ) {
@@ -454,6 +463,7 @@ fn iterate_method_candidates_with_autoref(
         env.clone(),
         krate,
         &traits_in_scope,
+        visible_from_module,
         name,
         &mut callback,
     ) {
@@ -470,6 +480,7 @@ fn iterate_method_candidates_with_autoref(
         env,
         krate,
         &traits_in_scope,
+        visible_from_module,
         name,
         &mut callback,
     ) {
@@ -485,6 +496,7 @@ fn iterate_method_candidates_by_receiver(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
@@ -492,7 +504,15 @@ fn iterate_method_candidates_by_receiver(
     // be found in any of the derefs of receiver_ty, so we have to go through
     // that.
     for self_ty in std::iter::once(receiver_ty).chain(rest_of_deref_chain) {
-        if iterate_inherent_methods(self_ty, db, name, Some(receiver_ty), krate, &mut callback) {
+        if iterate_inherent_methods(
+            self_ty,
+            db,
+            name,
+            Some(receiver_ty),
+            krate,
+            visible_from_module,
+            &mut callback,
+        ) {
             return true;
         }
     }
@@ -519,10 +539,12 @@ fn iterate_method_candidates_for_self_ty(
     env: Arc<TraitEnvironment>,
     krate: CrateId,
     traits_in_scope: &FxHashSet<TraitId>,
+    visible_from_module: Option<ModuleId>,
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
-    if iterate_inherent_methods(self_ty, db, name, None, krate, &mut callback) {
+    if iterate_inherent_methods(self_ty, db, name, None, krate, visible_from_module, &mut callback)
+    {
         return true;
     }
     iterate_trait_method_candidates(self_ty, db, env, krate, traits_in_scope, name, None, callback)
@@ -559,7 +581,9 @@ fn iterate_trait_method_candidates(
         // iteration
         let mut known_implemented = false;
         for (_name, item) in data.items.iter() {
-            if !is_valid_candidate(db, name, receiver_ty, *item, self_ty) {
+            // Don't pass a `visible_from_module` down to `is_valid_candidate`,
+            // since only inherent methods should be included into visibility checking.
+            if !is_valid_candidate(db, name, receiver_ty, *item, self_ty, None) {
                 continue;
             }
             if !known_implemented {
@@ -583,6 +607,7 @@ fn iterate_inherent_methods(
     name: Option<&Name>,
     receiver_ty: Option<&Canonical<Ty>>,
     krate: CrateId,
+    visible_from_module: Option<ModuleId>,
     callback: &mut dyn FnMut(&Ty, AssocItemId) -> bool,
 ) -> bool {
     let def_crates = match self_ty.value.def_crates(db, krate) {
@@ -594,7 +619,7 @@ fn iterate_inherent_methods(
 
         for &impl_def in impls.for_self_ty(&self_ty.value) {
             for &item in db.impl_data(impl_def).items.iter() {
-                if !is_valid_candidate(db, name, receiver_ty, item, self_ty) {
+                if !is_valid_candidate(db, name, receiver_ty, item, self_ty, visible_from_module) {
                     continue;
                 }
                 // we have to check whether the self type unifies with the type
@@ -639,6 +664,7 @@ fn is_valid_candidate(
     receiver_ty: Option<&Canonical<Ty>>,
     item: AssocItemId,
     self_ty: &Canonical<Ty>,
+    visible_from_module: Option<ModuleId>,
 ) -> bool {
     match item {
         AssocItemId::FunctionId(m) => {
@@ -660,6 +686,13 @@ fn is_valid_candidate(
                     return false;
                 }
             }
+            if let Some(from_module) = visible_from_module {
+                if !db.function_visibility(m).is_visible_from(db.upcast(), from_module) {
+                    cov_mark::hit!(autoderef_candidate_not_visible);
+                    return false;
+                }
+            }
+
             true
         }
         AssocItemId::ConstId(c) => {
diff --git a/crates/hir_ty/src/tests/macros.rs b/crates/hir_ty/src/tests/macros.rs
index 12951fb16..7eda51866 100644
--- a/crates/hir_ty/src/tests/macros.rs
+++ b/crates/hir_ty/src/tests/macros.rs
@@ -31,12 +31,12 @@ struct S;
 
 #[cfg(not(test))]
 impl S {
-    fn foo3(&self) -> i32 { 0 }
+    pub fn foo3(&self) -> i32 { 0 }
 }
 
 #[cfg(test)]
 impl S {
-    fn foo4(&self) -> i32 { 0 }
+    pub fn foo4(&self) -> i32 { 0 }
 }
 "#,
     );
diff --git a/crates/hir_ty/src/tests/method_resolution.rs b/crates/hir_ty/src/tests/method_resolution.rs
index 4e3f9a9b6..61f18b0d2 100644
--- a/crates/hir_ty/src/tests/method_resolution.rs
+++ b/crates/hir_ty/src/tests/method_resolution.rs
@@ -1173,3 +1173,122 @@ fn main() {
 "#,
     );
 }
+
+#[test]
+fn autoderef_visibility_field() {
+    check_infer(
+        r#"
+#[lang = "deref"]
+pub trait Deref {
+    type Target;
+    fn deref(&self) -> &Self::Target;
+}
+mod a {
+    pub struct Foo(pub char);
+    pub struct Bar(i32);
+    impl Bar {
+        pub fn new() -> Self {
+            Self(0)
+        }
+    }
+    impl super::Deref for Bar {
+        type Target = Foo;
+        fn deref(&self) -> &Foo {
+            &Foo('z')
+        }
+    }
+}
+mod b {
+    fn foo() {
+        let x = super::a::Bar::new().0;
+    }
+}
+        "#,
+        expect![[r#"
+            67..71 'self': &Self
+            200..231 '{     ...     }': Bar
+            214..218 'Self': Bar(i32) -> Bar
+            214..221 'Self(0)': Bar
+            219..220 '0': i32
+            315..319 'self': &Bar
+            329..362 '{     ...     }': &Foo
+            343..352 '&Foo('z')': &Foo
+            344..347 'Foo': Foo(char) -> Foo
+            344..352 'Foo('z')': Foo
+            348..351 ''z'': char
+            392..439 '{     ...     }': ()
+            406..407 'x': char
+            410..428 'super:...r::new': fn new() -> Bar
+            410..430 'super:...:new()': Bar
+            410..432 'super:...ew().0': char
+        "#]],
+    )
+}
+
+#[test]
+fn autoderef_visibility_method() {
+    cov_mark::check!(autoderef_candidate_not_visible);
+    check_infer(
+        r#"
+#[lang = "deref"]
+pub trait Deref {
+    type Target;
+    fn deref(&self) -> &Self::Target;
+}
+mod a {
+    pub struct Foo(pub char);
+    impl Foo {
+        pub fn mango(&self) -> char {
+            self.0
+        }
+    }
+    pub struct Bar(i32);
+    impl Bar {
+        pub fn new() -> Self {
+            Self(0)
+        }
+        fn mango(&self) -> i32 {
+            self.0
+        }
+    }
+    impl super::Deref for Bar {
+        type Target = Foo;
+        fn deref(&self) -> &Foo {
+            &Foo('z')
+        }
+    }
+}
+mod b {
+    fn foo() {
+        let x = super::a::Bar::new().mango();
+    }
+}
+        "#,
+        expect![[r#"
+            67..71 'self': &Self
+            168..172 'self': &Foo
+            182..212 '{     ...     }': char
+            196..200 'self': &Foo
+            196..202 'self.0': char
+            288..319 '{     ...     }': Bar
+            302..306 'Self': Bar(i32) -> Bar
+            302..309 'Self(0)': Bar
+            307..308 '0': i32
+            338..342 'self': &Bar
+            351..381 '{     ...     }': i32
+            365..369 'self': &Bar
+            365..371 'self.0': i32
+            465..469 'self': &Bar
+            479..512 '{     ...     }': &Foo
+            493..502 '&Foo('z')': &Foo
+            494..497 'Foo': Foo(char) -> Foo
+            494..502 'Foo('z')': Foo
+            498..501 ''z'': char
+            542..595 '{     ...     }': ()
+            556..557 'x': char
+            560..578 'super:...r::new': fn new() -> Bar
+            560..580 'super:...:new()': Bar
+            560..588 'super:...ango()': char
+        "#]],
+    )
+}
diff --git a/crates/hir_ty/src/tests/simple.rs b/crates/hir_ty/src/tests/simple.rs
index bcc43ed70..361cd6302 100644
--- a/crates/hir_ty/src/tests/simple.rs
+++ b/crates/hir_ty/src/tests/simple.rs
@@ -1103,7 +1103,7 @@ fn infer_inherent_method() {
 
         mod b {
             impl super::A {
-                fn bar(&self, x: u64) -> i64 {}
+                pub fn bar(&self, x: u64) -> i64 {}
             }
         }
 
@@ -1117,21 +1117,21 @@ fn infer_inherent_method() {
             31..35 'self': A
             37..38 'x': u32
             52..54 '{}': ()
-            102..106 'self': &A
-            108..109 'x': u64
-            123..125 '{}': ()
-            143..144 'a': A
-            149..197 '{     ...(1); }': ()
-            155..156 'a': A
-            155..163 'a.foo(1)': i32
-            161..162 '1': u32
-            169..180 '(&a).bar(1)': i64
-            170..172 '&a': &A
-            171..172 'a': A
-            178..179 '1': u64
-            186..187 'a': A
-            186..194 'a.bar(1)': i64
-            192..193 '1': u64
+            106..110 'self': &A
+            112..113 'x': u64
+            127..129 '{}': ()
+            147..148 'a': A
+            153..201 '{     ...(1); }': ()
+            159..160 'a': A
+            159..167 'a.foo(1)': i32
+            165..166 '1': u32
+            173..184 '(&a).bar(1)': i64
+            174..176 '&a': &A
+            175..176 'a': A
+            182..183 '1': u64
+            190..191 'a': A
+            190..198 'a.bar(1)': i64
+            196..197 '1': u64
         "#]],
     );
 }
diff --git a/crates/hir_ty/src/tests/traits.rs b/crates/hir_ty/src/tests/traits.rs
index 45a1958e3..2ba97f814 100644
--- a/crates/hir_ty/src/tests/traits.rs
+++ b/crates/hir_ty/src/tests/traits.rs
@@ -187,8 +187,8 @@ mod iter {
 mod collections {
     struct Vec<T> {}
     impl<T> Vec<T> {
-        fn new() -> Self { Vec {} }
-        fn push(&mut self, t: T) { }
+        pub fn new() -> Self { Vec {} }
+        pub fn push(&mut self, t: T) { }
     }
 
     impl<T> IntoIterator for Vec<T> {