diff --git a/CHANGELOG.md b/CHANGELOG.md index 801c96cc97ef..e8b75088c9ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 16146a4a7a46..66e42f6f0627 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 017e6e2a1423..6b4d8a4d3b27 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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` } diff --git a/clippy_lints/src/unneeded_struct_pattern.rs b/clippy_lints/src/unneeded_struct_pattern.rs new file mode 100644 index 000000000000..40ba70d451db --- /dev/null +++ b/clippy_lints/src/unneeded_struct_pattern.rs @@ -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); + + 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) { + 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, + ); + } + } + } +} diff --git a/tests/ui/auxiliary/non-exhaustive-enum.rs b/tests/ui/auxiliary/non-exhaustive-enum.rs index 420232f9f8d8..e3205193ccea 100644 --- a/tests/ui/auxiliary/non-exhaustive-enum.rs +++ b/tests/ui/auxiliary/non-exhaustive-enum.rs @@ -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, + }, +} diff --git a/tests/ui/unneeded_struct_pattern.fixed b/tests/ui/unneeded_struct_pattern.fixed new file mode 100644 index 000000000000..124a5c7d1ebf --- /dev/null +++ b/tests/ui/unneeded_struct_pattern.fixed @@ -0,0 +1,150 @@ +//@aux-build:non-exhaustive-enum.rs +#![allow(clippy::manual_unwrap_or_default, clippy::manual_unwrap_or)] +#![warn(clippy::unneeded_struct_pattern)] + +extern crate non_exhaustive_enum; +use non_exhaustive_enum::*; + +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, // Should be ignored + Custom::HasBracketsNoFields {} => 0, // Should be ignored + Custom::NoBrackets => 0, // Should be fixed + Custom::NoBracketsNonExhaustive => 0, // Should be fixed + _ => 0, + }; + + match Custom::Init { + Custom::HasFields { field: value } => value, // Should be ignored + Custom::HasBracketsNoFields { .. } => 0, // Should be ignored + Custom::NoBrackets => 0, // Should be fixed + Custom::NoBracketsNonExhaustive => 0, // Should be fixed + _ => 0, + }; + + match Custom::Init { + Custom::NoBrackets if true => 0, // Should be fixed + _ => 0, + }; + + match Custom::Init { + Custom::NoBrackets | Custom::NoBracketsNonExhaustive => 0, // Should be fixed + _ => 0, + }; + + if let Custom::HasFields { field: value } = Custom::Init {} // Should be ignored + if let Custom::HasBracketsNoFields {} = Custom::Init {} // Should be ignored + if let Custom::HasBracketsNoFields { .. } = Custom::Init {} // Should be ignored + if let Custom::NoBrackets = Custom::Init {} // Should be fixed + if let Custom::NoBrackets = Custom::Init {} // Should be fixed + if let Custom::NoBrackets | Custom::NoBracketsNonExhaustive = Custom::Init {} // Should be fixed + if let Custom::NoBracketsNonExhaustive = Custom::Init {} // Should be fixed + if let Custom::NoBracketsNonExhaustive = Custom::Init {} // Should be fixed + + // Should be ignored + let Custom::HasFields { field: value } = Custom::Init else { + panic!() + }; + // Should be ignored + let Custom::HasBracketsNoFields {} = Custom::Init else { + panic!() + }; + // Should be ignored + let Custom::HasBracketsNoFields { .. } = Custom::Init else { + panic!() + }; + let Custom::NoBrackets = Custom::Init else { panic!() }; // Should be fixed + // Should be fixed + let Custom::NoBrackets = Custom::Init else { + panic!() + }; + // Should be fixed + let Custom::NoBracketsNonExhaustive = Custom::Init else { + panic!() + }; + // Should be fixed + let Custom::NoBracketsNonExhaustive = Custom::Init else { + panic!() + }; + + enum Refutable { + Variant, + } + + fn pat_in_fn_param_1(Refutable::Variant: Refutable) {} // Should be fixed + fn pat_in_fn_param_2(Refutable::Variant: Refutable) {} // Should be fixed + + for Refutable::Variant in [] {} // Should be fixed + for Refutable::Variant in [] {} // Should be fixed +} + +fn external_crate() { + use ExtNonExhaustiveVariant::*; + + match ExhaustiveUnit { + // Expected + ExhaustiveUnit => 0, + _ => 0, + }; + + match ExhaustiveUnit { + // Exhaustive variant, should be fixed + ExhaustiveUnit => 0, + _ => 0, + }; + + match ExhaustiveUnit { + // Exhaustive variant, should be fixed + ExhaustiveUnit => 0, + _ => 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, + }; +} diff --git a/tests/ui/unneeded_struct_pattern.rs b/tests/ui/unneeded_struct_pattern.rs new file mode 100644 index 000000000000..b0efd35bcbc1 --- /dev/null +++ b/tests/ui/unneeded_struct_pattern.rs @@ -0,0 +1,150 @@ +//@aux-build:non-exhaustive-enum.rs +#![allow(clippy::manual_unwrap_or_default, clippy::manual_unwrap_or)] +#![warn(clippy::unneeded_struct_pattern)] + +extern crate non_exhaustive_enum; +use non_exhaustive_enum::*; + +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, // Should be ignored + Custom::HasBracketsNoFields {} => 0, // Should be ignored + Custom::NoBrackets {} => 0, // Should be fixed + Custom::NoBracketsNonExhaustive {} => 0, // Should be fixed + _ => 0, + }; + + match Custom::Init { + Custom::HasFields { field: value } => value, // Should be ignored + Custom::HasBracketsNoFields { .. } => 0, // Should be ignored + Custom::NoBrackets { .. } => 0, // Should be fixed + Custom::NoBracketsNonExhaustive { .. } => 0, // Should be fixed + _ => 0, + }; + + match Custom::Init { + Custom::NoBrackets {} if true => 0, // Should be fixed + _ => 0, + }; + + match Custom::Init { + Custom::NoBrackets {} | Custom::NoBracketsNonExhaustive {} => 0, // Should be fixed + _ => 0, + }; + + if let Custom::HasFields { field: value } = Custom::Init {} // Should be ignored + if let Custom::HasBracketsNoFields {} = Custom::Init {} // Should be ignored + if let Custom::HasBracketsNoFields { .. } = Custom::Init {} // Should be ignored + if let Custom::NoBrackets {} = Custom::Init {} // Should be fixed + if let Custom::NoBrackets { .. } = Custom::Init {} // Should be fixed + if let Custom::NoBrackets {} | Custom::NoBracketsNonExhaustive {} = Custom::Init {} // Should be fixed + if let Custom::NoBracketsNonExhaustive {} = Custom::Init {} // Should be fixed + if let Custom::NoBracketsNonExhaustive { .. } = Custom::Init {} // Should be fixed + + // Should be ignored + let Custom::HasFields { field: value } = Custom::Init else { + panic!() + }; + // Should be ignored + let Custom::HasBracketsNoFields {} = Custom::Init else { + panic!() + }; + // Should be ignored + let Custom::HasBracketsNoFields { .. } = Custom::Init else { + panic!() + }; + let Custom::NoBrackets {} = Custom::Init else { panic!() }; // Should be fixed + // Should be fixed + let Custom::NoBrackets { .. } = Custom::Init else { + panic!() + }; + // Should be fixed + let Custom::NoBracketsNonExhaustive {} = Custom::Init else { + panic!() + }; + // Should be fixed + let Custom::NoBracketsNonExhaustive { .. } = Custom::Init else { + panic!() + }; + + enum Refutable { + Variant, + } + + fn pat_in_fn_param_1(Refutable::Variant {}: Refutable) {} // Should be fixed + fn pat_in_fn_param_2(Refutable::Variant { .. }: Refutable) {} // Should be fixed + + for Refutable::Variant {} in [] {} // Should be fixed + for Refutable::Variant { .. } in [] {} // Should be fixed +} + +fn external_crate() { + use ExtNonExhaustiveVariant::*; + + match ExhaustiveUnit { + // Expected + ExhaustiveUnit => 0, + _ => 0, + }; + + match ExhaustiveUnit { + // Exhaustive variant, should be fixed + ExhaustiveUnit { .. } => 0, + _ => 0, + }; + + match ExhaustiveUnit { + // Exhaustive variant, should be fixed + ExhaustiveUnit {} => 0, + _ => 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, + }; +} diff --git a/tests/ui/unneeded_struct_pattern.stderr b/tests/ui/unneeded_struct_pattern.stderr new file mode 100644 index 000000000000..72603640ca28 --- /dev/null +++ b/tests/ui/unneeded_struct_pattern.stderr @@ -0,0 +1,203 @@ +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:11:13 + | +LL | None {} => 0, + | ^^^ help: remove the struct pattern + | + = note: `-D clippy::unneeded-struct-pattern` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unneeded_struct_pattern)]` + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:16:13 + | +LL | None { .. } => 0, + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:26:18 + | +LL | Some(None {}) => 0, + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:27:13 + | +LL | None {} => 0, + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:30:16 + | +LL | if let None {} = Some(0) {} + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:31:16 + | +LL | if let None { .. } = Some(0) {} + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:32:21 + | +LL | if let Some(None {}) = Some(Some(0)) {} + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:33:13 + | +LL | let None {} = Some(0) else { panic!() }; + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:34:13 + | +LL | let None { .. } = Some(0) else { panic!() }; + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:35:18 + | +LL | let Some(None {}) = Some(Some(0)) else { panic!() }; + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:51:27 + | +LL | Custom::NoBrackets {} => 0, // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:52:40 + | +LL | Custom::NoBracketsNonExhaustive {} => 0, // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:59:27 + | +LL | Custom::NoBrackets { .. } => 0, // Should be fixed + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:60:40 + | +LL | Custom::NoBracketsNonExhaustive { .. } => 0, // Should be fixed + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:65:27 + | +LL | Custom::NoBrackets {} if true => 0, // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:70:27 + | +LL | Custom::NoBrackets {} | Custom::NoBracketsNonExhaustive {} => 0, // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:70:64 + | +LL | Custom::NoBrackets {} | Custom::NoBracketsNonExhaustive {} => 0, // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:77:30 + | +LL | if let Custom::NoBrackets {} = Custom::Init {} // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:78:30 + | +LL | if let Custom::NoBrackets { .. } = Custom::Init {} // Should be fixed + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:79:30 + | +LL | if let Custom::NoBrackets {} | Custom::NoBracketsNonExhaustive {} = Custom::Init {} // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:79:67 + | +LL | if let Custom::NoBrackets {} | Custom::NoBracketsNonExhaustive {} = Custom::Init {} // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:80:43 + | +LL | if let Custom::NoBracketsNonExhaustive {} = Custom::Init {} // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:81:43 + | +LL | if let Custom::NoBracketsNonExhaustive { .. } = Custom::Init {} // Should be fixed + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:95:27 + | +LL | let Custom::NoBrackets {} = Custom::Init else { panic!() }; // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:97:27 + | +LL | let Custom::NoBrackets { .. } = Custom::Init else { + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:101:40 + | +LL | let Custom::NoBracketsNonExhaustive {} = Custom::Init else { + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:105:40 + | +LL | let Custom::NoBracketsNonExhaustive { .. } = Custom::Init else { + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:113:44 + | +LL | fn pat_in_fn_param_1(Refutable::Variant {}: Refutable) {} // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:114:44 + | +LL | fn pat_in_fn_param_2(Refutable::Variant { .. }: Refutable) {} // Should be fixed + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:116:27 + | +LL | for Refutable::Variant {} in [] {} // Should be fixed + | ^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:117:27 + | +LL | for Refutable::Variant { .. } in [] {} // Should be fixed + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:131:23 + | +LL | ExhaustiveUnit { .. } => 0, + | ^^^^^^^ help: remove the struct pattern + +error: struct pattern is not needed for a unit variant + --> tests/ui/unneeded_struct_pattern.rs:137:23 + | +LL | ExhaustiveUnit {} => 0, + | ^^^ help: remove the struct pattern + +error: aborting due to 33 previous errors +