From f79c47f28dd882b8c59353bc78758f0a1651799e Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 10 Jan 2020 10:42:21 +0100 Subject: [PATCH 1/5] Match underscore-prefixed variable also --- clippy_lints/src/matches.rs | 45 ++++++++++++++++++++-- tests/ui/matches.rs | 13 +++++++ tests/ui/matches.stderr | 74 +++++++++++++++++++++++++++++-------- 3 files changed, 113 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index c386b0c08b86..ce71b79e85a7 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -2,22 +2,24 @@ use crate::consts::{constant, miri_to_const, Constant}; use crate::utils::paths; use crate::utils::sugg::Sugg; use crate::utils::{ - expr_block, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg, remove_blocks, snippet, + expr_block, is_allowed, is_expn_of, match_qpath, match_type, match_var, multispan_sugg, remove_blocks, snippet, snippet_with_applicability, span_help_and_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, }; use if_chain::if_chain; -use rustc::lint::in_external_macro; +use rustc::hir::map::Map; +use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc::ty::{self, Ty}; use rustc_errors::Applicability; use rustc_hir::def::CtorKind; +use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use std::cmp::Ordering; use std::collections::Bound; -use syntax::ast::LitKind; +use syntax::ast::{self, LitKind}; declare_clippy_lint! { /// **What it does:** Checks for matches with a single arm where an `if let` @@ -468,6 +470,41 @@ fn is_wild<'tcx>(pat: &impl std::ops::Deref>) -> bool { } } +fn is_unused_underscored<'tcx>(patkind: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool { + match patkind { + PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => { + let mut visitor = UsedVisitor { + var: ident.name, + used: false, + }; + walk_expr(&mut visitor, body); + !visitor.used + }, + _ => false, + } +} + +struct UsedVisitor { + var: ast::Name, // var to look for + used: bool, // has the var been used otherwise? +} + +impl<'tcx> Visitor<'tcx> for UsedVisitor { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if match_var(expr, self.var) { + self.used = true; + } else { + walk_expr(self, expr); + } + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> { + NestedVisitorMap::None + } +} + fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex)); if match_type(cx, ex_ty, &paths::RESULT) { @@ -476,7 +513,7 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)); if_chain! { if path_str == "Err"; - if inner.iter().any(is_wild); + if inner.iter().any(is_wild) || inner.iter().any(|pat| is_unused_underscored(&pat.kind, arm.body)); if let ExprKind::Block(ref block, _) = arm.body.kind; if is_panic_block(block); then { diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs index 44725db97f7f..2ce0b574929e 100644 --- a/tests/ui/matches.rs +++ b/tests/ui/matches.rs @@ -28,6 +28,19 @@ fn match_wild_err_arm() { }, } + match x { + Ok(3) => println!("ok"), + Ok(_) => println!("ok"), + Err(_e) => panic!(), + } + + // Allowed when used in `panic!`. + match x { + Ok(3) => println!("ok"), + Ok(_) => println!("ok"), + Err(_e) => panic!("{}", _e), + } + // Allowed when not with `panic!` block. match x { Ok(3) => println!("ok"), diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index 75d050f316b5..4c723d709d01 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -78,37 +78,45 @@ LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +error: `Err(_)` will match all errors, maybe not a good idea + --> $DIR/matches.rs:34:9 + | +LL | Err(_e) => panic!(), + | ^^^^^^^ + | + = note: to remove this warning, match each error separately or use `unreachable!` macro + error: this `match` has identical arm bodies - --> $DIR/matches.rs:34:18 + --> $DIR/matches.rs:33:18 | LL | Ok(_) => println!("ok"), | ^^^^^^^^^^^^^^ | note: same as this - --> $DIR/matches.rs:33:18 + --> $DIR/matches.rs:32:18 | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ help: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:33:9 + --> $DIR/matches.rs:32:9 | LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: this `match` has identical arm bodies - --> $DIR/matches.rs:41:18 + --> $DIR/matches.rs:40:18 | LL | Ok(_) => println!("ok"), | ^^^^^^^^^^^^^^ | note: same as this - --> $DIR/matches.rs:40:18 + --> $DIR/matches.rs:39:18 | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ help: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:40:9 + --> $DIR/matches.rs:39:9 | LL | Ok(3) => println!("ok"), | ^^^^^ @@ -133,58 +141,94 @@ LL | Ok(3) => println!("ok"), = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: this `match` has identical arm bodies + --> $DIR/matches.rs:54:18 + | +LL | Ok(_) => println!("ok"), + | ^^^^^^^^^^^^^^ + | +note: same as this --> $DIR/matches.rs:53:18 | +LL | Ok(3) => println!("ok"), + | ^^^^^^^^^^^^^^ +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:53:9 + | +LL | Ok(3) => println!("ok"), + | ^^^^^ + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: this `match` has identical arm bodies + --> $DIR/matches.rs:60:18 + | +LL | Ok(_) => println!("ok"), + | ^^^^^^^^^^^^^^ + | +note: same as this + --> $DIR/matches.rs:59:18 + | +LL | Ok(3) => println!("ok"), + | ^^^^^^^^^^^^^^ +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:59:9 + | +LL | Ok(3) => println!("ok"), + | ^^^^^ + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: this `match` has identical arm bodies + --> $DIR/matches.rs:66:18 + | LL | Ok(_) => println!("ok"), | ^^^^^^^^^^^^^^ | note: same as this - --> $DIR/matches.rs:52:18 + --> $DIR/matches.rs:65:18 | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ help: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:52:9 + --> $DIR/matches.rs:65:9 | LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: this `match` has identical arm bodies - --> $DIR/matches.rs:76:29 + --> $DIR/matches.rs:89:29 | LL | (Ok(_), Some(x)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^^^^^ | note: same as this - --> $DIR/matches.rs:75:29 + --> $DIR/matches.rs:88:29 | LL | (Ok(x), Some(_)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^^^^^ help: consider refactoring into `(Ok(x), Some(_)) | (Ok(_), Some(x))` - --> $DIR/matches.rs:75:9 + --> $DIR/matches.rs:88:9 | LL | (Ok(x), Some(_)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: this `match` has identical arm bodies - --> $DIR/matches.rs:91:18 + --> $DIR/matches.rs:104:18 | LL | Ok(_) => println!("ok"), | ^^^^^^^^^^^^^^ | note: same as this - --> $DIR/matches.rs:90:18 + --> $DIR/matches.rs:103:18 | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ help: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:90:9 + --> $DIR/matches.rs:103:9 | LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: aborting due to 12 previous errors +error: aborting due to 15 previous errors From e5c9073f9c60095f9900f6826563c28858433d45 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 10 Jan 2020 10:56:09 +0100 Subject: [PATCH 2/5] Better binding name on Err for note --- clippy_lints/src/matches.rs | 62 +++++++++++++++++++++---------------- tests/ui/matches.stderr | 2 +- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index ce71b79e85a7..3d95b8f32714 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -17,6 +17,7 @@ use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; +use rustc_span::symbol::Ident; use std::cmp::Ordering; use std::collections::Bound; use syntax::ast::{self, LitKind}; @@ -470,18 +471,13 @@ fn is_wild<'tcx>(pat: &impl std::ops::Deref>) -> bool { } } -fn is_unused_underscored<'tcx>(patkind: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool { - match patkind { - PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => { - let mut visitor = UsedVisitor { - var: ident.name, - used: false, - }; - walk_expr(&mut visitor, body); - !visitor.used - }, - _ => false, - } +fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool { + let mut visitor = UsedVisitor { + var: ident.name, + used: false, + }; + walk_expr(&mut visitor, body); + !visitor.used } struct UsedVisitor { @@ -511,20 +507,34 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) for arm in arms { if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.kind { let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)); - if_chain! { - if path_str == "Err"; - if inner.iter().any(is_wild) || inner.iter().any(|pat| is_unused_underscored(&pat.kind, arm.body)); - if let ExprKind::Block(ref block, _) = arm.body.kind; - if is_panic_block(block); - then { - // `Err(_)` arm with `panic!` found - span_note_and_lint(cx, - MATCH_WILD_ERR_ARM, - arm.pat.span, - "`Err(_)` will match all errors, maybe not a good idea", - arm.pat.span, - "to remove this warning, match each error separately \ - or use `unreachable!` macro"); + if path_str == "Err" { + let mut matching_wild = inner.iter().any(is_wild); + let mut ident_bind_name = String::from("_"); + if !matching_wild { + // Looking for unused bindings (i.e.: `_e`) + inner.iter().for_each(|pat| { + if let PatKind::Binding(.., ident, None) = &pat.kind { + if ident.as_str().starts_with('_') && is_unused(ident, arm.body) { + ident_bind_name = (&ident.name.as_str()).to_string(); + matching_wild = true; + } + } + }); + } + if_chain! { + if matching_wild; + if let ExprKind::Block(ref block, _) = arm.body.kind; + if is_panic_block(block); + then { + // `Err(_)` or `Err(_e)` arm with `panic!` found + span_note_and_lint(cx, + MATCH_WILD_ERR_ARM, + arm.pat.span, + &format!("`Err({})` will match all errors, maybe not a good idea", &ident_bind_name), + arm.pat.span, + "to remove this warning, match each error separately \ + or use `unreachable!` macro"); + } } } } diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index 4c723d709d01..dd8014073df7 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -78,7 +78,7 @@ LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: `Err(_)` will match all errors, maybe not a good idea +error: `Err(_e)` will match all errors, maybe not a good idea --> $DIR/matches.rs:34:9 | LL | Err(_e) => panic!(), From 95cc500e9d4ff3bc707438b6a5e11397c232cd61 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 10 Jan 2020 15:09:37 +0100 Subject: [PATCH 3/5] Fix formatting --- clippy_lints/src/matches.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 3d95b8f32714..308401229ac7 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -528,12 +528,13 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) then { // `Err(_)` or `Err(_e)` arm with `panic!` found span_note_and_lint(cx, - MATCH_WILD_ERR_ARM, - arm.pat.span, - &format!("`Err({})` will match all errors, maybe not a good idea", &ident_bind_name), - arm.pat.span, - "to remove this warning, match each error separately \ - or use `unreachable!` macro"); + MATCH_WILD_ERR_ARM, + arm.pat.span, + &format!("`Err({})` will match all errors, maybe not a good idea", &ident_bind_name), + arm.pat.span, + "to remove this warning, match each error separately \ + or use `unreachable!` macro" + ); } } } From d3c76b5b2a803030d9be4e66c540120d86e3868a Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 10 Jan 2020 15:12:21 +0100 Subject: [PATCH 4/5] Change note message --- clippy_lints/src/matches.rs | 5 ++--- tests/ui/matches.stderr | 16 ++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 308401229ac7..ee7155c1a0bc 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -530,10 +530,9 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) span_note_and_lint(cx, MATCH_WILD_ERR_ARM, arm.pat.span, - &format!("`Err({})` will match all errors, maybe not a good idea", &ident_bind_name), + &format!("`Err({})` matches all errors", &ident_bind_name), arm.pat.span, - "to remove this warning, match each error separately \ - or use `unreachable!` macro" + "match each error separately or use the error output", ); } } diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index dd8014073df7..1c5c636fee61 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -1,11 +1,11 @@ -error: `Err(_)` will match all errors, maybe not a good idea +error: `Err(_)` matches all errors --> $DIR/matches.rs:14:9 | LL | Err(_) => panic!("err"), | ^^^^^^ | = note: `-D clippy::match-wild-err-arm` implied by `-D warnings` - = note: to remove this warning, match each error separately or use `unreachable!` macro + = note: match each error separately or use the error output error: this `match` has identical arm bodies --> $DIR/matches.rs:13:18 @@ -26,13 +26,13 @@ LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: `Err(_)` will match all errors, maybe not a good idea +error: `Err(_)` matches all errors --> $DIR/matches.rs:20:9 | LL | Err(_) => panic!(), | ^^^^^^ | - = note: to remove this warning, match each error separately or use `unreachable!` macro + = note: match each error separately or use the error output error: this `match` has identical arm bodies --> $DIR/matches.rs:19:18 @@ -52,13 +52,13 @@ LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: `Err(_)` will match all errors, maybe not a good idea +error: `Err(_)` matches all errors --> $DIR/matches.rs:26:9 | LL | Err(_) => { | ^^^^^^ | - = note: to remove this warning, match each error separately or use `unreachable!` macro + = note: match each error separately or use the error output error: this `match` has identical arm bodies --> $DIR/matches.rs:25:18 @@ -78,13 +78,13 @@ LL | Ok(3) => println!("ok"), | ^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: `Err(_e)` will match all errors, maybe not a good idea +error: `Err(_e)` matches all errors --> $DIR/matches.rs:34:9 | LL | Err(_e) => panic!(), | ^^^^^^^ | - = note: to remove this warning, match each error separately or use `unreachable!` macro + = note: match each error separately or use the error output error: this `match` has identical arm bodies --> $DIR/matches.rs:33:18 From 44fb8b5e8834a89e0d9f925b8ff235eec10526e3 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 10 Jan 2020 17:14:17 +0100 Subject: [PATCH 5/5] Extract visitor to utils --- clippy_lints/src/loops.rs | 60 +++++++++------------------------ clippy_lints/src/matches.rs | 47 +++----------------------- clippy_lints/src/utils/mod.rs | 8 +++++ clippy_lints/src/utils/usage.rs | 35 +++++++++++++++++++ 4 files changed, 62 insertions(+), 88 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 239d453a9f85..b2d6c178b54a 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1,22 +1,28 @@ +use crate::consts::{constant, Constant}; use crate::reexport::*; +use crate::utils::paths; +use crate::utils::usage::{is_unused, mutated_variables}; +use crate::utils::{ + get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, + is_integer_const, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, + snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, + span_lint_and_then, SpanlessEq, +}; +use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg}; use if_chain::if_chain; use itertools::Itertools; +use rustc::hir::map::Map; use rustc::lint::in_external_macro; use rustc::middle::region; +use rustc::ty::{self, Ty}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id; use rustc_hir::intravisit::{walk_block, walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -// use rustc::middle::region::CodeExtent; -use crate::consts::{constant, Constant}; -use crate::utils::usage::mutated_variables; -use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg}; -use rustc::hir::map::Map; -use rustc::ty::{self, Ty}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::Applicability; use rustc_span::source_map::Span; use rustc_span::{BytePos, Symbol}; use rustc_typeck::expr_use_visitor::*; @@ -24,14 +30,6 @@ use std::iter::{once, Iterator}; use std::mem; use syntax::ast; -use crate::utils::paths; -use crate::utils::{ - get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, - is_integer_const, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, - snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, - span_lint_and_then, SpanlessEq, -}; - declare_clippy_lint! { /// **What it does:** Checks for for-loops that manually copy items between /// slices that could be optimized by having a memcpy. @@ -1689,39 +1687,11 @@ fn check_for_mutation( fn pat_is_wild<'tcx>(pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool { match *pat { PatKind::Wild => true, - PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => { - let mut visitor = UsedVisitor { - var: ident.name, - used: false, - }; - walk_expr(&mut visitor, body); - !visitor.used - }, + PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => is_unused(&ident, body), _ => false, } } -struct UsedVisitor { - var: ast::Name, // var to look for - used: bool, // has the var been used otherwise? -} - -impl<'tcx> Visitor<'tcx> for UsedVisitor { - type Map = Map<'tcx>; - - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if match_var(expr, self.var) { - self.used = true; - } else { - walk_expr(self, expr); - } - } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> { - NestedVisitorMap::None - } -} - struct LocalUsedVisitor<'a, 'tcx> { cx: &'a LateContext<'a, 'tcx>, local: HirId, diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index ee7155c1a0bc..cddd479d7b72 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -1,26 +1,24 @@ use crate::consts::{constant, miri_to_const, Constant}; use crate::utils::paths; use crate::utils::sugg::Sugg; +use crate::utils::usage::is_unused; use crate::utils::{ - expr_block, is_allowed, is_expn_of, match_qpath, match_type, match_var, multispan_sugg, remove_blocks, snippet, + expr_block, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks, snippet, snippet_with_applicability, span_help_and_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, }; use if_chain::if_chain; -use rustc::hir::map::Map; -use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; +use rustc::lint::in_external_macro; use rustc::ty::{self, Ty}; use rustc_errors::Applicability; use rustc_hir::def::CtorKind; -use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; -use rustc_span::symbol::Ident; use std::cmp::Ordering; use std::collections::Bound; -use syntax::ast::{self, LitKind}; +use syntax::ast::LitKind; declare_clippy_lint! { /// **What it does:** Checks for matches with a single arm where an `if let` @@ -464,43 +462,6 @@ fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr<' } } -fn is_wild<'tcx>(pat: &impl std::ops::Deref>) -> bool { - match pat.kind { - PatKind::Wild => true, - _ => false, - } -} - -fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool { - let mut visitor = UsedVisitor { - var: ident.name, - used: false, - }; - walk_expr(&mut visitor, body); - !visitor.used -} - -struct UsedVisitor { - var: ast::Name, // var to look for - used: bool, // has the var been used otherwise? -} - -impl<'tcx> Visitor<'tcx> for UsedVisitor { - type Map = Map<'tcx>; - - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if match_var(expr, self.var) { - self.used = true; - } else { - walk_expr(self, expr); - } - } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> { - NestedVisitorMap::None - } -} - fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex)); if match_type(cx, ex_ty, &paths::RESULT) { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 797bccc2934c..eca2baa8c809 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -127,6 +127,14 @@ pub fn is_present_in_source(cx: &T, span: Span) -> bool { true } +/// Checks if given pattern is a wildcard (`_`) +pub fn is_wild<'tcx>(pat: &impl std::ops::Deref>) -> bool { + match pat.kind { + PatKind::Wild => true, + _ => false, + } +} + /// Checks if type is struct, enum or union type with the given def path. pub fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool { match ty.kind { diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index 18fb01c5b17d..7939d7d0863c 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -1,9 +1,14 @@ +use crate::utils::match_var; +use rustc::hir::map::Map; use rustc::ty; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::Res; +use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::LateContext; +use rustc_span::symbol::Ident; use rustc_typeck::expr_use_visitor::*; +use syntax::ast; /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &'a LateContext<'a, 'tcx>) -> Option> { @@ -70,3 +75,33 @@ impl<'tcx> Delegate<'tcx> for MutVarsDelegate { self.update(&cmt) } } + +pub struct UsedVisitor { + pub var: ast::Name, // var to look for + pub used: bool, // has the var been used otherwise? +} + +impl<'tcx> Visitor<'tcx> for UsedVisitor { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if match_var(expr, self.var) { + self.used = true; + } else { + walk_expr(self, expr); + } + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> { + NestedVisitorMap::None + } +} + +pub fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool { + let mut visitor = UsedVisitor { + var: ident.name, + used: false, + }; + walk_expr(&mut visitor, body); + !visitor.used +}