Auto merge of #39116 - mgattozzi:better-string-message, r=nrc

bors · bors · commit 6abe64871e6e · 2017-02-02T07:39:07.000Z
Add clearer error message using `&str + &str` This is the first part of #39018. One of the common things for new users coming from more dynamic languages like JavaScript, Python or Ruby is to use `+` to concatenate strings. However, this doesn't work that way in Rust unless the first type is a `String`. This commit adds a check for this use case and outputs a new error as well as a suggestion to guide the user towards the desired behavior. It also adds a new test case to test the output of the error.
diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs
@@ -13,7 +13,9 @@
 use super::FnCtxt;
 use hir::def_id::DefId;
 use rustc::ty::{Ty, TypeFoldable, PreferMutLvalue, TypeVariants};
+use rustc::ty::TypeVariants::{TyStr, TyRef};
 use rustc::infer::type_variable::TypeVariableOrigin;
+use errors;
 use syntax::ast;
 use syntax::symbol::Symbol;
 use rustc::hir;
@@ -237,9 +239,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                         };
 
                         if let Some(missing_trait) = missing_trait {
-                            span_note!(&mut err, lhs_expr.span,
-                                       "an implementation of `{}` might be missing for `{}`",
-                                        missing_trait, lhs_ty);
+                            if missing_trait == "std::ops::Add" &&
+                                self.check_str_addition(expr, lhs_expr, lhs_ty,
+                                                         rhs_expr, rhs_ty_var, &mut err) {
+                                // This has nothing here because it means we did string
+                                // concatenation (e.g. "Hello " + "World!"). This means
+                                // we don't want the span in the else clause to be emmitted
+                            } else {
+                                span_note!(&mut err, lhs_expr.span,
+                                            "an implementation of `{}` might be missing for `{}`",
+                                            missing_trait, lhs_ty);
+                            }
                         }
                         err.emit();
                     }
@@ -254,6 +264,47 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         (rhs_ty_var, return_ty)
     }
 
+    fn check_str_addition(&self,
+                          expr: &'gcx hir::Expr,
+                          lhs_expr: &'gcx hir::Expr,
+                          lhs_ty: Ty<'tcx>,
+                          rhs_expr: &'gcx hir::Expr,
+                          rhs_ty_var: Ty<'tcx>,
+                          mut err: &mut errors::DiagnosticBuilder) -> bool {
+        // If this function returns false it means we use it to make sure we print
+        // out the an "implementation of span_note!" above where this function is
+        // called and if true we don't.
+        let mut is_string_addition = false;
+        let rhs_ty = self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var);
+        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 {
+                    span_note!(&mut err, lhs_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),
+                                codemap.span_to_snippet(rhs_expr.span)) {
+                            (Ok(lstring), Ok(rstring)) =>
+                                format!("{}.to_owned() + {}", lstring, rstring),
+                            _ => format!("<expression>")
+                        };
+                    err.span_suggestion(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;
+                }
+
+            }
+
+        }
+
+        is_string_addition
+    }
+
     pub fn check_user_unop(&self,
                            op_str: &str,
                            mname: &str,
diff --git a/src/test/parse-fail/issue-39018.stderr b/src/test/parse-fail/issue-39018.stderr
@@ -0,0 +1,28 @@
+error[E0369]: binary operation `+` cannot be applied to type `&'static str`
+ --> src/test/ui/span/issue-39018.rs:2:13
+  |
+2 |     let x = "Hello " + "World!";
+  |             ^^^^^^^^
+  |
+note: `+` can't be used to concatenate two `&str` strings
+ --> src/test/ui/span/issue-39018.rs:2:13
+  |
+2 |     let x = "Hello " + "World!";
+  |             ^^^^^^^^
+help: to_owned() can be used to create an owned `String` from a string reference. This allows concatenation since the `String` is owned.
+  |     let x = "Hello ".to_owned() + "World!";
+
+error[E0369]: binary operation `+` cannot be applied to type `World`
+ --> src/test/ui/span/issue-39018.rs:7:13
+  |
+7 |     let y = World::Hello + World::Goodbye;
+  |             ^^^^^^^^^^^^
+  |
+note: an implementation of `std::ops::Add` might be missing for `World`
+ --> src/test/ui/span/issue-39018.rs:7:13
+  |
+7 |     let y = World::Hello + World::Goodbye;
+  |             ^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
diff --git a/src/test/ui/span/issue-39018.rs b/src/test/ui/span/issue-39018.rs
@@ -0,0 +1,23 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+pub fn main() {
+    let x = "Hello " + "World!";
+
+    // Make sure that the span outputs a warning
+    // for not having an implementation for std::ops::Add
+    // that won't output for the above string concatenation
+    let y = World::Hello + World::Goodbye;
+}
+
+enum World {
+    Hello,
+    Goodbye,
+}
diff --git a/src/test/ui/span/issue-39018.stderr b/src/test/ui/span/issue-39018.stderr
@@ -0,0 +1,28 @@
+error[E0369]: binary operation `+` cannot be applied to type `&'static str`
+  --> $DIR/issue-39018.rs:12:13
+   |
+12 |     let x = "Hello " + "World!";
+   |             ^^^^^^^^
+   |
+note: `+` can't be used to concatenate two `&str` strings
+  --> $DIR/issue-39018.rs:12:13
+   |
+12 |     let x = "Hello " + "World!";
+   |             ^^^^^^^^
+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.
+   |     let x = "Hello ".to_owned() + "World!";
+
+error[E0369]: binary operation `+` cannot be applied to type `World`
+  --> $DIR/issue-39018.rs:17:13
+   |
+17 |     let y = World::Hello + World::Goodbye;
+   |             ^^^^^^^^^^^^
+   |
+note: an implementation of `std::ops::Add` might be missing for `World`
+  --> $DIR/issue-39018.rs:17:13
+   |
+17 |     let y = World::Hello + World::Goodbye;
+   |             ^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
