Suggest fully qualified path on method name collision

compiler/rustc_hir_typeck/src/demand.rs

@@ -1,5 +1,5 @@
 use rustc_errors::{Applicability, Diag, MultiSpan, listify};
-use rustc_hir::def::Res;
+use rustc_hir::def::{DefKind, Res};
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::{self as hir, find_attr};
 use rustc_infer::infer::DefineOpaqueTypes;
@@ -29,7 +29,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         if expr_ty == expected {
             return;
         }
-        self.annotate_alternative_method_deref(err, expr, error);
+        self.annotate_alternative_method_deref_for_unop(err, expr, error);
         self.explain_self_literal(err, expr, expected, expr_ty);
 
         // Use `||` to give these suggestions a precedence
@@ -723,8 +723,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         hir::Path {
                             res:
                                 hir::def::Res::Def(
-                                    hir::def::DefKind::Static { .. }
-                                    | hir::def::DefKind::Const { .. },
+                                    DefKind::Static { .. } | DefKind::Const { .. },
                                     def_id,
                                 ),
                             ..
@@ -899,7 +898,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         false
     }
 
-    fn annotate_alternative_method_deref(
+    fn annotate_alternative_method_deref_for_unop(
         &self,
         err: &mut Diag<'_>,
         expr: &hir::Expr<'_>,
@@ -919,7 +918,17 @@ 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 {
+        self.annotate_alternative_method_deref(err, deref, Some(expected))
+    }
+
+    #[tracing::instrument(skip(self, err), level = "debug")]
+    pub(crate) fn annotate_alternative_method_deref(
+        &self,
+        err: &mut Diag<'_>,
+        expr: &hir::Expr<'_>,
+        expected: Option<Ty<'tcx>>,
+    ) {
+        let hir::ExprKind::MethodCall(path, base, args, _) = expr.kind else {
             return;
         };
         let Some(self_ty) = self.typeck_results.borrow().expr_ty_adjusted_opt(base) else {
@@ -929,7 +938,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let Ok(pick) = self.lookup_probe_for_diagnostic(
             path.ident,
             self_ty,
-            deref,
+            expr,
             probe::ProbeScope::TraitsInScope,
             None,
         ) else {
@@ -939,10 +948,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let Ok(in_scope_methods) = self.probe_for_name_many(
             probe::Mode::MethodCall,
             path.ident,
-            Some(expected),
+            expected,
             probe::IsSuggestion(true),
             self_ty,
-            deref.hir_id,
+            expr.hir_id,
             probe::ProbeScope::TraitsInScope,
         ) else {
             return;
@@ -954,45 +963,62 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let Ok(all_methods) = self.probe_for_name_many(
             probe::Mode::MethodCall,
             path.ident,
-            Some(expected),
+            expected,
             probe::IsSuggestion(true),
             self_ty,
-            deref.hir_id,
+            expr.hir_id,
             probe::ProbeScope::AllTraits,
         ) else {
             return;
         };
 
         let suggestions: Vec<_> = all_methods
             .into_iter()
-            .filter(|c| c.item.def_id != pick.item.def_id)
-            .map(|c| {
+            .filter_map(|c| {
+                if c.item.def_id == pick.item.def_id {
+                    return None;
+                }
                 let m = c.item;
                 let generic_args = ty::GenericArgs::for_item(self.tcx, m.def_id, |param, _| {
-                    self.var_for_def(deref.span, param)
+                    self.var_for_def(expr.span, param)
                 });
-                let mutability =
-                    match self.tcx.fn_sig(m.def_id).skip_binder().input(0).skip_binder().kind() {
-                        ty::Ref(_, _, hir::Mutability::Mut) => "&mut ",
-                        ty::Ref(_, _, _) => "&",
-                        _ => "",
-                    };
-                vec![
-                    (
-                        deref.span.until(base.span),
-                        format!(
-                            "{}({}",
-                            with_no_trimmed_paths!(
-                                self.tcx.def_path_str_with_args(m.def_id, generic_args,)
-                            ),
-                            mutability,
-                        ),
-                    ),
+                let fn_sig = self.tcx.fn_sig(m.def_id);
+                if fn_sig.skip_binder().inputs().skip_binder().len() != args.len() + 1 {
+                    return None;
+                }
+                let rcvr_ty = fn_sig.skip_binder().input(0).skip_binder();
+                let (mutability, ty) = match rcvr_ty.kind() {
+                    ty::Ref(_, ty, hir::Mutability::Mut) => ("&mut ", ty),
+                    ty::Ref(_, ty, _) => ("&", ty),
+                    _ => ("", &rcvr_ty),
+                };
+                let path = match self.tcx.assoc_parent(m.def_id) {
+                    Some((_, DefKind::Impl { of_trait: true })) => {
+                        // We have `impl Trait for T {}`, suggest `<T as Trait>::method`.
+                        self.tcx.def_path_str_with_args(m.def_id, generic_args).to_string()
+                    }
+                    Some((_, DefKind::Impl { of_trait: false })) => {
+                        if let ty::Adt(def, _) = ty.kind() {
+                            // We have `impl T {}`, suggest `T::method`.
+                            format!("{}::{}", self.tcx.def_path_str(def.did()), path.ident)
+                        } else {
+                            // This should be unreachable, as `impl &'a T {}` is invalid.
+                            format!("{ty}::{}", path.ident)
+                        }
+                    }
+                    // Fallback for arbitrary self types.
+                    _ => with_no_trimmed_paths!(
+                        self.tcx.def_path_str_with_args(m.def_id, generic_args)
+                    )
+                    .to_string(),
+                };
+                Some(vec![
+                    (expr.span.until(base.span), format!("{path}({}", mutability)),
                     match &args {
-                        [] => (base.span.shrink_to_hi().with_hi(deref.span.hi()), ")".to_string()),
+                        [] => (base.span.shrink_to_hi().with_hi(expr.span.hi()), ")".to_string()),
                         [first, ..] => (base.span.between(first.span), ", ".to_string()),
                     },
-                ]
+                ])
             })
             .collect();
         if suggestions.is_empty() {
@@ -1263,7 +1289,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let hir::def::Res::Def(kind, def_id) = path.res else {
                     return;
                 };
-                let callable_kind = if matches!(kind, hir::def::DefKind::Ctor(_, _)) {
+                let callable_kind = if matches!(kind, DefKind::Ctor(_, _)) {
                     CallableKind::Constructor
                 } else {
                     CallableKind::Function

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -2819,6 +2819,8 @@ impl<'a, 'b, 'tcx> ArgMatchingCtxt<'a, 'b, 'tcx> {
             );
             return;
         }
+
+        self.annotate_alternative_method_deref(err, self.call_expr, None);
     }
 
     /// A "softer" version of the `demand_compatible`, which checks types without persisting them,

tests/ui/methods/shadowed-intrinsic-method.rs

@@ -0,0 +1,37 @@
+// Can't use rustfix because we provide two suggestions:
+// to remove the arg for `Borrow::borrow` or to call `Type::borrow`.
+use std::borrow::Borrow;
+
+struct A;
+
+impl A { fn borrow(&mut self, _: ()) {} }
+
+struct B;
+
+fn main() {
+    // The fully-qualified path for items within functions is unnameable from outside that function.
+    impl B { fn borrow(&mut self, _: ()) {} }
+
+    struct C;
+    // The fully-qualified path for items within functions is unnameable from outside that function.
+    impl C { fn borrow(&mut self, _: ()) {} }
+
+    let mut a = A;
+    a.borrow(()); //~ ERROR E0061
+    // A::borrow(&mut a, ());
+    let mut b = B;
+    b.borrow(()); //~ ERROR E0061
+    // This currently suggests `main::<impl B>::borrow`, which is not correct, it should be
+    // B::borrow(&mut b, ());
+    let mut c = C;
+    c.borrow(()); //~ ERROR E0061
+    // This currently suggests `main::C::borrow`, which is not correct, it should be
+    // C::borrow(&mut c, ());
+}
+
+fn foo() {
+    let mut b = B;
+    b.borrow(()); //~ ERROR E0061
+    // This currently suggests `main::<impl B>::borrow`, which is not correct, it should be
+    // B::borrow(&mut b, ());
+}

tests/ui/methods/shadowed-intrinsic-method.stderr

@@ -0,0 +1,111 @@
+error[E0061]: this method takes 0 arguments but 1 argument was supplied
+  --> $DIR/shadowed-intrinsic-method.rs:20:7
+   |
+LL |     a.borrow(());
+   |       ^^^^^^ -- unexpected argument of type `()`
+   |
+note: the `borrow` call is resolved to the method in `std::borrow::Borrow`, shadowing the method of the same name on the inherent impl for `A`
+  --> $DIR/shadowed-intrinsic-method.rs:20:7
+   |
+LL | use std::borrow::Borrow;
+   |     ------------------- `std::borrow::Borrow` imported here
+...
+LL |     a.borrow(());
+   |       ^^^^^^ refers to `std::borrow::Borrow::borrow`
+note: method defined here
+  --> $SRC_DIR/core/src/borrow.rs:LL:COL
+help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly
+   |
+LL -     a.borrow(());
+LL +     A::borrow(&mut a, ());
+   |
+help: remove the extra argument
+   |
+LL -     a.borrow(());
+LL +     a.borrow();
+   |
+
+error[E0061]: this method takes 0 arguments but 1 argument was supplied
+  --> $DIR/shadowed-intrinsic-method.rs:23:7
+   |
+LL |     b.borrow(());
+   |       ^^^^^^ -- unexpected argument of type `()`
+   |
+note: the `borrow` call is resolved to the method in `std::borrow::Borrow`, shadowing the method of the same name on the inherent impl for `main::<impl B>`
+  --> $DIR/shadowed-intrinsic-method.rs:23:7
+   |
+LL | use std::borrow::Borrow;
+   |     ------------------- `std::borrow::Borrow` imported here
+...
+LL |     b.borrow(());
+   |       ^^^^^^ refers to `std::borrow::Borrow::borrow`
+note: method defined here
+  --> $SRC_DIR/core/src/borrow.rs:LL:COL
+help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly
+   |
+LL -     b.borrow(());
+LL +     B::borrow(&mut b, ());
+   |
+help: remove the extra argument
+   |
+LL -     b.borrow(());
+LL +     b.borrow();
+   |
+
+error[E0061]: this method takes 0 arguments but 1 argument was supplied
+  --> $DIR/shadowed-intrinsic-method.rs:27:7
+   |
+LL |     c.borrow(());
+   |       ^^^^^^ -- unexpected argument of type `()`
+   |
+note: the `borrow` call is resolved to the method in `std::borrow::Borrow`, shadowing the method of the same name on the inherent impl for `main::C`
+  --> $DIR/shadowed-intrinsic-method.rs:27:7
+   |
+LL | use std::borrow::Borrow;
+   |     ------------------- `std::borrow::Borrow` imported here
+...
+LL |     c.borrow(());
+   |       ^^^^^^ refers to `std::borrow::Borrow::borrow`
+note: method defined here
+  --> $SRC_DIR/core/src/borrow.rs:LL:COL
+help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly
+   |
+LL -     c.borrow(());
+LL +     C::borrow(&mut c, ());
+   |
+help: remove the extra argument
+   |
+LL -     c.borrow(());
+LL +     c.borrow();
+   |
+
+error[E0061]: this method takes 0 arguments but 1 argument was supplied
+  --> $DIR/shadowed-intrinsic-method.rs:34:7
+   |
+LL |     b.borrow(());
+   |       ^^^^^^ -- unexpected argument of type `()`
+   |
+note: the `borrow` call is resolved to the method in `std::borrow::Borrow`, shadowing the method of the same name on the inherent impl for `main::<impl B>`
+  --> $DIR/shadowed-intrinsic-method.rs:34:7
+   |
+LL | use std::borrow::Borrow;
+   |     ------------------- `std::borrow::Borrow` imported here
+...
+LL |     b.borrow(());
+   |       ^^^^^^ refers to `std::borrow::Borrow::borrow`
+note: method defined here
+  --> $SRC_DIR/core/src/borrow.rs:LL:COL
+help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly
+   |
+LL -     b.borrow(());
+LL +     B::borrow(&mut b, ());
+   |
+help: remove the extra argument
+   |
+LL -     b.borrow(());
+LL +     b.borrow();
+   |
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0061`.

tests/ui/mismatched_types/diagnostic-method-lookup-returns-sig-with-fewer-args.stderr

@@ -11,6 +11,20 @@ note: method defined here
    |
 LL |     pub fn get(&self, data: i32) {
    |            ^^^        ---------
+note: the `get` call is resolved to the method in `Target`, shadowing the method of the same name on trait `RandomTrait`
+  --> $DIR/diagnostic-method-lookup-returns-sig-with-fewer-args.rs:4:12
+   |
+LL |     target.get(10.0); // (used to crash here)
+   |            ^^^ refers to `Target::get`
+   = note: additionally, there are 1 other available methods that aren't in scope
+help: you might have meant to call one of the other methods; you can use the fully-qualified path to call one of them explicitly
+   |
+LL -     target.get(10.0); // (used to crash here)
+LL +     <_ as std::slice::SliceIndex<_>>::get(target, 10.0); // (used to crash here)
+   |
+LL -     target.get(10.0); // (used to crash here)
+LL +     <_ as object::read::elf::relocation::Relr>::get(&target, 10.0); // (used to crash here)
+   |
 
 error: aborting due to 1 previous error
 

tests/ui/suggestions/shadowed-lplace-method.fixed

@@ -6,5 +6,5 @@ use std::rc::Rc;
 
 fn main() {
     let rc = Rc::new(RefCell::new(true));
-    *std::cell::RefCell::<_>::borrow_mut(&rc) = false; //~ ERROR E0308
+    *RefCell::borrow_mut(&rc) = false; //~ ERROR E0308
 }

tests/ui/suggestions/shadowed-lplace-method.stderr

@@ -19,7 +19,7 @@ LL |     *rc.borrow_mut() = false;
 help: you might have meant to call the other method; you can use the fully-qualified path to call it explicitly
    |
 LL -     *rc.borrow_mut() = false;
-LL +     *std::cell::RefCell::<_>::borrow_mut(&rc) = false;
+LL +     *RefCell::borrow_mut(&rc) = false;
    |
 
 error: aborting due to 1 previous error