On type error of method call arguments, look at confusables for suggestion

Author: estebank

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -451,7 +451,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         call_expr: &'tcx hir::Expr<'tcx>,
     ) -> ErrorGuaranteed {
         // Next, let's construct the error
-        let (error_span, full_call_span, call_name, is_method) = match &call_expr.kind {
+        let (error_span, call_ident, full_call_span, call_name, is_method) = match &call_expr.kind {
             hir::ExprKind::Call(
                 hir::Expr { hir_id, span, kind: hir::ExprKind::Path(qpath), .. },
                 _,
@@ -463,20 +463,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         CtorOf::Struct => "struct",
                         CtorOf::Variant => "enum variant",
                     };
-                    (call_span, *span, name, false)
+                    (call_span, None, *span, name, false)
                 } else {
-                    (call_span, *span, "function", false)
+                    (call_span, None, *span, "function", false)
                 }
             }
-            hir::ExprKind::Call(hir::Expr { span, .. }, _) => (call_span, *span, "function", false),
+            hir::ExprKind::Call(hir::Expr { span, .. }, _) => {
+                (call_span, None, *span, "function", false)
+            }
             hir::ExprKind::MethodCall(path_segment, _, _, span) => {
                 let ident_span = path_segment.ident.span;
                 let ident_span = if let Some(args) = path_segment.args {
                     ident_span.with_hi(args.span_ext.hi())
                 } else {
                     ident_span
                 };
-                (*span, ident_span, "method", true)
+                (*span, Some(path_segment.ident), ident_span, "method", true)
             }
             k => span_bug!(call_span, "checking argument types on a non-call: `{:?}`", k),
         };
@@ -530,6 +532,28 @@ 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));
 
+        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
+                self.confusable_method_name(
+                    err,
+                    callee_ty.peel_refs(),
+                    call_name,
+                    Some(provided_arg_tys.iter().map(|(ty, _)| *ty).collect()),
+                )
+                .or_else(|| {
+                    self.confusable_method_name(err, callee_ty.peel_refs(), call_name, None)
+                });
+            }
+        };
         // A "softer" version of the `demand_compatible`, which checks types without persisting them,
         // and treats error types differently
         // This will allow us to "probe" for other argument orders that would likely have been correct
@@ -694,6 +718,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         Some(mismatch_idx),
                         is_method,
                     );
+                    suggest_confusable(&mut err);
                     return err.emit();
                 }
             }
@@ -718,7 +743,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             if cfg!(debug_assertions) {
                 span_bug!(error_span, "expected errors from argument matrix");
             } else {
-                return tcx.dcx().emit_err(errors::ArgMismatchIndeterminate { span: error_span });
+                let mut err =
+                    tcx.dcx().create_err(errors::ArgMismatchIndeterminate { span: error_span });
+                suggest_confusable(&mut err);
+                return err.emit();
             }
         }
 
@@ -733,7 +761,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             let trace =
                 mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
             if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308) {
-                reported = Some(self.err_ctxt().report_and_explain_type_error(trace, *e).emit());
+                let mut err = self.err_ctxt().report_and_explain_type_error(trace, *e);
+                suggest_confusable(&mut err);
+                reported = Some(err.emit());
                 return false;
             }
             true
@@ -801,6 +831,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 Some(expected_idx.as_usize()),
                 is_method,
             );
+            suggest_confusable(&mut err);
             return err.emit();
         }
 
@@ -828,6 +859,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 .with_code(err_code.to_owned())
         };
 
+        suggest_confusable(&mut err);
         // As we encounter issues, keep track of what we want to provide for the suggestion
         let mut labels = vec![];
         // If there is a single error, we give a specific suggestion; otherwise, we change to

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -1139,33 +1139,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             label_span_not_found(&mut err);
         }
 
-        let mut confusable_suggested = None;
-        if let ty::Adt(adt, _) = rcvr_ty.kind() {
-            'outer: 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()
-                {
-                    if let Some(attr) =
-                        self.tcx.get_attr(inherent_method.def_id, sym::rustc_confusables)
-                        && let Some(candidates) = parse_confusables(attr)
-                        && candidates.contains(&item_name.name)
-                    {
-                        {
-                            err.span_suggestion_verbose(
-                                item_name.span,
-                                format!("you might have meant to use `{}`", inherent_method.name),
-                                inherent_method.name,
-                                Applicability::MaybeIncorrect,
-                            );
-                            confusable_suggested = Some(inherent_method.name);
-                            break 'outer;
-                        }
-                    }
-                }
-            }
-        }
+        let confusable_suggested = self.confusable_method_name(&mut err, rcvr_ty, item_name, None);
 
         // Don't suggest (for example) `expr.field.clone()` if `expr.clone()`
         // can't be called due to `typeof(expr): Clone` not holding.
@@ -1347,6 +1321,52 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         Some(err)
     }
 
+    pub(crate) fn confusable_method_name(
+        &self,
+        err: &mut Diagnostic,
+        rcvr_ty: Ty<'tcx>,
+        item_name: Ident,
+        args: Option<Vec<Ty<'tcx>>>,
+    ) -> Option<Symbol> {
+        if let ty::Adt(adt, _) = 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()
+                {
+                    if let Some(attr) =
+                        self.tcx.get_attr(inherent_method.def_id, sym::rustc_confusables)
+                        && let Some(candidates) = parse_confusables(attr)
+                        && candidates.contains(&item_name.name)
+                    {
+                        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()
+                                .iter()
+                                .skip(1)
+                                .zip(args.into_iter())
+                                .all(|(expected, found)| self.can_coerce(*expected, *found));
+                        }
+                        if matches_args {
+                            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);
+                        }
+                    }
+                }
+            }
+        }
+        None
+    }
     fn note_candidates_on_method_error(
         &self,
         rcvr_ty: Ty<'tcx>,

library/alloc/src/string.rs

@@ -1049,6 +1049,7 @@ impl String {
     #[cfg(not(no_global_oom_handling))]
     #[inline]
     #[stable(feature = "rust1", since = "1.0.0")]
+    #[rustc_confusables("append", "push")]
     pub fn push_str(&mut self, string: &str) {
         self.vec.extend_from_slice(string.as_bytes())
     }

tests/ui/attributes/rustc_confusables_std_cases.rs

@@ -18,4 +18,7 @@ fn main() {
     //~^ HELP you might have meant to use `len`
     //~| HELP there is a method with a similar name
     String::new().push(""); //~ ERROR E0308
+    //~^ HELP you might have meant to use `push_str`
+    String::new().append(""); //~ ERROR E0599
+    //~^ HELP you might have meant to use `push_str`
 }

tests/ui/attributes/rustc_confusables_std_cases.stderr

@@ -67,8 +67,23 @@ LL |     String::new().push("");
    |
 note: method defined here
   --> $SRC_DIR/alloc/src/string.rs:LL:COL
+help: you might have meant to use `push_str`
+   |
+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
+   |
+LL |     String::new().append("");
+   |                   ^^^^^^ method not found in `String`
+   |
+help: you might have meant to use `push_str`
+   |
+LL |     String::new().push_str("");
+   |                   ~~~~~~~~
 
-error: aborting due to 6 previous errors
+error: aborting due to 7 previous errors
 
 Some errors have detailed explanations: E0308, E0599.
 For more information about an error, try `rustc --explain E0308`.