From 85ff8889e455c1e88723a6d7450042e17f91657a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 26 Dec 2022 12:00:22 -0800 Subject: [PATCH] Tweak wording --- compiler/rustc_hir_typeck/src/demand.rs | 101 +++++++++++++++--- compiler/rustc_hir_typeck/src/method/probe.rs | 20 ++-- .../suggestions/shadowed-lplace-method.stderr | 7 +- 3 files changed, 98 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index f7f492863ab..ea34cd94f8a 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -335,8 +335,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } let hir::ExprKind::Unary(hir::UnOp::Deref, deref) = lhs.kind else { return; }; let hir::ExprKind::MethodCall(path, base, args, _) = deref.kind else { return; }; - let self_ty = self.typeck_results.borrow().expr_ty_adjusted_opt(base).unwrap(); - let pick = self + let Some(self_ty) = self.typeck_results.borrow().expr_ty_adjusted_opt(base) else { return; }; + + let Ok(pick) = self .probe_for_name( probe::Mode::MethodCall, path.ident, @@ -344,9 +345,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self_ty, deref.hir_id, probe::ProbeScope::TraitsInScope, - ) - .unwrap(); - let methods = self.probe_for_name_many( + ) else { + return; + }; + let in_scope_methods = self.probe_for_name_many( + probe::Mode::MethodCall, + path.ident, + probe::IsSuggestion(true), + self_ty, + deref.hir_id, + probe::ProbeScope::TraitsInScope, + ); + let other_methods_in_scope: Vec<_> = + in_scope_methods.iter().filter(|c| c.item.def_id != pick.item.def_id).collect(); + + let all_methods = self.probe_for_name_many( probe::Mode::MethodCall, path.ident, probe::IsSuggestion(true), @@ -354,10 +367,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { deref.hir_id, probe::ProbeScope::AllTraits, ); - let suggestions: Vec<_> = methods + let suggestions: Vec<_> = all_methods .into_iter() - .filter(|m| m.def_id != pick.item.def_id) - .map(|m| { + .filter(|c| c.item.def_id != pick.item.def_id) + .map(|c| { + let m = c.item; let substs = ty::InternalSubsts::for_item(self.tcx, m.def_id, |param, _| { self.var_for_def(deref.span, param) }); @@ -389,21 +403,74 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut path_span: MultiSpan = path.ident.span.into(); path_span.push_span_label( path.ident.span, - format!( + with_no_trimmed_paths!(format!( "refers to `{}`", - with_no_trimmed_paths!(self.tcx.def_path_str(pick.item.def_id)), - ), + self.tcx.def_path_str(pick.item.def_id), + )), ); + let container_id = pick.item.container_id(self.tcx); + let container = with_no_trimmed_paths!(self.tcx.def_path_str(container_id)); + for def_id in pick.import_ids { + let hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id); + path_span.push_span_label( + self.tcx.hir().span(hir_id), + format!("`{container}` imported here"), + ); + } + let tail = with_no_trimmed_paths!(match &other_methods_in_scope[..] { + [] => return, + [candidate] => format!( + "the method of the same name on {} `{}`", + match candidate.kind { + probe::CandidateKind::InherentImplCandidate(..) => "the inherent impl for", + _ => "trait", + }, + self.tcx.def_path_str(candidate.item.container_id(self.tcx)) + ), + [.., last] if other_methods_in_scope.len() < 5 => { + format!( + "the methods of the same name on {} and `{}`", + other_methods_in_scope[..other_methods_in_scope.len() - 1] + .iter() + .map(|c| format!( + "`{}`", + self.tcx.def_path_str(c.item.container_id(self.tcx)) + )) + .collect::>() + .join(", "), + self.tcx.def_path_str(last.item.container_id(self.tcx)) + ) + } + _ => format!( + "the methods of the same name on {} other traits", + other_methods_in_scope.len() + ), + }); err.span_note( path_span, &format!( - "there are multiple methods with the same name, `{}` refers to `{}` in the method call", - path.ident, - with_no_trimmed_paths!(self.tcx.def_path_str(pick.item.def_id)), - )); + "the `{}` call is resolved to the method in `{container}`, shadowing {tail}", + path.ident, + ), + ); + if suggestions.len() > other_methods_in_scope.len() { + err.note(&format!( + "additionally, there are {} other available methods that aren't in scope", + suggestions.len() - other_methods_in_scope.len() + )); + } err.multipart_suggestions( - "you might have meant to invoke a different method, you can use the fully-qualified path", - suggestions, + &format!( + "you might have meant to call {}; you can use the fully-qualified path to call {} \ + explicitly", + if suggestions.len() == 1 { + "the other method" + } else { + "one of the other methods" + }, + if suggestions.len() == 1 { "it" } else { "one of them" }, + ), + suggestions, Applicability::MaybeIncorrect, ); } diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index a7574d4e1af..6b3fa664d9c 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -97,7 +97,7 @@ impl<'a, 'tcx> Deref for ProbeContext<'a, 'tcx> { } #[derive(Debug, Clone)] -struct Candidate<'tcx> { +pub(crate) struct Candidate<'tcx> { // Candidates are (I'm not quite sure, but they are mostly) basically // some metadata on top of a `ty::AssocItem` (without substs). // @@ -131,13 +131,13 @@ struct Candidate<'tcx> { // if `T: Sized`. xform_self_ty: Ty<'tcx>, xform_ret_ty: Option>, - item: ty::AssocItem, - kind: CandidateKind<'tcx>, - import_ids: SmallVec<[LocalDefId; 1]>, + pub(crate) item: ty::AssocItem, + pub(crate) kind: CandidateKind<'tcx>, + pub(crate) import_ids: SmallVec<[LocalDefId; 1]>, } #[derive(Debug, Clone)] -enum CandidateKind<'tcx> { +pub(crate) enum CandidateKind<'tcx> { InherentImplCandidate( SubstsRef<'tcx>, // Normalize obligations @@ -323,7 +323,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } #[instrument(level = "debug", skip(self))] - pub fn probe_for_name_many( + pub(crate) fn probe_for_name_many( &self, mode: Mode, item_name: Ident, @@ -331,7 +331,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self_ty: Ty<'tcx>, scope_expr_id: hir::HirId, scope: ProbeScope, - ) -> Vec { + ) -> Vec> { self.probe_op( item_name.span, mode, @@ -344,10 +344,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { |probe_cx| { Ok(probe_cx .inherent_candidates - .iter() - .chain(&probe_cx.extension_candidates) - // .filter(|candidate| candidate_filter(&candidate.item)) - .map(|candidate| candidate.item) + .into_iter() + .chain(probe_cx.extension_candidates) .collect()) }, ) diff --git a/src/test/ui/suggestions/shadowed-lplace-method.stderr b/src/test/ui/suggestions/shadowed-lplace-method.stderr index 080600128a3..91d0d1200d4 100644 --- a/src/test/ui/suggestions/shadowed-lplace-method.stderr +++ b/src/test/ui/suggestions/shadowed-lplace-method.stderr @@ -8,12 +8,15 @@ LL | *rc.borrow_mut() = false; | = note: expected struct `Rc>` found type `bool` -note: there are multiple methods with the same name, `borrow_mut` refers to `std::borrow::BorrowMut::borrow_mut` in the method call +note: the `borrow_mut` call is resolved to the method in `std::borrow::BorrowMut`, shadowing the method of the same name on the inherent impl for `std::cell::RefCell` --> $DIR/shadowed-lplace-method.rs:9:9 | +LL | use std::borrow::BorrowMut; + | ---------------------- `std::borrow::BorrowMut` imported here +... LL | *rc.borrow_mut() = false; | ^^^^^^^^^^ refers to `std::borrow::BorrowMut::borrow_mut` -help: you might have meant to invoke a different method, you can use the fully-qualified path +help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly | LL | *std::cell::RefCell::<_>::borrow_mut(&rc) = false; | +++++++++++++++++++++++++++++++++++++ ~