Auto merge of #5258 - ThibsG:UselessBindingInStruct638, r=flip1995

Add lint for .. use in fully binded struct

This PR adds the lint `match-wild-in-fully-binded-struct` to prevent the use of the `..` pattern when all fields of the struct are already binded.

Fixes: #638

changelog: Add [`rest_pat_in_fully_bound_structs`] lint to warn against the use of `..` in fully binded struct

CHANGELOG.md

@@ -1321,6 +1321,7 @@ Released 2018-09-13
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
+[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
 [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
 [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else

README.md

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

@@ -610,6 +610,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &matches::MATCH_REF_PATS,
         &matches::MATCH_SINGLE_BINDING,
         &matches::MATCH_WILD_ERR_ARM,
+        &matches::REST_PAT_IN_FULLY_BOUND_STRUCTS,
         &matches::SINGLE_MATCH,
         &matches::SINGLE_MATCH_ELSE,
         &matches::WILDCARD_ENUM_MATCH_ARM,
@@ -1026,6 +1027,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&integer_division::INTEGER_DIVISION),
         LintId::of(&let_underscore::LET_UNDERSCORE_MUST_USE),
         LintId::of(&literal_representation::DECIMAL_LITERAL_REPRESENTATION),
+        LintId::of(&matches::REST_PAT_IN_FULLY_BOUND_STRUCTS),
         LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM),
         LintId::of(&mem_forget::MEM_FORGET),
         LintId::of(&methods::CLONE_ON_REF_PTR),

clippy_lints/src/matches.rs

@@ -14,8 +14,8 @@ use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::def::CtorKind;
 use rustc_hir::{
-    print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, PatKind, QPath,
-    RangeEnd,
+    print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Pat, PatKind,
+    QPath, RangeEnd,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
@@ -311,6 +311,36 @@ declare_clippy_lint! {
     "a match with a single binding instead of using `let` statement"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for unnecessary '..' pattern binding on struct when all fields are explicitly matched.
+    ///
+    /// **Why is this bad?** Correctness and readability. It's like having a wildcard pattern after
+    /// matching all enum variants explicitly.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # struct A { a: i32 }
+    /// let a = A { a: 5 };
+    ///
+    /// // Bad
+    /// match a {
+    ///     A { a: 5, .. } => {},
+    ///     _ => {},
+    /// }
+    ///
+    /// // Good
+    /// match a {
+    ///     A { a: 5 } => {},
+    ///     _ => {},
+    /// }
+    /// ```
+    pub REST_PAT_IN_FULLY_BOUND_STRUCTS,
+    restriction,
+    "a match on a struct that binds all fields but still uses the wildcard pattern"
+}
+
 #[derive(Default)]
 pub struct Matches {
     infallible_destructuring_match_linted: bool,
@@ -327,7 +357,8 @@ impl_lint_pass!(Matches => [
     WILDCARD_ENUM_MATCH_ARM,
     WILDCARD_IN_OR_PATTERNS,
     MATCH_SINGLE_BINDING,
-    INFALLIBLE_DESTRUCTURING_MATCH
+    INFALLIBLE_DESTRUCTURING_MATCH,
+    REST_PAT_IN_FULLY_BOUND_STRUCTS
 ]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
@@ -388,6 +419,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
             }
         }
     }
+
+    fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat<'_>) {
+        if_chain! {
+            if let PatKind::Struct(ref qpath, fields, true) = pat.kind;
+            if let QPath::Resolved(_, ref path) = qpath;
+            let def_id = path.res.def_id();
+            let ty = cx.tcx.type_of(def_id);
+            if let ty::Adt(def, _) = ty.kind;
+            if fields.len() == def.non_enum_variant().fields.len();
+
+            then {
+                span_lint_and_help(
+                    cx,
+                    REST_PAT_IN_FULLY_BOUND_STRUCTS,
+                    pat.span,
+                    "unnecessary use of `..` pattern in struct binding. All fields were already bound",
+                    "consider removing `..` from this binding",
+                );
+            }
+        }
+    }
 }
 
 #[rustfmt::skip]

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; 358] = [
+pub const ALL_LINTS: [Lint; 359] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1785,6 +1785,13 @@ pub const ALL_LINTS: [Lint; 358] = [
         deprecation: None,
         module: "replace_consts",
     },
+    Lint {
+        name: "rest_pat_in_fully_bound_structs",
+        group: "restriction",
+        desc: "a match on a struct that binds all fields but still uses the wildcard pattern",
+        deprecation: None,
+        module: "matches",
+    },
     Lint {
         name: "result_expect_used",
         group: "restriction",

tests/ui/rest_pat_in_fully_bound_structs.rs

@@ -0,0 +1,30 @@
+#![warn(clippy::rest_pat_in_fully_bound_structs)]
+
+struct A {
+    a: i32,
+    b: i64,
+    c: &'static str,
+}
+
+fn main() {
+    let a_struct = A { a: 5, b: 42, c: "A" };
+
+    match a_struct {
+        A { a: 5, b: 42, c: "", .. } => {}, // Lint
+        A { a: 0, b: 0, c: "", .. } => {},  // Lint
+        _ => {},
+    }
+
+    match a_struct {
+        A { a: 5, b: 42, .. } => {},
+        A { a: 0, b: 0, c: "", .. } => {}, // Lint
+        _ => {},
+    }
+
+    // No lint
+    match a_struct {
+        A { a: 5, .. } => {},
+        A { a: 0, b: 0, .. } => {},
+        _ => {},
+    }
+}

tests/ui/rest_pat_in_fully_bound_structs.stderr

@@ -0,0 +1,27 @@
+error: unnecessary use of `..` pattern in struct binding. All fields were already bound
+  --> $DIR/rest_pat_in_fully_bound_structs.rs:13:9
+   |
+LL |         A { a: 5, b: 42, c: "", .. } => {}, // Lint
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::rest-pat-in-fully-bound-structs` implied by `-D warnings`
+   = help: consider removing `..` from this binding
+
+error: unnecessary use of `..` pattern in struct binding. All fields were already bound
+  --> $DIR/rest_pat_in_fully_bound_structs.rs:14:9
+   |
+LL |         A { a: 0, b: 0, c: "", .. } => {},  // Lint
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing `..` from this binding
+
+error: unnecessary use of `..` pattern in struct binding. All fields were already bound
+  --> $DIR/rest_pat_in_fully_bound_structs.rs:20:9
+   |
+LL |         A { a: 0, b: 0, c: "", .. } => {}, // Lint
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing `..` from this binding
+
+error: aborting due to 3 previous errors
+