From 017dac23017e2dcf8fe350b66821a9e50d39bbd1 Mon Sep 17 00:00:00 2001
From: Georg Brandl <georg@python.org>
Date: Fri, 21 Aug 2015 19:49:00 +0200
Subject: [PATCH] new lint: using &Ref patterns instead of matching on *expr
 (fixes #187)

---
 README.md                                     |  1 +
 src/lib.rs                                    |  1 +
 src/matches.rs                                | 30 +++++++++++++++++--
 .../{match_if_let.rs => matches.rs}           | 23 +++++++++++++-
 4 files changed, 51 insertions(+), 4 deletions(-)
 rename tests/compile-fail/{match_if_let.rs => matches.rs} (58%)

diff --git a/README.md b/README.md
index 559761040..72ce27392 100644
--- a/README.md
+++ b/README.md
@@ -29,6 +29,7 @@ len_zero                 | warn    | checking `.len() == 0` or `.len() > 0` (or
 let_and_return           | warn    | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a function
 let_unit_value           | warn    | creating a let binding to a value of unit type, which usually can't be used afterwards
 linkedlist               | warn    | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a RingBuf
+match_ref_pats           | warn    | a match has all arms prefixed with `&`; the match expression can be dereferenced instead
 modulo_one               | warn    | taking a number modulo 1, which always returns 0
 mut_mut                  | warn    | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)
 needless_bool            | warn    | if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
diff --git a/src/lib.rs b/src/lib.rs
index fbeebb210..26af063c9 100755
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -88,6 +88,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         loops::EXPLICIT_ITER_LOOP,
         loops::ITER_NEXT_LOOP,
         loops::NEEDLESS_RANGE_LOOP,
+        matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,
         methods::OPTION_UNWRAP_USED,
         methods::RESULT_UNWRAP_USED,
diff --git a/src/matches.rs b/src/matches.rs
index b9f6cbc43..b704a2b47 100644
--- a/src/matches.rs
+++ b/src/matches.rs
@@ -3,23 +3,27 @@ use syntax::ast;
 use syntax::ast::*;
 use std::borrow::Cow;
 
-use utils::{snippet, snippet_block, span_help_and_lint};
+use utils::{snippet, snippet_block, span_lint, span_help_and_lint};
 
 declare_lint!(pub SINGLE_MATCH, Warn,
               "a match statement with a single nontrivial arm (i.e, where the other arm \
                is `_ => {}`) is used; recommends `if let` instead");
+declare_lint!(pub MATCH_REF_PATS, Warn,
+              "a match has all arms prefixed with `&`; the match expression can be \
+               dereferenced instead");
 
 #[allow(missing_copy_implementations)]
 pub struct MatchPass;
 
 impl LintPass for MatchPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(SINGLE_MATCH)
+        lint_array!(SINGLE_MATCH, MATCH_REF_PATS)
     }
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
         if let ExprMatch(ref ex, ref arms, ast::MatchSource::Normal) = expr.node {
-            // check preconditions: only two arms
+            // check preconditions for SINGLE_MATCH
+                // only two arms
             if arms.len() == 2 &&
                 // both of the arms have a single pattern and no guard
                 arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
@@ -48,6 +52,13 @@ impl LintPass for MatchPass {
                                 body_code)
                 );
             }
+
+            // check preconditions for MATCH_REF_PATS
+            if has_only_ref_pats(arms) {
+                span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
+                    "instead of prefixing all patterns with `&`, you can dereference the \
+                     expression to match: `match *{} {{ ...`", snippet(cx, ex.span, "..")));
+            }
         }
     }
 }
@@ -59,3 +70,16 @@ fn is_unit_expr(expr: &Expr) -> bool {
         _ => false,
     }
 }
+
+fn has_only_ref_pats(arms: &[Arm]) -> bool {
+    for arm in arms {
+        for pat in &arm.pats {
+            match pat.node {
+                PatRegion(..) => (),  // &-patterns
+                PatWild(..) => (),    // an "anything" wildcard is also fine
+                _ => return false,
+            }
+        }
+    }
+    true
+}
diff --git a/tests/compile-fail/match_if_let.rs b/tests/compile-fail/matches.rs
similarity index 58%
rename from tests/compile-fail/match_if_let.rs
rename to tests/compile-fail/matches.rs
index bf2e7e43a..43cf43b68 100755
--- a/tests/compile-fail/match_if_let.rs
+++ b/tests/compile-fail/matches.rs
@@ -2,8 +2,9 @@
 
 #![plugin(clippy)]
 #![deny(clippy)]
+#![allow(unused)]
 
-fn main(){
+fn single_match(){
     let x = Some(1u8);
     match x {  //~ ERROR you seem to be trying to use match
                //~^ HELP try
@@ -36,3 +37,23 @@ fn main(){
         _ => println!("nope"),
     }
 }
+
+fn ref_pats() {
+    let ref v = Some(0);
+    match v {  //~ERROR instead of prefixing all patterns with `&`
+        &Some(v) => println!("{:?}", v),
+        &None => println!("none"),
+    }
+    match v {  // this doesn't trigger, we have a different pattern
+        &Some(v) => println!("some"),
+        other => println!("other"),
+    }
+    let ref tup = (1, 2);
+    match tup {  //~ERROR instead of prefixing all patterns with `&`
+        &(v, 1) => println!("{}", v),
+        _ => println!("none"),
+    }
+}
+
+fn main() {
+}