Merge pull request #275 from Manishearth/shadow

extended pattern matching

src/shadow.rs

@@ -6,7 +6,7 @@ use syntax::visit::FnKind;
 use rustc::lint::{Context, LintArray, LintPass};
 use rustc::middle::def::Def::{DefVariant, DefStruct};
 
-use utils::{in_external_macro, snippet, span_lint};
+use utils::{in_external_macro, snippet, span_lint, span_note_and_lint};
 
 declare_lint!(pub SHADOW_SAME, Allow,
     "rebinding a name to itself, e.g. `let mut x = &mut x`");
@@ -60,8 +60,12 @@ fn check_decl(cx: &Context, decl: &Decl, bindings: &mut Vec<Name>) {
     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 o) = init { check_expr(cx, o, bindings) }
+        if let &Some(ref o) = init {
-        check_pat(cx, pat, init, span, bindings);
+            check_expr(cx, o, bindings);
+            check_pat(cx, pat, &Some(o), span, bindings);
+        } else {
+            check_pat(cx, pat, &None, span, bindings);
+        }
     }
 }
 
@@ -72,8 +76,8 @@ fn is_binding(cx: &Context, pat: &Pat) -> bool {
     }
 }
 
-fn check_pat<T>(cx: &Context, pat: &Pat, init: &Option<T>, span: Span,
+fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span,
-        bindings: &mut Vec<Name>) where T: Deref<Target=Expr> {
+        bindings: &mut Vec<Name>) {
     //TODO: match more stuff / destructuring
     match pat.node {
         PatIdent(_, ref ident, ref inner) => {
@@ -88,22 +92,55 @@ fn check_pat<T>(cx: &Context, pat: &Pat, init: &Option<T>, span: Span,
             if let Some(ref p) = *inner { check_pat(cx, p, init, span, bindings); }
         },
         //PatEnum(Path, Option<Vec<P<Pat>>>),
-        //PatQPath(QSelf, Path),
+        PatStruct(_, ref pfields, _) =>
-        //PatStruct(Path, Vec<Spanned<FieldPat>>, bool),
+            if let Some(ref init_struct) = *init {
-        //PatTup(Vec<P<Pat>>),
+                if let ExprStruct(_, ref efields, _) = init_struct.node {
+                    for field in pfields {
+                        let ident = field.node.ident;
+                        let efield = efields.iter()
+                            .find(|ref f| f.ident.node == ident)
+                            .map(|f| &*f.expr);
+                        check_pat(cx, &field.node.pat, &efield, span, bindings);
+                    }
+                } else {
+                    for field in pfields {
+                        check_pat(cx, &field.node.pat, init, span, bindings);
+                    }
+                }
+            } else {
+                for field in pfields {
+                    check_pat(cx, &field.node.pat, &None, span, bindings);
+                }
+            },
+        PatTup(ref inner) =>
+            if let Some(ref init_tup) = *init {
+                if let ExprTup(ref tup) = init_tup.node {
+                    for (i, p) in inner.iter().enumerate() {
+                        check_pat(cx, p, &Some(&tup[i]), p.span, bindings);
+                    }
+                } else {
+                    for p in inner {
+                        check_pat(cx, p, init, span, bindings);
+                    }
+                }
+            } else {
+                for p in inner {
+                    check_pat(cx, p, &None, span, bindings);
+                }
+            },
         PatBox(ref inner) => {
             if let Some(ref initp) = *init {
-                match initp.node {
+                if let ExprBox(_, ref inner_init) = initp.node {
-                    ExprBox(_, ref inner_init) =>
+                    check_pat(cx, inner, &Some(&**inner_init), span, bindings);
-                        check_pat(cx, inner, &Some(&**inner_init), span, bindings),
+                } else {
-                    //TODO: ExprCall on Box::new
+                    check_pat(cx, inner, init, span, bindings);
-                    _ => check_pat(cx, inner, init, span, bindings),
                 }
             } else {
                 check_pat(cx, inner, init, span, bindings);
             }
         },
-        //PatRegion(P<Pat>, Mutability),
+        PatRegion(ref inner, _) =>
+            check_pat(cx, inner, init, span, bindings),
         //PatRange(P<Expr>, P<Expr>),
         //PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),
         _ => (),
@@ -112,7 +149,7 @@ fn check_pat<T>(cx: &Context, pat: &Pat, init: &Option<T>, span: Span,
 
 fn lint_shadow<T>(cx: &Context, name: Name, span: Span, lspan: Span, init:
         &Option<T>) where T: Deref<Target=Expr> {
-    if let &Some(ref expr) = init {
+    if let Some(ref expr) = *init {
         if is_self_shadow(name, expr) {
             span_lint(cx, SHADOW_SAME, span, &format!(
                 "{} is shadowed by itself in {}",
@@ -120,20 +157,22 @@ fn lint_shadow<T>(cx: &Context, name: Name, span: Span, lspan: Span, init:
                 snippet(cx, expr.span, "..")));
         } else {
             if contains_self(name, expr) {
-                span_lint(cx, SHADOW_REUSE, span, &format!(
+                span_note_and_lint(cx, SHADOW_REUSE, lspan, &format!(
                     "{} is shadowed by {} which reuses the original value",
                     snippet(cx, lspan, "_"),
-                    snippet(cx, expr.span, "..")));
+                    snippet(cx, expr.span, "..")),
+                    expr.span, "initialization happens here");
             } else {
-                span_lint(cx, SHADOW_UNRELATED, span, &format!(
+                span_note_and_lint(cx, SHADOW_UNRELATED, lspan, &format!(
-                    "{} is shadowed by {} in this declaration",
+                    "{} is shadowed by {}",
                     snippet(cx, lspan, "_"),
-                    snippet(cx, expr.span, "..")));
+                    snippet(cx, expr.span, "..")),
+                    expr.span, "initialization happens here");
             }
         }
     } else {
         span_lint(cx, SHADOW_UNRELATED, span, &format!(
-            "{} is shadowed in this declaration", snippet(cx, lspan, "_")));
+            "{} shadows a previous declaration", snippet(cx, lspan, "_")));
     }
 }
 

src/utils.rs

@@ -23,7 +23,7 @@ pub fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool {
                 if info.callee.name() == "closure expansion" {
                     return false;
                 }
-            }, 
+            },
             ExpnFormat::MacroAttribute(..) => {
                 // these are all plugins
                 return true;
@@ -177,7 +177,7 @@ pub fn span_lint(cx: &Context, lint: &'static Lint, sp: Span, msg: &str) {
 
 pub fn span_help_and_lint(cx: &Context, lint: &'static Lint, span: Span,
         msg: &str, help: &str) {
-    span_lint(cx, lint, span, msg);
+    cx.span_lint(lint, span, msg);
     if cx.current_level(lint) != Level::Allow {
         cx.sess().fileline_help(span, &format!("{}\nfor further information \
             visit https://github.com/Manishearth/rust-clippy/wiki#{}",
@@ -185,6 +185,21 @@ pub fn span_help_and_lint(cx: &Context, lint: &'static Lint, span: Span,
     }
 }
 
+pub fn span_note_and_lint(cx: &Context, lint: &'static Lint, span: Span,
+        msg: &str, note_span: Span, note: &str) {
+    cx.span_lint(lint, span, msg);
+    if cx.current_level(lint) != Level::Allow {
+        if note_span == span {
+            cx.sess().fileline_note(note_span, note)
+        } else {
+            cx.sess().span_note(note_span, note)
+        }
+        cx.sess().fileline_help(span, &format!("for further information visit \
+            https://github.com/Manishearth/rust-clippy/wiki#{}",
+            lint.name_lower()))
+    }
+}
+
 /// return the base type for references and raw pointers
 pub fn walk_ptrs_ty(ty: ty::Ty) -> ty::Ty {
     match ty.sty {

tests/compile-fail/matches.rs

@@ -39,14 +39,16 @@ fn single_match(){
 }
 
 fn ref_pats() {
-    let ref v = Some(0);
+    {
-    match v {  //~ERROR instead of prefixing all patterns with `&`
+        let ref v = Some(0);
-        &Some(v) => println!("{:?}", v),
+        match v {  //~ERROR instead of prefixing all patterns with `&`
-        &None => println!("none"),
+            &Some(v) => println!("{:?}", v),
-    }
+            &None => println!("none"),
-    match v {  // this doesn't trigger, we have a different pattern
+        }
-        &Some(v) => println!("some"),
+        match v {  // this doesn't trigger, we have a different pattern
-        other => println!("other"),
+            &Some(v) => println!("some"),
+            other => println!("other"),
+        }
     }
     let ref tup = (1, 2);
     match tup {  //~ERROR instead of prefixing all patterns with `&`

tests/compile-fail/shadow.rs

@@ -18,7 +18,7 @@ fn main() {
     let x = (1, x); //~ERROR: x is shadowed by (1, x) which reuses
     let x = first(x); //~ERROR: x is shadowed by first(x) which reuses
     let y = 1;
-    let x = y; //~ERROR: x is shadowed by y in this declaration
+    let x = y; //~ERROR: x is shadowed by y
 
     let o = Some(1u8);