Diagnose unresolved method calls

8 files changed, 320 insertions, 48 deletions

diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs
index a663a568b91..22dcea8fcd4 100644
--- 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
index 531a359a96a..02024e1ea78 100644
--- 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
index 58d02479e57..b30c664e24f 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -51,6 +51,7 @@ diagnostics![
     UnresolvedField,
     UnresolvedImport,
     UnresolvedMacroCall,
+    UnresolvedMethodCall,
     UnresolvedModule,
     UnresolvedProcMacro,
 ];
@@ -147,6 +148,14 @@ pub struct UnresolvedField {
 }
 
 #[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>>,
     pub field: Field,
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index bac6d4cd854..269c45943e5 100644
--- 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
index 8e338061df4..936354f2961 100644
--- 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
index 9826e1c707e..a0c276cc332 100644
--- 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
new file mode 100644
index 00000000000..0d1f91f02c3
--- /dev/null
+++ b/crates/ide-diagnostics/src/handlers/unresolved_method.rs
@@ -0,0 +1,130 @@
+use hir::{db::AstDatabase, HirDisplay};
+use ide_db::{
+    assists::{Assist, AssistId, AssistKind},
+    base_db::FileRange,
+    label::Label,
+    source_change::SourceChange,
+};
+use syntax::{ast, AstNode, TextRange};
+use text_edit::TextEdit;
+
+use crate::{Diagnostic, DiagnosticsContext};
+
+// Diagnostic: unresolved-method
+//
+// This diagnostic is triggered if a method does not exist on a given type.
+pub(crate) fn unresolved_method(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::UnresolvedMethodCall,
+) -> Diagnostic {
+    let field_suffix = if d.field_with_same_name.is_some() {
+        ", but a field with a similar name exists"
+    } else {
+        ""
+    };
+    Diagnostic::new(
+        "unresolved-method",
+        format!(
+            "no method `{}` on type `{}`{field_suffix}",
+            d.name,
+            d.receiver.display(ctx.sema.db)
+        ),
+        ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
+    )
+    .with_fixes(fixes(ctx, d))
+}
+
+fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option<Vec<Assist>> {
+    if let Some(ty) = &d.field_with_same_name {
+        field_fix(ctx, d, ty)
+    } else {
+        // FIXME: add quickfix
+        None
+    }
+}
+
+fn field_fix(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::UnresolvedMethodCall,
+    ty: &hir::Type,
+) -> Option<Vec<Assist>> {
+    if !ty.impls_fnonce(ctx.sema.db) {
+        return None;
+    }
+    let expr_ptr = &d.expr;
+    let root = ctx.sema.db.parse_or_expand(expr_ptr.file_id)?;
+    let expr = expr_ptr.value.to_node(&root);
+    let (file_id, range) = match expr {
+        ast::Expr::MethodCallExpr(mcall) => {
+            let FileRange { range, file_id } =
+                ctx.sema.original_range_opt(mcall.receiver()?.syntax())?;
+            let FileRange { range: range2, file_id: file_id2 } =
+                ctx.sema.original_range_opt(mcall.name_ref()?.syntax())?;
+            if file_id != file_id2 {
+                return None;
+            }
+            (file_id, TextRange::new(range.start(), range2.end()))
+        }
+        _ => return None,
+    };
+    Some(vec![Assist {
+        id: AssistId("expected-method-found-field-fix", AssistKind::QuickFix),
+        label: Label::new("Use parentheses to call the value of the field".to_string()),
+        group: None,
+        target: range,
+        source_change: Some(SourceChange::from_iter([
+            (file_id, TextEdit::insert(range.start(), "(".to_owned())),
+            (file_id, TextEdit::insert(range.end(), ")".to_owned())),
+        ])),
+        trigger_signature_help: false,
+    }])
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_diagnostics, check_fix};
+
+    #[test]
+    fn smoke_test() {
+        check_diagnostics(
+            r#"
+fn main() {
+    ().foo();
+ // ^^^^^^^^ error: no method `foo` on type `()`
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn field() {
+        check_diagnostics(
+            r#"
+struct Foo { bar: i32 }
+fn foo() {
+    Foo { bar: i32 }.bar();
+ // ^^^^^^^^^^^^^^^^^^^^^^ error: no method `bar` on type `Foo`, but a field with a similar name exists
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn callable_field() {
+        check_fix(
+            r#"
+//- minicore: fn
+struct Foo { bar: fn() }
+fn foo() {
+    Foo { bar: foo }.b$0ar();
+}
+"#,
+            r#"
+struct Foo { bar: fn() }
+fn foo() {
+    (Foo { bar: foo }.bar)();
+}
+"#,
+        );
+    }
+}
diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
index a0e8bca5cb9..c8635ff8011 100644
--- a/crates/ide-diagnostics/src/lib.rs
+++ b/crates/ide-diagnostics/src/lib.rs
@@ -45,6 +45,7 @@ mod handlers {
     pub(crate) mod unimplemented_builtin_macro;
     pub(crate) mod unresolved_extern_crate;
     pub(crate) mod unresolved_field;
+    pub(crate) mod unresolved_method;
     pub(crate) mod unresolved_import;
     pub(crate) mod unresolved_macro_call;
     pub(crate) mod unresolved_module;
@@ -271,6 +272,7 @@ pub fn diagnostics(
             AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d, config.proc_macros_enabled, config.proc_attr_macros_enabled),
             AnyDiagnostic::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d),
             AnyDiagnostic::UnresolvedField(d) => handlers::unresolved_field::unresolved_field(&ctx, &d),
+            AnyDiagnostic::UnresolvedMethodCall(d) => handlers::unresolved_method::unresolved_method(&ctx, &d),
 
             AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) {
                 Some(it) => it,