Diagnose unresolved method calls

Veykril · Veykril · commit e7485a04169f · 2023-03-03T20:43:13.000+01:00
diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs
@@ -164,14 +164,45 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>;
 
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum InferenceDiagnostic {
-    NoSuchField { expr: ExprId },
-    PrivateField { expr: ExprId, field: FieldId },
-    PrivateAssocItem { id: ExprOrPatId, item: AssocItemId },
-    UnresolvedField { expr: ExprId, receiver: Ty, name: Name, method_with_same_name_exists: bool },
+    NoSuchField {
+        expr: ExprId,
+    },
+    PrivateField {
+        expr: ExprId,
+        field: FieldId,
+    },
+    PrivateAssocItem {
+        id: ExprOrPatId,
+        item: AssocItemId,
+    },
+    UnresolvedField {
+        expr: ExprId,
+        receiver: Ty,
+        name: Name,
+        method_with_same_name_exists: bool,
+    },
+    UnresolvedMethodCall {
+        expr: ExprId,
+        receiver: Ty,
+        name: Name,
+        /// Contains the type the field resolves to
+        field_with_same_name: Option<Ty>,
+    },
     // FIXME: Make this proper
-    BreakOutsideOfLoop { expr: ExprId, is_break: bool, bad_value_break: bool },
-    MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize },
-    ExpectedFunction { call_expr: ExprId, found: Ty },
+    BreakOutsideOfLoop {
+        expr: ExprId,
+        is_break: bool,
+        bad_value_break: bool,
+    },
+    MismatchedArgCount {
+        call_expr: ExprId,
+        expected: usize,
+        found: usize,
+    },
+    ExpectedFunction {
+        call_expr: ExprId,
+        found: Ty,
+    },
 }
 
 /// A mismatch between an expected and an inferred type.
@@ -509,12 +540,28 @@ impl<'a> InferenceContext<'a> {
         }
         result.diagnostics.retain_mut(|diagnostic| {
             if let InferenceDiagnostic::ExpectedFunction { found: ty, .. }
-            | InferenceDiagnostic::UnresolvedField { receiver: ty, .. } = diagnostic
+            | InferenceDiagnostic::UnresolvedField { receiver: ty, .. }
+            | InferenceDiagnostic::UnresolvedMethodCall { receiver: ty, .. } = diagnostic
             {
                 *ty = table.resolve_completely(ty.clone());
+                // FIXME: Remove this when we are on par with rustc in terms of inference
                 if ty.is_unknown() {
                     return false;
                 }
+
+                if let InferenceDiagnostic::UnresolvedMethodCall { field_with_same_name, .. } =
+                    diagnostic
+                {
+                    let clear = if let Some(ty) = field_with_same_name {
+                        *ty = table.resolve_completely(ty.clone());
+                        ty.is_unknown()
+                    } else {
+                        false
+                    };
+                    if clear {
+                        *field_with_same_name = None;
+                    }
+                }
             }
             true
         });
diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs
@@ -1212,12 +1212,14 @@ impl<'a> InferenceContext<'a> {
         }
     }
 
-    fn infer_field_access(&mut self, tgt_expr: ExprId, expr: ExprId, name: &Name) -> Ty {
-        let receiver_ty = self.infer_expr_inner(expr, &Expectation::none());
-
+    fn lookup_field(
+        &mut self,
+        receiver_ty: &Ty,
+        name: &Name,
+    ) -> Option<(Ty, Option<FieldId>, Vec<Adjustment>, bool)> {
         let mut autoderef = Autoderef::new(&mut self.table, receiver_ty.clone());
         let mut private_field = None;
-        let ty = autoderef.by_ref().find_map(|(derefed_ty, _)| {
+        let res = autoderef.by_ref().find_map(|(derefed_ty, _)| {
             let (field_id, parameters) = match derefed_ty.kind(Interner) {
                 TyKind::Tuple(_, substs) => {
                     return name.as_tuple_index().and_then(|idx| {
@@ -1226,6 +1228,7 @@ impl<'a> InferenceContext<'a> {
                             .get(idx)
                             .map(|a| a.assert_ty_ref(Interner))
                             .cloned()
+                            .map(|ty| (None, ty))
                     });
                 }
                 TyKind::Adt(AdtId(hir_def::AdtId::StructId(s)), parameters) => {
@@ -1244,58 +1247,81 @@ impl<'a> InferenceContext<'a> {
                 .is_visible_from(self.db.upcast(), self.resolver.module());
             if !is_visible {
                 if private_field.is_none() {
-                    private_field = Some(field_id);
+                    private_field = Some((field_id, parameters));
                 }
                 return None;
             }
-            // can't have `write_field_resolution` here because `self.table` is borrowed :(
-            self.result.field_resolutions.insert(tgt_expr, field_id);
             let ty = self.db.field_types(field_id.parent)[field_id.local_id]
                 .clone()
                 .substitute(Interner, &parameters);
-            Some(ty)
+            Some((Some(field_id), ty))
         });
-        let ty = match ty {
-            Some(ty) => {
+
+        Some(match res {
+            Some((field_id, ty)) => {
+                let adjustments = auto_deref_adjust_steps(&autoderef);
+                let ty = self.insert_type_vars(ty);
+                let ty = self.normalize_associated_types_in(ty);
+
+                (ty, field_id, adjustments, true)
+            }
+            None => {
+                let (field_id, subst) = private_field?;
                 let adjustments = auto_deref_adjust_steps(&autoderef);
-                self.write_expr_adj(expr, adjustments);
+                let ty = self.db.field_types(field_id.parent)[field_id.local_id]
+                    .clone()
+                    .substitute(Interner, &subst);
                 let ty = self.insert_type_vars(ty);
                 let ty = self.normalize_associated_types_in(ty);
+
+                (ty, Some(field_id), adjustments, false)
+            }
+        })
+    }
+
+    fn infer_field_access(&mut self, tgt_expr: ExprId, receiver: ExprId, name: &Name) -> Ty {
+        let receiver_ty = self.infer_expr_inner(receiver, &Expectation::none());
+        match self.lookup_field(&receiver_ty, name) {
+            Some((ty, field_id, adjustments, is_public)) => {
+                self.write_expr_adj(receiver, adjustments);
+                if let Some(field_id) = field_id {
+                    self.result.field_resolutions.insert(tgt_expr, field_id);
+                }
+                if !is_public {
+                    if let Some(field) = field_id {
+                        // FIXME: Merge this diagnostic into UnresolvedField?
+                        self.result
+                            .diagnostics
+                            .push(InferenceDiagnostic::PrivateField { expr: tgt_expr, field });
+                    }
+                }
                 ty
             }
-            _ => {
-                // Write down the first private field resolution if we found no field
-                // This aids IDE features for private fields like goto def
-                if let Some(field) = private_field {
-                    self.result.field_resolutions.insert(tgt_expr, field);
-                    // FIXME: Merge this diagnostic into UnresolvedField
-                    self.result
-                        .diagnostics
-                        .push(InferenceDiagnostic::PrivateField { expr: tgt_expr, field });
-                } else {
-                    // no field found, try looking for a method of the same name
+            None => {
+                // no field found,
+                let method_with_same_name_exists = {
                     let canonicalized_receiver = self.canonicalize(receiver_ty.clone());
                     let traits_in_scope = self.resolver.traits_in_scope(self.db.upcast());
 
-                    let resolved = method_resolution::lookup_method(
+                    method_resolution::lookup_method(
                         self.db,
                         &canonicalized_receiver.value,
                         self.trait_env.clone(),
                         &traits_in_scope,
                         VisibleFromModule::Filter(self.resolver.module()),
                         name,
-                    );
-                    self.result.diagnostics.push(InferenceDiagnostic::UnresolvedField {
-                        expr: tgt_expr,
-                        receiver: receiver_ty,
-                        name: name.clone(),
-                        method_with_same_name_exists: resolved.is_some(),
-                    });
-                }
+                    )
+                    .is_some()
+                };
+                self.result.diagnostics.push(InferenceDiagnostic::UnresolvedField {
+                    expr: tgt_expr,
+                    receiver: receiver_ty,
+                    name: name.clone(),
+                    method_with_same_name_exists,
+                });
                 self.err_ty()
             }
-        };
-        ty
+        }
     }
 
     fn infer_method_call(
@@ -1335,11 +1361,30 @@ impl<'a> InferenceContext<'a> {
                 }
                 (ty, self.db.value_ty(func.into()), substs)
             }
-            None => (
-                receiver_ty,
-                Binders::empty(Interner, self.err_ty()),
-                Substitution::empty(Interner),
-            ),
+            None => {
+                let field_with_same_name_exists = match self.lookup_field(&receiver_ty, method_name)
+                {
+                    Some((ty, field_id, adjustments, _public)) => {
+                        self.write_expr_adj(receiver, adjustments);
+                        if let Some(field_id) = field_id {
+                            self.result.field_resolutions.insert(tgt_expr, field_id);
+                        }
+                        Some(ty)
+                    }
+                    None => None,
+                };
+                self.result.diagnostics.push(InferenceDiagnostic::UnresolvedMethodCall {
+                    expr: tgt_expr,
+                    receiver: receiver_ty.clone(),
+                    name: method_name.clone(),
+                    field_with_same_name: field_with_same_name_exists,
+                });
+                (
+                    receiver_ty,
+                    Binders::empty(Interner, self.err_ty()),
+                    Substitution::empty(Interner),
+                )
+            }
         };
         let method_ty = method_ty.substitute(Interner, &substs);
         self.register_obligations_for_call(&method_ty);
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
@@ -51,6 +51,7 @@ diagnostics![
     UnresolvedField,
     UnresolvedImport,
     UnresolvedMacroCall,
+    UnresolvedMethodCall,
     UnresolvedModule,
     UnresolvedProcMacro,
 ];
@@ -146,6 +147,14 @@ pub struct UnresolvedField {
     pub method_with_same_name_exists: bool,
 }
 
+#[derive(Debug)]
+pub struct UnresolvedMethodCall {
+    pub expr: InFile<AstPtr<ast::Expr>>,
+    pub receiver: Type,
+    pub name: Name,
+    pub field_with_same_name: Option<Type>,
+}
+
 #[derive(Debug)]
 pub struct PrivateField {
     pub expr: InFile<AstPtr<ast::Expr>>,
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -89,7 +89,7 @@ pub use crate::{
         MissingMatchArms, MissingUnsafe, NoSuchField, PrivateAssocItem, PrivateField,
         ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro,
         UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
-        UnresolvedModule, UnresolvedProcMacro,
+        UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},
@@ -1441,6 +1441,26 @@ impl DefWithBody {
                         .into(),
                     )
                 }
+                hir_ty::InferenceDiagnostic::UnresolvedMethodCall {
+                    expr,
+                    receiver,
+                    name,
+                    field_with_same_name,
+                } => {
+                    let expr = expr_syntax(*expr);
+
+                    acc.push(
+                        UnresolvedMethodCall {
+                            expr,
+                            name: name.clone(),
+                            receiver: Type::new(db, DefWithBodyId::from(self), receiver.clone()),
+                            field_with_same_name: field_with_same_name
+                                .clone()
+                                .map(|ty| Type::new(db, DefWithBodyId::from(self), ty)),
+                        }
+                        .into(),
+                    )
+                }
             }
         }
         for (pat_or_expr, mismatch) in infer.type_mismatches() {
diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs
@@ -83,6 +83,14 @@ impl From<NoHashHashMap<FileId, TextEdit>> for SourceChange {
     }
 }
 
+impl FromIterator<(FileId, TextEdit)> for SourceChange {
+    fn from_iter<T: IntoIterator<Item = (FileId, TextEdit)>>(iter: T) -> Self {
+        let mut this = SourceChange::default();
+        this.extend(iter);
+        this
+    }
+}
+
 pub struct SourceChangeBuilder {
     pub edit: TextEditBuilder,
     pub file_id: FileId,
diff --git a/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs b/crates/ide-diagnostics/src/handlers/replace_filter_map_next_with_find_map.rs
@@ -55,7 +55,18 @@ fn fixes(
 
 #[cfg(test)]
 mod tests {
-    use crate::tests::{check_diagnostics, check_fix};
+    use crate::{
+        tests::{check_diagnostics_with_config, check_fix},
+        DiagnosticsConfig,
+    };
+
+    #[track_caller]
+    pub(crate) fn check_diagnostics(ra_fixture: &str) {
+        let mut config = DiagnosticsConfig::test_sample();
+        config.disabled.insert("inactive-code".to_string());
+        config.disabled.insert("unresolved-method".to_string());
+        check_diagnostics_with_config(config, ra_fixture)
+    }
 
     #[test]
     fn replace_filter_map_next_with_find_map2() {
diff --git a/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/crates/ide-diagnostics/src/handlers/unresolved_method.rs
diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs

Original file line number	Diff line number	Diff line change
`@@ -83,6 +83,14 @@ impl From<NoHashHashMap<FileId, TextEdit>> for SourceChange {`
`83`	`83`	`}`
`84`	`84`	`}`
`85`	`85`
	`86`	`+impl FromIterator<(FileId, TextEdit)> for SourceChange {`
	`87`	`+ fn from_iter<T: IntoIterator<Item = (FileId, TextEdit)>>(iter: T) -> Self {`
	`88`	`+ let mut this = SourceChange::default();`
	`89`	`+ this.extend(iter);`
	`90`	`+ this`
	`91`	`+ }`
	`92`	`+}`
	`93`	`+`
`86`	`94`	`pub struct SourceChangeBuilder {`
`87`	`95`	`pub edit: TextEditBuilder,`
`88`	`96`	`pub file_id: FileId,`