added string_add lint and fixed string_add_assign + test

README.md

@@ -30,6 +30,7 @@ Lints included in this crate:
  - `collapsible_if`: Warns on cases where two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }`
  - `zero_width_space`: Warns on encountering a unicode zero-width space
  - `string_add_assign`: Warns on `x = x + ..` where `x` is a `String` and suggests using `push_str(..)` instead.
+ - `string_add`: Matches `x + ..` where `x` is a `String` and where `string_add_assign` doesn't warn. Allowed by default.
  - `needless_return`: Warns on using `return expr;` when a simple `expr` would suffice.
  - `let_and_return`: Warns on doing `let x = expr; x` at the end of a function.
  - `option_unwrap_used`: Warns when `Option.unwrap()` is used, and suggests `.expect()`.

src/lib.rs

@@ -58,6 +58,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box misc::ModuloOne as LintPassObject);
     reg.register_lint_pass(box unicode::Unicode as LintPassObject);
     reg.register_lint_pass(box strings::StringAdd as LintPassObject);
+    reg.register_lint_pass(box strings::StringAddAssign as LintPassObject);
     reg.register_lint_pass(box returns::ReturnPass as LintPassObject);
     reg.register_lint_pass(box methods::MethodsPass as LintPassObject);
     reg.register_lint_pass(box types::LetPass as LintPassObject);

src/strings.rs

@@ -9,7 +9,7 @@ use syntax::ast::*;
 use syntax::codemap::{Span, Spanned};
 use eq_op::is_exp_equal;
 use types::match_ty_unwrap;
-use utils::{match_def_path, span_lint, walk_ptrs_ty};
+use utils::{match_def_path, span_lint, walk_ptrs_ty, get_parent_expr};
 
 declare_lint! {
     pub STRING_ADD_ASSIGN,
@@ -17,10 +17,48 @@ declare_lint! {
     "Warn on `x = x + ..` where x is a `String`"
 }
 
-#[derive(Copy,Clone)]
+declare_lint! {
+	pub STRING_ADD,
+	Allow,
+	"Warn on `x + ..` where x is a `String`"
+}
+
+#[derive(Copy, Clone)]
 pub struct StringAdd;
 
 impl LintPass for StringAdd {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(STRING_ADD)
+    }
+
+    fn check_expr(&mut self, cx: &Context, e: &Expr) {
+		if let &ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) = &e.node {
+			if is_string(cx, left) {
+				if let Allow = cx.current_level(STRING_ADD_ASSIGN) {
+					// the string_add_assign is allow, so no duplicates
+				} else {
+					let parent = get_parent_expr(cx, e);
+					if let Some(ref p) = parent {
+						if let &ExprAssign(ref target, _) = &p.node {
+							// avoid duplicate matches
+							if is_exp_equal(target, left) { return; }
+						}
+					}
+				}
+				//TODO check for duplicates
+				 span_lint(cx, STRING_ADD, e.span,
+						"you add something to a string. \
+						Consider using `String::push_str()` instead.")
+			}
+		}
+	}
+}
+            
+
+#[derive(Copy, Clone)]
+pub struct StringAddAssign;
+
+impl LintPass for StringAddAssign {
     fn get_lints(&self) -> LintArray {
         lint_array!(STRING_ADD_ASSIGN)
     }
@@ -37,8 +75,9 @@ impl LintPass for StringAdd {
 }
 
 fn is_string(cx: &Context, e: &Expr) -> bool {
-    if let TyStruct(did, _) = walk_ptrs_ty(cx.tcx.expr_ty(e)).sty {
-        match_def_path(cx, did.did, &["std", "string", "String"])
+	let ty = walk_ptrs_ty(cx.tcx.expr_ty(e));
+    if let TyStruct(did, _) = ty.sty {
+        match_def_path(cx, did.did, &["collections", "string", "String"])
     } else { false }
 }
 

tests/compile-fail/strings.rs

@@ -2,11 +2,16 @@
 #![plugin(clippy)]
 
 #![deny(string_add_assign)]
-
+#![deny(string_add)]
 fn main() {
-    let x = "".to_owned();
+    let mut x = "".to_owned();
 
-    for i in (1..3) {
-        x = x + "."; //~ERROR
+    for _ in (1..3) {
+        x = x + "."; //~ERROR you assign the result of adding something to this string.
     }
+    
+    let y = "".to_owned();
+    let z = y + "..."; //~ERROR you add something to a string.
+    
+    assert_eq!(&x, &z);
 }