Skip to content

Commit 9369b52

Browse files
estebankestebank
committed
Do not suggest using to_owned() on &str += &str 
1 parent 0ad6179 commit 9369b52

File tree

3 files changed

+38
-42
lines changed

3 files changed

+38
-42
lines changed

‎src/librustc_typeck/check/op.rs

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
300300
if let Some(missing_trait) = missing_trait {
301301
if op.node == hir::BinOpKind::Add &&
302302
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
303-
rhs_ty, &mut err) {
303+
rhs_ty, &mut err, true) {
304304
// This has nothing here because it means we did string
305-
// concatenation (e.g. "Hello " + "World!"). This means
305+
// concatenation (e.g. "Hello " += "World!"). This means
306306
// we don't want the note in the else clause to be emitted
307307
} else if let ty::TyParam(_) = lhs_ty.sty {
308308
// FIXME: point to span of param
@@ -374,7 +374,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
374374
if let Some(missing_trait) = missing_trait {
375375
if op.node == hir::BinOpKind::Add &&
376376
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
377-
rhs_ty, &mut err) {
377+
rhs_ty, &mut err, false) {
378378
// This has nothing here because it means we did string
379379
// concatenation (e.g. "Hello " + "World!"). This means
380380
// 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> {
403403
(lhs_ty, rhs_ty, return_ty)
404404
}
405405

406-
fn check_str_addition(&self,
407-
expr: &'gcx hir::Expr,
408-
lhs_expr: &'gcx hir::Expr,
409-
rhs_expr: &'gcx hir::Expr,
410-
lhs_ty: Ty<'tcx>,
411-
rhs_ty: Ty<'tcx>,
412-
err: &mut errors::DiagnosticBuilder) -> bool {
406+
fn check_str_addition(
407+
&self,
408+
expr: &'gcx hir::Expr,
409+
lhs_expr: &'gcx hir::Expr,
410+
rhs_expr: &'gcx hir::Expr,
411+
lhs_ty: Ty<'tcx>,
412+
rhs_ty: Ty<'tcx>,
413+
err: &mut errors::DiagnosticBuilder,
414+
is_assign: bool,
415+
) -> bool {
413416
let codemap = self.tcx.sess.codemap();
414417
let msg = "`to_owned()` can be used to create an owned `String` \
415418
from a string reference. String concatenation \
@@ -421,34 +424,36 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
421424
match (&lhs_ty.sty, &rhs_ty.sty) {
422425
(&TyRef(_, l_ty, _), &TyRef(_, r_ty, _))
423426
if l_ty.sty == TyStr && r_ty.sty == TyStr => {
424-
err.span_label(expr.span,
425-
"`+` can't be used to concatenate two `&str` strings");
426-
match codemap.span_to_snippet(lhs_expr.span) {
427-
Ok(lstring) => err.span_suggestion(lhs_expr.span,
428-
msg,
429-
format!("{}.to_owned()", lstring)),
430-
_ => err.help(msg),
431-
};
427+
if !is_assign {
428+
err.span_label(expr.span,
429+
"`+` can't be used to concatenate two `&str` strings");
430+
match codemap.span_to_snippet(lhs_expr.span) {
431+
Ok(lstring) => err.span_suggestion(lhs_expr.span,
432+
msg,
433+
format!("{}.to_owned()", lstring)),
434+
_ => err.help(msg),
435+
};
436+
}
432437
true
433438
}
434439
(&TyRef(_, l_ty, _), &TyAdt(..))
435440
if l_ty.sty == TyStr && &format!("{:?}", rhs_ty) == "std::string::String" => {
436441
err.span_label(expr.span,
437442
"`+` can't be used to concatenate a `&str` with a `String`");
438-
match codemap.span_to_snippet(lhs_expr.span) {
439-
Ok(lstring) => err.span_suggestion(lhs_expr.span,
440-
msg,
441-
format!("{}.to_owned()", lstring)),
442-
_ => err.help(msg),
443-
};
444-
match codemap.span_to_snippet(rhs_expr.span) {
445-
Ok(rstring) => {
446-
err.span_suggestion(rhs_expr.span,
447-
"you also need to borrow the `String` on the right to \
448-
get a `&str`",
449-
format!("&{}", rstring));
443+
match (
444+
codemap.span_to_snippet(lhs_expr.span),
445+
codemap.span_to_snippet(rhs_expr.span),
446+
is_assign,
447+
) {
448+
(Ok(l), Ok(r), false) => {
449+
err.multipart_suggestion(msg, vec![
450+
(lhs_expr.span, format!("{}.to_owned()", l)),
451+
(rhs_expr.span, format!("&{}", r)),
452+
]);
453+
}
454+
_ => {
455+
err.help(msg);
450456
}
451-
_ => {}
452457
};
453458
true
454459
}

‎src/test/ui/issue-10401.stderr

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,6 @@ LL | a += { "b" };
55
| -^^^^^^^^^^^
66
| |
77
| cannot use `+=` on type `&str`
8-
| `+` can't be used to concatenate two `&str` strings
9-
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
10-
|
11-
LL | a.to_owned() += { "b" };
12-
| ^^^^^^^^^^^^
138

149
error: aborting due to previous error
1510

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,8 @@ LL | let x = "Hello " + "World!".to_owned();
2323
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `+` can't be used to concatenate a `&str` with a `String`
2424
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
2525
|
26-
LL | let x = "Hello ".to_owned() + "World!".to_owned();
27-
| ^^^^^^^^^^^^^^^^^^^
28-
help: you also need to borrow the `String` on the right to get a `&str`
29-
|
30-
LL | let x = "Hello " + &"World!".to_owned();
31-
| ^^^^^^^^^^^^^^^^^^^^
26+
LL | let x = "Hello ".to_owned() + &"World!".to_owned();
27+
| ^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
3228

3329
error: aborting due to 3 previous errors
3430

0 commit comments

Comments
 (0)