squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Rollup merge of #52605 - estebank:str-plus-eq, r=oli-obk

Browse source 
Do not suggest using `to_owned()` on `&str += &str`

 - Don't provide incorrect suggestion for `&str += &str` (fix #52410)
 - On `&str + String` suggest `&str.to_owned() + &String` as a single suggestion
This commit is contained in:
kennytm 2018-07-24 09:49:52 +08:00 committed by GitHub
parent a98c19e24b 9369b52b0f
commit 5b7e3a1746
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
src/librustc_typeck/check/op.rs
View file

@ -307,9 +307,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
@ -381,7 +381,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
@ -410,13 +410,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 \
@ -428,34 +431,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
}

5
src/test/ui/issue-10401.stderr
View file

@ -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

8
src/test/ui/span/issue-39018.stderr
View file

@ -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