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 new lint unneeded_struct_pattern #13465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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 @@ -6133,6 +6133,7 @@ Released 2018-09-13
[`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
[`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps
[`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern
[`unneeded_struct_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_struct_pattern
[`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern
[`unnested_or_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns
[`unreachable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreachable
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/arbitrary_source_item_ordering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,8 @@ fn convert_assoc_item_kind(value: AssocItemKind) -> SourceItemOrderingTraitAssoc
#[allow(clippy::enum_glob_use)] // Very local glob use for legibility.
use SourceItemOrderingTraitAssocItemKind::*;
match value {
AssocItemKind::Const { .. } => Const,
AssocItemKind::Type { .. } => Type,
AssocItemKind::Const => Const,
AssocItemKind::Type => Type,
AssocItemKind::Fn { .. } => Fn,
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
crate::unneeded_struct_pattern::UNNEEDED_STRUCT_PATTERN_INFO,
crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,
crate::unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME_INFO,
crate::unused_async::UNUSED_ASYNC_INFO,
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 @@ -374,6 +374,7 @@ mod unnecessary_owned_empty_strings;
mod unnecessary_self_imports;
mod unnecessary_struct_initialization;
mod unnecessary_wraps;
mod unneeded_struct_pattern;
mod unnested_or_patterns;
mod unsafe_removed_from_name;
mod unused_async;
Expand Down Expand Up @@ -967,5 +968,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf)));
store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern));
// add lints here, do not remove this comment, it's used in `new_lint`
}
76 changes: 76 additions & 0 deletions clippy_lints/src/unneeded_struct_pattern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_from_proc_macro;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Pat, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;

declare_clippy_lint! {
/// ### What it does
/// Checks for struct patterns that match against unit variant.
///
/// ### Why is this bad?
/// Struct pattern `{ }` or `{ .. }` is not needed for unit variant.
///
/// ### Example
/// ```no_run
/// match Some(42) {
/// Some(v) => v,
/// None { .. } => 0,
/// };
/// // Or
/// match Some(42) {
/// Some(v) => v,
/// None { } => 0,
/// };
/// ```
/// Use instead:
/// ```no_run
/// match Some(42) {
/// Some(v) => v,
/// None => 0,
/// };
/// ```
#[clippy::version = "1.83.0"]
pub UNNEEDED_STRUCT_PATTERN,
style,
"using struct pattern to match against unit variant"
}

declare_lint_pass!(UnneededStructPattern => [UNNEEDED_STRUCT_PATTERN]);

impl LateLintPass<'_> for UnneededStructPattern {
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) {
if !pat.span.from_expansion()
&& let PatKind::Struct(path, [], _) = &pat.kind
&& let QPath::Resolved(_, path) = path
&& let Res::Def(DefKind::Variant, did) = path.res
{
let enum_did = cx.tcx.parent(did);
let variant = cx.tcx.adt_def(enum_did).variant_with_id(did);
Centri3 marked this conversation as resolved.
Show resolved Hide resolved

let has_only_fields_brackets = variant.ctor.is_some() && variant.fields.is_empty();
let non_exhaustive_activated = !variant.def_id.is_local() && variant.is_field_list_non_exhaustive();
if !has_only_fields_brackets || non_exhaustive_activated {
return;
}

if is_from_proc_macro(cx, *path) {
return;
}

if let Some(brackets_span) = pat.span.trim_start(path.span) {
Centri3 marked this conversation as resolved.
Show resolved Hide resolved
span_lint_and_sugg(
cx,
UNNEEDED_STRUCT_PATTERN,
brackets_span,
"struct pattern is not needed for a unit variant",
"remove the struct pattern",
String::new(),
Applicability::MachineApplicable,
);
}
}
}
}
21 changes: 21 additions & 0 deletions tests/ui/auxiliary/non-exhaustive-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,24 @@ pub enum ErrorKind {
#[doc(hidden)]
Uncategorized,
}

#[non_exhaustive]
pub enum ExtNonExhaustiveEnum {
Unit,
Tuple(i32),
Struct { field: i32 },
}

pub enum ExtNonExhaustiveVariant {
ExhaustiveUnit,
#[non_exhaustive]
Unit,
#[non_exhaustive]
Tuple(i32),
#[non_exhaustive]
StructNoField {},
#[non_exhaustive]
Struct {
field: i32,
},
}
177 changes: 177 additions & 0 deletions tests/ui/unneeded_struct_pattern.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
//@aux-build:non-exhaustive-enum.rs
#![allow(
clippy::manual_unwrap_or_default,
clippy::manual_unwrap_or,
clippy::redundant_pattern_matching
)]
#![warn(clippy::unneeded_struct_pattern)]

extern crate non_exhaustive_enum;
use non_exhaustive_enum::*;

fn noop() {}

fn main() {
match Some(114514) {
Some(v) => v,
None => 0,
};

match Some(1919810) {
Some(v) => v,
None => 0,
};

match Some(123456) {
Some(v) => v,
None => 0,
};

match Some(Some(123456)) {
Some(Some(v)) => v,
Some(None) => 0,
None => 0,
};

if let None = Some(0) {}
if let None = Some(0) {}
if let Some(None) = Some(Some(0)) {}
let None = Some(0) else { panic!() };
let None = Some(0) else { panic!() };
let Some(None) = Some(Some(0)) else { panic!() };

enum Custom {
HasFields {
field: i32,
},
HasBracketsNoFields {},
NoBrackets,
#[non_exhaustive]
NoBracketsNonExhaustive,
Init,
};

match Custom::Init {
Custom::HasFields { field: value } => value,
Custom::HasBracketsNoFields {} => 0,
Custom::NoBrackets => 0, //~ ERROR: struct pattern is not needed for a unit variant
Custom::NoBracketsNonExhaustive => 0, //~ ERROR: struct pattern is not needed for a unit variant
_ => 0,
};

match Custom::Init {
Custom::HasFields { field: value } => value,
Custom::HasBracketsNoFields { .. } => 0,
Custom::NoBrackets => 0, //~ ERROR: struct pattern is not needed for a unit variant
Custom::NoBracketsNonExhaustive => 0, //~ ERROR: struct pattern is not needed for a unit variant
_ => 0,
};

match Custom::Init {
Custom::NoBrackets if true => 0, //~ ERROR: struct pattern is not needed for a unit variant
_ => 0,
};

match Custom::Init {
Custom::NoBrackets | Custom::NoBracketsNonExhaustive => 0, //~ ERROR: struct pattern is not needed for a unit variant
_ => 0,
};

if let Custom::HasFields { field: value } = Custom::Init {
noop();
}
if let Custom::HasBracketsNoFields {} = Custom::Init {
noop();
}
if let Custom::HasBracketsNoFields { .. } = Custom::Init {
noop();
}
if let Custom::NoBrackets = Custom::Init {
//~^ ERROR: struct pattern is not needed for a unit variant
noop();
}
if let Custom::NoBrackets = Custom::Init {
//~^ ERROR: struct pattern is not needed for a unit variant
noop();
}
if let Custom::NoBrackets | Custom::NoBracketsNonExhaustive = Custom::Init {
//~^ ERROR: struct pattern is not needed for a unit variant
noop();
}
if let Custom::NoBracketsNonExhaustive = Custom::Init {
//~^ ERROR: struct pattern is not needed for a unit variant
noop();
}
if let Custom::NoBracketsNonExhaustive = Custom::Init {
//~^ ERROR: struct pattern is not needed for a unit variant
noop();
}

let Custom::HasFields { field: value } = Custom::Init else {
panic!()
};

let Custom::HasBracketsNoFields {} = Custom::Init else {
panic!()
};

let Custom::HasBracketsNoFields { .. } = Custom::Init else {
panic!()
};
let Custom::NoBrackets = Custom::Init else { panic!() }; //~ ERROR: struct pattern is not needed for a unit variant

let Custom::NoBrackets = Custom::Init else {
//~^ ERROR: struct pattern is not needed for a unit variant
panic!()
};
let Custom::NoBracketsNonExhaustive = Custom::Init else {
//~^ ERROR: struct pattern is not needed for a unit variant
panic!()
};
let Custom::NoBracketsNonExhaustive = Custom::Init else {
//~^ ERROR: struct pattern is not needed for a unit variant
panic!()
};

enum Refutable {
Variant,
}

fn pat_in_fn_param_1(Refutable::Variant: Refutable) {} //~ ERROR: struct pattern is not needed for a unit variant
fn pat_in_fn_param_2(Refutable::Variant: Refutable) {} //~ ERROR: struct pattern is not needed for a unit variant

for Refutable::Variant in [] {} //~ ERROR: struct pattern is not needed for a unit variant
for Refutable::Variant in [] {} //~ ERROR: struct pattern is not needed for a unit variant
}

fn external_crate() {
use ExtNonExhaustiveVariant::*;

match ExhaustiveUnit {
// Expected
ExhaustiveUnit => 0,
_ => 0,
};

match ExhaustiveUnit {
// Exhaustive variant
ExhaustiveUnit => 0, //~ ERROR: struct pattern is not needed for a unit variant
_ => 0,
};

match ExhaustiveUnit {
// Exhaustive variant
ExhaustiveUnit => 0, //~ ERROR: struct pattern is not needed for a unit variant
_ => 0,
};

match ExhaustiveUnit {
ExhaustiveUnit => 0,
// vvvvv Non-exhaustive variants, should all be ignored
Unit { .. } => 0,
Tuple { 0: field, .. } => field,
StructNoField { .. } => 0,
Struct { field, .. } => field,
_ => 0,
};
}
Loading
Loading