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
+