diff --git a/README.md b/README.md index 0a947e12ecfc..4644bac94cde 100644 --- a/README.md +++ b/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()`. diff --git a/src/lib.rs b/src/lib.rs index 7b47edaa4965..f2872a17b56e 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,6 +56,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); diff --git a/src/strings.rs b/src/strings.rs index 97016f362681..60ea23556ac3 100644 --- a/src/strings.rs +++ b/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 } } diff --git a/tests/compile-fail/strings.rs b/tests/compile-fail/strings.rs index 4b6f0bc884f6..e898a087d08b 100755 --- a/tests/compile-fail/strings.rs +++ b/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); }