Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint for .. use in fully binded struct #5258

Merged
merged 1 commit into from
Mar 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
59 changes: 56 additions & 3 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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]
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/rest_pat_in_fully_bound_structs.rs
Original file line number Diff line number Diff line change
@@ -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, .. } => {},
_ => {},
}
}
27 changes: 27 additions & 0 deletions tests/ui/rest_pat_in_fully_bound_structs.stderr
Original file line number Diff line number Diff line change
@@ -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