From 20bc72e693372940183e97a32e7d07d2f9180182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 20 Feb 2018 22:46:51 -0800 Subject: [PATCH] Handle custom diagnostic for `&str + String` --- src/librustc_typeck/check/op.rs | 69 ++++++++++++++++++----------- src/test/ui/span/issue-39018.rs | 3 ++ src/test/ui/span/issue-39018.stderr | 16 ++++++- 3 files changed, 61 insertions(+), 27 deletions(-) diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index a6776a0fe86..646cd061a07 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -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; @@ -301,7 +301,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 @@ -330,37 +330,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!("") - }; - 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, diff --git a/src/test/ui/span/issue-39018.rs b/src/test/ui/span/issue-39018.rs index 4c9d10ba46b..7b3288fd29c 100644 --- a/src/test/ui/span/issue-39018.rs +++ b/src/test/ui/span/issue-39018.rs @@ -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 { diff --git a/src/test/ui/span/issue-39018.stderr b/src/test/ui/span/issue-39018.stderr index db662a1df59..70f8ecf42cb 100644 --- a/src/test/ui/span/issue-39018.stderr +++ b/src/test/ui/span/issue-39018.stderr @@ -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