From 08fb953e1a9ac43720db6cf1d75bd19f5aa3cff5 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 2 Sep 2015 01:36:37 +0200 Subject: [PATCH 1/5] extended pattern matching --- src/shadow.rs | 52 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/src/shadow.rs b/src/shadow.rs index eba7a9da187..e7097f9daf6 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -60,8 +60,12 @@ fn check_decl(cx: &Context, decl: &Decl, bindings: &mut Vec) { 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) } - check_pat(cx, pat, init, span, bindings); + if let &Some(ref o) = init { + 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(cx: &Context, pat: &Pat, init: &Option, span: Span, - bindings: &mut Vec) where T: Deref { +fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, + bindings: &mut Vec) { //TODO: match more stuff / destructuring match pat.node { PatIdent(_, ref ident, ref inner) => { @@ -88,9 +92,43 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option, span: Span, if let Some(ref p) = *inner { check_pat(cx, p, init, span, bindings); } }, //PatEnum(Path, Option>>), - //PatQPath(QSelf, Path), - //PatStruct(Path, Vec>, bool), - //PatTup(Vec>), + PatStruct(_, ref pfields, _) => + if let Some(ref init_struct) = *init { // TODO follow + if let ExprStruct(_, ref efields, ref _base) = init_struct.node { + // TODO: follow base + 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, &None, 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 { //TODO: follow + 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, &None, 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 { From e2e89bf800b263e7a7ba5639d62b402ce5dd3bba Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 2 Sep 2015 01:36:37 +0200 Subject: [PATCH 2/5] extended pattern matching --- src/shadow.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/shadow.rs b/src/shadow.rs index e7097f9daf6..2276fb1da5d 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -60,7 +60,7 @@ fn check_decl(cx: &Context, decl: &Decl, bindings: &mut Vec) { 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 { + if let &Some(ref o) = init { check_expr(cx, o, bindings); check_pat(cx, pat, &Some(o), span, bindings); } else { @@ -92,10 +92,9 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, if let Some(ref p) = *inner { check_pat(cx, p, init, span, bindings); } }, //PatEnum(Path, Option>>), - PatStruct(_, ref pfields, _) => - if let Some(ref init_struct) = *init { // TODO follow - if let ExprStruct(_, ref efields, ref _base) = init_struct.node { - // TODO: follow base + PatStruct(_, ref pfields, _) => + if let Some(ref init_struct) = *init { + if let ExprStruct(_, ref efields, _) = init_struct.node { for field in pfields { let ident = field.node.ident; let efield = efields.iter() @@ -105,7 +104,7 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, } } else { for field in pfields { - check_pat(cx, &field.node.pat, &None, span, bindings); + check_pat(cx, &field.node.pat, init, span, bindings); } } } else { @@ -114,14 +113,14 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, } }, PatTup(ref inner) => - if let Some(ref init_tup) = *init { //TODO: follow + if let Some(ref init_tup) = *init { if let ExprTup(ref tup) = init_tup.node { - for (i, p) in inner.iter().enumerate() { + 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, &None, span, bindings); + check_pat(cx, p, init, span, bindings); } } } else { From 1ab733cfa1b8ccad239aaa1a9e9a0fbf69a5ee18 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 2 Sep 2015 01:36:37 +0200 Subject: [PATCH 3/5] extended pattern matching --- src/shadow.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/shadow.rs b/src/shadow.rs index e7097f9daf6..2276fb1da5d 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -60,7 +60,7 @@ fn check_decl(cx: &Context, decl: &Decl, bindings: &mut Vec) { 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 { + if let &Some(ref o) = init { check_expr(cx, o, bindings); check_pat(cx, pat, &Some(o), span, bindings); } else { @@ -92,10 +92,9 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, if let Some(ref p) = *inner { check_pat(cx, p, init, span, bindings); } }, //PatEnum(Path, Option>>), - PatStruct(_, ref pfields, _) => - if let Some(ref init_struct) = *init { // TODO follow - if let ExprStruct(_, ref efields, ref _base) = init_struct.node { - // TODO: follow base + PatStruct(_, ref pfields, _) => + if let Some(ref init_struct) = *init { + if let ExprStruct(_, ref efields, _) = init_struct.node { for field in pfields { let ident = field.node.ident; let efield = efields.iter() @@ -105,7 +104,7 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, } } else { for field in pfields { - check_pat(cx, &field.node.pat, &None, span, bindings); + check_pat(cx, &field.node.pat, init, span, bindings); } } } else { @@ -114,14 +113,14 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, } }, PatTup(ref inner) => - if let Some(ref init_tup) = *init { //TODO: follow + if let Some(ref init_tup) = *init { if let ExprTup(ref tup) = init_tup.node { - for (i, p) in inner.iter().enumerate() { + 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, &None, span, bindings); + check_pat(cx, p, init, span, bindings); } } } else { From bc1eb8481029856d41df3ae2a404cbfe51b80016 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 2 Sep 2015 07:56:13 +0200 Subject: [PATCH 4/5] match region patterns --- src/shadow.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shadow.rs b/src/shadow.rs index 2276fb1da5d..d64f6840db8 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -130,17 +130,17 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, }, PatBox(ref inner) => { if let Some(ref initp) = *init { - match initp.node { - ExprBox(_, ref inner_init) => - check_pat(cx, inner, &Some(&**inner_init), span, bindings), - //TODO: ExprCall on Box::new - _ => check_pat(cx, inner, init, span, bindings), + if let ExprBox(_, ref inner_init) = initp.node { + check_pat(cx, inner, &Some(&**inner_init), span, bindings), + } else { + check_pat(cx, inner, init, span, bindings), } } else { check_pat(cx, inner, init, span, bindings); } }, - //PatRegion(P, Mutability), + PatRegion(ref inner, _) => + check_pat(cx, inner, init, span, bindings), //PatRange(P, P), //PatVec(Vec>, Option>, Vec>), _ => (), From 0fb7d1d2d98016b496c36606ab003fd5d0b7e994 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 2 Sep 2015 08:19:47 +0200 Subject: [PATCH 5/5] reporting improvements --- src/shadow.rs | 22 ++++++++++++---------- src/utils.rs | 19 +++++++++++++++++-- tests/compile-fail/matches.rs | 18 ++++++++++-------- tests/compile-fail/shadow.rs | 2 +- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/shadow.rs b/src/shadow.rs index d64f6840db8..751077ec1d9 100644 --- a/src/shadow.rs +++ b/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`"); @@ -131,9 +131,9 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, PatBox(ref inner) => { if let Some(ref initp) = *init { if let ExprBox(_, ref inner_init) = initp.node { - check_pat(cx, inner, &Some(&**inner_init), span, bindings), + check_pat(cx, inner, &Some(&**inner_init), span, bindings); } else { - check_pat(cx, inner, init, span, bindings), + check_pat(cx, inner, init, span, bindings); } } else { check_pat(cx, inner, init, span, bindings); @@ -149,7 +149,7 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option<&Expr>, span: Span, fn lint_shadow(cx: &Context, name: Name, span: Span, lspan: Span, init: &Option) where T: Deref { - 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 {}", @@ -157,20 +157,22 @@ fn lint_shadow(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!( - "{} is shadowed by {} in this declaration", + span_note_and_lint(cx, SHADOW_UNRELATED, lspan, &format!( + "{} 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, "_"))); } } diff --git a/src/utils.rs b/src/utils.rs index f16387f606d..2f9b86964c2 100644 --- a/src/utils.rs +++ b/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 { diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs index 3cc540992c9..07dc7c9ef83 100755 --- a/tests/compile-fail/matches.rs +++ b/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 `&` - &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 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 `&` diff --git a/tests/compile-fail/shadow.rs b/tests/compile-fail/shadow.rs index 8ac9a93b140..80d48f84163 100755 --- a/tests/compile-fail/shadow.rs +++ b/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);