Skip to content

Commit

Permalink
Rollup merge of rust-lang#48392 - estebank:string, r=petrochenkov
Browse files Browse the repository at this point in the history 
Handle custom diagnostic for `&str + String`

Now all of `&str + &str`, `&str + String` and `String + String` have relevant diagnostic output.
  • Loading branch information
@Manishearth
Manishearth authored Feb 24, 2018
2 parents d45c4a6 + 20bc72e commit 7738eb4
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 27 deletions.
69 changes: 43 additions & 26 deletions src/librustc_typeck/check/op.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use super::{FnCtxt, Needs};
use super::method::MethodCallee;
use rustc::ty::{self, Ty, TypeFoldable, TypeVariants};
use rustc::ty::TypeVariants::{TyStr, TyRef};
use rustc::ty::TypeVariants::{TyStr, TyRef, TyAdt};
use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability};
use rustc::infer::type_variable::TypeVariableOrigin;
use errors;
Expand Down Expand Up @@ -299,7 +299,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {

if let Some(missing_trait) = missing_trait {
if missing_trait == "std::ops::Add" &&
self.check_str_addition(expr, lhs_expr, lhs_ty,
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
rhs_ty, &mut err) {
// This has nothing here because it means we did string
// concatenation (e.g. "Hello " + "World!"). This means
Expand Down Expand Up @@ -328,37 +328,54 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
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 {
let codemap = self.tcx.sess.codemap();
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 mut is_string_addition = false;
if let TyRef(_, l_ty) = lhs_ty.sty {
if let TyRef(_, r_ty) = rhs_ty.sty {
if l_ty.ty.sty == TyStr && r_ty.ty.sty == TyStr {
err.span_label(expr.span,
"`+` can't be used to concatenate two `&str` strings");
let codemap = self.tcx.sess.codemap();
let suggestion =
match codemap.span_to_snippet(lhs_expr.span) {
Ok(lstring) => format!("{}.to_owned()", lstring),
_ => format!("<expression>")
};
err.span_suggestion(lhs_expr.span,
&format!("`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"), suggestion);
is_string_addition = true;
}

match (&lhs_ty.sty, &rhs_ty.sty) {
(&TyRef(_, ref l_ty), &TyRef(_, ref r_ty))
if l_ty.ty.sty == TyStr && r_ty.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),
};
true
}

(&TyRef(_, ref l_ty), &TyAdt(..))
if l_ty.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));
}
_ => {}
};
true
}
_ => false,
}

is_string_addition
}

pub fn check_user_unop(&self,
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/span/issue-39018.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ pub fn main() {
// that won't output for the above string concatenation
let y = World::Hello + World::Goodbye;
//~^ ERROR cannot be applied to type

let x = "Hello " + "World!".to_owned();
//~^ ERROR cannot be applied to type
}

enum World {
Expand Down
16 changes: 15 additions & 1 deletion src/test/ui/span/issue-39018.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,19 @@ error[E0369]: binary operation `+` cannot be applied to type `World`
|
= note: an implementation of `std::ops::Add` might be missing for `World`

error: aborting due to 2 previous errors
error[E0369]: binary operation `+` cannot be applied to type `&str`
--> $DIR/issue-39018.rs:21:13
|
21 | 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
|
21 | let x = "Hello ".to_owned() + "World!".to_owned();
| ^^^^^^^^^^^^^^^^^^^
help: you also need to borrow the `String` on the right to get a `&str`
|
21 | let x = "Hello " + &"World!".to_owned();
| ^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

0 comments on commit 7738eb4

Please sign in to comment.