Merge pull request #470 from sanxiyn/match-ref-pats

Extend match_ref_pats to desugared matches

src/approx_const.rs

@@ -44,7 +44,7 @@ impl LintPass for ApproxConstant {
 
 impl LateLintPass for ApproxConstant {
     fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
-        if let &ExprLit(ref lit) = &e.node {
+        if let ExprLit(ref lit) = e.node {
             check_lit(cx, lit, e);
         }
     }

src/attrs.rs

@@ -42,7 +42,7 @@ impl LateLintPass for AttrPass {
 }
 
 fn is_relevant_item(item: &Item) -> bool {
-    if let &ItemFn(_, _, _, _, _, ref block) = &item.node {
+    if let ItemFn(_, _, _, _, _, ref block) = item.node {
         is_relevant_block(block)
     } else { false }
 }

src/bit_mask.rs

@@ -182,7 +182,7 @@ fn check_ineffective_gt(cx: &LateContext, span: Span, m: u64, c: u64, op: &str)
 fn fetch_int_literal(cx: &LateContext, lit : &Expr) -> Option<u64> {
     match lit.node {
         ExprLit(ref lit_ptr) => {
-            if let &LitInt(value, _) = &lit_ptr.node {
+            if let LitInt(value, _) = lit_ptr.node {
                 Option::Some(value) //TODO: Handle sign
             } else { Option::None }
         }

src/len_zero.rs

@@ -125,7 +125,7 @@ fn check_len_zero(cx: &LateContext, span: Span, name: &Name,
 fn has_is_empty(cx: &LateContext, expr: &Expr) -> bool {
     /// get a ImplOrTraitItem and return true if it matches is_empty(self)
     fn is_is_empty(cx: &LateContext, id: &ImplOrTraitItemId) -> bool {
-        if let &MethodTraitItemId(def_id) = id {
+        if let MethodTraitItemId(def_id) = *id {
             if let ty::MethodTraitItem(ref method) =
                 cx.tcx.impl_or_trait_item(def_id) {
                     method.name.as_str() == "is_empty"

src/lifetimes.rs

@@ -168,7 +168,7 @@ impl <'v, 't> RefVisitor<'v, 't>  {
     }
 
     fn record(&mut self, lifetime: &Option<Lifetime>) {
-        if let &Some(ref lt) = lifetime {
+        if let Some(ref lt) = *lifetime {
             if lt.name.as_str() == "'static" {
                 self.lts.push(Static);
             } else {

src/matches.rs

@@ -2,6 +2,7 @@ use rustc::lint::*;
 use rustc_front::hir::*;
 use rustc::middle::ty;
 use syntax::ast::Lit_::LitBool;
+use syntax::codemap::Span;
 
 use utils::{snippet, span_lint, span_help_and_lint, in_external_macro, expr_block};
 
@@ -25,9 +26,8 @@ impl LintPass for MatchPass {
 
 impl LateLintPass for MatchPass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
+        if in_external_macro(cx, expr.span) { return; }
         if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node {
-            if in_external_macro(cx, expr.span) { return; }
-
             // check preconditions for SINGLE_MATCH
                 // only two arms
             if arms.len() == 2 &&
@@ -53,19 +53,6 @@ impl LateLintPass for MatchPass {
                                             expr_block(cx, &arms[0].body, None, "..")));
             }
 
-            // check preconditions for MATCH_REF_PATS
-            if has_only_ref_pats(arms) {
-                if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
-                    span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
-                        "you don't need to add `&` to both the expression to match \
-                         and the patterns: use `match {} {{ ...`", snippet(cx, inner.span, "..")));
-                } else {
-                    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, "..")));
-                }
-            }
-
             // check preconditions for MATCH_BOOL
             // type of expression == bool
             if cx.tcx.expr_ty(ex).sty == ty::TyBool {
@@ -123,6 +110,22 @@ impl LateLintPass for MatchPass {
                 }
             }
         }
+        if let ExprMatch(ref ex, ref arms, source) = expr.node {
+            // check preconditions for MATCH_REF_PATS
+            if has_only_ref_pats(arms) {
+                if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
+                    let template = match_template(cx, expr.span, source, "", inner);
+                    span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
+                        "you don't need to add `&` to both the expression \
+                         and the patterns: use `{}`", template));
+                } else {
+                    let template = match_template(cx, expr.span, source, "*", ex);
+                    span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
+                        "instead of prefixing all patterns with `&`, you can dereference the \
+                         expression: `{}`", template));
+                }
+            }
+        }
     }
 }
 
@@ -143,3 +146,25 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
     // look for Some(v) where there's at least one true element
     mapped.map_or(false, |v| v.iter().any(|el| *el))
 }
+
+fn match_template(cx: &LateContext,
+                  span: Span,
+                  source: MatchSource,
+                  op: &str,
+                  expr: &Expr) -> String {
+    let expr_snippet = snippet(cx, expr.span, "..");
+    match source {
+        MatchSource::Normal => {
+            format!("match {}{} {{ ...", op, expr_snippet)
+        }
+        MatchSource::IfLetDesugar { .. } => {
+            format!("if let ... = {}{} {{", op, expr_snippet)
+        }
+        MatchSource::WhileLetDesugar => {
+            format!("while let ... = {}{} {{", op, expr_snippet)
+        }
+        MatchSource::ForLoopDesugar => {
+            cx.sess().span_bug(span, "for loop desugared to match with &-patterns!")
+        }
+    }
+}

src/misc.rs

@@ -84,10 +84,10 @@ impl LateLintPass for CmpNan {
     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) {
-                if let &ExprPath(_, ref path) = &left.node {
+                if let ExprPath(_, ref path) = left.node {
                     check_nan(cx, path, expr.span);
                 }
-                if let &ExprPath(_, ref path) = &right.node {
+                if let ExprPath(_, ref path) = right.node {
                     check_nan(cx, path, expr.span);
                 }
             }
@@ -189,7 +189,7 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other_span: Span, left: bool, o
                 }
         }
         ExprCall(ref path, ref v) if v.len() == 1 => {
-            if let &ExprPath(None, ref path) = &path.node {
+            if let ExprPath(None, ref path) = path.node {
                 if match_path(path, &["String", "from_str"]) ||
                     match_path(path, &["String", "from"]) {
                             snippet(cx, v[0].span, "..")
@@ -235,7 +235,7 @@ impl LintPass for ModuloOne {
 impl LateLintPass for ModuloOne {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if let ExprBinary(ref cmp, _, ref right) = expr.node {
-            if let &Spanned {node: BinOp_::BiRem, ..} = cmp {
+            if let Spanned {node: BinOp_::BiRem, ..} = *cmp {
                 if is_integer_literal(right, 1) {
                     cx.span_lint(MODULO_ONE, expr.span, "any number modulo 1 will be 0");
                 }

src/mutex_atomic.rs

@@ -34,7 +34,7 @@ pub struct MutexAtomic;
 impl LateLintPass for MutexAtomic {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         let ty = cx.tcx.expr_ty(expr);
-        if let &ty::TyStruct(_, subst) = &ty.sty {
+        if let ty::TyStruct(_, subst) = ty.sty {
             if match_type(cx, ty, &MUTEX_PATH) {
                 let mutex_param = &subst.types.get(ParamSpace::TypeSpace, 0).sty;
                 if let Some(atomic_name) = get_atomic_name(mutex_param) {

src/ptr_arg.rs

@@ -28,13 +28,13 @@ impl LintPass for PtrArg {
 
 impl LateLintPass for PtrArg {
     fn check_item(&mut self, cx: &LateContext, item: &Item) {
-        if let &ItemFn(ref decl, _, _, _, _, _) = &item.node {
+        if let ItemFn(ref decl, _, _, _, _, _) = item.node {
             check_fn(cx, decl);
         }
     }
 
     fn check_impl_item(&mut self, cx: &LateContext, item: &ImplItem) {
-        if let &ImplItemKind::Method(ref sig, _) = &item.node {
+        if let ImplItemKind::Method(ref sig, _) = item.node {
             if let Some(Node::NodeItem(it)) = cx.tcx.map.find(cx.tcx.map.get_parent(item.id)) {
                 if let ItemImpl(_, _, _, Some(_), _, _) = it.node {
                     return; // ignore trait impls
@@ -45,7 +45,7 @@ impl LateLintPass for PtrArg {
     }
 
     fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) {
-        if let &MethodTraitItem(ref sig, _) = &item.node {
+        if let MethodTraitItem(ref sig, _) = item.node {
             check_fn(cx, &sig.decl);
         }
     }

src/ranges.rs

@@ -40,10 +40,10 @@ impl LateLintPass for StepByZero {
                 if_let_chain! {
                     [
                         // .iter() call
-                        let &ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = iter,
+                        let ExprMethodCall( Spanned { node: ref iter_name, .. }, _, ref iter_args ) = *iter,
                         iter_name.as_str() == "iter",
                         // range expression in .zip() call: 0..x.len()
-                        let &ExprRange(Some(ref from), Some(ref to)) = zip_arg,
+                        let ExprRange(Some(ref from), Some(ref to)) = *zip_arg,
                         is_integer_literal(from, 0),
                         // .len() call
                         let ExprMethodCall(Spanned { node: ref len_name, .. }, _, ref len_args) = to.node,

src/shadow.rs

@@ -63,8 +63,8 @@ fn check_decl(cx: &LateContext, decl: &Decl, bindings: &mut Vec<(Name, Span)>) {
     if is_from_for_desugar(decl) { return; }
     if let DeclLocal(ref local) = decl.node {
         let Local{ ref pat, ref ty, ref init, id: _, span } = **local;
-        if let &Some(ref t) = ty { check_ty(cx, t, bindings) }
+        if let Some(ref t) = *ty { check_ty(cx, t, bindings) }
-        if let &Some(ref o) = init {
+        if let Some(ref o) = *init {
             check_expr(cx, o, bindings);
             check_pat(cx, pat, &Some(o), span, bindings);
         } else {
@@ -210,7 +210,7 @@ fn check_expr(cx: &LateContext, expr: &Expr, bindings: &mut Vec<(Name, Span)>) {
         ExprIf(ref cond, ref then, ref otherwise) => {
             check_expr(cx, cond, bindings);
             check_block(cx, then, bindings);
-            if let &Some(ref o) = otherwise { check_expr(cx, o, bindings); }
+            if let Some(ref o) = *otherwise { check_expr(cx, o, bindings); }
         }
         ExprWhile(ref cond, ref block, _) => {
             check_expr(cx, cond, bindings);

src/strings.rs

@@ -34,14 +34,14 @@ impl LintPass for StringAdd {
 
 impl LateLintPass for StringAdd {
     fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
-        if let &ExprBinary(Spanned{ node: BiAdd, .. }, ref left, _) = &e.node {
+        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 {
+                        if let ExprAssign(ref target, _) = p.node {
                             // avoid duplicate matches
                             if is_exp_equal(cx, target, left) { return; }
                         }
@@ -51,7 +51,7 @@ impl LateLintPass for StringAdd {
                     "you added something to a string. \
                      Consider using `String::push_str()` instead")
             }
-        } else if let &ExprAssign(ref target, ref src) = &e.node {
+        } else if let ExprAssign(ref target, ref src) = e.node {
             if is_string(cx, target) && is_add(cx, src, target) {
                 span_lint(cx, STRING_ADD_ASSIGN, e.span,
                     "you assigned the result of adding something to this string. \

tests/compile-fail/matches.rs

@@ -78,7 +78,7 @@ fn match_bool() {
 fn ref_pats() {
     {
         let v = &Some(0);
-        match v {  //~ERROR instead of prefixing all patterns with `&`
+        match v {  //~ERROR dereference the expression: `match *v { ...`
             &Some(v) => println!("{:?}", v),
             &None => println!("none"),
         }
@@ -88,13 +88,13 @@ fn ref_pats() {
         }
     }
     let tup =& (1, 2);
-    match tup {  //~ERROR instead of prefixing all patterns with `&`
+    match tup {  //~ERROR dereference the expression: `match *tup { ...`
         &(v, 1) => println!("{}", v),
         _ => println!("none"),
     }
     // special case: using & both in expr and pats
     let w = Some(0);
-    match &w {  //~ERROR you don't need to add `&` to both
+    match &w {  //~ERROR use `match w { ...`
         &Some(v) => println!("{:?}", v),
         &None => println!("none"),
     }
@@ -103,6 +103,16 @@ fn ref_pats() {
     match w {
         _ => println!("none"),
     }
+
+    let a = &Some(0);
+    if let &None = a { //~ERROR dereference the expression: `if let ... = *a {`
+        println!("none");
+    }
+
+    let b = Some(0);
+    if let &None = &b { //~ERROR use `if let ... = b {`
+        println!("none");
+    }
 }
 
 fn main() {