Provide more and more accurate suggestions when calling the wrong method

estebank · estebank · commit d30dfb0af72a · 2024-02-22T18:04:55.000Z
```
error[E0308]: mismatched types
  --&gt; $DIR/rustc_confusables_std_cases.rs:20:14
   |
LL |     x.append(42);
   |       ------ ^^ expected `&amp;mut Vec&lt;{integer}&gt;`, found integer
   |       |
   |       arguments to this method are incorrect
   |
   = note: expected mutable reference `&amp;mut Vec&lt;{integer}&gt;`
                           found type `{integer}`
note: method defined here
  --&gt; $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
help: you might have meant to use `push`
   |
LL |     x.push(42);
   |       ~~~~
```
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
@@ -1,7 +1,11 @@
 use crate::coercion::CoerceMany;
 use crate::errors::SuggestPtrNullMut;
 use crate::fn_ctxt::arg_matrix::{ArgMatrix, Compatibility, Error, ExpectedIdx, ProvidedIdx};
+use crate::fn_ctxt::infer::FnCall;
 use crate::gather_locals::Declaration;
+use crate::method::probe::IsSuggestion;
+use crate::method::probe::Mode::MethodCall;
+use crate::method::probe::ProbeScope::TraitsInScope;
 use crate::method::MethodCallee;
 use crate::TupleArgumentsFlag::*;
 use crate::{errors, Expectation::*};
@@ -532,25 +536,111 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let callee_ty = callee_expr
             .and_then(|callee_expr| self.typeck_results.borrow().expr_ty_adjusted_opt(callee_expr));
 
+        // Obtain another method on `Self` that have similar name.
+        let similar_assoc = |call_name: Ident| -> Option<(ty::AssocItem, ty::FnSig<'_>)> {
+            if let Some(callee_ty) = callee_ty
+                && let Ok(Some(assoc)) = self.probe_op(
+                    call_name.span,
+                    MethodCall,
+                    Some(call_name),
+                    None,
+                    IsSuggestion(true),
+                    callee_ty.peel_refs(),
+                    callee_expr.unwrap().hir_id,
+                    TraitsInScope,
+                    |mut ctxt| ctxt.probe_for_similar_candidate(),
+                )
+                && let ty::AssocKind::Fn = assoc.kind
+                && assoc.fn_has_self_parameter
+            {
+                let fn_sig =
+                    if let ty::Adt(_, args) = callee_ty.peel_refs().kind() {
+                        let args = ty::GenericArgs::identity_for_item(tcx, assoc.def_id)
+                            .rebase_onto(tcx, assoc.container_id(tcx), args);
+                        tcx.fn_sig(assoc.def_id).instantiate(tcx, args)
+                    } else {
+                        tcx.fn_sig(assoc.def_id).instantiate_identity()
+                    };
+                let fn_sig =
+                    self.instantiate_binder_with_fresh_vars(call_name.span, FnCall, fn_sig);
+                Some((assoc, fn_sig));
+            }
+            None
+        };
+
         let suggest_confusable = |err: &mut Diagnostic| {
             if let Some(call_name) = call_ident
                 && let Some(callee_ty) = callee_ty
             {
-                // FIXME: check in the following order
-                //        - methods marked as `rustc_confusables` with the provided arguments (done)
-                //        - methods marked as `rustc_confusables` with the right number of arguments
-                //        - methods marked as `rustc_confusables` (done)
-                //        - methods with the same argument type/count and short levenshtein distance
-                //        - methods with short levenshtein distance
-                //        - methods with the same argument type/count
+                let input_types: Vec<Ty<'_>> = provided_arg_tys.iter().map(|(ty, _)| *ty).collect();
+                // Check for other methods in the following order
+                //  - methods marked as `rustc_confusables` with the provided arguments
+                //  - methods with the same argument type/count and short levenshtein distance
+                //  - methods marked as `rustc_confusables` (done)
+                //  - methods with short levenshtein distance
+
+                // Look for commonly confusable method names considering arguments.
                 self.confusable_method_name(
                     err,
                     callee_ty.peel_refs(),
                     call_name,
-                    Some(provided_arg_tys.iter().map(|(ty, _)| *ty).collect()),
+                    Some(input_types.clone()),
                 )
                 .or_else(|| {
+                    // Look for method names with short levenshtein distance, considering arguments.
+                    if let Some((assoc, fn_sig)) = similar_assoc(call_name)
+                        && fn_sig.inputs()[1..]
+                            .iter()
+                            .zip(input_types.iter())
+                            .all(|(expected, found)| self.can_coerce(*expected, *found))
+                        && fn_sig.inputs()[1..].len() == input_types.len()
+                    {
+                        err.span_suggestion_verbose(
+                            call_name.span,
+                            format!("you might have meant to use `{}`", assoc.name),
+                            assoc.name,
+                            Applicability::MaybeIncorrect,
+                        );
+                        return Some(assoc.name);
+                    }
+                    None
+                })
+                .or_else(|| {
+                    // Look for commonly confusable method names disregarding arguments.
                     self.confusable_method_name(err, callee_ty.peel_refs(), call_name, None)
+                })
+                .or_else(|| {
+                    // Look for similarly named methods with levenshtein distance with the right
+                    // number of arguments.
+                    if let Some((assoc, fn_sig)) = similar_assoc(call_name)
+                        && fn_sig.inputs()[1..].len() == input_types.len()
+                    {
+                        err.span_note(
+                            tcx.def_span(assoc.def_id),
+                            format!(
+                                "there's is a method with similar name `{}`, but the arguments \
+                                 don't match",
+                                assoc.name,
+                            ),
+                        );
+                        return Some(assoc.name);
+                    }
+                    None
+                })
+                .or_else(|| {
+                    // Fallthrough: look for similarly named methods with levenshtein distance.
+                    if let Some((assoc, _)) = similar_assoc(call_name) {
+                        err.span_note(
+                            tcx.def_span(assoc.def_id),
+                            format!(
+                                "there's is a method with similar name `{}`, but their argument \
+                                 count doesn't match",
+                                assoc.name,
+                            ),
+                        );
+                        return Some(assoc.name);
+                    }
+                    None
                 });
             }
         };
diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs
@@ -54,7 +54,7 @@ pub use self::PickKind::*;
 #[derive(Clone, Copy, Debug)]
 pub struct IsSuggestion(pub bool);
 
-struct ProbeContext<'a, 'tcx> {
+pub(crate) struct ProbeContext<'a, 'tcx> {
     fcx: &'a FnCtxt<'a, 'tcx>,
     span: Span,
     mode: Mode,
@@ -355,7 +355,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         .unwrap()
     }
 
-    fn probe_op<OP, R>(
+    pub(crate) fn probe_op<OP, R>(
         &'a self,
         span: Span,
         mode: Mode,
@@ -1750,7 +1750,9 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
     /// Similarly to `probe_for_return_type`, this method attempts to find the best matching
     /// candidate method where the method name may have been misspelled. Similarly to other
     /// edit distance based suggestions, we provide at most one such suggestion.
-    fn probe_for_similar_candidate(&mut self) -> Result<Option<ty::AssocItem>, MethodError<'tcx>> {
+    pub(crate) fn probe_for_similar_candidate(
+        &mut self,
+    ) -> Result<Option<ty::AssocItem>, MethodError<'tcx>> {
         debug!("probing for method names similar to {:?}", self.method_name);
 
         self.probe(|_| {
@@ -1942,7 +1944,21 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         let hir_id = self.fcx.tcx.local_def_id_to_hir_id(local_def_id);
         let attrs = self.fcx.tcx.hir().attrs(hir_id);
         for attr in attrs {
-            let sym::doc = attr.name_or_empty() else {
+            if sym::doc == attr.name_or_empty() {
+            } else if sym::rustc_confusables == attr.name_or_empty() {
+                let Some(confusables) = attr.meta_item_list() else {
+                    continue;
+                };
+                // #[rustc_confusables("foo", "bar"))]
+                for n in confusables {
+                    if let Some(lit) = n.lit()
+                        && name.as_str() == lit.symbol.as_str()
+                    {
+                        return true;
+                    }
+                }
+                continue;
+            } else {
                 continue;
             };
             let Some(values) = attr.meta_item_list() else {
diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs
@@ -23,6 +23,7 @@ use rustc_hir::PatKind::Binding;
 use rustc_hir::PathSegment;
 use rustc_hir::{ExprKind, Node, QPath};
 use rustc_infer::infer::{
+    self,
     type_variable::{TypeVariableOrigin, TypeVariableOriginKind},
     RegionVariableOrigin,
 };
@@ -1234,7 +1235,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             label_span_not_found(&mut err);
         }
 
-        let confusable_suggested = self.confusable_method_name(&mut err, rcvr_ty, item_name, None);
+        let confusable_suggested = self.confusable_method_name(
+            &mut err,
+            rcvr_ty,
+            item_name,
+            args.map(|args| {
+                args.iter()
+                    .map(|expr| {
+                        self.node_ty_opt(expr.hir_id).unwrap_or_else(|| {
+                            self.next_ty_var(TypeVariableOrigin {
+                                kind: TypeVariableOriginKind::MiscVariable,
+                                span: expr.span,
+                            })
+                        })
+                    })
+                    .collect()
+            }),
+        );
 
         // Don't suggest (for example) `expr.field.clone()` if `expr.clone()`
         // can't be called due to `typeof(expr): Clone` not holding.
@@ -1421,9 +1438,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         err: &mut Diagnostic,
         rcvr_ty: Ty<'tcx>,
         item_name: Ident,
-        args: Option<Vec<Ty<'tcx>>>,
+        call_args: Option<Vec<Ty<'tcx>>>,
     ) -> Option<Symbol> {
-        if let ty::Adt(adt, _) = rcvr_ty.kind() {
+        if let ty::Adt(adt, adt_args) = rcvr_ty.kind() {
             for inherent_impl_did in self.tcx.inherent_impls(adt.did()).into_iter().flatten() {
                 for inherent_method in
                     self.tcx.associated_items(inherent_impl_did).in_definition_order()
@@ -1432,29 +1449,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         self.tcx.get_attr(inherent_method.def_id, sym::rustc_confusables)
                         && let Some(candidates) = parse_confusables(attr)
                         && candidates.contains(&item_name.name)
+                        && let ty::AssocKind::Fn = inherent_method.kind
                     {
-                        let mut matches_args = args.is_none();
-                        if let ty::AssocKind::Fn = inherent_method.kind
-                            && let Some(ref args) = args
-                        {
-                            let fn_sig =
-                                self.tcx.fn_sig(inherent_method.def_id).instantiate_identity();
-                            matches_args = fn_sig
-                                .inputs()
-                                .skip_binder()
+                        let args =
+                            ty::GenericArgs::identity_for_item(self.tcx, inherent_method.def_id)
+                                .rebase_onto(
+                                    self.tcx,
+                                    inherent_method.container_id(self.tcx),
+                                    adt_args,
+                                );
+                        let fn_sig =
+                            self.tcx.fn_sig(inherent_method.def_id).instantiate(self.tcx, args);
+                        let fn_sig = self.instantiate_binder_with_fresh_vars(
+                            item_name.span,
+                            infer::FnCall,
+                            fn_sig,
+                        );
+                        if let Some(ref args) = call_args
+                            && fn_sig.inputs()[1..]
                                 .iter()
-                                .skip(1)
                                 .zip(args.into_iter())
-                                .all(|(expected, found)| self.can_coerce(*expected, *found));
-                        }
-                        if matches_args {
+                                .all(|(expected, found)| self.can_coerce(*expected, *found))
+                            && fn_sig.inputs()[1..].len() == args.len()
+                        {
                             err.span_suggestion_verbose(
                                 item_name.span,
                                 format!("you might have meant to use `{}`", inherent_method.name),
                                 inherent_method.name,
                                 Applicability::MaybeIncorrect,
                             );
                             return Some(inherent_method.name);
+                        } else if let None = call_args {
+                            err.span_note(
+                                self.tcx.def_span(inherent_method.def_id),
+                                format!(
+                                    "you might have meant to use method `{}`",
+                                    inherent_method.name,
+                                ),
+                            );
+                            return Some(inherent_method.name);
                         }
                     }
                 }
diff --git a/tests/ui/attributes/rustc_confusables_std_cases.rs b/tests/ui/attributes/rustc_confusables_std_cases.rs
@@ -17,6 +17,8 @@ fn main() {
     x.size(); //~ ERROR E0599
     //~^ HELP you might have meant to use `len`
     //~| HELP there is a method with a similar name
+    x.append(42); //~ ERROR E0308
+    //~^ HELP you might have meant to use `push`
     String::new().push(""); //~ ERROR E0308
     //~^ HELP you might have meant to use `push_str`
     String::new().append(""); //~ ERROR E0599
diff --git a/tests/ui/attributes/rustc_confusables_std_cases.stderr b/tests/ui/attributes/rustc_confusables_std_cases.stderr
@@ -58,7 +58,24 @@ LL |     x.resize();
    |       ~~~~~~
 
 error[E0308]: mismatched types
-  --> $DIR/rustc_confusables_std_cases.rs:20:24
+  --> $DIR/rustc_confusables_std_cases.rs:20:14
+   |
+LL |     x.append(42);
+   |       ------ ^^ expected `&mut Vec<{integer}>`, found integer
+   |       |
+   |       arguments to this method are incorrect
+   |
+   = note: expected mutable reference `&mut Vec<{integer}>`
+                           found type `{integer}`
+note: method defined here
+  --> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
+help: you might have meant to use `push`
+   |
+LL |     x.push(42);
+   |       ~~~~
+
+error[E0308]: mismatched types
+  --> $DIR/rustc_confusables_std_cases.rs:22:24
    |
 LL |     String::new().push("");
    |                   ---- ^^ expected `char`, found `&str`
@@ -73,7 +90,7 @@ LL |     String::new().push_str("");
    |                   ~~~~~~~~
 
 error[E0599]: no method named `append` found for struct `String` in the current scope
-  --> $DIR/rustc_confusables_std_cases.rs:22:19
+  --> $DIR/rustc_confusables_std_cases.rs:24:19
    |
 LL |     String::new().append("");
    |                   ^^^^^^ method not found in `String`
@@ -83,7 +100,7 @@ help: you might have meant to use `push_str`
 LL |     String::new().push_str("");
    |                   ~~~~~~~~
 
-error: aborting due to 7 previous errors
+error: aborting due to 8 previous errors
 
 Some errors have detailed explanations: E0308, E0599.
 For more information about an error, try `rustc --explain E0308`.
