Rollup merge of #60901 - estebank:str-str-str, r=Centril

Handle more string addition cases with appropriate suggestions

src/librustc_typeck/check/op.rs

@@ -305,8 +305,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                             };
                             if let Some(missing_trait) = missing_trait {
                                 if op.node == hir::BinOpKind::Add &&
-                                    self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
+                                    self.check_str_addition(
-                                                            rhs_ty, &mut err, true, op) {
+                                        lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op) {
                                     // This has nothing here because it means we did string
                                     // concatenation (e.g., "Hello " += "World!"). This means
                                     // we don't want the note in the else clause to be emitted
@@ -400,8 +400,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                             };
                             if let Some(missing_trait) = missing_trait {
                                 if op.node == hir::BinOpKind::Add &&
-                                    self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
+                                    self.check_str_addition(
-                                                            rhs_ty, &mut err, false, op) {
+                                        lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op) {
                                     // This has nothing here because it means we did string
                                     // concatenation (e.g., "Hello " + "World!"). This means
                                     // we don't want the note in the else clause to be emitted
@@ -502,9 +502,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         false
     }
 
+    /// Provide actionable suggestions when trying to add two strings with incorrect types,
+    /// like `&str + &str`, `String + String` and `&str + &String`.
+    ///
+    /// If this function returns `true` it means a note was printed, so we don't need
+    /// to print the normal "implementation of `std::ops::Add` might be missing" note
     fn check_str_addition(
         &self,
-        expr: &'gcx hir::Expr,
         lhs_expr: &'gcx hir::Expr,
         rhs_expr: &'gcx hir::Expr,
         lhs_ty: Ty<'tcx>,
@@ -514,45 +518,78 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         op: hir::BinOp,
     ) -> bool {
         let source_map = self.tcx.sess.source_map();
+        let remove_borrow_msg = "String concatenation appends the string on the right to the \
+                                 string on the left and may require reallocation. This \
+                                 requires ownership of the string on the left";
+
         let msg = "`to_owned()` can be used to create an owned `String` \
                    from a string reference. String concatenation \
                    appends the string on the right to the string \
                    on the left and may require reallocation. This \
                    requires ownership of the string on the left";
-        // If this function returns true it means a note was printed, so we don't need
+
-        // to print the normal "implementation of `std::ops::Add` might be missing" note
+        let is_std_string = |ty| &format!("{:?}", ty) == "std::string::String";
+
         match (&lhs_ty.sty, &rhs_ty.sty) {
-            (&Ref(_, l_ty, _), &Ref(_, r_ty, _))
+            (&Ref(_, l_ty, _), &Ref(_, r_ty, _)) // &str or &String + &str, &String or &&str
-            if l_ty.sty == Str && r_ty.sty == Str => {
+                if (l_ty.sty == Str || is_std_string(l_ty)) && (
-                if !is_assign {
+                        r_ty.sty == Str || is_std_string(r_ty) ||
-                    err.span_label(op.span,
+                        &format!("{:?}", rhs_ty) == "&&str"
-                                   "`+` can't be used to concatenate two `&str` strings");
+                    ) =>
+            {
+                if !is_assign { // Do not supply this message if `&str += &str`
+                    err.span_label(
+                        op.span,
+                        "`+` cannot be used to concatenate two `&str` strings",
+                    );
                     match source_map.span_to_snippet(lhs_expr.span) {
-                        Ok(lstring) => err.span_suggestion(
+                        Ok(lstring) => {
-                            lhs_expr.span,
+                            err.span_suggestion(
-                            msg,
+                                lhs_expr.span,
-                            format!("{}.to_owned()", lstring),
+                                if lstring.starts_with("&") {
-                            Applicability::MachineApplicable,
+                                    remove_borrow_msg
-                        ),
+                                } else {
+                                    msg
+                                },
+                                if lstring.starts_with("&") {
+                                    // let a = String::new();
+                                    // let _ = &a + "bar";
+                                    format!("{}", &lstring[1..])
+                                } else {
+                                    format!("{}.to_owned()", lstring)
+                                },
+                                Applicability::MachineApplicable,
+                            )
+                        }
                         _ => err.help(msg),
                     };
                 }
                 true
             }
-            (&Ref(_, l_ty, _), &Adt(..))
+            (&Ref(_, l_ty, _), &Adt(..)) // Handle `&str` & `&String` + `String`
-            if l_ty.sty == Str && &format!("{:?}", rhs_ty) == "std::string::String" => {
+                if (l_ty.sty == Str || is_std_string(l_ty)) && is_std_string(rhs_ty) =>
-                err.span_label(expr.span,
+            {
-                    "`+` can't be used to concatenate a `&str` with a `String`");
+                err.span_label(
+                    op.span,
+                    "`+` cannot be used to concatenate a `&str` with a `String`",
+                );
                 match (
                     source_map.span_to_snippet(lhs_expr.span),
                     source_map.span_to_snippet(rhs_expr.span),
                     is_assign,
                 ) {
                     (Ok(l), Ok(r), false) => {
+                        let to_string = if l.starts_with("&") {
+                            // let a = String::new(); let b = String::new();
+                            // let _ = &a + b;
+                            format!("{}", &l[1..])
+                        } else {
+                            format!("{}.to_owned()", l)
+                        };
                         err.multipart_suggestion(
                             msg,
                             vec![
-                                (lhs_expr.span, format!("{}.to_owned()", l)),
+                                (lhs_expr.span, to_string),
                                 (rhs_expr.span, format!("&{}", r)),
                             ],
                             Applicability::MachineApplicable,

src/libsyntax/parse/parser.rs

@@ -3497,8 +3497,7 @@ impl<'a> Parser<'a> {
                     let binary = self.mk_binary(source_map::respan(cur_op_span, ast_op), lhs, rhs);
                     self.mk_expr(span, binary, ThinVec::new())
                 }
-                AssocOp::Assign =>
+                AssocOp::Assign => self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()),
-                    self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()),
                 AssocOp::ObsoleteInPlace =>
                     self.mk_expr(span, ExprKind::ObsoleteInPlace(lhs, rhs), ThinVec::new()),
                 AssocOp::AssignOp(k) => {

src/test/ui/issues/issue-47377.stderr

@@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
 LL |      let _a = b + ", World!";
    |               - ^ ---------- &str
    |               | |
-   |               | `+` can't be used to concatenate two `&str` strings
+   |               | `+` cannot be used to concatenate two `&str` strings
    |               &str
 help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
    |

src/test/ui/issues/issue-47380.stderr

@@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
 LL |     println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
    |                                      - ^ ---------- &str
    |                                      | |
-   |                                      | `+` can't be used to concatenate two `&str` strings
+   |                                      | `+` cannot be used to concatenate two `&str` strings
    |                                      &str
 help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
    |

src/test/ui/span/issue-39018.rs

@@ -16,3 +16,23 @@ enum World {
     Hello,
     Goodbye,
 }
+
+fn foo() {
+    let a = String::new();
+    let b = String::new();
+    let c = "";
+    let d = "";
+    let e = &a;
+    let _ = &a + &b; //~ ERROR binary operation
+    let _ = &a + b; //~ ERROR binary operation
+    let _ = a + &b; // ok
+    let _ = a + b; //~ ERROR mismatched types
+    let _ = e + b; //~ ERROR binary operation
+    let _ = e + &b; //~ ERROR binary operation
+    let _ = e + d; //~ ERROR binary operation
+    let _ = e + &d; //~ ERROR binary operation
+    let _ = &c + &d; //~ ERROR binary operation
+    let _ = &c + d; //~ ERROR binary operation
+    let _ = c + &d; //~ ERROR binary operation
+    let _ = c + d; //~ ERROR binary operation
+}

src/test/ui/span/issue-39018.stderr

@@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
 LL |     let x = "Hello " + "World!";
    |             -------- ^ -------- &str
    |             |        |
-   |             |        `+` can't be used to concatenate two `&str` strings
+   |             |        `+` cannot be used to concatenate two `&str` strings
    |             &str
 help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
    |
@@ -25,16 +25,152 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
   --> $DIR/issue-39018.rs:11:22
    |
 LL |     let x = "Hello " + "World!".to_owned();
-   |             ---------^--------------------
+   |             -------- ^ ------------------- std::string::String
-   |             |          |
+   |             |        |
-   |             |          std::string::String
+   |             |        `+` cannot be used to concatenate a `&str` with a `String`
    |             &str
-   |             `+` can't be used to concatenate a `&str` with a `String`
 help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
    |
 LL |     let x = "Hello ".to_owned() + &"World!".to_owned();
    |             ^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 3 previous errors
+error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
+  --> $DIR/issue-39018.rs:26:16
+   |
+LL |     let _ = &a + &b;
+   |             -- ^ -- &std::string::String
+   |             |  |
+   |             |  `+` cannot be used to concatenate two `&str` strings
+   |             &std::string::String
+help: String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = a + &b;
+   |             ^
 
-For more information about this error, try `rustc --explain E0369`.
+error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
+  --> $DIR/issue-39018.rs:27:16
+   |
+LL |     let _ = &a + b;
+   |             -- ^ - std::string::String
+   |             |  |
+   |             |  `+` cannot be used to concatenate a `&str` with a `String`
+   |             &std::string::String
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = a + &b;
+   |             ^   ^^
+
+error[E0308]: mismatched types
+  --> $DIR/issue-39018.rs:29:17
+   |
+LL |     let _ = a + b;
+   |                 ^
+   |                 |
+   |                 expected &str, found struct `std::string::String`
+   |                 help: consider borrowing here: `&b`
+   |
+   = note: expected type `&str`
+              found type `std::string::String`
+
+error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
+  --> $DIR/issue-39018.rs:30:15
+   |
+LL |     let _ = e + b;
+   |             - ^ - std::string::String
+   |             | |
+   |             | `+` cannot be used to concatenate a `&str` with a `String`
+   |             &std::string::String
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = e.to_owned() + &b;
+   |             ^^^^^^^^^^^^   ^^
+
+error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
+  --> $DIR/issue-39018.rs:31:15
+   |
+LL |     let _ = e + &b;
+   |             - ^ -- &std::string::String
+   |             | |
+   |             | `+` cannot be used to concatenate two `&str` strings
+   |             &std::string::String
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = e.to_owned() + &b;
+   |             ^^^^^^^^^^^^
+
+error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
+  --> $DIR/issue-39018.rs:32:15
+   |
+LL |     let _ = e + d;
+   |             - ^ - &str
+   |             | |
+   |             | `+` cannot be used to concatenate two `&str` strings
+   |             &std::string::String
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = e.to_owned() + d;
+   |             ^^^^^^^^^^^^
+
+error[E0369]: binary operation `+` cannot be applied to type `&std::string::String`
+  --> $DIR/issue-39018.rs:33:15
+   |
+LL |     let _ = e + &d;
+   |             - ^ -- &&str
+   |             | |
+   |             | `+` cannot be used to concatenate two `&str` strings
+   |             &std::string::String
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = e.to_owned() + &d;
+   |             ^^^^^^^^^^^^
+
+error[E0369]: binary operation `+` cannot be applied to type `&&str`
+  --> $DIR/issue-39018.rs:34:16
+   |
+LL |     let _ = &c + &d;
+   |             -- ^ -- &&str
+   |             |
+   |             &&str
+   |
+   = note: an implementation of `std::ops::Add` might be missing for `&&str`
+
+error[E0369]: binary operation `+` cannot be applied to type `&&str`
+  --> $DIR/issue-39018.rs:35:16
+   |
+LL |     let _ = &c + d;
+   |             -- ^ - &str
+   |             |
+   |             &&str
+   |
+   = note: an implementation of `std::ops::Add` might be missing for `&&str`
+
+error[E0369]: binary operation `+` cannot be applied to type `&str`
+  --> $DIR/issue-39018.rs:36:15
+   |
+LL |     let _ = c + &d;
+   |             - ^ -- &&str
+   |             | |
+   |             | `+` cannot be used to concatenate two `&str` strings
+   |             &str
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = c.to_owned() + &d;
+   |             ^^^^^^^^^^^^
+
+error[E0369]: binary operation `+` cannot be applied to type `&str`
+  --> $DIR/issue-39018.rs:37:15
+   |
+LL |     let _ = c + d;
+   |             - ^ - &str
+   |             | |
+   |             | `+` cannot be used to concatenate two `&str` strings
+   |             &str
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
+   |
+LL |     let _ = c.to_owned() + d;
+   |             ^^^^^^^^^^^^
+
+error: aborting due to 14 previous errors
+
+Some errors have detailed explanations: E0308, E0369.
+For more information about an error, try `rustc --explain E0308`.

src/test/ui/str/str-concat-on-double-ref.stderr

@@ -3,10 +3,13 @@ error[E0369]: binary operation `+` cannot be applied to type `&std::string::Stri
    |
 LL |     let c = a + b;
    |             - ^ - &str
-   |             |
+   |             | |
+   |             | `+` cannot be used to concatenate two `&str` strings
    |             &std::string::String
+help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
    |
-   = note: an implementation of `std::ops::Add` might be missing for `&std::string::String`
+LL |     let c = a.to_owned() + b;
+   |             ^^^^^^^^^^^^
 
 error: aborting due to previous error