New string_add_assign lint (first part of #121), also formatting & refactoring

diff --git a/README.md b/README.md
index 87339f464646b0f40d8ce80fa4738f918c7cd498..fcd8d38a3b3913033403c93a74166584796080b0 100644 (file)
--- a/README.md
+++ b/README.md
@@ -29,6 +29,7 @@ Lints included in this crate:
  - `inline_always`: Warns on `#[inline(always)]`, because in most cases it is a bad idea
  - `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.
 
 To use, add the following lines to your Cargo.toml:
 

diff --git a/src/eq_op.rs b/src/eq_op.rs
index bd9787bd51c707720567ef4b9608783a5adbb276..1000d310e39480ff046daf6236d03e13becfac10 100644 (file)
--- a/src/eq_op.rs
+++ b/src/eq_op.rs
@@ -23,238 +23,237 @@ fn check_expr(&mut self, cx: &Context, e: &Expr) {
         if let ExprBinary(ref op, ref left, ref right) = e.node {
             if is_cmp_or_bit(op) && is_exp_equal(left, right) {
                 span_lint(cx, EQ_OP, e.span, &format!(
-                                       "equal expressions as operands to {}", 
-                                               ast_util::binop_to_string(op.node)));
+                    "equal expressions as operands to {}", 
+                        ast_util::binop_to_string(op.node)));
             }
         }
     }
 }
 
-fn is_exp_equal(left : &Expr, right : &Expr) -> bool {
-       match (&left.node, &right.node) {
-               (&ExprBinary(ref lop, ref ll, ref lr), 
-                               &ExprBinary(ref rop, ref rl, ref rr)) => 
-                       lop.node == rop.node && 
-                       is_exp_equal(ll, rl) && is_exp_equal(lr, rr),
-               (&ExprBox(ref lpl, ref lbox), &ExprBox(ref rpl, ref rbox)) => 
-                       both(lpl, rpl, |l, r| is_exp_equal(l, r)) && 
-                               is_exp_equal(lbox, rbox),
-               (&ExprCall(ref lcallee, ref largs), 
-                &ExprCall(ref rcallee, ref rargs)) => is_exp_equal(lcallee, 
-                       rcallee) && is_exps_equal(largs, rargs),
-               (&ExprCast(ref lc, ref lty), &ExprCast(ref rc, ref rty)) => 
-                       is_ty_equal(lty, rty) && is_exp_equal(lc, rc),
-               (&ExprField(ref lfexp, ref lfident), 
-                               &ExprField(ref rfexp, ref rfident)) => 
-                       lfident.node == rfident.node && is_exp_equal(lfexp, rfexp),
-               (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node,
-               (&ExprMethodCall(ref lident, ref lcty, ref lmargs), 
-                               &ExprMethodCall(ref rident, ref rcty, ref rmargs)) => 
-                       lident.node == rident.node && is_tys_equal(lcty, rcty) && 
-                               is_exps_equal(lmargs, rmargs),
-               (&ExprParen(ref lparen), _) => is_exp_equal(lparen, right),
-               (_, &ExprParen(ref rparen)) => is_exp_equal(left, rparen),
-               (&ExprPath(ref lqself, ref lsubpath), 
-                               &ExprPath(ref rqself, ref rsubpath)) => 
-                       both(lqself, rqself, |l, r| is_qself_equal(l, r)) && 
-                               is_path_equal(lsubpath, rsubpath),              
-               (&ExprTup(ref ltup), &ExprTup(ref rtup)) => 
-                       is_exps_equal(ltup, rtup),
-               (&ExprUnary(lunop, ref l), &ExprUnary(runop, ref r)) => 
-                       lunop == runop && is_exp_equal(l, r), 
-               (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(l, r),
-               _ => false
-       }
+pub fn is_exp_equal(left : &Expr, right : &Expr) -> bool {
+    match (&left.node, &right.node) {
+        (&ExprBinary(ref lop, ref ll, ref lr), 
+                &ExprBinary(ref rop, ref rl, ref rr)) => 
+            lop.node == rop.node && 
+            is_exp_equal(ll, rl) && is_exp_equal(lr, rr),
+        (&ExprBox(ref lpl, ref lbox), &ExprBox(ref rpl, ref rbox)) => 
+            both(lpl, rpl, |l, r| is_exp_equal(l, r)) && 
+                is_exp_equal(lbox, rbox),
+        (&ExprCall(ref lcallee, ref largs), 
+         &ExprCall(ref rcallee, ref rargs)) => is_exp_equal(lcallee, 
+            rcallee) && is_exps_equal(largs, rargs),
+        (&ExprCast(ref lc, ref lty), &ExprCast(ref rc, ref rty)) => 
+            is_ty_equal(lty, rty) && is_exp_equal(lc, rc),
+        (&ExprField(ref lfexp, ref lfident), 
+                &ExprField(ref rfexp, ref rfident)) => 
+            lfident.node == rfident.node && is_exp_equal(lfexp, rfexp),
+        (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node,
+        (&ExprMethodCall(ref lident, ref lcty, ref lmargs), 
+                &ExprMethodCall(ref rident, ref rcty, ref rmargs)) => 
+            lident.node == rident.node && is_tys_equal(lcty, rcty) && 
+                is_exps_equal(lmargs, rmargs),
+        (&ExprParen(ref lparen), _) => is_exp_equal(lparen, right),
+        (_, &ExprParen(ref rparen)) => is_exp_equal(left, rparen),
+        (&ExprPath(ref lqself, ref lsubpath), 
+                &ExprPath(ref rqself, ref rsubpath)) => 
+            both(lqself, rqself, |l, r| is_qself_equal(l, r)) && 
+                is_path_equal(lsubpath, rsubpath),      
+        (&ExprTup(ref ltup), &ExprTup(ref rtup)) => 
+            is_exps_equal(ltup, rtup),
+        (&ExprUnary(lunop, ref l), &ExprUnary(runop, ref r)) => 
+            lunop == runop && is_exp_equal(l, r), 
+        (&ExprVec(ref l), &ExprVec(ref r)) => is_exps_equal(l, r),
+        _ => false
+    }
 }
 
 fn is_exps_equal(left : &[P<Expr>], right : &[P<Expr>]) -> bool {
-       over(left, right, |l, r| is_exp_equal(l, r))
+    over(left, right, |l, r| is_exp_equal(l, r))
 }
 
 fn is_path_equal(left : &Path, right : &Path) -> bool {
     // The == of idents doesn't work with different contexts,
-    // we have to be explicit about hygeine
-       left.global == right.global
-    && left.segments.iter().zip(right.segments.iter())
-           .all( |(l,r)| l.identifier.name == r.identifier.name
-                         && l.identifier.ctxt == r.identifier.ctxt
-                         && l.parameters == r.parameters)
+    // we have to be explicit about hygiene
+    left.global == right.global && over(&left.segments, &right.segments, 
+        |l, r| l.identifier.name == r.identifier.name
+              && l.identifier.ctxt == r.identifier.ctxt
+               && l.parameters == r.parameters)
 }
 
 fn is_qself_equal(left : &QSelf, right : &QSelf) -> bool {
-       left.ty.node == right.ty.node && left.position == right.position
+    left.ty.node == right.ty.node && left.position == right.position
 }
 
 fn is_ty_equal(left : &Ty, right : &Ty) -> bool {
-       match (&left.node, &right.node) {
-       (&TyVec(ref lvec), &TyVec(ref rvec)) => is_ty_equal(lvec, rvec),
-       (&TyFixedLengthVec(ref lfvty, ref lfvexp), 
-                       &TyFixedLengthVec(ref rfvty, ref rfvexp)) => 
-               is_ty_equal(lfvty, rfvty) && is_exp_equal(lfvexp, rfvexp),
-       (&TyPtr(ref lmut), &TyPtr(ref rmut)) => is_mut_ty_equal(lmut, rmut),
-       (&TyRptr(ref ltime, ref lrmut), &TyRptr(ref rtime, ref rrmut)) => 
-               both(ltime, rtime, is_lifetime_equal) && 
-               is_mut_ty_equal(lrmut, rrmut),
-       (&TyBareFn(ref lbare), &TyBareFn(ref rbare)) => 
-               is_bare_fn_ty_equal(lbare, rbare),
+    match (&left.node, &right.node) {
+    (&TyVec(ref lvec), &TyVec(ref rvec)) => is_ty_equal(lvec, rvec),
+    (&TyFixedLengthVec(ref lfvty, ref lfvexp), 
+            &TyFixedLengthVec(ref rfvty, ref rfvexp)) => 
+        is_ty_equal(lfvty, rfvty) && is_exp_equal(lfvexp, rfvexp),
+    (&TyPtr(ref lmut), &TyPtr(ref rmut)) => is_mut_ty_equal(lmut, rmut),
+    (&TyRptr(ref ltime, ref lrmut), &TyRptr(ref rtime, ref rrmut)) => 
+        both(ltime, rtime, is_lifetime_equal) && 
+        is_mut_ty_equal(lrmut, rrmut),
+    (&TyBareFn(ref lbare), &TyBareFn(ref rbare)) => 
+        is_bare_fn_ty_equal(lbare, rbare),
     (&TyTup(ref ltup), &TyTup(ref rtup)) => is_tys_equal(ltup, rtup),
-       (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => 
-               both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath),
+    (&TyPath(ref lq, ref lpath), &TyPath(ref rq, ref rpath)) => 
+        both(lq, rq, is_qself_equal) && is_path_equal(lpath, rpath),
     (&TyObjectSum(ref lsumty, ref lobounds), 
-                       &TyObjectSum(ref rsumty, ref robounds)) => 
-               is_ty_equal(lsumty, rsumty) && 
-               is_param_bounds_equal(lobounds, robounds),
-       (&TyPolyTraitRef(ref ltbounds), &TyPolyTraitRef(ref rtbounds)) => 
-               is_param_bounds_equal(ltbounds, rtbounds),
+            &TyObjectSum(ref rsumty, ref robounds)) => 
+        is_ty_equal(lsumty, rsumty) && 
+        is_param_bounds_equal(lobounds, robounds),
+    (&TyPolyTraitRef(ref ltbounds), &TyPolyTraitRef(ref rtbounds)) => 
+        is_param_bounds_equal(ltbounds, rtbounds),
     (&TyParen(ref lty), &TyParen(ref rty)) => is_ty_equal(lty, rty),
     (&TyTypeof(ref lof), &TyTypeof(ref rof)) => is_exp_equal(lof, rof),
-       (&TyInfer, &TyInfer) => true,
-       _ => false
-       }
+    (&TyInfer, &TyInfer) => true,
+    _ => false
+    }
 }
 
 fn is_param_bound_equal(left : &TyParamBound, right : &TyParamBound) 
-               -> bool {
-       match(left, right) {
-       (&TraitTyParamBound(ref lpoly, ref lmod), 
-                       &TraitTyParamBound(ref rpoly, ref rmod)) => 
-               lmod == rmod && is_poly_traitref_equal(lpoly, rpoly),
+        -> bool {
+    match(left, right) {
+    (&TraitTyParamBound(ref lpoly, ref lmod), 
+            &TraitTyParamBound(ref rpoly, ref rmod)) => 
+        lmod == rmod && is_poly_traitref_equal(lpoly, rpoly),
     (&RegionTyParamBound(ref ltime), &RegionTyParamBound(ref rtime)) => 
-               is_lifetime_equal(ltime, rtime),
+        is_lifetime_equal(ltime, rtime),
     _ => false
-       }
+    }
 }
 
 fn is_poly_traitref_equal(left : &PolyTraitRef, right : &PolyTraitRef)
-               -> bool {
-       is_lifetimedefs_equal(&left.bound_lifetimes, &right.bound_lifetimes)
-               && is_path_equal(&left.trait_ref.path, &right.trait_ref.path)
+        -> bool {
+    is_lifetimedefs_equal(&left.bound_lifetimes, &right.bound_lifetimes)
+        && is_path_equal(&left.trait_ref.path, &right.trait_ref.path)
 }
 
 fn is_param_bounds_equal(left : &TyParamBounds, right : &TyParamBounds)
-               -> bool {
-       over(left, right, is_param_bound_equal)
+        -> bool {
+    over(left, right, is_param_bound_equal)
 }
 
 fn is_mut_ty_equal(left : &MutTy, right : &MutTy) -> bool {
-       left.mutbl == right.mutbl && is_ty_equal(&left.ty, &right.ty)
+    left.mutbl == right.mutbl && is_ty_equal(&left.ty, &right.ty)
 }
 
 fn is_bare_fn_ty_equal(left : &BareFnTy, right : &BareFnTy) -> bool {
-       left.unsafety == right.unsafety && left.abi == right.abi && 
-               is_lifetimedefs_equal(&left.lifetimes, &right.lifetimes) && 
-                       is_fndecl_equal(&left.decl, &right.decl)
+    left.unsafety == right.unsafety && left.abi == right.abi && 
+        is_lifetimedefs_equal(&left.lifetimes, &right.lifetimes) && 
+            is_fndecl_equal(&left.decl, &right.decl)
 } 
 
 fn is_fndecl_equal(left : &P<FnDecl>, right : &P<FnDecl>) -> bool {
-       left.variadic == right.variadic && 
-               is_args_equal(&left.inputs, &right.inputs) && 
-               is_fnret_ty_equal(&left.output, &right.output)
+    left.variadic == right.variadic && 
+        is_args_equal(&left.inputs, &right.inputs) && 
+        is_fnret_ty_equal(&left.output, &right.output)
 }
 
 fn is_fnret_ty_equal(left : &FunctionRetTy, right : &FunctionRetTy) 
-               -> bool {
-       match (left, right) {
-       (&NoReturn(_), &NoReturn(_)) | 
-       (&DefaultReturn(_), &DefaultReturn(_)) => true,
-       (&Return(ref lty), &Return(ref rty)) => is_ty_equal(lty, rty),
-       _ => false      
-       }
+        -> bool {
+    match (left, right) {
+    (&NoReturn(_), &NoReturn(_)) | 
+    (&DefaultReturn(_), &DefaultReturn(_)) => true,
+    (&Return(ref lty), &Return(ref rty)) => is_ty_equal(lty, rty),
+    _ => false  
+    }
 }
 
 fn is_arg_equal(l: &Arg, r : &Arg) -> bool {
-       is_ty_equal(&l.ty, &r.ty) && is_pat_equal(&l.pat, &r.pat)
+    is_ty_equal(&l.ty, &r.ty) && is_pat_equal(&l.pat, &r.pat)
 }
 
 fn is_args_equal(left : &[Arg], right : &[Arg]) -> bool {
-       over(left, right, is_arg_equal)
+    over(left, right, is_arg_equal)
 }
 
 fn is_pat_equal(left : &Pat, right : &Pat) -> bool {
-       match(&left.node, &right.node) {
-       (&PatWild(lwild), &PatWild(rwild)) => lwild == rwild,
-       (&PatIdent(ref lmode, ref lident, Option::None), 
-                       &PatIdent(ref rmode, ref rident, Option::None)) =>
-               lmode == rmode && is_ident_equal(&lident.node, &rident.node),
-       (&PatIdent(ref lmode, ref lident, Option::Some(ref lpat)), 
-                       &PatIdent(ref rmode, ref rident, Option::Some(ref rpat))) =>
-               lmode == rmode && is_ident_equal(&lident.node, &rident.node) && 
-                       is_pat_equal(lpat, rpat),
+    match(&left.node, &right.node) {
+    (&PatWild(lwild), &PatWild(rwild)) => lwild == rwild,
+    (&PatIdent(ref lmode, ref lident, Option::None), 
+            &PatIdent(ref rmode, ref rident, Option::None)) =>
+        lmode == rmode && is_ident_equal(&lident.node, &rident.node),
+    (&PatIdent(ref lmode, ref lident, Option::Some(ref lpat)), 
+            &PatIdent(ref rmode, ref rident, Option::Some(ref rpat))) =>
+        lmode == rmode && is_ident_equal(&lident.node, &rident.node) && 
+            is_pat_equal(lpat, rpat),
     (&PatEnum(ref lpath, ref lenum), &PatEnum(ref rpath, ref renum)) => 
-               is_path_equal(lpath, rpath) && both(lenum, renum, |l, r| 
-                       is_pats_equal(l, r)),
+        is_path_equal(lpath, rpath) && both(lenum, renum, |l, r| 
+            is_pats_equal(l, r)),
     (&PatStruct(ref lpath, ref lfieldpat, lbool), 
-                       &PatStruct(ref rpath, ref rfieldpat, rbool)) =>
-               lbool == rbool && is_path_equal(lpath, rpath) && 
-                       is_spanned_fieldpats_equal(lfieldpat, rfieldpat),
+            &PatStruct(ref rpath, ref rfieldpat, rbool)) =>
+        lbool == rbool && is_path_equal(lpath, rpath) && 
+            is_spanned_fieldpats_equal(lfieldpat, rfieldpat),
     (&PatTup(ref ltup), &PatTup(ref rtup)) => is_pats_equal(ltup, rtup), 
     (&PatBox(ref lboxed), &PatBox(ref rboxed)) => 
-               is_pat_equal(lboxed, rboxed),
+        is_pat_equal(lboxed, rboxed),
     (&PatRegion(ref lpat, ref lmut), &PatRegion(ref rpat, ref rmut)) => 
-               is_pat_equal(lpat, rpat) && lmut == rmut,
-       (&PatLit(ref llit), &PatLit(ref rlit)) => is_exp_equal(llit, rlit),
+        is_pat_equal(lpat, rpat) && lmut == rmut,
+    (&PatLit(ref llit), &PatLit(ref rlit)) => is_exp_equal(llit, rlit),
     (&PatRange(ref lfrom, ref lto), &PatRange(ref rfrom, ref rto)) =>
-               is_exp_equal(lfrom, rfrom) && is_exp_equal(lto, rto),
+        is_exp_equal(lfrom, rfrom) && is_exp_equal(lto, rto),
     (&PatVec(ref lfirst, Option::None, ref llast), 
-                       &PatVec(ref rfirst, Option::None, ref rlast)) =>
-               is_pats_equal(lfirst, rfirst) && is_pats_equal(llast, rlast),
+            &PatVec(ref rfirst, Option::None, ref rlast)) =>
+        is_pats_equal(lfirst, rfirst) && is_pats_equal(llast, rlast),
     (&PatVec(ref lfirst, Option::Some(ref lpat), ref llast), 
-                       &PatVec(ref rfirst, Option::Some(ref rpat), ref rlast)) =>
-               is_pats_equal(lfirst, rfirst) && is_pat_equal(lpat, rpat) && 
-                       is_pats_equal(llast, rlast),
-       // I don't match macros for now, the code is slow enough as is ;-)
-       _ => false
-       }
+            &PatVec(ref rfirst, Option::Some(ref rpat), ref rlast)) =>
+        is_pats_equal(lfirst, rfirst) && is_pat_equal(lpat, rpat) && 
+            is_pats_equal(llast, rlast),
+    // I don't match macros for now, the code is slow enough as is ;-)
+    _ => false
+    }
 }
 
 fn is_spanned_fieldpats_equal(left : &[code::Spanned<FieldPat>], 
-               right : &[code::Spanned<FieldPat>]) -> bool {
-       over(left, right, |l, r| is_fieldpat_equal(&l.node, &r.node))
+        right : &[code::Spanned<FieldPat>]) -> bool {
+    over(left, right, |l, r| is_fieldpat_equal(&l.node, &r.node))
 }
 
 fn is_fieldpat_equal(left : &FieldPat, right : &FieldPat) -> bool {
-       left.is_shorthand == right.is_shorthand && 
-               is_ident_equal(&left.ident, &right.ident) && 
-               is_pat_equal(&left.pat, &right.pat) 
+    left.is_shorthand == right.is_shorthand && 
+        is_ident_equal(&left.ident, &right.ident) && 
+        is_pat_equal(&left.pat, &right.pat) 
 }
 
 fn is_ident_equal(left : &Ident, right : &Ident) -> bool {
-       &left.name == &right.name && left.ctxt == right.ctxt
+    &left.name == &right.name && left.ctxt == right.ctxt
 }
 
 fn is_pats_equal(left : &[P<Pat>], right : &[P<Pat>]) -> bool {
-       over(left, right, |l, r| is_pat_equal(l, r))
+    over(left, right, |l, r| is_pat_equal(l, r))
 }
 
 fn is_lifetimedef_equal(left : &LifetimeDef, right : &LifetimeDef)
-               -> bool {
-       is_lifetime_equal(&left.lifetime, &right.lifetime) && 
-               over(&left.bounds, &right.bounds, is_lifetime_equal)
+        -> bool {
+    is_lifetime_equal(&left.lifetime, &right.lifetime) && 
+        over(&left.bounds, &right.bounds, is_lifetime_equal)
 }
 
 fn is_lifetimedefs_equal(left : &[LifetimeDef], right : &[LifetimeDef]) 
-               -> bool {
-       over(left, right, is_lifetimedef_equal)
+        -> bool {
+    over(left, right, is_lifetimedef_equal)
 }
 
 fn is_lifetime_equal(left : &Lifetime, right : &Lifetime) -> bool {
-       left.name == right.name
+    left.name == right.name
 }
 
 fn is_tys_equal(left : &[P<Ty>], right : &[P<Ty>]) -> bool {
-       over(left, right, |l, r| is_ty_equal(l, r))
+    over(left, right, |l, r| is_ty_equal(l, r))
 }
 
 fn over<X, F>(left: &[X], right: &[X], mut eq_fn: F) -> bool 
-               where F: FnMut(&X, &X) -> bool {
+        where F: FnMut(&X, &X) -> bool {
     left.len() == right.len() && left.iter().zip(right).all(|(x, y)| 
-               eq_fn(x, y))
+        eq_fn(x, y))
 }
 
 fn both<X, F>(l: &Option<X>, r: &Option<X>, mut eq_fn : F) -> bool 
-               where F: FnMut(&X, &X) -> bool {
-       l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false,
-               |y| eq_fn(x, y)))
+        where F: FnMut(&X, &X) -> bool {
+    l.as_ref().map_or_else(|| r.is_none(), |x| r.as_ref().map_or(false,
+        |y| eq_fn(x, y)))
 }
 
 fn is_cmp_or_bit(op : &BinOp) -> bool {

diff --git a/src/lib.rs b/src/lib.rs
index 647128e0f0c4bfe3346d8b6725360f6ade2c3044..d0d0b6374689afb84776c0458f35279b4903cc8a 100644 (file)
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -28,6 +28,7 @@
 pub mod collapsible_if;
 pub mod unicode;
 pub mod utils;
+pub mod strings;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
@@ -51,6 +52,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box attrs::AttrPass as LintPassObject);
     reg.register_lint_pass(box collapsible_if::CollapsibleIf as LintPassObject);
     reg.register_lint_pass(box unicode::Unicode as LintPassObject);
+    reg.register_lint_pass(box strings::StringAdd as LintPassObject);
     
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH, misc::STR_TO_STRING,
@@ -70,5 +72,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            attrs::INLINE_ALWAYS,
                                            collapsible_if::COLLAPSIBLE_IF,
                                            unicode::ZERO_WIDTH_SPACE,
+                                           strings::STRING_ADD_ASSIGN,
                                            ]);
 }

diff --git a/src/strings.rs b/src/strings.rs
new file mode 100644 (file)
index 0000000..8bd882c
--- /dev/null
+++ b/src/strings.rs
@@ -0,0 +1,55 @@
+//! This LintPass catches both string addition and string addition + assignment
+//! 
+//! Note that since we have two lints where one subsumes the other, we try to
+//! disable the subsumed lint unless it has a higher level
+
+use rustc::lint::*;
+use rustc::middle::ty::TypeVariants::TyStruct;
+use syntax::ast::*;
+use syntax::codemap::{Span, Spanned};
+use eq_op::is_exp_equal;
+use misc::walk_ty;
+use types::match_ty_unwrap;
+use utils::{match_def_path, span_lint};
+
+declare_lint! {
+    pub STRING_ADD_ASSIGN,
+    Warn,
+    "Warn on `x = 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_ASSIGN)
+    }
+    
+    fn check_expr(&mut self, cx: &Context, e: &Expr) {
+        if let &ExprAssign(ref target, ref  src) =&e.node {
+            if is_string(cx, target) && is_add(src, target) { 
+                span_lint(cx, STRING_ADD_ASSIGN, e.span, 
+                    "You assign the result of adding something to this string. \
+                    Consider using `String::push_str(..) instead.")
+            }
+        }
+    }
+}
+
+fn is_string(cx: &Context, e: &Expr) -> bool {
+    if let TyStruct(def_id, _) = walk_ty(cx.tcx.expr_ty(e)).sty {
+        match_def_path(cx, def_id, &["std", "string", "String"])
+    } else { false }
+}
+
+fn is_add(src: &Expr, target: &Expr) -> bool {
+    match &src.node {
+        &ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) =>
+            is_exp_equal(target, left),
+        &ExprBlock(ref block) => block.stmts.is_empty() && 
+            block.expr.as_ref().map_or(false, |expr| is_add(&*expr, target)),
+        &ExprParen(ref expr) => is_add(&*expr, target),
+        _ => false
+    }
+}

diff --git a/src/utils.rs b/src/utils.rs
index d62e082c1fc2ed2f9c9aa996f666046ad023a39b..4a87f4b3a2ef6d549c82a36f10bab47eff43b464 100644 (file)
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -41,7 +41,7 @@ pub fn match_def_path(cx: &Context, def_id: DefId, path: &[&str]) -> bool {
 /// `match_path(path, &["std", "rt", "begin_unwind"])`
 pub fn match_path(path: &Path, segments: &[&str]) -> bool {
        path.segments.iter().rev().zip(segments.iter().rev()).all(
-               |(a,b)| a.identifier.name == b)
+               |(a,b)| &a.identifier.name.as_str() == b)
 }
 
 /// convert a span to a code snippet if available, otherwise use default, e.g.

diff --git a/tests/compile-fail/strings.rs b/tests/compile-fail/strings.rs
new file mode 100644 (file)
index 0000000..2b200f1
--- /dev/null
+++ b/tests/compile-fail/strings.rs
@@ -0,0 +1,12 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(string_add_assign)]
+
+fn main() {
+       let x = "".to_owned();
+       
+       for i in (1..3) {
+               x = x + "."; //~ERROR
+       }
+}