Auto merge of rust-lang#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 rust-lang#5556

CHANGELOG.md

@@ -1439,6 +1439,7 @@ Released 2018-09-13
 [`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
 [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
 [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
+[`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
 [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
 [`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
 [`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget

clippy_lints/src/float_literal.rs

@@ -77,7 +77,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatLiteral {
                 let type_suffix = match lit_float_ty {
                     LitFloatType::Suffixed(FloatTy::F32) => Some("f32"),
                     LitFloatType::Suffixed(FloatTy::F64) => Some("f64"),
-                    _ => None
+                    LitFloatType::Unsuffixed => None
                 };
                 let (is_whole, mut float_str) = match fty {
                     FloatTy::F32 => {

clippy_lints/src/lib.rs

@@ -638,6 +638,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &matches::MATCH_OVERLAPPING_ARM,
         &matches::MATCH_REF_PATS,
         &matches::MATCH_SINGLE_BINDING,
+        &matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
         &matches::MATCH_WILD_ERR_ARM,
         &matches::REST_PAT_IN_FULLY_BOUND_STRUCTS,
         &matches::SINGLE_MATCH,
@@ -1139,6 +1140,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&macro_use::MACRO_USE_IMPORTS),
         LintId::of(&match_on_vec_items::MATCH_ON_VEC_ITEMS),
         LintId::of(&matches::MATCH_BOOL),
+        LintId::of(&matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
         LintId::of(&matches::SINGLE_MATCH_ELSE),
         LintId::of(&methods::FILTER_MAP),
         LintId::of(&methods::FILTER_MAP_NEXT),

clippy_lints/src/matches.rs

@@ -220,7 +220,7 @@ declare_clippy_lint! {
     /// # enum Foo { A(usize), B(usize) }
     /// # let x = Foo::B(1);
     /// match x {
-    ///     A => {},
+    ///     Foo::A(_) => {},
     ///     _ => {},
     /// }
     /// ```
@@ -229,6 +229,40 @@ declare_clippy_lint! {
     "a wildcard enum match arm using `_`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for wildcard enum matches for a single variant.
+    ///
+    /// **Why is this bad?** New enum variants added by library updates can be missed.
+    ///
+    /// **Known problems:** Suggested replacements may not use correct path to enum
+    /// if it's not present in the current scope.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// # enum Foo { A, B, C }
+    /// # let x = Foo::B;
+    /// match x {
+    ///     Foo::A => {},
+    ///     Foo::B => {},
+    ///     _ => {},
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # enum Foo { A, B, C }
+    /// # let x = Foo::B;
+    /// match x {
+    ///     Foo::A => {},
+    ///     Foo::B => {},
+    ///     Foo::C => {},
+    /// }
+    /// ```
+    pub MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
+    pedantic,
+    "a wildcard enum match for a single variant"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for wildcard pattern used with others patterns in same match arm.
     ///
@@ -356,6 +390,7 @@ impl_lint_pass!(Matches => [
     MATCH_WILD_ERR_ARM,
     MATCH_AS_REF,
     WILDCARD_ENUM_MATCH_ARM,
+    MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
     WILDCARD_IN_OR_PATTERNS,
     MATCH_SINGLE_BINDING,
     INFALLIBLE_DESTRUCTURING_MATCH,
@@ -729,9 +764,21 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_
                 if let QPath::Resolved(_, p) = path {
                     missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
                 }
-            } else if let PatKind::TupleStruct(ref path, ..) = arm.pat.kind {
+            } else if let PatKind::TupleStruct(ref path, ref patterns, ..) = arm.pat.kind {
                 if let QPath::Resolved(_, p) = path {
-                    missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
+                    // Some simple checks for exhaustive patterns.
+                    // There is a room for improvements to detect more cases,
+                    // but it can be more expensive to do so.
+                    let is_pattern_exhaustive = |pat: &&Pat<'_>| {
+                        if let PatKind::Wild | PatKind::Binding(.., None) = pat.kind {
+                            true
+                        } else {
+                            false
+                        }
+                    };
+                    if patterns.iter().all(is_pattern_exhaustive) {
+                        missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
+                    }
                 }
             }
         }
@@ -766,14 +813,27 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_
             }
         }
 
+        if suggestion.len() == 1 {
+            // No need to check for non-exhaustive enum as in that case len would be greater than 1
+            span_lint_and_sugg(
+                cx,
+                MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
+                wildcard_span,
+                message,
+                "try this",
+                suggestion[0].clone(),
+                Applicability::MaybeIncorrect,
+            )
+        };
+
         span_lint_and_sugg(
             cx,
             WILDCARD_ENUM_MATCH_ARM,
             wildcard_span,
             message,
             "try this",
             suggestion.join(" | "),
-            Applicability::MachineApplicable,
+            Applicability::MaybeIncorrect,
         )
     }
 }

clippy_lints/src/misc_early.rs

@@ -379,7 +379,7 @@ impl EarlyLintPass for MiscEarlyLints {
             let left_binding = match left {
                 BindingMode::ByRef(Mutability::Mut) => "ref mut ",
                 BindingMode::ByRef(Mutability::Not) => "ref ",
-                _ => "",
+                BindingMode::ByValue(..) => "",
             };
 
             if let PatKind::Wild = right.kind {

clippy_lints/src/missing_const_for_fn.rs

@@ -113,7 +113,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn {
                     return;
                 }
             },
-            _ => return,
+            FnKind::Closure(..) => return,
         }
 
         let mir = cx.tcx.optimized_mir(def_id);

clippy_lints/src/needless_pass_by_value.rs

@@ -86,7 +86,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
                 }
             },
             FnKind::Method(..) => (),
-            _ => return,
+            FnKind::Closure(..) => return,
         }
 
         // Exclude non-inherent impls

clippy_lints/src/trivially_copy_pass_by_ref.rs

@@ -161,7 +161,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef {
                 }
             },
             FnKind::Method(..) => (),
-            _ => return,
+            FnKind::Closure(..) => return,
         }
 
         // Exclude non-inherent impls

clippy_lints/src/utils/mod.rs

@@ -358,7 +358,7 @@ pub fn trait_ref_of_method<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> O
 pub fn has_drop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
     match ty.ty_adt_def() {
         Some(def) => def.has_dtor(cx.tcx),
-        _ => false,
+        None => false,
     }
 }
 

src/lintlist/mod.rs

@@ -1200,6 +1200,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         module: "matches",
     },
+    Lint {
+        name: "match_wildcard_for_single_variants",
+        group: "pedantic",
+        desc: "a wildcard enum match for a single variant",
+        deprecation: None,
+        module: "matches",
+    },
     Lint {
         name: "maybe_infinite_iter",
         group: "pedantic",

tests/compile-test.rs

@@ -44,7 +44,7 @@ fn third_party_crates() -> String {
     for entry in fs::read_dir(dep_dir).unwrap() {
         let path = match entry {
             Ok(entry) => entry.path(),
-            _ => continue,
+            Err(_) => continue,
         };
         if let Some(name) = path.file_name().and_then(OsStr::to_str) {
             for dep in CRATES {

tests/ui/match_wildcard_for_single_variants.fixed

@@ -0,0 +1,59 @@
+// run-rustfix
+
+#![warn(clippy::match_wildcard_for_single_variants)]
+#![allow(dead_code)]
+
+enum Foo {
+    A,
+    B,
+    C,
+}
+
+enum Color {
+    Red,
+    Green,
+    Blue,
+    Rgb(u8, u8, u8),
+}
+
+fn main() {
+    let f = Foo::A;
+    match f {
+        Foo::A => {},
+        Foo::B => {},
+        Foo::C => {},
+    }
+
+    let color = Color::Red;
+
+    // check exhaustive bindings
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(_r, _g, _b) => {},
+        Color::Blue => {},
+    }
+
+    // check exhaustive wild
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(..) => {},
+        Color::Blue => {},
+    }
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(_, _, _) => {},
+        Color::Blue => {},
+    }
+
+    // shouldn't lint as there is one missing variant
+    // and one that isn't exhaustively covered
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(255, _, _) => {},
+        _ => {},
+    }
+}

tests/ui/match_wildcard_for_single_variants.rs

@@ -0,0 +1,59 @@
+// run-rustfix
+
+#![warn(clippy::match_wildcard_for_single_variants)]
+#![allow(dead_code)]
+
+enum Foo {
+    A,
+    B,
+    C,
+}
+
+enum Color {
+    Red,
+    Green,
+    Blue,
+    Rgb(u8, u8, u8),
+}
+
+fn main() {
+    let f = Foo::A;
+    match f {
+        Foo::A => {},
+        Foo::B => {},
+        _ => {},
+    }
+
+    let color = Color::Red;
+
+    // check exhaustive bindings
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(_r, _g, _b) => {},
+        _ => {},
+    }
+
+    // check exhaustive wild
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(..) => {},
+        _ => {},
+    }
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(_, _, _) => {},
+        _ => {},
+    }
+
+    // shouldn't lint as there is one missing variant
+    // and one that isn't exhaustively covered
+    match color {
+        Color::Red => {},
+        Color::Green => {},
+        Color::Rgb(255, _, _) => {},
+        _ => {},
+    }
+}

tests/ui/match_wildcard_for_single_variants.stderr

@@ -0,0 +1,28 @@
+error: wildcard match will miss any future added variants
+  --> $DIR/match_wildcard_for_single_variants.rs:24:9
+   |
+LL |         _ => {},
+   |         ^ help: try this: `Foo::C`
+   |
+   = note: `-D clippy::match-wildcard-for-single-variants` implied by `-D warnings`
+
+error: wildcard match will miss any future added variants
+  --> $DIR/match_wildcard_for_single_variants.rs:34:9
+   |
+LL |         _ => {},
+   |         ^ help: try this: `Color::Blue`
+
+error: wildcard match will miss any future added variants
+  --> $DIR/match_wildcard_for_single_variants.rs:42:9
+   |
+LL |         _ => {},
+   |         ^ help: try this: `Color::Blue`
+
+error: wildcard match will miss any future added variants
+  --> $DIR/match_wildcard_for_single_variants.rs:48:9
+   |
+LL |         _ => {},
+   |         ^ help: try this: `Color::Blue`
+
+error: aborting due to 4 previous errors
+