From 9369b52b0ff48eeb2bbc51f033fcfe2bb55e537c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 21 Jul 2018 22:33:32 -0700 Subject: [PATCH] Do not suggest using `to_owned()` on `&str += &str` --- src/librustc_typeck/check/op.rs | 67 ++++++++++++++++------------- src/test/ui/issue-10401.stderr | 5 --- src/test/ui/span/issue-39018.stderr | 8 +--- 3 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 46746d4bd29..89d36d28a10 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -300,9 +300,9 @@ 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) { + rhs_ty, &mut err, true) { // This has nothing here because it means we did string - // concatenation (e.g. "Hello " + "World!"). This means + // concatenation (e.g. "Hello " += "World!"). This means // we don't want the note in the else clause to be emitted } else if let ty::TyParam(_) = lhs_ty.sty { // FIXME: point to span of param @@ -374,7 +374,7 @@ 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) { + rhs_ty, &mut err, false) { // 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 @@ -403,13 +403,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { (lhs_ty, rhs_ty, return_ty) } - fn check_str_addition(&self, - expr: &'gcx hir::Expr, - lhs_expr: &'gcx hir::Expr, - rhs_expr: &'gcx hir::Expr, - lhs_ty: Ty<'tcx>, - rhs_ty: Ty<'tcx>, - err: &mut errors::DiagnosticBuilder) -> bool { + fn check_str_addition( + &self, + expr: &'gcx hir::Expr, + lhs_expr: &'gcx hir::Expr, + rhs_expr: &'gcx hir::Expr, + lhs_ty: Ty<'tcx>, + rhs_ty: Ty<'tcx>, + err: &mut errors::DiagnosticBuilder, + is_assign: bool, + ) -> bool { let codemap = self.tcx.sess.codemap(); let msg = "`to_owned()` can be used to create an owned `String` \ from a string reference. String concatenation \ @@ -421,34 +424,36 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match (&lhs_ty.sty, &rhs_ty.sty) { (&TyRef(_, l_ty, _), &TyRef(_, r_ty, _)) if l_ty.sty == TyStr && r_ty.sty == TyStr => { - err.span_label(expr.span, - "`+` can't be used to concatenate two `&str` strings"); - match codemap.span_to_snippet(lhs_expr.span) { - Ok(lstring) => err.span_suggestion(lhs_expr.span, - msg, - format!("{}.to_owned()", lstring)), - _ => err.help(msg), - }; + if !is_assign { + err.span_label(expr.span, + "`+` can't be used to concatenate two `&str` strings"); + match codemap.span_to_snippet(lhs_expr.span) { + Ok(lstring) => err.span_suggestion(lhs_expr.span, + msg, + format!("{}.to_owned()", lstring)), + _ => err.help(msg), + }; + } true } (&TyRef(_, l_ty, _), &TyAdt(..)) if l_ty.sty == TyStr && &format!("{:?}", rhs_ty) == "std::string::String" => { err.span_label(expr.span, "`+` can't be used to concatenate a `&str` with a `String`"); - match codemap.span_to_snippet(lhs_expr.span) { - Ok(lstring) => err.span_suggestion(lhs_expr.span, - msg, - format!("{}.to_owned()", lstring)), - _ => err.help(msg), - }; - match codemap.span_to_snippet(rhs_expr.span) { - Ok(rstring) => { - err.span_suggestion(rhs_expr.span, - "you also need to borrow the `String` on the right to \ - get a `&str`", - format!("&{}", rstring)); + match ( + codemap.span_to_snippet(lhs_expr.span), + codemap.span_to_snippet(rhs_expr.span), + is_assign, + ) { + (Ok(l), Ok(r), false) => { + err.multipart_suggestion(msg, vec![ + (lhs_expr.span, format!("{}.to_owned()", l)), + (rhs_expr.span, format!("&{}", r)), + ]); + } + _ => { + err.help(msg); } - _ => {} }; true } diff --git a/src/test/ui/issue-10401.stderr b/src/test/ui/issue-10401.stderr index 8c91c11a67c..94d13d5f268 100644 --- a/src/test/ui/issue-10401.stderr +++ b/src/test/ui/issue-10401.stderr @@ -5,11 +5,6 @@ LL | a += { "b" }; | -^^^^^^^^^^^ | | | cannot use `+=` on type `&str` - | `+` can't be used to concatenate two `&str` strings -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 | a.to_owned() += { "b" }; - | ^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/span/issue-39018.stderr b/src/test/ui/span/issue-39018.stderr index ee6334e8164..bd4e7cf574f 100644 --- a/src/test/ui/span/issue-39018.stderr +++ b/src/test/ui/span/issue-39018.stderr @@ -23,12 +23,8 @@ LL | let x = "Hello " + "World!".to_owned(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `+` 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(); - | ^^^^^^^^^^^^^^^^^^^ -help: you also need to borrow the `String` on the right to get a `&str` - | -LL | let x = "Hello " + &"World!".to_owned(); - | ^^^^^^^^^^^^^^^^^^^^ +LL | let x = "Hello ".to_owned() + &"World!".to_owned(); + | ^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^ error: aborting due to 3 previous errors