From 2aa14c9beb48ad780412df747b0a64b90eb5f13e Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sun, 1 Mar 2020 17:36:14 +0100 Subject: [PATCH] Add restrictive pat use in full binded struct --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/lib.rs | 2 + clippy_lints/src/matches.rs | 59 ++++++++++++++++++- src/lintlist/mod.rs | 9 ++- tests/ui/rest_pat_in_fully_bound_structs.rs | 30 ++++++++++ .../ui/rest_pat_in_fully_bound_structs.stderr | 27 +++++++++ 7 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 tests/ui/rest_pat_in_fully_bound_structs.rs create mode 100644 tests/ui/rest_pat_in_fully_bound_structs.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 99e84ea5193..4d8e2494389 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 1300c5ad47b..6915b1bde02 100644 --- a/README.md +++ b/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: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4157d33079c..327cc56cafa 100644 --- a/clippy_lints/src/lib.rs +++ b/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), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 6f1efe0fd85..9668c5d8349 100644 --- a/clippy_lints/src/matches.rs +++ b/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,28 @@ 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; + if let Some(def_id) = path.res.opt_def_id(); + let ty = cx.tcx.type_of(def_id); + if let ty::Adt(def, _) = ty.kind; + if def.is_struct() || def.is_union(); + 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] diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 15e6a4b6036..2b93e4279f0 100644 --- a/src/lintlist/mod.rs +++ b/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", diff --git a/tests/ui/rest_pat_in_fully_bound_structs.rs b/tests/ui/rest_pat_in_fully_bound_structs.rs new file mode 100644 index 00000000000..22e4d4edd78 --- /dev/null +++ b/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, .. } => {}, + _ => {}, + } +} diff --git a/tests/ui/rest_pat_in_fully_bound_structs.stderr b/tests/ui/rest_pat_in_fully_bound_structs.stderr new file mode 100644 index 00000000000..effa46b4b0f --- /dev/null +++ b/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 +