diff --git a/src/lib.rs b/src/lib.rs index e22ee28e7..cf5def800 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,6 +44,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box identity_op::IdentityOp as LintPassObject); reg.register_lint_pass(box mut_mut::MutMut as LintPassObject); reg.register_lint_pass(box len_zero::LenZero as LintPassObject); + reg.register_lint_pass(box misc::CmpOwned as LintPassObject); reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST, misc::SINGLE_MATCH, misc::STR_TO_STRING, @@ -54,7 +55,7 @@ pub fn plugin_registrar(reg: &mut Registry) { needless_bool::NEEDLESS_BOOL, approx_const::APPROX_CONSTANT, misc::CMP_NAN, misc::FLOAT_CMP, - misc::PRECEDENCE, + misc::PRECEDENCE, misc::CMP_OWNED, eta_reduction::REDUNDANT_CLOSURE, identity_op::IDENTITY_OP, mut_mut::MUT_MUT, diff --git a/src/misc.rs b/src/misc.rs index 8cde5104f..5f5b07a15 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -211,3 +211,52 @@ fn is_arith_op(op : BinOp_) -> bool { _ => false } } + +declare_lint!(pub CMP_OWNED, Warn, + "Warn on creating an owned string just for comparison"); + +#[derive(Copy,Clone)] +pub struct CmpOwned; + +impl LintPass for CmpOwned { + fn get_lints(&self) -> LintArray { + lint_array!(CMP_OWNED) + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let ExprBinary(ref cmp, ref left, ref right) = expr.node { + if is_comparison_binop(cmp.node) { + check_to_owned(cx, left, right.span); + check_to_owned(cx, right, left.span) + } + } + } +} + +fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) { + match &expr.node { + &ExprMethodCall(Spanned{node: ref ident, ..}, _, _) => { + let name = ident.as_str(); + if name == "to_string" || name == "to_owned" { + cx.span_lint(CMP_OWNED, expr.span, &format!( + "this creates an owned instance just for comparison. + Consider using {}.as_slice() to compare without allocation", + cx.sess().codemap().span_to_snippet(other_span).unwrap_or( + "..".to_string()))) + } + }, + &ExprCall(ref path, _) => { + if let &ExprPath(None, ref path) = &path.node { + if path.segments.iter().zip(["String", "from_str"].iter()).all( + |(seg, name)| &seg.identifier.as_str() == name) { + cx.span_lint(CMP_OWNED, expr.span, &format!( + "this creates an owned instance just for comparison. + Consider using {}.as_slice() to compare without allocation", + cx.sess().codemap().span_to_snippet(other_span).unwrap_or( + "..".to_string()))) + } + } + }, + _ => () + } +} diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs new file mode 100644 index 000000000..a8b0cb32f --- /dev/null +++ b/tests/compile-fail/cmp_owned.rs @@ -0,0 +1,17 @@ +#![feature(plugin, collections)] +#![plugin(clippy)] + +#[deny(cmp_owned)] +fn main() { + let x = "oh"; + + #[allow(str_to_string)] + fn with_to_string(x : &str) { + x != "foo".to_string(); //~ERROR this creates an owned instance + } + with_to_string(x); + + x != "foo".to_owned(); //~ERROR this creates an owned instance + + x != String::from_str("foo"); //~ERROR this creates an owned instance +}