Merge pull request #19160 from Veykril/push-f3601671f6a468a8cc0774253ddaddff

Improve error recovery when method-calling an assoc function

Author: Veykril

crates/hir-ty/src/infer.rs

@@ -236,7 +236,7 @@ pub enum InferenceDiagnostic {
         name: Name,
         /// Contains the type the field resolves to
         field_with_same_name: Option<Ty>,
-        assoc_func_with_same_name: Option<AssocItemId>,
+        assoc_func_with_same_name: Option<FunctionId>,
     },
     UnresolvedAssocItem {
         id: ExprOrPatId,

crates/hir-ty/src/infer/expr.rs

@@ -1922,21 +1922,32 @@ impl InferenceContext<'_> {
             VisibleFromModule::Filter(self.resolver.module()),
             method_name,
         );
-        let (receiver_ty, method_ty, substs) = match resolved {
+        match resolved {
             Some((adjust, func, visible)) => {
-                let (ty, adjustments) = adjust.apply(&mut self.table, receiver_ty);
-                let generics = generics(self.db.upcast(), func.into());
-                let substs = self.substs_for_method_call(generics, generic_args);
-                self.write_expr_adj(receiver, adjustments);
-                self.write_method_resolution(tgt_expr, func, substs.clone());
                 if !visible {
                     self.push_diagnostic(InferenceDiagnostic::PrivateAssocItem {
                         id: tgt_expr.into(),
                         item: func.into(),
                     })
                 }
-                (ty, self.db.value_ty(func.into()).unwrap(), substs)
+
+                let (ty, adjustments) = adjust.apply(&mut self.table, receiver_ty);
+                self.write_expr_adj(receiver, adjustments);
+
+                let generics = generics(self.db.upcast(), func.into());
+                let substs = self.substs_for_method_call(generics, generic_args);
+                self.write_method_resolution(tgt_expr, func, substs.clone());
+                self.check_method_call(
+                    tgt_expr,
+                    args,
+                    self.db.value_ty(func.into()).expect("we have a function def"),
+                    substs,
+                    ty,
+                    expected,
+                )
             }
+            // Failed to resolve, report diagnostic and try to resolve as call to field access or
+            // assoc function
             None => {
                 let field_with_same_name_exists = match self.lookup_field(&receiver_ty, method_name)
                 {
@@ -1956,12 +1967,11 @@ impl InferenceContext<'_> {
                     VisibleFromModule::Filter(self.resolver.module()),
                     Some(method_name),
                     method_resolution::LookupMode::Path,
-                    |_ty, item, visible| {
-                        if visible {
-                            Some(item)
-                        } else {
-                            None
+                    |_ty, item, visible| match item {
+                        hir_def::AssocItemId::FunctionId(function_id) if visible => {
+                            Some(function_id)
                         }
+                        _ => None,
                     },
                 );
 
@@ -1973,31 +1983,41 @@ impl InferenceContext<'_> {
                     assoc_func_with_same_name,
                 });
 
-                return match field_with_same_name_exists {
-                    Some(field_ty) => match field_ty.callable_sig(self.db) {
-                        Some(sig) => self.check_call(
-                            tgt_expr,
-                            args,
-                            field_ty,
-                            sig.params(),
-                            sig.ret().clone(),
-                            &[],
-                            true,
-                            expected,
-                        ),
-                        None => {
-                            self.check_call_arguments(tgt_expr, args, &[], &[], &[], true);
-                            field_ty
-                        }
-                    },
+                let recovered = match assoc_func_with_same_name {
+                    Some(f) => {
+                        let generics = generics(self.db.upcast(), f.into());
+                        let substs = self.substs_for_method_call(generics, generic_args);
+                        let f = self
+                            .db
+                            .value_ty(f.into())
+                            .expect("we have a function def")
+                            .substitute(Interner, &substs);
+                        let sig = f.callable_sig(self.db).expect("we have a function def");
+                        Some((f, sig, true))
+                    }
+                    None => field_with_same_name_exists.and_then(|field_ty| {
+                        let callable_sig = field_ty.callable_sig(self.db)?;
+                        Some((field_ty, callable_sig, false))
+                    }),
+                };
+                match recovered {
+                    Some((callee_ty, sig, strip_first)) => self.check_call(
+                        tgt_expr,
+                        args,
+                        callee_ty,
+                        sig.params().get(strip_first as usize..).unwrap_or(&[]),
+                        sig.ret().clone(),
+                        &[],
+                        true,
+                        expected,
+                    ),
                     None => {
                         self.check_call_arguments(tgt_expr, args, &[], &[], &[], true);
                         self.err_ty()
                     }
-                };
+                }
             }
-        };
-        self.check_method_call(tgt_expr, args, method_ty, substs, receiver_ty, expected)
+        }
     }
 
     fn check_method_call(
@@ -2067,9 +2087,10 @@ impl InferenceContext<'_> {
         expected_inputs: &[Ty],
         param_tys: &[Ty],
         skip_indices: &[u32],
-        is_varargs: bool,
+        ignore_arg_param_mismatch: bool,
     ) {
-        let arg_count_mismatch = args.len() != param_tys.len() + skip_indices.len() && !is_varargs;
+        let arg_count_mismatch =
+            !ignore_arg_param_mismatch && args.len() != param_tys.len() + skip_indices.len();
         if arg_count_mismatch {
             self.push_diagnostic(InferenceDiagnostic::MismatchedArgCount {
                 call_expr: expr,
@@ -2098,7 +2119,7 @@ impl InferenceContext<'_> {
                     continue;
                 }
 
-                while skip_indices.peek().is_some_and(|i| *i < idx as u32) {
+                while skip_indices.peek().is_some_and(|&i| i < idx as u32) {
                     skip_indices.next();
                 }
                 if skip_indices.peek().copied() == Some(idx as u32) {

crates/hir-ty/src/tests/diagnostics.rs

@@ -159,22 +159,47 @@ fn method_call_on_field() {
     check(
         r#"
 struct S {
-    field: fn() -> u32,
+    field: fn(f32) -> u32,
     field2: u32
 }
 
 fn main() {
-    let s = S { field: || 0, field2: 0 };
+    let s = S { field: |_| 0, field2: 0 };
     s.field(0);
-         // ^ type: i32
+         // ^ expected f32, got i32
  // ^^^^^^^^^^ type: u32
     s.field2(0);
           // ^ type: i32
- // ^^^^^^^^^^^ type: u32
+ // ^^^^^^^^^^^ type: {unknown}
     s.not_a_field(0);
                // ^ type: i32
  // ^^^^^^^^^^^^^^^^ type: {unknown}
 }
 "#,
     );
 }
+
+#[test]
+fn method_call_on_assoc() {
+    check(
+        r#"
+struct S;
+
+impl S {
+    fn not_a_method() -> f32 { 0.0 }
+    fn not_a_method2(this: Self, param: f32) -> Self { this }
+    fn not_a_method3(param: f32) -> Self { S }
+}
+
+fn main() {
+    S.not_a_method(0);
+ // ^^^^^^^^^^^^^^^^^ type: f32
+    S.not_a_method2(0);
+                 // ^ expected f32, got i32
+ // ^^^^^^^^^^^^^^^^^^ type: S
+    S.not_a_method3(0);
+ // ^^^^^^^^^^^^^^^^^^ type: S
+}
+"#,
+    );
+}

crates/hir-ty/src/tests/method_resolution.rs

@@ -1210,7 +1210,7 @@ impl<T> Slice<T> {
 fn main() {
     let foo: Slice<u32>;
     foo.into_vec(); // we shouldn't crash on this at least
-} //^^^^^^^^^^^^^^ {unknown}
+} //^^^^^^^^^^^^^^ ()
 "#,
     );
 }

crates/hir/src/diagnostics.rs

@@ -10,7 +10,7 @@ use hir_def::{
     hir::ExprOrPatId,
     path::{hir_segment_to_ast_segment, ModPath},
     type_ref::TypesSourceMap,
-    AssocItemId, DefWithBodyId, SyntheticSyntax,
+    DefWithBodyId, SyntheticSyntax,
 };
 use hir_expand::{name::Name, HirFileId, InFile};
 use hir_ty::{
@@ -25,7 +25,7 @@ use syntax::{
 };
 use triomphe::Arc;
 
-use crate::{AssocItem, Field, Local, Trait, Type};
+use crate::{AssocItem, Field, Function, Local, Trait, Type};
 
 pub use hir_def::VariantId;
 pub use hir_ty::{
@@ -253,7 +253,7 @@ pub struct UnresolvedMethodCall {
     pub receiver: Type,
     pub name: Name,
     pub field_with_same_name: Option<Type>,
-    pub assoc_func_with_same_name: Option<AssocItemId>,
+    pub assoc_func_with_same_name: Option<Function>,
 }
 
 #[derive(Debug)]
@@ -623,7 +623,7 @@ impl AnyDiagnostic {
                     field_with_same_name: field_with_same_name
                         .clone()
                         .map(|ty| Type::new(db, def, ty)),
-                    assoc_func_with_same_name: *assoc_func_with_same_name,
+                    assoc_func_with_same_name: assoc_func_with_same_name.map(Into::into),
                 }
                 .into()
             }

crates/ide-diagnostics/src/handlers/unresolved_method.rs

@@ -1,4 +1,4 @@
-use hir::{db::ExpandDatabase, AssocItem, FileRange, HirDisplay, InFile};
+use hir::{db::ExpandDatabase, FileRange, HirDisplay, InFile};
 use ide_db::text_edit::TextEdit;
 use ide_db::{
     assists::{Assist, AssistId, AssistKind},
@@ -112,7 +112,7 @@ fn field_fix(
 }
 
 fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -> Option<Assist> {
-    if let Some(assoc_item_id) = d.assoc_func_with_same_name {
+    if let Some(f) = d.assoc_func_with_same_name {
         let db = ctx.sema.db;
 
         let expr_ptr = &d.expr;
@@ -127,30 +127,25 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -
         let receiver = call.receiver()?;
         let receiver_type = &ctx.sema.type_of_expr(&receiver)?.original;
 
-        let need_to_take_receiver_as_first_arg = match hir::AssocItem::from(assoc_item_id) {
-            AssocItem::Function(f) => {
-                let assoc_fn_params = f.assoc_fn_params(db);
-                if assoc_fn_params.is_empty() {
-                    false
-                } else {
-                    assoc_fn_params
-                        .first()
-                        .map(|first_arg| {
-                            // For generic type, say `Box`, take `Box::into_raw(b: Self)` as example,
-                            // type of `b` is `Self`, which is `Box<T, A>`, containing unspecified generics.
-                            // However, type of `receiver` is specified, it could be `Box<i32, Global>` or something like that,
-                            // so `first_arg.ty() == receiver_type` evaluate to `false` here.
-                            // Here add `first_arg.ty().as_adt() == receiver_type.as_adt()` as guard,
-                            // apply `.as_adt()` over `Box<T, A>` or `Box<i32, Global>` gets `Box`, so we get `true` here.
-
-                            // FIXME: it fails when type of `b` is `Box` with other generic param different from `receiver`
-                            first_arg.ty() == receiver_type
-                                || first_arg.ty().as_adt() == receiver_type.as_adt()
-                        })
-                        .unwrap_or(false)
-                }
-            }
-            _ => false,
+        let assoc_fn_params = f.assoc_fn_params(db);
+        let need_to_take_receiver_as_first_arg = if assoc_fn_params.is_empty() {
+            false
+        } else {
+            assoc_fn_params
+                .first()
+                .map(|first_arg| {
+                    // For generic type, say `Box`, take `Box::into_raw(b: Self)` as example,
+                    // type of `b` is `Self`, which is `Box<T, A>`, containing unspecified generics.
+                    // However, type of `receiver` is specified, it could be `Box<i32, Global>` or something like that,
+                    // so `first_arg.ty() == receiver_type` evaluate to `false` here.
+                    // Here add `first_arg.ty().as_adt() == receiver_type.as_adt()` as guard,
+                    // apply `.as_adt()` over `Box<T, A>` or `Box<i32, Global>` gets `Box`, so we get `true` here.
+
+                    // FIXME: it fails when type of `b` is `Box` with other generic param different from `receiver`
+                    first_arg.ty() == receiver_type
+                        || first_arg.ty().as_adt() == receiver_type.as_adt()
+                })
+                .unwrap_or(false)
         };
 
         let mut receiver_type_adt_name =