match_wildcard_for_single_variants: remove empty line at start of lint example.

[rust.git] / clippy_lints / src / matches.rs

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index bbf14374a1f7f0c741fd2336e20900be5e386cf1..6d7af45a47224d54a99e6949c50185e792acf9bc 100644 (file)
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -36,10 +36,17 @@
     /// ```rust
     /// # fn bar(stool: &str) {}
     /// # let x = Some("abc");
+    ///
+    /// // Bad
     /// match x {
     ///     Some(ref foo) => bar(foo),
     ///     _ => (),
     /// }
+    ///
+    /// // Good
+    /// if let Some(ref foo) = x {
+    ///     bar(foo);
+    /// }
     /// ```
     pub SINGLE_MATCH,
     style,
@@ -97,11 +104,19 @@
     ///
     /// **Example:**
     /// ```rust,ignore
+    /// // Bad
     /// match x {
     ///     &A(ref y) => foo(y),
     ///     &B => bar(),
     ///     _ => frob(&x),
     /// }
+    ///
+    /// // Good
+    /// match *x {
+    ///     A(ref y) => foo(y),
+    ///     B => bar(),
+    ///     _ => frob(x),
+    /// }
     /// ```
     pub MATCH_REF_PATS,
     style,
@@ -168,7 +183,7 @@
     /// **What it does:** Checks for arm which matches all errors with `Err(_)`
     /// and take drastic actions like `panic!`.
     ///
-    /// **Why is this bad?** It is generally a bad practice, just like
+    /// **Why is this bad?** It is generally a bad practice, similar to
     /// catching all exceptions in java with `catch(Exception)`
     ///
     /// **Known problems:** None.
@@ -182,7 +197,7 @@
     /// }
     /// ```
     pub MATCH_WILD_ERR_ARM,
-    style,
+    pedantic,
     "a `match` with `Err(_)` arm and take drastic actions"
 }
 
@@ -197,10 +212,15 @@
     /// **Example:**
     /// ```rust
     /// let x: Option<()> = None;
+    ///
+    /// // Bad
     /// let r: Option<&()> = match x {
     ///     None => None,
     ///     Some(ref v) => Some(v),
     /// };
+    ///
+    /// // Good
+    /// let r: Option<&()> = x.as_ref();
     /// ```
     pub MATCH_AS_REF,
     complexity,
@@ -219,16 +239,56 @@
     /// ```rust
     /// # enum Foo { A(usize), B(usize) }
     /// # let x = Foo::B(1);
+    ///
+    /// // Bad
     /// match x {
-    ///     A => {},
+    ///     Foo::A(_) => {},
     ///     _ => {},
     /// }
+    ///
+    /// // Good
+    /// match x {
+    ///     Foo::A(_) => {},
+    ///     Foo::B(_) => {},
+    /// }
     /// ```
     pub WILDCARD_ENUM_MATCH_ARM,
     restriction,
     "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;
+    /// // Bad
+    /// match x {
+    ///     Foo::A => {},
+    ///     Foo::B => {},
+    ///     _ => {},
+    /// }
+    ///
+    /// // Good
+    /// 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.
     ///
@@ -239,10 +299,17 @@
     ///
     /// **Example:**
     /// ```rust
+    /// // Bad
     /// match "foo" {
     ///     "a" => {},
     ///     "bar" | _ => {},
     /// }
+    ///
+    /// // Good
+    /// match "foo" {
+    ///     "a" => {},
+    ///     _ => {},
+    /// }
     /// ```
     pub WILDCARD_IN_OR_PATTERNS,
     complexity,
@@ -356,6 +423,7 @@ pub struct 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,
@@ -676,7 +744,7 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>])
                                 arm.pat.span,
                                 &format!("`Err({})` matches all errors", &ident_bind_name),
                                 None,
-                                "match each error separately or use the error output",
+                                "match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable",
                             );
                         }
                     }
@@ -729,9 +797,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,6 +846,19 @@ 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,
@@ -773,7 +866,7 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_
             message,
             "try this",
             suggestion.join(" | "),
-            Applicability::MachineApplicable,
+            Applicability::MaybeIncorrect,
         )
     }
 }