Skip to content

Commit

Permalink
Auto merge of #3652 - Aehmlo:where_the_wild_things_are, r=phansch
Browse files Browse the repository at this point in the history
Add wildcard_match_arm lint

This lint prevents using a wildcard in a match arm. Implemented as a restriction currently, because this is pretty much an edge case. See #3649 for more information.

Didn't add any tests because I wasn't sure how, but if someone wants to point me in the right direction, I'd be happy to!
  • Loading branch information
bors committed Jan 29, 2019
2 parents 6b1a2a9 + 7fa50fb commit 6ce78d1
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,7 @@ All notable changes to this project will be documented in this file.
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
[`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator
[`wildcard_dependencies`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_dependencies
[`wildcard_enum_match_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_enum_match_arm
[`write_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#write_literal
[`write_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#write_with_newline
[`writeln_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#writeln_empty_string
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 293 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 294 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
indexing_slicing::INDEXING_SLICING,
inherent_impl::MULTIPLE_INHERENT_IMPL,
literal_representation::DECIMAL_LITERAL_REPRESENTATION,
matches::WILDCARD_ENUM_MATCH_ARM,
mem_forget::MEM_FORGET,
methods::CLONE_ON_REF_PTR,
methods::OPTION_UNWRAP_USED,
Expand Down
40 changes: 39 additions & 1 deletion clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,25 @@ declare_clippy_lint! {
"a match on an Option value instead of using `as_ref()` or `as_mut`"
}

/// **What it does:** Checks for wildcard enum matches using `_`.
///
/// **Why is this bad?** New enum variants added by library updates can be missed.
///
/// **Known problems:** Nested wildcards a la `Foo(_)` are currently not detected.
///
/// **Example:**
/// ```rust
/// match x {
/// A => {},
/// _ => {},
/// }
/// ```
declare_clippy_lint! {
pub WILDCARD_ENUM_MATCH_ARM,
restriction,
"a wildcard enum match arm using `_`"
}

#[allow(missing_copy_implementations)]
pub struct MatchPass;

Expand All @@ -199,7 +218,8 @@ impl LintPass for MatchPass {
SINGLE_MATCH_ELSE,
MATCH_OVERLAPPING_ARM,
MATCH_WILD_ERR_ARM,
MATCH_AS_REF
MATCH_AS_REF,
WILDCARD_ENUM_MATCH_ARM
)
}

Expand All @@ -218,6 +238,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MatchPass {
check_match_bool(cx, ex, arms, expr);
check_overlapping_arms(cx, ex, arms);
check_wild_err_arm(cx, ex, arms);
check_wild_enum_match(cx, ex, arms);
check_match_as_ref(cx, ex, arms, expr);
}
if let ExprKind::Match(ref ex, ref arms, _) = expr.node {
Expand Down Expand Up @@ -442,6 +463,23 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
}
}

fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
if cx.tables.expr_ty(ex).is_enum() {
for arm in arms {
if is_wild(&arm.pats[0]) {
span_note_and_lint(
cx,
WILDCARD_ENUM_MATCH_ARM,
arm.pats[0].span,
"wildcard match will miss any future added variants.",
arm.pats[0].span,
"to resolve, match each variant explicitly",
);
}
}
}
}

// If the block contains only a `panic!` macro (as expression or statement)
fn is_panic_block(block: &Block) -> bool {
match (&block.expr, block.stmts.len(), block.stmts.first()) {
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/wildcard_enum_match_arm.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#![deny(clippy::wildcard_enum_match_arm)]

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum Color {
Red,
Green,
Blue,
Rgb(u8, u8, u8),
Cyan,
}

impl Color {
fn is_monochrome(self) -> bool {
match self {
Color::Red | Color::Green | Color::Blue => true,
Color::Rgb(r, g, b) => r | g == 0 || r | b == 0 || g | b == 0,
Color::Cyan => false,
}
}
}

fn main() {
let color = Color::Rgb(0, 0, 127);
match color {
Color::Red => println!("Red"),
_ => eprintln!("Not red"),
};
match color {
Color::Red => {},
Color::Green => {},
Color::Blue => {},
Color::Cyan => {},
c if c.is_monochrome() => {},
Color::Rgb(_, _, _) => {},
};
let x: u8 = unimplemented!();
match x {
0 => {},
140 => {},
_ => {},
};
}
15 changes: 15 additions & 0 deletions tests/ui/wildcard_enum_match_arm.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: wildcard match will miss any future added variants.
--> $DIR/wildcard_enum_match_arm.rs:26:9
|
LL | _ => eprintln!("Not red"),
| ^
|
note: lint level defined here
--> $DIR/wildcard_enum_match_arm.rs:1:9
|
LL | #![deny(clippy::wildcard_enum_match_arm)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: to resolve, match each variant explicitly

error: aborting due to previous error

0 comments on commit 6ce78d1

Please sign in to comment.