From 567d5a7293f2a3ab6672919fb0ce8f94c28a6136 Mon Sep 17 00:00:00 2001
From: Manish Goregaokar <manishsmail@gmail.com>
Date: Tue, 13 Oct 2015 04:16:05 +0530
Subject: [PATCH] Improve cmp_owned suggestions (fixes #386)

---
 src/misc.rs                     | 42 +++++++++++++++++++++------------
 tests/compile-fail/cmp_owned.rs |  6 ++++-
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/src/misc.rs b/src/misc.rs
index cf4504d2f67..497bb6692c7 100644
--- a/src/misc.rs
+++ b/src/misc.rs
@@ -165,37 +165,49 @@ impl LateLintPass for CmpOwned {
     fn check_expr(&mut self, cx: &LateContext, 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)
+                check_to_owned(cx, left, right.span, true, cmp.span);
+                check_to_owned(cx, right, left.span, false, cmp.span)
             }
         }
     }
 }
 
-fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span) {
-    match expr.node {
-        ExprMethodCall(Spanned{node: ref name, ..}, _, ref args) => {
+fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, op: Span) {
+    let snip = match expr.node {
+        ExprMethodCall(Spanned{node: ref name, ..}, _, ref args) if args.len() == 1 => {
             if name.as_str() == "to_string" ||
                 name.as_str() == "to_owned" && is_str_arg(cx, args) {
-                    span_lint(cx, CMP_OWNED, expr.span, &format!(
-                        "this creates an owned instance just for comparison. \
-                         Consider using `{}.as_slice()` to compare without allocation",
-                        snippet(cx, other_span, "..")))
+                    snippet(cx, args[0].span, "..")
+                } else {
+                    return
                 }
         },
-        ExprCall(ref path, _) => {
+        ExprCall(ref path, ref v) if v.len() == 1 => {
             if let &ExprPath(None, ref path) = &path.node {
                 if match_path(path, &["String", "from_str"]) ||
                     match_path(path, &["String", "from"]) {
-                        span_lint(cx, CMP_OWNED, expr.span, &format!(
-                            "this creates an owned instance just for comparison. \
-                             Consider using `{}.as_slice()` to compare without allocation",
-                            snippet(cx, other_span, "..")))
+                            snippet(cx, v[0].span, "..")
+                    } else {
+                        return
                     }
+            } else {
+                return
             }
         },
-        _ => ()
+        _ => return
+    };
+    if left {
+        span_lint(cx, CMP_OWNED, expr.span, &format!(
+        "this creates an owned instance just for comparison. Consider using \
+        `{} {} {}` to compare without allocation", snip,
+        snippet(cx, op, "=="), snippet(cx, other_span, "..")));
+    } else {
+        span_lint(cx, CMP_OWNED, expr.span, &format!(
+        "this creates an owned instance just for comparison. Consider using \
+        `{} {} {}` to compare without allocation",
+        snippet(cx, other_span, ".."), snippet(cx, op, "=="),  snip));
     }
+
 }
 
 fn is_str_arg(cx: &LateContext, args: &[P<Expr>]) -> bool {
diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs
index 2765da5cf23..afca83e1d32 100755
--- a/tests/compile-fail/cmp_owned.rs
+++ b/tests/compile-fail/cmp_owned.rs
@@ -7,7 +7,11 @@ fn main() {
 
     #[allow(str_to_string)]
     fn with_to_string(x : &str) {
-        x != "foo".to_string(); //~ERROR this creates an owned instance
+        x != "foo".to_string();
+        //~^ ERROR this creates an owned instance just for comparison. Consider using `x != "foo"` to compare without allocation
+
+        "foo".to_string() != x;
+        //~^ ERROR this creates an owned instance just for comparison. Consider using `"foo" != x` to compare without allocation
     }
     with_to_string(x);