From 0e894656729662450b1cf8f879f901fd6c2d3f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 7 Feb 2024 05:13:06 +0000 Subject: [PATCH] On type error of method call arguments, look at confusables for suggestion --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 46 ++++++++++-- .../rustc_hir_typeck/src/method/suggest.rs | 74 ++++++++++++------- library/alloc/src/string.rs | 1 + .../attributes/rustc_confusables_std_cases.rs | 3 + .../rustc_confusables_std_cases.stderr | 17 ++++- 5 files changed, 106 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 2d9ec9f6bab..15dc0ba444b 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -451,7 +451,7 @@ fn report_arg_errors( 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,12 +463,14 @@ fn report_arg_errors( 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 { @@ -476,7 +478,7 @@ fn report_arg_errors( } 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 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { 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 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { Some(mismatch_idx), is_method, ); + suggest_confusable(&mut err); return err.emit(); } } @@ -718,7 +743,10 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { 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 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { 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 @@ -802,6 +832,7 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { Some(expected_idx.as_usize()), is_method, ); + suggest_confusable(&mut err); return err.emit(); } @@ -829,6 +860,7 @@ fn has_error_or_infer<'tcx>(tys: impl IntoIterator>) -> bool { .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 diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index b7ca5413243..6226f2ee796 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -1234,33 +1234,7 @@ pub fn report_no_match_method_error( 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. @@ -1442,6 +1416,52 @@ pub fn report_no_match_method_error( Some(err) } + pub(crate) fn confusable_method_name( + &self, + err: &mut Diagnostic, + rcvr_ty: Ty<'tcx>, + item_name: Ident, + args: Option>>, + ) -> Option { + 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>, diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 6ad783bee27..6dadbc8e364 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -1049,6 +1049,7 @@ pub fn as_mut_str(&mut self) -> &mut str { #[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()) } diff --git a/tests/ui/attributes/rustc_confusables_std_cases.rs b/tests/ui/attributes/rustc_confusables_std_cases.rs index f408a09a81f..eda35e4b633 100644 --- a/tests/ui/attributes/rustc_confusables_std_cases.rs +++ b/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` } diff --git a/tests/ui/attributes/rustc_confusables_std_cases.stderr b/tests/ui/attributes/rustc_confusables_std_cases.stderr index ce3bf3921dc..f69b79b4028 100644 --- a/tests/ui/attributes/rustc_confusables_std_cases.stderr +++ b/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: aborting due to 6 previous errors +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 7 previous errors Some errors have detailed explanations: E0308, E0599. For more information about an error, try `rustc --explain E0308`.