Rework logic and provide structured suggestion

Author: estebank

compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs

@@ -1187,6 +1187,13 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         has_custom_message: bool,
     ) -> bool {
         let span = obligation.cause.span;
+        let param_env = obligation.param_env;
+
+        let mk_result = |trait_pred_and_new_ty| {
+            let obligation =
+                self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty);
+            self.predicate_must_hold_modulo_regions(&obligation)
+        };
 
         let code = match obligation.cause.code() {
             ObligationCauseCode::FunctionArg { parent_code, .. } => parent_code,
@@ -1195,13 +1202,74 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             c @ ObligationCauseCode::WhereClauseInExpr(_, _, hir_id, _)
                 if self.tcx.hir_span(*hir_id).lo() == span.lo() =>
             {
+                // `hir_id` corresponds to the HIR node that introduced a `where`-clause obligation.
+                // If that obligation comes from a type in an associated method call, we need
+                // special handling here.
                 if let hir::Node::Expr(expr) = self.tcx.parent_hir_node(*hir_id)
                     && let hir::ExprKind::Call(base, _) = expr.kind
-                    && let hir::ExprKind::Path(hir::QPath::TypeRelative(ty, _)) = base.kind
+                    && let hir::ExprKind::Path(hir::QPath::TypeRelative(ty, segment)) = base.kind
+                    && let hir::Node::Expr(outer) = self.tcx.parent_hir_node(expr.hir_id)
+                    && let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, mtbl, _) = outer.kind
                     && ty.span == span
                 {
-                    // Do not suggest borrowing when we already do so. This would happen with
-                    // `let _ = &str::from("");` where the expression corresponds to the `str`.
+                    // We've encountered something like `&str::from("")`, where the intended code
+                    // was likely `<&str>::from("")`. The former is interpreted as "call method
+                    // `from` on `str` and borrow the result", while the latter means "call method
+                    // `from` on `&str`".
+
+                    let trait_pred_and_imm_ref = poly_trait_pred.map_bound(|p| {
+                        (p, Ty::new_imm_ref(self.tcx, self.tcx.lifetimes.re_static, p.self_ty()))
+                    });
+                    let trait_pred_and_mut_ref = poly_trait_pred.map_bound(|p| {
+                        (p, Ty::new_mut_ref(self.tcx, self.tcx.lifetimes.re_static, p.self_ty()))
+                    });
+
+                    let imm_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_imm_ref);
+                    let mut_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_mut_ref);
+                    let sugg_msg = |pre: &str| {
+                        format!(
+                            "you likely meant to call the associated function `{FN}` for type \
+                             `&{pre}{TY}`, but the code as written calls associated function `{FN}` on \
+                             type `{TY}`",
+                            FN = segment.ident,
+                            TY = poly_trait_pred.self_ty(),
+                        )
+                    };
+                    match (imm_ref_self_ty_satisfies_pred, mut_ref_self_ty_satisfies_pred, mtbl) {
+                        (true, _, hir::Mutability::Not) | (_, true, hir::Mutability::Mut) => {
+                            err.multipart_suggestion_verbose(
+                                sugg_msg(mtbl.prefix_str()),
+                                vec![
+                                    (outer.span.shrink_to_lo(), "<".to_string()),
+                                    (span.shrink_to_hi(), ">".to_string()),
+                                ],
+                                Applicability::MachineApplicable,
+                            );
+                        }
+                        (true, _, hir::Mutability::Mut) => {
+                            // There's an associated function found on the immutable borrow of the
+                            err.multipart_suggestion_verbose(
+                                sugg_msg("mut "),
+                                vec![
+                                    (outer.span.shrink_to_lo().until(span), "<&".to_string()),
+                                    (span.shrink_to_hi(), ">".to_string()),
+                                ],
+                                Applicability::MachineApplicable,
+                            );
+                        }
+                        (_, true, hir::Mutability::Not) => {
+                            err.multipart_suggestion_verbose(
+                                sugg_msg(""),
+                                vec![
+                                    (outer.span.shrink_to_lo().until(span), "<&mut ".to_string()),
+                                    (span.shrink_to_hi(), ">".to_string()),
+                                ],
+                                Applicability::MachineApplicable,
+                            );
+                        }
+                        _ => {}
+                    }
+                    // If we didn't return early here, we would instead suggest `&&str::from("")`.
                     return false;
                 }
                 c
@@ -1229,8 +1297,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             never_suggest_borrow.push(def_id);
         }
 
-        let param_env = obligation.param_env;
-
         // Try to apply the original trait bound by borrowing.
         let mut try_borrowing = |old_pred: ty::PolyTraitPredicate<'tcx>,
                                  blacklist: &[DefId]|
@@ -1252,11 +1318,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                 )
             });
 
-            let mk_result = |trait_pred_and_new_ty| {
-                let obligation =
-                    self.mk_trait_obligation_with_new_self_ty(param_env, trait_pred_and_new_ty);
-                self.predicate_must_hold_modulo_regions(&obligation)
-            };
             let imm_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_imm_ref);
             let mut_ref_self_ty_satisfies_pred = mk_result(trait_pred_and_mut_ref);
 

tests/ui/suggestions/dont-suggest-borrowing-existing-borrow.fixed

@@ -0,0 +1,17 @@
+//@ run-rustfix
+
+struct S;
+trait Trait {
+    fn foo() {}
+}
+impl Trait for &S {}
+impl Trait for &mut S {}
+fn main() {
+    let _ = <&str>::from("value");
+    //~^ ERROR the trait bound `str: From<_>` is not satisfied
+    //~| ERROR the size for values of type `str` cannot be known at compilation time
+    let _ = <&mut S>::foo();
+    //~^ ERROR the trait bound `S: Trait` is not satisfied
+    let _ = <&S>::foo();
+    //~^ ERROR the trait bound `S: Trait` is not satisfied
+}

tests/ui/suggestions/dont-suggest-borrowing-existing-borrow.rs

@@ -1,5 +1,17 @@
+//@ run-rustfix
+
+struct S;
+trait Trait {
+    fn foo() {}
+}
+impl Trait for &S {}
+impl Trait for &mut S {}
 fn main() {
     let _ = &str::from("value");
     //~^ ERROR the trait bound `str: From<_>` is not satisfied
     //~| ERROR the size for values of type `str` cannot be known at compilation time
+    let _ = &mut S::foo();
+    //~^ ERROR the trait bound `S: Trait` is not satisfied
+    let _ = &S::foo();
+    //~^ ERROR the trait bound `S: Trait` is not satisfied
 }

tests/ui/suggestions/dont-suggest-borrowing-existing-borrow.stderr

@@ -1,5 +1,5 @@
 error[E0277]: the trait bound `str: From<_>` is not satisfied
-  --> $DIR/dont-suggest-borrowing-existing-borrow.rs:2:14
+  --> $DIR/dont-suggest-borrowing-existing-borrow.rs:10:14
    |
 LL |     let _ = &str::from("value");
    |              ^^^ the trait `From<_>` is not implemented for `str`
@@ -11,16 +11,48 @@ LL |     let _ = &str::from("value");
              `String` implements `From<Box<str>>`
              `String` implements `From<Cow<'_, str>>`
              `String` implements `From<char>`
+help: you likely meant to call the associated function `from` for type `&str`, but the code as written calls associated function `from` on type `str`
+   |
+LL |     let _ = <&str>::from("value");
+   |             +    +
+
+error[E0277]: the trait bound `S: Trait` is not satisfied
+  --> $DIR/dont-suggest-borrowing-existing-borrow.rs:13:18
+   |
+LL |     let _ = &mut S::foo();
+   |                  ^ the trait `Trait` is not implemented for `S`
+   |
+   = help: the following other types implement trait `Trait`:
+             &S
+             &mut S
+help: you likely meant to call the associated function `foo` for type `&mut S`, but the code as written calls associated function `foo` on type `S`
+   |
+LL |     let _ = <&mut S>::foo();
+   |             +      +
+
+error[E0277]: the trait bound `S: Trait` is not satisfied
+  --> $DIR/dont-suggest-borrowing-existing-borrow.rs:15:14
+   |
+LL |     let _ = &S::foo();
+   |              ^ the trait `Trait` is not implemented for `S`
+   |
+   = help: the following other types implement trait `Trait`:
+             &S
+             &mut S
+help: you likely meant to call the associated function `foo` for type `&S`, but the code as written calls associated function `foo` on type `S`
+   |
+LL |     let _ = <&S>::foo();
+   |             +  +
 
 error[E0277]: the size for values of type `str` cannot be known at compilation time
-  --> $DIR/dont-suggest-borrowing-existing-borrow.rs:2:14
+  --> $DIR/dont-suggest-borrowing-existing-borrow.rs:10:14
    |
 LL |     let _ = &str::from("value");
    |              ^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `str`
    = note: the return type of a function must have a statically known size
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0277`.