From 3ebd2bc2e4fac8f56e1399872ea449594a280daa Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sat, 25 Dec 2021 16:52:58 +0100 Subject: [PATCH] new lint: `init-numbered-fields` --- CHANGELOG.md | 1 + clippy_lints/src/init_numbered_fields.rs | 80 ++++++++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 8 ++- clippy_lints/src/macro_use.rs | 6 +- clippy_utils/src/lib.rs | 7 +++ tests/ui/numbered_fields.fixed | 33 ++++++++++ tests/ui/numbered_fields.rs | 41 ++++++++++++ tests/ui/numbered_fields.stderr | 26 ++++++++ 11 files changed, 198 insertions(+), 7 deletions(-) create mode 100644 clippy_lints/src/init_numbered_fields.rs create mode 100644 tests/ui/numbered_fields.fixed create mode 100644 tests/ui/numbered_fields.rs create mode 100644 tests/ui/numbered_fields.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index c09310e5556..27bac4718b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3026,6 +3026,7 @@ Released 2018-09-13 [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string [`inherent_to_string_shadow_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string_shadow_display +[`init_numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#init_numbered_fields [`inline_always`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_always [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax diff --git a/clippy_lints/src/init_numbered_fields.rs b/clippy_lints/src/init_numbered_fields.rs new file mode 100644 index 00000000000..5fe6725b581 --- /dev/null +++ b/clippy_lints/src/init_numbered_fields.rs @@ -0,0 +1,80 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::in_macro; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use std::borrow::Cow; +use std::cmp::Reverse; +use std::collections::BinaryHeap; + +declare_clippy_lint! { + /// ### What it does + /// Checks for tuple structs initialized with field syntax. + /// It will however not lint if a base initializer is present. + /// The lint will also ignore code in macros. + /// + /// ### Why is this bad? + /// This may be confusing to the uninitiated and adds no + /// benefit as opposed to tuple initializers + /// + /// ### Example + /// ```rust + /// struct TupleStruct(u8, u16); + /// + /// let _ = TupleStruct { + /// 0: 1, + /// 1: 23, + /// }; + /// + /// // should be written as + /// let base = TupleStruct(1, 23); + /// + /// // This is OK however + /// let _ = TupleStruct { 0: 42, ..base }; + /// ``` + #[clippy::version = "1.59.0"] + pub INIT_NUMBERED_FIELDS, + style, + "numbered fields in tuple struct initializer" +} + +declare_lint_pass!(NumberedFields => [INIT_NUMBERED_FIELDS]); + +impl<'tcx> LateLintPass<'tcx> for NumberedFields { + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if let ExprKind::Struct(path, fields, None) = e.kind { + if !fields.is_empty() + && !in_macro(e.span) + && fields + .iter() + .all(|f| f.ident.as_str().as_bytes().iter().all(u8::is_ascii_digit)) + { + let expr_spans = fields + .iter() + .map(|f| (Reverse(f.ident.as_str().parse::().unwrap()), f.expr.span)) + .collect::>(); + let mut appl = Applicability::MachineApplicable; + let snippet = format!( + "{}({})", + snippet_with_applicability(cx, path.span(), "..", &mut appl), + expr_spans + .into_iter_sorted() + .map(|(_, span)| snippet_with_applicability(cx, span, "..", &mut appl)) + .intersperse(Cow::Borrowed(", ")) + .collect::() + ); + span_lint_and_sugg( + cx, + INIT_NUMBERED_FIELDS, + e.span, + "used a field initializer for a tuple struct", + "try this instead", + snippet, + appl, + ); + } + } + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 3d3999d4cc0..944411087e9 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -81,6 +81,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(infinite_iter::INFINITE_ITER), LintId::of(inherent_to_string::INHERENT_TO_STRING), LintId::of(inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY), + LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS), LintId::of(inline_fn_without_body::INLINE_FN_WITHOUT_BODY), LintId::of(int_plus_one::INT_PLUS_ONE), LintId::of(large_const_arrays::LARGE_CONST_ARRAYS), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 766c5ba1bcb..002122793f3 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -178,6 +178,7 @@ store.register_lints(&[ inherent_impl::MULTIPLE_INHERENT_IMPL, inherent_to_string::INHERENT_TO_STRING, inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY, + init_numbered_fields::INIT_NUMBERED_FIELDS, inline_fn_without_body::INLINE_FN_WITHOUT_BODY, int_plus_one::INT_PLUS_ONE, integer_division::INTEGER_DIVISION, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index ea87e7e7a73..1a0b869d40a 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -29,6 +29,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(functions::MUST_USE_UNIT), LintId::of(functions::RESULT_UNIT_ERR), LintId::of(inherent_to_string::INHERENT_TO_STRING), + LintId::of(init_numbered_fields::INIT_NUMBERED_FIELDS), LintId::of(len_zero::COMPARISON_TO_EMPTY), LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(len_zero::LEN_ZERO), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d1c7956a7a5..d4687a1e287 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,13 +1,15 @@ // error-pattern:cargo-clippy +#![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] +#![feature(control_flow_enum)] #![feature(drain_filter)] #![feature(in_band_lifetimes)] +#![feature(iter_intersperse)] +#![feature(let_else)] #![feature(once_cell)] #![feature(rustc_private)] #![feature(stmt_expr_attributes)] -#![feature(control_flow_enum)] -#![feature(let_else)] #![recursion_limit = "512"] #![cfg_attr(feature = "deny-warnings", deny(warnings))] #![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] @@ -242,6 +244,7 @@ mod indexing_slicing; mod infinite_iter; mod inherent_impl; mod inherent_to_string; +mod init_numbered_fields; mod inline_fn_without_body; mod int_plus_one; mod integer_division; @@ -854,6 +857,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes)); store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit)); store.register_late_pass(|| Box::new(return_self_not_must_use::ReturnSelfNotMustUse)); + store.register_late_pass(|| Box::new(init_numbered_fields::NumberedFields)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index 262a26951c6..bbc14fc535d 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::in_macro; use clippy_utils::source::snippet; use hir::def::{DefKind, Res}; use if_chain::if_chain; @@ -8,7 +9,6 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::hygiene::ExpnKind; use rustc_span::{edition::Edition, sym, Span}; declare_clippy_lint! { @@ -214,7 +214,3 @@ impl<'tcx> LateLintPass<'tcx> for MacroUseImports { } } } - -fn in_macro(span: Span) -> bool { - span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) -} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 4af870d3155..a04db7d570e 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -142,6 +142,13 @@ macro_rules! extract_msrv_attr { }; } +/// Returns `true` if the span comes from a macro expansion, no matter if from a +/// macro by example or from a procedural macro +#[must_use] +pub fn in_macro(span: Span) -> bool { + span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) +} + /// Returns `true` if the two spans come from differing expansions (i.e., one is /// from a macro and one isn't). #[must_use] diff --git a/tests/ui/numbered_fields.fixed b/tests/ui/numbered_fields.fixed new file mode 100644 index 00000000000..1da97e96879 --- /dev/null +++ b/tests/ui/numbered_fields.fixed @@ -0,0 +1,33 @@ +//run-rustfix +#![warn(clippy::init_numbered_fields)] + +#[derive(Default)] +struct TupleStruct(u32, u32, u8); + +// This shouldn't lint because it's in a macro +macro_rules! tuple_struct_init { + () => { + TupleStruct { 0: 0, 1: 1, 2: 2 } + }; +} + +fn main() { + let tuple_struct = TupleStruct::default(); + + // This should lint + let _ = TupleStruct(1u32, 42, 23u8); + + // This should also lint and order the fields correctly + let _ = TupleStruct(1u32, 3u32, 2u8); + + // Ok because of default initializer + let _ = TupleStruct { 0: 42, ..tuple_struct }; + + let _ = TupleStruct { + 1: 23, + ..TupleStruct::default() + }; + + // Ok because it's in macro + let _ = tuple_struct_init!(); +} diff --git a/tests/ui/numbered_fields.rs b/tests/ui/numbered_fields.rs new file mode 100644 index 00000000000..08ec405a560 --- /dev/null +++ b/tests/ui/numbered_fields.rs @@ -0,0 +1,41 @@ +//run-rustfix +#![warn(clippy::init_numbered_fields)] + +#[derive(Default)] +struct TupleStruct(u32, u32, u8); + +// This shouldn't lint because it's in a macro +macro_rules! tuple_struct_init { + () => { + TupleStruct { 0: 0, 1: 1, 2: 2 } + }; +} + +fn main() { + let tuple_struct = TupleStruct::default(); + + // This should lint + let _ = TupleStruct { + 0: 1u32, + 1: 42, + 2: 23u8, + }; + + // This should also lint and order the fields correctly + let _ = TupleStruct { + 0: 1u32, + 2: 2u8, + 1: 3u32, + }; + + // Ok because of default initializer + let _ = TupleStruct { 0: 42, ..tuple_struct }; + + let _ = TupleStruct { + 1: 23, + ..TupleStruct::default() + }; + + // Ok because it's in macro + let _ = tuple_struct_init!(); +} diff --git a/tests/ui/numbered_fields.stderr b/tests/ui/numbered_fields.stderr new file mode 100644 index 00000000000..01691c8b141 --- /dev/null +++ b/tests/ui/numbered_fields.stderr @@ -0,0 +1,26 @@ +error: used a field initializer for a tuple struct + --> $DIR/numbered_fields.rs:18:13 + | +LL | let _ = TupleStruct { + | _____________^ +LL | | 0: 1u32, +LL | | 1: 42, +LL | | 2: 23u8, +LL | | }; + | |_____^ help: try this instead: `TupleStruct(1u32, 42, 23u8)` + | + = note: `-D clippy::init-numbered-fields` implied by `-D warnings` + +error: used a field initializer for a tuple struct + --> $DIR/numbered_fields.rs:25:13 + | +LL | let _ = TupleStruct { + | _____________^ +LL | | 0: 1u32, +LL | | 2: 2u8, +LL | | 1: 3u32, +LL | | }; + | |_____^ help: try this instead: `TupleStruct(1u32, 3u32, 2u8)` + +error: aborting due to 2 previous errors +