Merge pull request #63 from Manishearth/cmp_owned

new lint: cmp_owned

README.md

@@ -25,6 +25,7 @@ Lints included in this crate:
  - `mut_mut`: Warns on `&mut &mut` which is either a copy'n'paste error, or shows a fundamental misunderstanding of references
  - `len_zero`: Warns on `_.len() == 0` and suggests using `_.is_empty()` (or similar comparisons with `>` or `!=`)
  - `len_without_is_empty`: Warns on traits or impls that have a `.len()` but no `.is_empty()` method
+ - `cmp_owned`: Warns on creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
 
 To use, add the following lines to your Cargo.toml:
 

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,

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())))
+				}
+			}
+		},
+		_ => ()
+	}
+}

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
+}