Skip to content

Commit

Permalink
Match underscore-prefixed variable also
Browse files Browse the repository at this point in the history
  • Loading branch information
ThibsG committed Jan 13, 2020
1 parent 920cdb5 commit f79c47f
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 19 deletions.
45 changes: 41 additions & 4 deletions clippy_lints/src/matches.rs
Expand Up @@ -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`
Expand Down Expand Up @@ -468,6 +470,41 @@ fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> 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) {
Expand All @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/matches.rs
Expand Up @@ -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"),
Expand Down
74 changes: 59 additions & 15 deletions tests/ui/matches.stderr
Expand Up @@ -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"),
| ^^^^^
Expand All @@ -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

0 comments on commit f79c47f

Please sign in to comment.