diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index c386b0c08b8..ce71b79e85a 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 44725db97f7..2ce0b574929 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 75d050f316b..4c723d709d0 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:53:18 + --> $DIR/matches.rs:54:18 | LL | Ok(_) => println!("ok"), | ^^^^^^^^^^^^^^ | note: same as this - --> $DIR/matches.rs:52:18 + --> $DIR/matches.rs:53:18 | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ help: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:52:9 + --> $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:76:29 + --> $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:65:18 + | +LL | Ok(3) => println!("ok"), + | ^^^^^^^^^^^^^^ +help: consider refactoring into `Ok(3) | Ok(_)` + --> $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: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