From 91ec91b6cb52294494e3372bc3b9642f3e2cd6eb Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sat, 25 Dec 2021 16:52:58 +0100 Subject: [PATCH] new lint: numbered-fields --- CHANGELOG.md | 1 + 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_lints/src/numbered_fields.rs | 80 ++++++++++++++++++++++++++ clippy_utils/src/lib.rs | 7 +++ tests/ui/numbered_fields.fixed | 30 ++++++++++ tests/ui/numbered_fields.rs | 34 +++++++++++ tests/ui/numbered_fields.stderr | 15 +++++ 11 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 clippy_lints/src/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 c09310e55568..230add2c0cd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3171,6 +3171,7 @@ Released 2018-09-13 [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref +[`numbered_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#numbered_fields [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 3d3999d4cc0d..2e595885fffe 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -221,6 +221,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), + LintId::of(numbered_fields::NUMBERED_FIELDS), LintId::of(octal_escapes::OCTAL_ESCAPES), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 766c5ba1bcb0..ce055142150e 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -382,6 +382,7 @@ store.register_lints(&[ non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS, non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY, nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES, + numbered_fields::NUMBERED_FIELDS, octal_escapes::OCTAL_ESCAPES, open_options::NONSENSICAL_OPEN_OPTIONS, option_env_unwrap::OPTION_ENV_UNWRAP, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index ea87e7e7a736..9b1b39a068db 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -88,6 +88,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST), LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), + LintId::of(numbered_fields::NUMBERED_FIELDS), LintId::of(ptr::CMP_NULL), LintId::of(ptr::PTR_ARG), LintId::of(ptr_eq::PTR_EQ), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d1c7956a7a5c..c90642942142 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)] @@ -312,6 +314,7 @@ mod non_expressive_names; mod non_octal_unix_permissions; mod non_send_fields_in_send_ty; mod nonstandard_macro_braces; +mod numbered_fields; mod octal_escapes; mod open_options; mod option_env_unwrap; @@ -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(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 262a26951c68..bbc14fc535da 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_lints/src/numbered_fields.rs b/clippy_lints/src/numbered_fields.rs new file mode 100644 index 000000000000..4f12d87d5e4f --- /dev/null +++ b/clippy_lints/src/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 NUMBERED_FIELDS, + style, + "numbered fields in tuple struct initializer" +} + +declare_lint_pass!(NumberedFields => [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, + NUMBERED_FIELDS, + e.span, + "used a field initializer for a tuple struct", + "try this instead", + snippet, + appl, + ); + } + } + } +} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 4af870d31554..a04db7d570e4 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 000000000000..dfb99b02213b --- /dev/null +++ b/tests/ui/numbered_fields.fixed @@ -0,0 +1,30 @@ +//run-rustfix +#![warn(clippy::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); + + // 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 000000000000..29ff11346602 --- /dev/null +++ b/tests/ui/numbered_fields.rs @@ -0,0 +1,34 @@ +//run-rustfix +#![warn(clippy::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, + }; + + // 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 000000000000..37d635e14d9f --- /dev/null +++ b/tests/ui/numbered_fields.stderr @@ -0,0 +1,15 @@ +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::numbered-fields` implied by `-D warnings` + +error: aborting due to previous error +