Auto merge of #4537 - mikerite:unneeded_wildcard_pattern, r=phansch

Add `unneeded-wildcard-pattern` lint

changelog: Add `unneeded-wildcard-pattern` lint

CHANGELOG.md

@@ -1190,6 +1190,7 @@ Released 2018-09-13
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
 [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
 [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern
+[`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern
 [`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal
 [`unsafe_removed_from_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_removed_from_name
 [`unsafe_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_vector_initialization

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 315 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 316 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:
 

clippy_lints/src/eval_order_dependence.rs

@@ -101,7 +101,7 @@ struct DivergenceVisitor<'a, 'tcx> {
 impl<'a, 'tcx> DivergenceVisitor<'a, 'tcx> {
     fn maybe_walk_expr(&mut self, e: &'tcx Expr) {
         match e.node {
-            ExprKind::Closure(.., _) => {},
+            ExprKind::Closure(..) => {},
             ExprKind::Match(ref e, ref arms, _) => {
                 self.visit_expr(e);
                 for arm in arms {

clippy_lints/src/lib.rs

@@ -828,6 +828,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         misc_early::REDUNDANT_CLOSURE_CALL,
         misc_early::REDUNDANT_PATTERN,
         misc_early::UNNEEDED_FIELD_PATTERN,
+        misc_early::UNNEEDED_WILDCARD_PATTERN,
         misc_early::ZERO_PREFIXED_LITERAL,
         mut_reference::UNNECESSARY_MUT_PASSED,
         mutex_atomic::MUTEX_ATOMIC,
@@ -1047,6 +1048,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         methods::USELESS_ASREF,
         misc::SHORT_CIRCUIT_STATEMENT,
         misc_early::REDUNDANT_CLOSURE_CALL,
+        misc_early::UNNEEDED_WILDCARD_PATTERN,
         misc_early::ZERO_PREFIXED_LITERAL,
         needless_bool::BOOL_COMPARISON,
         needless_bool::NEEDLESS_BOOL,

clippy_lints/src/misc_early.rs

@@ -195,6 +195,44 @@ declare_clippy_lint! {
     "using `name @ _` in a pattern"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for tuple patterns with a wildcard
+    /// pattern (`_`) is next to a rest pattern (`..`).
+    ///
+    /// _NOTE_: While `_, ..` means there is at least one element left, `..`
+    /// means there are 0 or more elements left. This can make a difference
+    /// when refactoring, but shouldn't result in errors in the refactored code,
+    /// since the wildcard pattern isn't used anyway.
+    /// **Why is this bad?** The wildcard pattern is unneeded as the rest pattern
+    /// can match that element as well.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # struct TupleStruct(u32, u32, u32);
+    /// # let t = TupleStruct(1, 2, 3);
+    ///
+    /// match t {
+    ///     TupleStruct(0, .., _) => (),
+    ///     _ => (),
+    /// }
+    /// ```
+    /// can be written as
+    /// ```rust
+    /// # struct TupleStruct(u32, u32, u32);
+    /// # let t = TupleStruct(1, 2, 3);
+    ///
+    /// match t {
+    ///     TupleStruct(0, ..) => (),
+    ///     _ => (),
+    /// }
+    /// ```
+    pub UNNEEDED_WILDCARD_PATTERN,
+    complexity,
+    "tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`)"
+}
+
 declare_lint_pass!(MiscEarlyLints => [
     UNNEEDED_FIELD_PATTERN,
     DUPLICATE_UNDERSCORE_ARGUMENT,
@@ -204,7 +242,8 @@ declare_lint_pass!(MiscEarlyLints => [
     UNSEPARATED_LITERAL_SUFFIX,
     ZERO_PREFIXED_LITERAL,
     BUILTIN_TYPE_SHADOW,
-    REDUNDANT_PATTERN
+    REDUNDANT_PATTERN,
+    UNNEEDED_WILDCARD_PATTERN,
 ]);
 
 // Used to find `return` statements or equivalents e.g., `?`
@@ -326,6 +365,8 @@ impl EarlyLintPass for MiscEarlyLints {
                 );
             }
         }
+
+        check_unneeded_wildcard_pattern(cx, pat);
     }
 
     fn check_fn(&mut self, cx: &EarlyContext<'_>, _: FnKind<'_>, decl: &FnDecl, _: Span, _: NodeId) {
@@ -520,3 +561,54 @@ impl MiscEarlyLints {
         }
     }
 }
+
+fn check_unneeded_wildcard_pattern(cx: &EarlyContext<'_>, pat: &Pat) {
+    if let PatKind::TupleStruct(_, ref patterns) | PatKind::Tuple(ref patterns) = pat.node {
+        fn span_lint(cx: &EarlyContext<'_>, span: Span, only_one: bool) {
+            span_lint_and_sugg(
+                cx,
+                UNNEEDED_WILDCARD_PATTERN,
+                span,
+                if only_one {
+                    "this pattern is unneeded as the `..` pattern can match that element"
+                } else {
+                    "these patterns are unneeded as the `..` pattern can match those elements"
+                },
+                if only_one { "remove it" } else { "remove them" },
+                "".to_string(),
+                Applicability::MachineApplicable,
+            );
+        }
+
+        #[allow(clippy::trivially_copy_pass_by_ref)]
+        fn is_wild<P: std::ops::Deref<Target = Pat>>(pat: &&P) -> bool {
+            if let PatKind::Wild = pat.node {
+                true
+            } else {
+                false
+            }
+        }
+
+        if let Some(rest_index) = patterns.iter().position(|pat| pat.is_rest()) {
+            if let Some((left_index, left_pat)) = patterns[..rest_index]
+                .iter()
+                .rev()
+                .take_while(is_wild)
+                .enumerate()
+                .last()
+            {
+                span_lint(cx, left_pat.span.until(patterns[rest_index].span), left_index == 0);
+            }
+
+            if let Some((right_index, right_pat)) =
+                patterns[rest_index + 1..].iter().take_while(is_wild).enumerate().last()
+            {
+                span_lint(
+                    cx,
+                    patterns[rest_index].span.shrink_to_hi().to(right_pat.span),
+                    right_index == 0,
+                );
+            }
+        }
+    }
+}

clippy_lints/src/no_effect.rs

@@ -47,7 +47,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
         return false;
     }
     match expr.node {
-        ExprKind::Lit(..) | ExprKind::Closure(.., _) => true,
+        ExprKind::Lit(..) | ExprKind::Closure(..) => true,
         ExprKind::Path(..) => !has_drop(cx, cx.tables.expr_ty(expr)),
         ExprKind::Index(ref a, ref b) | ExprKind::Binary(_, ref a, ref b) => {
             has_no_effect(cx, a) && has_no_effect(cx, b)

clippy_lints/src/utils/sugg.rs

@@ -93,7 +93,7 @@ impl<'a> Sugg<'a> {
         match expr.node {
             hir::ExprKind::AddrOf(..)
             | hir::ExprKind::Box(..)
-            | hir::ExprKind::Closure(.., _)
+            | hir::ExprKind::Closure(..)
             | hir::ExprKind::Unary(..)
             | hir::ExprKind::Match(..) => Sugg::MaybeParen(snippet),
             hir::ExprKind::Continue(..)

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 315] = [
+pub const ALL_LINTS: [Lint; 316] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1988,6 +1988,13 @@ pub const ALL_LINTS: [Lint; 315] = [
         deprecation: None,
         module: "misc_early",
     },
+    Lint {
+        name: "unneeded_wildcard_pattern",
+        group: "complexity",
+        desc: "tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`)",
+        deprecation: None,
+        module: "misc_early",
+    },
     Lint {
         name: "unreadable_literal",
         group: "style",

tests/ui/unneeded_wildcard_pattern.fixed

@@ -0,0 +1,45 @@
+// run-rustfix
+#![feature(stmt_expr_attributes)]
+#![deny(clippy::unneeded_wildcard_pattern)]
+
+fn main() {
+    let t = (0, 1, 2, 3);
+
+    if let (0, ..) = t {};
+    if let (0, ..) = t {};
+    if let (.., 0) = t {};
+    if let (.., 0) = t {};
+    if let (0, ..) = t {};
+    if let (0, ..) = t {};
+    if let (_, 0, ..) = t {};
+    if let (.., 0, _) = t {};
+    if let (0, _, _, _) = t {};
+    if let (0, ..) = t {};
+    if let (.., 0) = t {};
+
+    #[rustfmt::skip]
+    {
+        if let (0, ..,) = t {};
+    }
+
+    struct S(usize, usize, usize, usize);
+
+    let s = S(0, 1, 2, 3);
+
+    if let S(0, ..) = s {};
+    if let S(0, ..) = s {};
+    if let S(.., 0) = s {};
+    if let S(.., 0) = s {};
+    if let S(0, ..) = s {};
+    if let S(0, ..) = s {};
+    if let S(_, 0, ..) = s {};
+    if let S(.., 0, _) = s {};
+    if let S(0, _, _, _) = s {};
+    if let S(0, ..) = s {};
+    if let S(.., 0) = s {};
+
+    #[rustfmt::skip]
+    {
+        if let S(0, ..,) = s {};
+    }
+}

tests/ui/unneeded_wildcard_pattern.rs

@@ -0,0 +1,45 @@
+// run-rustfix
+#![feature(stmt_expr_attributes)]
+#![deny(clippy::unneeded_wildcard_pattern)]
+
+fn main() {
+    let t = (0, 1, 2, 3);
+
+    if let (0, .., _) = t {};
+    if let (0, _, ..) = t {};
+    if let (_, .., 0) = t {};
+    if let (.., _, 0) = t {};
+    if let (0, _, _, ..) = t {};
+    if let (0, .., _, _) = t {};
+    if let (_, 0, ..) = t {};
+    if let (.., 0, _) = t {};
+    if let (0, _, _, _) = t {};
+    if let (0, ..) = t {};
+    if let (.., 0) = t {};
+
+    #[rustfmt::skip]
+    {
+        if let (0, .., _, _,) = t {};
+    }
+
+    struct S(usize, usize, usize, usize);
+
+    let s = S(0, 1, 2, 3);
+
+    if let S(0, .., _) = s {};
+    if let S(0, _, ..) = s {};
+    if let S(_, .., 0) = s {};
+    if let S(.., _, 0) = s {};
+    if let S(0, _, _, ..) = s {};
+    if let S(0, .., _, _) = s {};
+    if let S(_, 0, ..) = s {};
+    if let S(.., 0, _) = s {};
+    if let S(0, _, _, _) = s {};
+    if let S(0, ..) = s {};
+    if let S(.., 0) = s {};
+
+    #[rustfmt::skip]
+    {
+        if let S(0, .., _, _,) = s {};
+    }
+}

tests/ui/unneeded_wildcard_pattern.stderr

@@ -0,0 +1,92 @@
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:8:18
+   |
+LL |     if let (0, .., _) = t {};
+   |                  ^^^ help: remove it
+   |
+note: lint level defined here
+  --> $DIR/unneeded_wildcard_pattern.rs:3:9
+   |
+LL | #![deny(clippy::unneeded_wildcard_pattern)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:9:16
+   |
+LL |     if let (0, _, ..) = t {};
+   |                ^^^ help: remove it
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:10:13
+   |
+LL |     if let (_, .., 0) = t {};
+   |             ^^^ help: remove it
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:11:15
+   |
+LL |     if let (.., _, 0) = t {};
+   |               ^^^ help: remove it
+
+error: these patterns are unneeded as the `..` pattern can match those elements
+  --> $DIR/unneeded_wildcard_pattern.rs:12:16
+   |
+LL |     if let (0, _, _, ..) = t {};
+   |                ^^^^^^ help: remove them
+
+error: these patterns are unneeded as the `..` pattern can match those elements
+  --> $DIR/unneeded_wildcard_pattern.rs:13:18
+   |
+LL |     if let (0, .., _, _) = t {};
+   |                  ^^^^^^ help: remove them
+
+error: these patterns are unneeded as the `..` pattern can match those elements
+  --> $DIR/unneeded_wildcard_pattern.rs:22:22
+   |
+LL |         if let (0, .., _, _,) = t {};
+   |                      ^^^^^^ help: remove them
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:29:19
+   |
+LL |     if let S(0, .., _) = s {};
+   |                   ^^^ help: remove it
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:30:17
+   |
+LL |     if let S(0, _, ..) = s {};
+   |                 ^^^ help: remove it
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:31:14
+   |
+LL |     if let S(_, .., 0) = s {};
+   |              ^^^ help: remove it
+
+error: this pattern is unneeded as the `..` pattern can match that element
+  --> $DIR/unneeded_wildcard_pattern.rs:32:16
+   |
+LL |     if let S(.., _, 0) = s {};
+   |                ^^^ help: remove it
+
+error: these patterns are unneeded as the `..` pattern can match those elements
+  --> $DIR/unneeded_wildcard_pattern.rs:33:17
+   |
+LL |     if let S(0, _, _, ..) = s {};
+   |                 ^^^^^^ help: remove them
+
+error: these patterns are unneeded as the `..` pattern can match those elements
+  --> $DIR/unneeded_wildcard_pattern.rs:34:19
+   |
+LL |     if let S(0, .., _, _) = s {};
+   |                   ^^^^^^ help: remove them
+
+error: these patterns are unneeded as the `..` pattern can match those elements
+  --> $DIR/unneeded_wildcard_pattern.rs:43:23
+   |
+LL |         if let S(0, .., _, _,) = s {};
+   |                       ^^^^^^ help: remove them
+
+error: aborting due to 14 previous errors
+