From 0fb7d1d2d98016b496c36606ab003fd5d0b7e994 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 2 Sep 2015 08:19:47 +0200 Subject: [PATCH] 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 d64f6840d..751077ec1 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 f16387f60..2f9b86964 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 3cc540992..07dc7c9ef 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 8ac9a93b1..80d48f841 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);