Rollup merge of #106150 - estebank:issue-39232, r=compiler-errors

Detect when method call on LHS might be shadowed

Address #39232.

compiler/rustc_hir_typeck/src/demand.rs

@@ -1,5 +1,6 @@
 use crate::FnCtxt;
 use rustc_ast::util::parser::PREC_POSTFIX;
+use rustc_errors::MultiSpan;
 use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
@@ -30,12 +31,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr_ty: Ty<'tcx>,
         expected: Ty<'tcx>,
         expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
-        _error: Option<TypeError<'tcx>>,
+        error: Option<TypeError<'tcx>>,
     ) {
         if expr_ty == expected {
             return;
         }
 
+        self.annotate_alternative_method_deref(err, expr, error);
+
         // Use `||` to give these suggestions a precedence
         let _ = self.suggest_missing_parentheses(err, expr)
             || self.suggest_remove_last_method_call(err, expr, expected)
@@ -316,6 +319,162 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    fn annotate_alternative_method_deref(
+        &self,
+        err: &mut Diagnostic,
+        expr: &hir::Expr<'_>,
+        error: Option<TypeError<'tcx>>,
+    ) {
+        let parent = self.tcx.hir().get_parent_node(expr.hir_id);
+        let Some(TypeError::Sorts(ExpectedFound { expected, .. })) = error else {return;};
+        let Some(hir::Node::Expr(hir::Expr {
+                    kind: hir::ExprKind::Assign(lhs, rhs, _), ..
+                })) = self.tcx.hir().find(parent) else {return; };
+        if rhs.hir_id != expr.hir_id || expected.is_closure() {
+            return;
+        }
+        let hir::ExprKind::Unary(hir::UnOp::Deref, deref) = lhs.kind else { return; };
+        let hir::ExprKind::MethodCall(path, base, args, _) = deref.kind else { return; };
+        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,
+                probe::IsSuggestion(true),
+                self_ty,
+                deref.hir_id,
+                probe::ProbeScope::TraitsInScope,
+            ) 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),
+            self_ty,
+            deref.hir_id,
+            probe::ProbeScope::AllTraits,
+        );
+        let suggestions: Vec<_> = all_methods
+            .into_iter()
+            .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)
+                });
+                vec![
+                    (
+                        deref.span.until(base.span),
+                        format!(
+                            "{}({}",
+                            with_no_trimmed_paths!(
+                                self.tcx.def_path_str_with_substs(m.def_id, substs,)
+                            ),
+                            match self.tcx.fn_sig(m.def_id).input(0).skip_binder().kind() {
+                                ty::Ref(_, _, hir::Mutability::Mut) => "&mut ",
+                                ty::Ref(_, _, _) => "&",
+                                _ => "",
+                            },
+                        ),
+                    ),
+                    match &args[..] {
+                        [] => (base.span.shrink_to_hi().with_hi(deref.span.hi()), ")".to_string()),
+                        [first, ..] => (base.span.between(first.span), ", ".to_string()),
+                    },
+                ]
+            })
+            .collect();
+        if suggestions.is_empty() {
+            return;
+        }
+        let mut path_span: MultiSpan = path.ident.span.into();
+        path_span.push_span_label(
+            path.ident.span,
+            with_no_trimmed_paths!(format!(
+                "refers to `{}`",
+                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::<Vec<String>>()
+                        .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!(
+                "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(
+            &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,
+        );
+    }
+
     /// If the expected type is an enum (Issue #55250) with any variants whose
     /// sole field is of the found type, suggest such variants. (Issue #42764)
     fn suggest_compatible_variants(

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<Ty<'tcx>>,
-    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
@@ -322,6 +322,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         )
     }
 
+    #[instrument(level = "debug", skip(self))]
+    pub(crate) fn probe_for_name_many(
+        &self,
+        mode: Mode,
+        item_name: Ident,
+        is_suggestion: IsSuggestion,
+        self_ty: Ty<'tcx>,
+        scope_expr_id: hir::HirId,
+        scope: ProbeScope,
+    ) -> Vec<Candidate<'tcx>> {
+        self.probe_op(
+            item_name.span,
+            mode,
+            Some(item_name),
+            None,
+            is_suggestion,
+            self_ty,
+            scope_expr_id,
+            scope,
+            |probe_cx| {
+                Ok(probe_cx
+                    .inherent_candidates
+                    .into_iter()
+                    .chain(probe_cx.extension_candidates)
+                    .collect())
+            },
+        )
+        .unwrap()
+    }
+
     fn probe_op<OP, R>(
         &'a self,
         span: Span,

src/test/ui/suggestions/shadowed-lplace-method-2.rs

@@ -0,0 +1,23 @@
+#![allow(unused)]
+
+struct X {
+    x: (),
+}
+pub trait A {
+    fn foo(&mut self, _: usize) -> &mut ();
+}
+impl A for X {
+    fn foo(&mut self, _: usize) -> &mut () {
+        &mut self.x
+    }
+}
+impl X {
+    fn foo(&mut self, _: usize) -> &mut Self {
+        self
+    }
+}
+
+fn main() {
+    let mut x = X { x: () };
+    *x.foo(0) = (); //~ ERROR E0308
+}

src/test/ui/suggestions/shadowed-lplace-method-2.stderr

@@ -0,0 +1,25 @@
+error[E0308]: mismatched types
+  --> $DIR/shadowed-lplace-method-2.rs:22:17
+   |
+LL |     *x.foo(0) = ();
+   |     ---------   ^^ expected struct `X`, found `()`
+   |     |
+   |     expected due to the type of this binding
+   |
+note: the `foo` call is resolved to the method in `X`, shadowing the method of the same name on trait `A`
+  --> $DIR/shadowed-lplace-method-2.rs:22:8
+   |
+LL |     *x.foo(0) = ();
+   |        ^^^ refers to `X::foo`
+help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly
+   |
+LL |     *<_ as A>::foo(&mut x, 0) = ();
+   |      ++++++++++++++++++  ~
+help: try wrapping the expression in `X`
+   |
+LL |     *x.foo(0) = X { x: () };
+   |                 ++++++    +
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.

src/test/ui/suggestions/shadowed-lplace-method.fixed

@@ -0,0 +1,10 @@
+// run-rustfix
+#![allow(unused_imports)]
+use std::borrow::BorrowMut;
+use std::cell::RefCell;
+use std::rc::Rc;
+
+fn main() {
+    let rc = Rc::new(RefCell::new(true));
+    *std::cell::RefCell::<_>::borrow_mut(&rc) = false; //~ ERROR E0308
+}

src/test/ui/suggestions/shadowed-lplace-method.rs

@@ -0,0 +1,10 @@
+// run-rustfix
+#![allow(unused_imports)]
+use std::borrow::BorrowMut;
+use std::cell::RefCell;
+use std::rc::Rc;
+
+fn main() {
+    let rc = Rc::new(RefCell::new(true));
+    *rc.borrow_mut() = false; //~ ERROR E0308
+}

src/test/ui/suggestions/shadowed-lplace-method.stderr

@@ -0,0 +1,26 @@
+error[E0308]: mismatched types
+  --> $DIR/shadowed-lplace-method.rs:9:24
+   |
+LL |     *rc.borrow_mut() = false;
+   |     ----------------   ^^^^^ expected struct `Rc`, found `bool`
+   |     |
+   |     expected due to the type of this binding
+   |
+   = note: expected struct `Rc<RefCell<bool>>`
+                found type `bool`
+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<T>`
+  --> $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 call the other method; you can use the fully-qualified path to call it explicitly
+   |
+LL |     *std::cell::RefCell::<_>::borrow_mut(&rc) = false;
+   |      +++++++++++++++++++++++++++++++++++++  ~
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.