Skip to content

Commit e5c9073

Browse files
committed
Better binding name on Err for note
1 parent f79c47f commit e5c9073

File tree

2 files changed

+37
-27
lines changed

2 files changed

+37
-27
lines changed

clippy_lints/src/matches.rs

+36-26
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use rustc_hir::*;
1717
use rustc_lint::{LateContext, LateLintPass, LintContext};
1818
use rustc_session::{declare_lint_pass, declare_tool_lint};
1919
use rustc_span::source_map::Span;
20+
use rustc_span::symbol::Ident;
2021
use std::cmp::Ordering;
2122
use std::collections::Bound;
2223
use syntax::ast::{self, LitKind};
@@ -470,18 +471,13 @@ fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> bool {
470471
}
471472
}
472473

473-
fn is_unused_underscored<'tcx>(patkind: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
474-
match patkind {
475-
PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => {
476-
let mut visitor = UsedVisitor {
477-
var: ident.name,
478-
used: false,
479-
};
480-
walk_expr(&mut visitor, body);
481-
!visitor.used
482-
},
483-
_ => false,
484-
}
474+
fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool {
475+
let mut visitor = UsedVisitor {
476+
var: ident.name,
477+
used: false,
478+
};
479+
walk_expr(&mut visitor, body);
480+
!visitor.used
485481
}
486482

487483
struct UsedVisitor {
@@ -511,20 +507,34 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>])
511507
for arm in arms {
512508
if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.kind {
513509
let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false));
514-
if_chain! {
515-
if path_str == "Err";
516-
if inner.iter().any(is_wild) || inner.iter().any(|pat| is_unused_underscored(&pat.kind, arm.body));
517-
if let ExprKind::Block(ref block, _) = arm.body.kind;
518-
if is_panic_block(block);
519-
then {
520-
// `Err(_)` arm with `panic!` found
521-
span_note_and_lint(cx,
522-
MATCH_WILD_ERR_ARM,
523-
arm.pat.span,
524-
"`Err(_)` will match all errors, maybe not a good idea",
525-
arm.pat.span,
526-
"to remove this warning, match each error separately \
527-
or use `unreachable!` macro");
510+
if path_str == "Err" {
511+
let mut matching_wild = inner.iter().any(is_wild);
512+
let mut ident_bind_name = String::from("_");
513+
if !matching_wild {
514+
// Looking for unused bindings (i.e.: `_e`)
515+
inner.iter().for_each(|pat| {
516+
if let PatKind::Binding(.., ident, None) = &pat.kind {
517+
if ident.as_str().starts_with('_') && is_unused(ident, arm.body) {
518+
ident_bind_name = (&ident.name.as_str()).to_string();
519+
matching_wild = true;
520+
}
521+
}
522+
});
523+
}
524+
if_chain! {
525+
if matching_wild;
526+
if let ExprKind::Block(ref block, _) = arm.body.kind;
527+
if is_panic_block(block);
528+
then {
529+
// `Err(_)` or `Err(_e)` arm with `panic!` found
530+
span_note_and_lint(cx,
531+
MATCH_WILD_ERR_ARM,
532+
arm.pat.span,
533+
&format!("`Err({})` will match all errors, maybe not a good idea", &ident_bind_name),
534+
arm.pat.span,
535+
"to remove this warning, match each error separately \
536+
or use `unreachable!` macro");
537+
}
528538
}
529539
}
530540
}

tests/ui/matches.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ LL | Ok(3) => println!("ok"),
7878
| ^^^^^
7979
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
8080

81-
error: `Err(_)` will match all errors, maybe not a good idea
81+
error: `Err(_e)` will match all errors, maybe not a good idea
8282
--> $DIR/matches.rs:34:9
8383
|
8484
LL | Err(_e) => panic!(),

0 commit comments

Comments
 (0)