diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index d1ca05780930a..cd207478f8f6f 100644 --- a/src/librustc_typeck/check/op.rs +++ b/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, - rhs_ty, &mut err, true, op) { + self.check_str_addition( + 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, - rhs_ty, &mut err, false, op) { + self.check_str_addition( + 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, _)) - if l_ty.sty == Str && r_ty.sty == Str => { - if !is_assign { - err.span_label(op.span, - "`+` can't be used to concatenate two `&str` strings"); + (&Ref(_, l_ty, _), &Ref(_, r_ty, _)) // &str or &String + &str, &String or &&str + if (l_ty.sty == Str || is_std_string(l_ty)) && ( + r_ty.sty == Str || is_std_string(r_ty) || + &format!("{:?}", rhs_ty) == "&&str" + ) => + { + 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( - lhs_expr.span, - msg, - format!("{}.to_owned()", lstring), - Applicability::MachineApplicable, - ), + Ok(lstring) => { + err.span_suggestion( + lhs_expr.span, + if lstring.starts_with("&") { + 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(..)) - if l_ty.sty == Str && &format!("{:?}", rhs_ty) == "std::string::String" => { - err.span_label(expr.span, - "`+` can't be used to concatenate a `&str` with a `String`"); + (&Ref(_, l_ty, _), &Adt(..)) // Handle `&str` & `&String` + `String` + if (l_ty.sty == Str || is_std_string(l_ty)) && is_std_string(rhs_ty) => + { + 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, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 99e8db9d8e6d2..a80fe4b2fd8e5 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3540,8 +3540,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 => - self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()), + AssocOp::Assign => 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) => { diff --git a/src/test/ui/issues/issue-47377.stderr b/src/test/ui/issues/issue-47377.stderr index 88466131e3144..7d11a8c802128 100644 --- a/src/test/ui/issues/issue-47377.stderr +++ b/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 | diff --git a/src/test/ui/issues/issue-47380.stderr b/src/test/ui/issues/issue-47380.stderr index d69101eab4c46..89a154c5109d8 100644 --- a/src/test/ui/issues/issue-47380.stderr +++ b/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 | diff --git a/src/test/ui/span/issue-39018.rs b/src/test/ui/span/issue-39018.rs index 6dbc8d39976ad..a3b1d1d81799f 100644 --- a/src/test/ui/span/issue-39018.rs +++ b/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 +} diff --git a/src/test/ui/span/issue-39018.stderr b/src/test/ui/span/issue-39018.stderr index a5b91f090d2c0..d8fbf841b6157 100644 --- a/src/test/ui/span/issue-39018.stderr +++ b/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; + | ^ + +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 -For more information about this error, try `rustc --explain E0369`. +Some errors have detailed explanations: E0308, E0369. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/str/str-concat-on-double-ref.stderr b/src/test/ui/str/str-concat-on-double-ref.stderr index 61ebcfdefc316..3e53cdc4d98ca 100644 --- a/src/test/ui/str/str-concat-on-double-ref.stderr +++ b/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