Skip to content

Commit cafa946

Browse files
borsbors
committed
Auto merge of #5582 - vtmargaryan:match_wildcard_for_single_variants, r=flip1995
New lint: `match_wildcard_for_single_variants` changelog: Added a new lint match_wildcard_for_single_variants to warn on enum matches where a wildcard is used to match a single variant Closes #5556
2 parents 34ba597 + d906253 commit cafa946

14 files changed

+227
-11
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,7 @@ Released 2018-09-13
14391439
[`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
14401440
[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
14411441
[`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
1442+
[`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
14421443
[`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
14431444
[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
14441445
[`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget

‎clippy_lints/src/float_literal.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatLiteral {
7777
let type_suffix = match lit_float_ty {
7878
LitFloatType::Suffixed(FloatTy::F32) => Some("f32"),
7979
LitFloatType::Suffixed(FloatTy::F64) => Some("f64"),
80-
_ => None
80+
LitFloatType::Unsuffixed => None
8181
};
8282
let (is_whole, mut float_str) = match fty {
8383
FloatTy::F32 => {

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
638638
&matches::MATCH_OVERLAPPING_ARM,
639639
&matches::MATCH_REF_PATS,
640640
&matches::MATCH_SINGLE_BINDING,
641+
&matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
641642
&matches::MATCH_WILD_ERR_ARM,
642643
&matches::REST_PAT_IN_FULLY_BOUND_STRUCTS,
643644
&matches::SINGLE_MATCH,
@@ -1139,6 +1140,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11391140
LintId::of(&macro_use::MACRO_USE_IMPORTS),
11401141
LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
11411142
LintId::of(&matches::MATCH_BOOL),
1143+
LintId::of(&matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
11421144
LintId::of(&matches::SINGLE_MATCH_ELSE),
11431145
LintId::of(&methods::FILTER_MAP),
11441146
LintId::of(&methods::FILTER_MAP_NEXT),

‎clippy_lints/src/matches.rs

+64-4
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ declare_clippy_lint! {
220220
/// # enum Foo { A(usize), B(usize) }
221221
/// # let x = Foo::B(1);
222222
/// match x {
223-
/// A => {},
223+
/// Foo::A(_) => {},
224224
/// _ => {},
225225
/// }
226226
/// ```
@@ -229,6 +229,40 @@ declare_clippy_lint! {
229229
"a wildcard enum match arm using `_`"
230230
}
231231

232+
declare_clippy_lint! {
233+
/// **What it does:** Checks for wildcard enum matches for a single variant.
234+
///
235+
/// **Why is this bad?** New enum variants added by library updates can be missed.
236+
///
237+
/// **Known problems:** Suggested replacements may not use correct path to enum
238+
/// if it's not present in the current scope.
239+
///
240+
/// **Example:**
241+
///
242+
/// ```rust
243+
/// # enum Foo { A, B, C }
244+
/// # let x = Foo::B;
245+
/// match x {
246+
/// Foo::A => {},
247+
/// Foo::B => {},
248+
/// _ => {},
249+
/// }
250+
/// ```
251+
/// Use instead:
252+
/// ```rust
253+
/// # enum Foo { A, B, C }
254+
/// # let x = Foo::B;
255+
/// match x {
256+
/// Foo::A => {},
257+
/// Foo::B => {},
258+
/// Foo::C => {},
259+
/// }
260+
/// ```
261+
pub MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
262+
pedantic,
263+
"a wildcard enum match for a single variant"
264+
}
265+
232266
declare_clippy_lint! {
233267
/// **What it does:** Checks for wildcard pattern used with others patterns in same match arm.
234268
///
@@ -356,6 +390,7 @@ impl_lint_pass!(Matches => [
356390
MATCH_WILD_ERR_ARM,
357391
MATCH_AS_REF,
358392
WILDCARD_ENUM_MATCH_ARM,
393+
MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
359394
WILDCARD_IN_OR_PATTERNS,
360395
MATCH_SINGLE_BINDING,
361396
INFALLIBLE_DESTRUCTURING_MATCH,
@@ -729,9 +764,21 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_
729764
if let QPath::Resolved(_, p) = path {
730765
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
731766
}
732-
} else if let PatKind::TupleStruct(ref path, ..) = arm.pat.kind {
767+
} else if let PatKind::TupleStruct(ref path, ref patterns, ..) = arm.pat.kind {
733768
if let QPath::Resolved(_, p) = path {
734-
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
769+
// Some simple checks for exhaustive patterns.
770+
// There is a room for improvements to detect more cases,
771+
// but it can be more expensive to do so.
772+
let is_pattern_exhaustive = |pat: &&Pat<'_>| {
773+
if let PatKind::Wild | PatKind::Binding(.., None) = pat.kind {
774+
true
775+
} else {
776+
false
777+
}
778+
};
779+
if patterns.iter().all(is_pattern_exhaustive) {
780+
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
781+
}
735782
}
736783
}
737784
}
@@ -766,14 +813,27 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_
766813
}
767814
}
768815

816+
if suggestion.len() == 1 {
817+
// No need to check for non-exhaustive enum as in that case len would be greater than 1
818+
span_lint_and_sugg(
819+
cx,
820+
MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
821+
wildcard_span,
822+
message,
823+
"try this",
824+
suggestion[0].clone(),
825+
Applicability::MaybeIncorrect,
826+
)
827+
};
828+
769829
span_lint_and_sugg(
770830
cx,
771831
WILDCARD_ENUM_MATCH_ARM,
772832
wildcard_span,
773833
message,
774834
"try this",
775835
suggestion.join(" | "),
776-
Applicability::MachineApplicable,
836+
Applicability::MaybeIncorrect,
777837
)
778838
}
779839
}

‎clippy_lints/src/misc_early.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ impl EarlyLintPass for MiscEarlyLints {
379379
let left_binding = match left {
380380
BindingMode::ByRef(Mutability::Mut) => "ref mut ",
381381
BindingMode::ByRef(Mutability::Not) => "ref ",
382-
_ => "",
382+
BindingMode::ByValue(..) => "",
383383
};
384384

385385
if let PatKind::Wild = right.kind {

‎clippy_lints/src/missing_const_for_fn.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn {
113113
return;
114114
}
115115
},
116-
_ => return,
116+
FnKind::Closure(..) => return,
117117
}
118118

119119
let mir = cx.tcx.optimized_mir(def_id);

‎clippy_lints/src/needless_pass_by_value.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
8686
}
8787
},
8888
FnKind::Method(..) => (),
89-
_ => return,
89+
FnKind::Closure(..) => return,
9090
}
9191

9292
// Exclude non-inherent impls

‎clippy_lints/src/trivially_copy_pass_by_ref.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef {
161161
}
162162
},
163163
FnKind::Method(..) => (),
164-
_ => return,
164+
FnKind::Closure(..) => return,
165165
}
166166

167167
// Exclude non-inherent impls

‎clippy_lints/src/utils/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> O
358358
pub fn has_drop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
359359
match ty.ty_adt_def() {
360360
Some(def) => def.has_dtor(cx.tcx),
361-
_ => false,
361+
None => false,
362362
}
363363
}
364364

‎src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1200,6 +1200,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
12001200
deprecation: None,
12011201
module: "matches",
12021202
},
1203+
Lint {
1204+
name: "match_wildcard_for_single_variants",
1205+
group: "pedantic",
1206+
desc: "a wildcard enum match for a single variant",
1207+
deprecation: None,
1208+
module: "matches",
1209+
},
12031210
Lint {
12041211
name: "maybe_infinite_iter",
12051212
group: "pedantic",

‎tests/compile-test.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ fn third_party_crates() -> String {
4444
for entry in fs::read_dir(dep_dir).unwrap() {
4545
let path = match entry {
4646
Ok(entry) => entry.path(),
47-
_ => continue,
47+
Err(_) => continue,
4848
};
4949
if let Some(name) = path.file_name().and_then(OsStr::to_str) {
5050
for dep in CRATES {

‎tests/ui/match_wildcard_for_single_variants.fixed

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::match_wildcard_for_single_variants)]
4+
#![allow(dead_code)]
5+
6+
enum Foo {
7+
A,
8+
B,
9+
C,
10+
}
11+
12+
enum Color {
13+
Red,
14+
Green,
15+
Blue,
16+
Rgb(u8, u8, u8),
17+
}
18+
19+
fn main() {
20+
let f = Foo::A;
21+
match f {
22+
Foo::A => {},
23+
Foo::B => {},
24+
Foo::C => {},
25+
}
26+
27+
let color = Color::Red;
28+
29+
// check exhaustive bindings
30+
match color {
31+
Color::Red => {},
32+
Color::Green => {},
33+
Color::Rgb(_r, _g, _b) => {},
34+
Color::Blue => {},
35+
}
36+
37+
// check exhaustive wild
38+
match color {
39+
Color::Red => {},
40+
Color::Green => {},
41+
Color::Rgb(..) => {},
42+
Color::Blue => {},
43+
}
44+
match color {
45+
Color::Red => {},
46+
Color::Green => {},
47+
Color::Rgb(_, _, _) => {},
48+
Color::Blue => {},
49+
}
50+
51+
// shouldn't lint as there is one missing variant
52+
// and one that isn't exhaustively covered
53+
match color {
54+
Color::Red => {},
55+
Color::Green => {},
56+
Color::Rgb(255, _, _) => {},
57+
_ => {},
58+
}
59+
}

‎tests/ui/match_wildcard_for_single_variants.rs

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::match_wildcard_for_single_variants)]
4+
#![allow(dead_code)]
5+
6+
enum Foo {
7+
A,
8+
B,
9+
C,
10+
}
11+
12+
enum Color {
13+
Red,
14+
Green,
15+
Blue,
16+
Rgb(u8, u8, u8),
17+
}
18+
19+
fn main() {
20+
let f = Foo::A;
21+
match f {
22+
Foo::A => {},
23+
Foo::B => {},
24+
_ => {},
25+
}
26+
27+
let color = Color::Red;
28+
29+
// check exhaustive bindings
30+
match color {
31+
Color::Red => {},
32+
Color::Green => {},
33+
Color::Rgb(_r, _g, _b) => {},
34+
_ => {},
35+
}
36+
37+
// check exhaustive wild
38+
match color {
39+
Color::Red => {},
40+
Color::Green => {},
41+
Color::Rgb(..) => {},
42+
_ => {},
43+
}
44+
match color {
45+
Color::Red => {},
46+
Color::Green => {},
47+
Color::Rgb(_, _, _) => {},
48+
_ => {},
49+
}
50+
51+
// shouldn't lint as there is one missing variant
52+
// and one that isn't exhaustively covered
53+
match color {
54+
Color::Red => {},
55+
Color::Green => {},
56+
Color::Rgb(255, _, _) => {},
57+
_ => {},
58+
}
59+
}

‎tests/ui/match_wildcard_for_single_variants.stderr

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error: wildcard match will miss any future added variants
2+
--> $DIR/match_wildcard_for_single_variants.rs:24:9
3+
|
4+
LL | _ => {},
5+
| ^ help: try this: `Foo::C`
6+
|
7+
= note: `-D clippy::match-wildcard-for-single-variants` implied by `-D warnings`
8+
9+
error: wildcard match will miss any future added variants
10+
--> $DIR/match_wildcard_for_single_variants.rs:34:9
11+
|
12+
LL | _ => {},
13+
| ^ help: try this: `Color::Blue`
14+
15+
error: wildcard match will miss any future added variants
16+
--> $DIR/match_wildcard_for_single_variants.rs:42:9
17+
|
18+
LL | _ => {},
19+
| ^ help: try this: `Color::Blue`
20+
21+
error: wildcard match will miss any future added variants
22+
--> $DIR/match_wildcard_for_single_variants.rs:48:9
23+
|
24+
LL | _ => {},
25+
| ^ help: try this: `Color::Blue`
26+
27+
error: aborting due to 4 previous errors
28+

0 commit comments

Comments
 (0)