diff --git a/CHANGELOG.md b/CHANGELOG.md index d1d70f4ddfb8..af236daac590 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6046,6 +6046,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/README.md b/README.md index ec76a6dfb08e..1690e2beb16f 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 over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. diff --git a/book/src/README.md b/book/src/README.md index 7bdfb97c3acf..23527ba896af 100644 --- a/book/src/README.md +++ b/book/src/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 3c4e75df8abe..a2c411806c29 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -744,6 +744,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 153820140129..dcb5fce89739 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -372,6 +372,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; @@ -951,5 +952,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(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(|_| 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..89c376960eae --- /dev/null +++ b/clippy_lints/src/unneeded_struct_pattern.rs @@ -0,0 +1,71 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +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, + nursery, + "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_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_fields_brackets || non_exhaustive_activated { + 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..7ebd152daffd --- /dev/null +++ b/tests/ui/unneeded_struct_pattern.fixed @@ -0,0 +1,98 @@ +//@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, + }; + + 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, + }; +} + +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..3d62113f936f --- /dev/null +++ b/tests/ui/unneeded_struct_pattern.rs @@ -0,0 +1,98 @@ +//@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, + }; + + 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, + }; +} + +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..ba0d647d3fff --- /dev/null +++ b/tests/ui/unneeded_struct_pattern.stderr @@ -0,0 +1,83 @@ +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:44: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:45: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:52: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:53: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:58: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:63: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:63: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:79: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:85:23 + | +LL | ExhaustiveUnit {} => 0, + | ^^^ help: remove the struct pattern + +error: aborting due to 13 previous errors +