Merge #11540

11540: fix: Resolve private fields in type inference r=flodiebold a=Veykril

Fixes #10253 (comment)
(the same issue probably exists for method calls, but I think fixing that might be trickier)

Visibility checks were introduced in #7841 for autoderef to work properly, so now we just record the first field we find unconditionally, and then overwrite it if autoderef manages to find another field in a later cycle.

Co-authored-by: Lukas Wirth <[email protected]>

Author: bors[bot]

crates/hir_ty/src/infer.rs

@@ -13,8 +13,8 @@
 //! to certain types. To record this, we use the union-find implementation from
 //! the `ena` crate, which is extracted from rustc.
 
-use std::ops::Index;
 use std::sync::Arc;
+use std::{collections::hash_map::Entry, ops::Index};
 
 use chalk_ir::{cast::Cast, DebruijnIndex, Mutability, Safety, Scalar, TypeFlags};
 use hir_def::{
@@ -459,6 +459,12 @@ impl<'a> InferenceContext<'a> {
         self.result.field_resolutions.insert(expr, field);
     }
 
+    fn write_field_resolution_if_empty(&mut self, expr: ExprId, field: FieldId) {
+        if let Entry::Vacant(entry) = self.result.field_resolutions.entry(expr) {
+            entry.insert(field);
+        }
+    }
+
     fn write_variant_resolution(&mut self, id: ExprOrPatId, variant: VariantId) {
         self.result.variant_resolutions.insert(id, variant);
     }

crates/hir_ty/src/infer/expr.rs

@@ -532,6 +532,11 @@ impl<'a> InferenceContext<'a> {
                                         .substitute(Interner, &parameters),
                                 )
                             } else {
+                                // Write down the first field resolution even if it is not visible
+                                // This aids IDE features for private fields like goto def and in
+                                // case of autoderef finding an applicable field, this will be
+                                // overwritten in a following cycle
+                                self.write_field_resolution_if_empty(tgt_expr, field);
                                 None
                             }
                         }
@@ -546,6 +551,11 @@ impl<'a> InferenceContext<'a> {
                                         .substitute(Interner, &parameters),
                                 )
                             } else {
+                                // Write down the first field resolution even if it is not visible
+                                // This aids IDE features for private fields like goto def and in
+                                // case of autoderef finding an applicable field, this will be
+                                // overwritten in a following cycle
+                                self.write_field_resolution_if_empty(tgt_expr, field);
                                 None
                             }
                         }

crates/ide_db/src/defs.rs

@@ -206,8 +206,23 @@ impl NameClass {
 
         let parent = name.syntax().parent()?;
 
-        let def = if let Some(item) = ast::Item::cast(parent.clone()) {
-            match item {
+        let definition = match_ast! {
+            match parent {
+                ast::Item(it) => classify_item(sema, it)?,
+                ast::IdentPat(it) => return classify_ident_pat(sema, it),
+                ast::Rename(it) => classify_rename(sema, it)?,
+                ast::SelfParam(it) => Definition::Local(sema.to_def(&it)?),
+                ast::RecordField(it) => Definition::Field(sema.to_def(&it)?),
+                ast::Variant(it) => Definition::Variant(sema.to_def(&it)?),
+                ast::TypeParam(it) => Definition::GenericParam(sema.to_def(&it)?.into()),
+                ast::ConstParam(it) => Definition::GenericParam(sema.to_def(&it)?.into()),
+                _ => return None,
+            }
+        };
+        return Some(NameClass::Definition(definition));
+
+        fn classify_item(sema: &Semantics<RootDatabase>, item: ast::Item) -> Option<Definition> {
+            let definition = match item {
                 ast::Item::MacroRules(it) => {
                     Definition::Macro(sema.to_def(&ast::Macro::MacroRules(it))?)
                 }
@@ -229,14 +244,20 @@ impl NameClass {
                 ast::Item::Struct(it) => Definition::Adt(hir::Adt::Struct(sema.to_def(&it)?)),
                 ast::Item::Union(it) => Definition::Adt(hir::Adt::Union(sema.to_def(&it)?)),
                 _ => return None,
-            }
-        } else if let Some(it) = ast::IdentPat::cast(parent.clone()) {
-            if let Some(def) = sema.resolve_bind_pat_to_const(&it) {
+            };
+            Some(definition)
+        }
+
+        fn classify_ident_pat(
+            sema: &Semantics<RootDatabase>,
+            ident_pat: ast::IdentPat,
+        ) -> Option<NameClass> {
+            if let Some(def) = sema.resolve_bind_pat_to_const(&ident_pat) {
                 return Some(NameClass::ConstReference(Definition::from(def)));
             }
 
-            let local = sema.to_def(&it)?;
-            let pat_parent = it.syntax().parent();
+            let local = sema.to_def(&ident_pat)?;
+            let pat_parent = ident_pat.syntax().parent();
             if let Some(record_pat_field) = pat_parent.and_then(ast::RecordPatField::cast) {
                 if record_pat_field.name_ref().is_none() {
                     if let Some(field) = sema.resolve_record_pat_field(&record_pat_field) {
@@ -247,57 +268,23 @@ impl NameClass {
                     }
                 }
             }
+            Some(NameClass::Definition(Definition::Local(local)))
+        }
 
-            Definition::Local(local)
-        } else if let Some(it) = ast::Rename::cast(parent.clone()) {
-            if let Some(use_tree) = it.syntax().parent().and_then(ast::UseTree::cast) {
+        fn classify_rename(
+            sema: &Semantics<RootDatabase>,
+            rename: ast::Rename,
+        ) -> Option<Definition> {
+            if let Some(use_tree) = rename.syntax().parent().and_then(ast::UseTree::cast) {
                 let path = use_tree.path()?;
-                let path_segment = path.segment()?;
-                let name_ref = path_segment.name_ref()?;
-                let name_ref = if name_ref.self_token().is_some() {
-                    use_tree
-                        .syntax()
-                        .parent()
-                        .as_ref()
-                        // Skip over UseTreeList
-                        .and_then(|it| {
-                            let use_tree = it.parent().and_then(ast::UseTree::cast)?;
-                            let path = use_tree.path()?;
-                            let path_segment = path.segment()?;
-                            path_segment.name_ref()
-                        })
-                        .unwrap_or(name_ref)
-                } else {
-                    name_ref
-                };
-                let name_ref_class = NameRefClass::classify(sema, &name_ref)?;
-
-                match name_ref_class {
-                    NameRefClass::Definition(def) => def,
-                    NameRefClass::FieldShorthand { local_ref: _, field_ref } => {
-                        Definition::Field(field_ref)
-                    }
-                }
+                sema.resolve_path(&path).map(Definition::from)
             } else {
-                let extern_crate = it.syntax().parent().and_then(ast::ExternCrate::cast)?;
+                let extern_crate = rename.syntax().parent().and_then(ast::ExternCrate::cast)?;
                 let krate = sema.resolve_extern_crate(&extern_crate)?;
                 let root_module = krate.root_module(sema.db);
-                Definition::Module(root_module)
-            }
-        } else {
-            match_ast! {
-                match parent {
-                    ast::SelfParam(it) => Definition::Local(sema.to_def(&it)?),
-                    ast::RecordField(it) => Definition::Field(sema.to_def(&it)?),
-                    ast::Variant(it) => Definition::Variant(sema.to_def(&it)?),
-                    ast::TypeParam(it) => Definition::GenericParam(sema.to_def(&it)?.into()),
-                    ast::ConstParam(it) => Definition::GenericParam(sema.to_def(&it)?.into()),
-                    _ => return None,
-                }
+                Some(Definition::Module(root_module))
             }
-        };
-
-        Some(NameClass::Definition(def))
+        }
     }
 
     pub fn classify_lifetime(
@@ -307,19 +294,14 @@ impl NameClass {
         let _p = profile::span("classify_lifetime").detail(|| lifetime.to_string());
         let parent = lifetime.syntax().parent()?;
 
-        match_ast! {
-            match parent {
-                ast::LifetimeParam(it) => {
-                    let def = sema.to_def(&it)?;
-                    Some(NameClass::Definition(Definition::GenericParam(def.into())))
-                },
-                ast::Label(it) => {
-                    let def = sema.to_def(&it)?;
-                    Some(NameClass::Definition(Definition::Label(def)))
-                },
-                _ => None,
-            }
+        if let Some(it) = ast::LifetimeParam::cast(parent.clone()) {
+            sema.to_def(&it).map(Into::into).map(Definition::GenericParam)
+        } else if let Some(it) = ast::Label::cast(parent.clone()) {
+            sema.to_def(&it).map(Definition::Label)
+        } else {
+            None
         }
+        .map(NameClass::Definition)
     }
 }