From 240ec241d5ea5aee7e85a474fd07687618f1410d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 4 Apr 2026 10:59:06 +0200 Subject: [PATCH 1/6] Add clippy lint: use_destructuring --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/use_destructuring.rs | 222 ++++++++++++++++++++++++++ tests/ui/use_destructuring.rs | 155 ++++++++++++++++++ tests/ui/use_destructuring.stderr | 60 +++++++ 6 files changed, 441 insertions(+) create mode 100644 clippy_lints/src/use_destructuring.rs create mode 100644 tests/ui/use_destructuring.rs create mode 100644 tests/ui/use_destructuring.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 748e283edffb..c71c63af15cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7276,6 +7276,7 @@ Released 2018-09-13 [`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug +[`use_destructuring`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_destructuring [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding [`used_underscore_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_items diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index c164241673a3..2c46b985761c 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -788,6 +788,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::unwrap::UNNECESSARY_UNWRAP_INFO, crate::unwrap_in_result::UNWRAP_IN_RESULT_INFO, crate::upper_case_acronyms::UPPER_CASE_ACRONYMS_INFO, + crate::use_destructuring::USE_DESTRUCTURING_INFO, crate::use_self::USE_SELF_INFO, crate::useless_concat::USELESS_CONCAT_INFO, crate::useless_conversion::USELESS_CONVERSION_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 61c54678c4b2..a53d887575a6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -390,6 +390,7 @@ mod unused_unit; mod unwrap; mod unwrap_in_result; mod upper_case_acronyms; +mod use_destructuring; mod use_self; mod useless_concat; mod useless_conversion; @@ -865,6 +866,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))), Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))), Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)), + Box::new(|_| Box::new(use_destructuring::UseDestructuring)), Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))), Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))), // add late passes here, used by `cargo dev new_lint` diff --git a/clippy_lints/src/use_destructuring.rs b/clippy_lints/src/use_destructuring.rs new file mode 100644 index 000000000000..7dbc0f00f420 --- /dev/null +++ b/clippy_lints/src/use_destructuring.rs @@ -0,0 +1,222 @@ +use std::ops::ControlFlow; + +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::visitors::for_each_expr_without_closures; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_hir::def::Res; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, ExprKind, FnDecl, HirId, Node, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::def_id::LocalDefId; +use rustc_span::{Span, Symbol}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for functions that access all fields of a struct individually + /// and suggests using destructuring instead. + /// + /// ### Why is this bad? + /// Destructuring makes field usage explicit and ensures the compiler + /// errors when new fields are added to the struct, helping keep code + /// in sync with struct definitions. + /// + /// ### Example + /// ```no_run + /// use std::fmt; + /// + /// struct Point { + /// x: f32, + /// y: f32, + /// } + /// + /// impl fmt::Display for Point { + /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + /// write!(f, "({}, {})", self.x, self.y) + /// } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// use std::fmt; + /// + /// struct Point { + /// x: f32, + /// y: f32, + /// } + /// + /// impl fmt::Display for Point { + /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + /// let Self { x, y } = self; + /// write!(f, "({x}, {y})") + /// } + /// } + /// ``` + #[clippy::version = "1.89.0"] + pub USE_DESTRUCTURING, + pedantic, + "accessing all fields of a struct individually instead of destructuring" +} + +declare_lint_pass!(UseDestructuring => [USE_DESTRUCTURING]); + +/// Tracks field accesses on a single local variable within a function body. +struct LocalInfo<'tcx> { + /// The type of the local after peeling references (i.e. the struct type). + ty: Ty<'tcx>, + + /// The set of field names (or tuple indices) accessed on this local. + accessed_fields: FxHashSet, + + /// Span of the first field access, used for the lint diagnostic. + first_field_access_span: Span, +} + +impl<'tcx> LateLintPass<'tcx> for UseDestructuring { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _kind: FnKind<'tcx>, + _decl: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + _span: Span, + _def_id: LocalDefId, + ) { + let typeck_results = cx.typeck_results(); + + // Map from local HirId -> info about field accesses on that local + let mut locals: FxHashMap> = FxHashMap::default(); + + // Locals that are used in non-field-access contexts (e.g. passed to a function) + let mut non_field_uses: FxHashSet = FxHashSet::default(); + + // Locals that have field mutations (e.g. self.x = 5) + let mut mutated_locals: FxHashSet = FxHashSet::default(); + + for_each_expr_without_closures(body.value, |expr| { + match expr.kind { + ExprKind::Field(base, ident) => { + // Check if the base is a direct local variable reference + if let ExprKind::Path(QPath::Resolved(_, path)) = base.kind + && let Res::Local(local_hir_id) = path.res + && !expr.span.from_expansion() + { + let base_ty = typeck_results.expr_ty_adjusted(base).peel_refs(); + let info = locals.entry(local_hir_id).or_insert_with(|| LocalInfo { + ty: base_ty, + accessed_fields: FxHashSet::default(), + first_field_access_span: expr.span, + }); + info.accessed_fields.insert(ident.name); + + // Check if this field access is on the LHS of an assignment + if let Node::Expr(parent) = cx.tcx.parent_hir_node(expr.hir_id) { + let is_assign_target = matches!( + parent.kind, + ExprKind::Assign(lhs, _, _) + | ExprKind::AssignOp(_, lhs, _) + if lhs.hir_id == expr.hir_id + ); + if is_assign_target { + mutated_locals.insert(local_hir_id); + } + } + } + }, + ExprKind::Path(QPath::Resolved(_, path)) => { + // A direct use of a local variable (not as part of field access) + if let Res::Local(local_hir_id) = path.res { + // Check if the parent expression is a field access on this local. + // If so, this is already handled above — not a "non-field use". + let is_field_base = matches!( + cx.tcx.parent_hir_node(expr.hir_id), + Node::Expr(parent_expr) if matches!(parent_expr.kind, ExprKind::Field(_, _)) + ); + if !is_field_base { + non_field_uses.insert(local_hir_id); + } + } + }, + _ => {}, + } + ControlFlow::::Continue(()) + }); + + #[expect(rustc::potential_query_instability)] // order-independent + for (local_hir_id, info) in &locals { + if non_field_uses.contains(local_hir_id) || mutated_locals.contains(local_hir_id) { + continue; + } + check_local(cx, *local_hir_id, info); + } + } +} + +fn check_local<'tcx>(cx: &LateContext<'tcx>, local_hir_id: HirId, info: &LocalInfo<'tcx>) { + let ty::Adt(adt_def, _) = info.ty.kind() else { + return; + }; + + if !adt_def.is_struct() { + return; + } + + let variant = adt_def.non_enum_variant(); + + if variant.is_field_list_non_exhaustive() { + return; + } + + let struct_fields = &variant.fields; + + if struct_fields.len() < 2 { + return; + } + + // Check that ALL fields are accessed + // TODO(emilk): future improvement is to emit this lint if the _majority_ + // of fields are accessed. This could catch bugs, where the writer forgot about a field! + // But the cutoff for this needs to be configurable in `clippy.toml` + if info.accessed_fields.len() != struct_fields.len() { + return; + } + + if !struct_fields.iter().all(|f| info.accessed_fields.contains(&f.name)) { + return; + } + + // Build the suggestion string + let type_name = cx.tcx.item_name(adt_def.did()); + + let var_name = if let Node::Pat(pat) = cx.tcx.hir_node(local_hir_id) + && let Some(ident) = pat.simple_ident() + { + ident.name.to_string() + } else { + "..".to_string() + }; + + let is_tuple_struct = variant.ctor.is_some(); + let help_msg = if is_tuple_struct { + let binding_names: Vec = (0..struct_fields.len()).map(|i| format!("field_{i}")).collect(); + let bindings_str = binding_names.join(", "); + format!("consider using `let {type_name}({bindings_str}) = {var_name};`") + } else { + let field_names: Vec<&str> = struct_fields.iter().map(|f| f.name.as_str()).collect(); + let fields_str = field_names.join(", "); + format!("consider using `let {type_name} {{ {fields_str} }} = {var_name};`") + }; + + span_lint_and_help( + cx, + USE_DESTRUCTURING, + info.first_field_access_span, + format!( + "all {} fields of `{type_name}` are accessed individually; consider destructuring", + struct_fields.len() + ), + None, + help_msg, + ); +} diff --git a/tests/ui/use_destructuring.rs b/tests/ui/use_destructuring.rs new file mode 100644 index 000000000000..1edc8dc0e2e3 --- /dev/null +++ b/tests/ui/use_destructuring.rs @@ -0,0 +1,155 @@ +#![warn(clippy::use_destructuring)] +#![allow(unused, clippy::no_effect, clippy::needless_pass_by_value)] + +use std::fmt; + +// ============ SHOULD LINT ============ + +struct Vec3 { + x: f32, + y: f32, + z: f32, +} + +impl Vec3 { + fn sum(self) -> f32 { + self.x + self.y + self.z + //~^ use_destructuring + } + + fn sum_ref(&self) -> f32 { + self.x + self.y + self.z + //~^ use_destructuring + } +} + +// Field access passed as arguments to a macro +impl fmt::Display for Vec3 { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "({}, {}, {})", self.x, self.y, self.z) + //~^ use_destructuring + } +} + +struct Pair { + a: i32, + b: i32, +} + +impl fmt::Display for Pair { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "({}, {})", self.a, self.b) + //~^ use_destructuring + } +} + +// All fields accessed on a function parameter +fn add_pair(p: Pair) -> i32 { + p.a + p.b + //~^ use_destructuring +} + +// All fields accessed on a local variable +fn use_local() -> i32 { + let p = Pair { a: 1, b: 2 }; + p.a + p.b + //~^ use_destructuring +} + +// Tuple struct +struct TupleStruct(i32, i32); + +impl TupleStruct { + fn sum(self) -> i32 { + self.0 + self.1 + //~^ use_destructuring + } +} + +// ============ SHOULD NOT LINT ============ + +// --- Only one field (fewer than 2) --- +struct Single { + x: f32, +} + +impl Single { + fn get(self) -> f32 { + self.x + } +} + +// --- Not all fields accessed --- +struct Triple { + a: i32, + b: i32, + c: i32, +} + +impl Triple { + fn partial(self) -> i32 { + self.a + self.b + } +} + +// --- Variable used in non-field-access context --- +fn take_vec3(_v: Vec3) {} + +fn pass_whole(v: Vec3) -> f32 { + let _ = v.x + v.y + v.z; + take_vec3(v); + 0.0 +} + +// --- Field is mutated --- +impl Pair { + fn mutate(&mut self) { + self.a = 1; + self.b = 2; + } +} + +// --- Tuple struct (should still lint) --- + +// --- Non-exhaustive struct --- +#[non_exhaustive] +struct NonExhaustive { + x: i32, + y: i32, +} + +impl NonExhaustive { + fn sum(self) -> i32 { + self.x + self.y + } +} + +// --- Union --- +union MyUnion { + a: i32, + b: f32, +} + +// --- From macro expansion --- +macro_rules! access_fields { + ($v:expr) => { + $v.x + $v.y + $v.z + }; +} + +fn use_macro(v: Vec3) -> f32 { + access_fields!(v) +} + +// --- Variable also returned/moved --- +fn return_whole(v: Vec3) -> Vec3 { + let _ = v.x + v.y + v.z; + v +} + +// --- Only some fields accessed multiple times but not all --- +impl Triple { + fn double_a(self) -> i32 { + self.a + self.a + } +} diff --git a/tests/ui/use_destructuring.stderr b/tests/ui/use_destructuring.stderr new file mode 100644 index 000000000000..0b9238307107 --- /dev/null +++ b/tests/ui/use_destructuring.stderr @@ -0,0 +1,60 @@ +error: all 3 fields of `Vec3` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:16:9 + | +LL | self.x + self.y + self.z + | ^^^^^^ + | + = help: consider using `let Vec3 { x, y, z } = self;` + = note: `-D clippy::use-destructuring` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::use_destructuring)]` + +error: all 3 fields of `Vec3` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:21:9 + | +LL | self.x + self.y + self.z + | ^^^^^^ + | + = help: consider using `let Vec3 { x, y, z } = self;` + +error: all 3 fields of `Vec3` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:29:35 + | +LL | write!(f, "({}, {}, {})", self.x, self.y, self.z) + | ^^^^^^ + | + = help: consider using `let Vec3 { x, y, z } = self;` + +error: all 2 fields of `Pair` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:41:31 + | +LL | write!(f, "({}, {})", self.a, self.b) + | ^^^^^^ + | + = help: consider using `let Pair { a, b } = self;` + +error: all 2 fields of `Pair` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:48:5 + | +LL | p.a + p.b + | ^^^ + | + = help: consider using `let Pair { a, b } = p;` + +error: all 2 fields of `Pair` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:55:5 + | +LL | p.a + p.b + | ^^^ + | + = help: consider using `let Pair { a, b } = p;` + +error: all 2 fields of `TupleStruct` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:64:9 + | +LL | self.0 + self.1 + | ^^^^^^ + | + = help: consider using `let TupleStruct(field_0, field_1) = self;` + +error: aborting due to 7 previous errors + From 0a16fe92be8277089fc7d55943e7c68d161b5d90 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 4 Apr 2026 12:48:09 +0200 Subject: [PATCH 2/6] Add configurable limit --- CHANGELOG.md | 1 + book/src/lint_configuration.md | 10 ++ clippy_config/src/conf.rs | 3 + clippy_lints/src/lib.rs | 2 +- clippy_lints/src/use_destructuring.rs | 151 ++++++++++-------- .../toml_unknown_key/conf_unknown_key.stderr | 3 + tests/ui/use_destructuring.rs | 55 ++++--- tests/ui/use_destructuring.stderr | 34 ++-- 8 files changed, 148 insertions(+), 111 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c71c63af15cb..18d64ab8b27c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7411,6 +7411,7 @@ Released 2018-09-13 [`unnecessary-box-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unnecessary-box-size [`unreadable-literal-lint-fractions`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unreadable-literal-lint-fractions [`upper-case-acronyms-aggressive`]: https://doc.rust-lang.org/clippy/lint_configuration.html#upper-case-acronyms-aggressive +[`use-destructuring-min-fields`]: https://doc.rust-lang.org/clippy/lint_configuration.html#use-destructuring-min-fields [`vec-box-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#vec-box-size-threshold [`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold [`warn-on-all-wildcard-imports`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-on-all-wildcard-imports diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 64d0bf9b62f6..557a60c5d1ed 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -1185,6 +1185,16 @@ Enables verbose mode. Triggers if there is more than one uppercase char next to * [`upper_case_acronyms`](https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms) +## `use-destructuring-min-fields` +The minimum number of struct fields for the `use_destructuring` lint to trigger. + +**Default Value:** `3` + +--- +**Affected lints:** +* [`use_destructuring`](https://rust-lang.github.io/rust-clippy/master/index.html#use_destructuring) + + ## `vec-box-size-threshold` The size of the boxed type in bytes, where boxing in a `Vec` is allowed diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 465e88a783ed..c15499bd0a4a 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -906,6 +906,9 @@ define_Conf! { /// Enables verbose mode. Triggers if there is more than one uppercase char next to each other #[lints(upper_case_acronyms)] upper_case_acronyms_aggressive: bool = false, + /// The minimum number of struct fields for the `use_destructuring` lint to trigger. + #[lints(use_destructuring)] + use_destructuring_min_fields: u64 = 3, /// The size of the boxed type in bytes, where boxing in a `Vec` is allowed #[lints(vec_box)] vec_box_size_threshold: u64 = 4096, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a53d887575a6..1eb4d517669e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -866,7 +866,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))), Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))), Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)), - Box::new(|_| Box::new(use_destructuring::UseDestructuring)), + Box::new(move |_| Box::new(use_destructuring::UseDestructuring::new(conf))), Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))), Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))), // add late passes here, used by `cargo dev new_lint` diff --git a/clippy_lints/src/use_destructuring.rs b/clippy_lints/src/use_destructuring.rs index 7dbc0f00f420..9ff593911512 100644 --- a/clippy_lints/src/use_destructuring.rs +++ b/clippy_lints/src/use_destructuring.rs @@ -1,5 +1,6 @@ use std::ops::ControlFlow; +use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::visitors::for_each_expr_without_closures; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -8,7 +9,7 @@ use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, ExprKind, FnDecl, HirId, Node, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, Ty}; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; use rustc_span::def_id::LocalDefId; use rustc_span::{Span, Symbol}; @@ -22,6 +23,12 @@ declare_clippy_lint! { /// errors when new fields are added to the struct, helping keep code /// in sync with struct definitions. /// + /// ### Configuration + /// This lint has the following configuration variables: + /// + /// - `use-destructuring-min-fields`: The minimum number of struct fields + /// required for the lint to trigger (default: `3`). + /// /// ### Example /// ```no_run /// use std::fmt; @@ -29,11 +36,12 @@ declare_clippy_lint! { /// struct Point { /// x: f32, /// y: f32, + /// z: f32, /// } /// /// impl fmt::Display for Point { /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - /// write!(f, "({}, {})", self.x, self.y) + /// write!(f, "({}, {}, {})", self.x, self.y, self.z) /// } /// } /// ``` @@ -44,12 +52,13 @@ declare_clippy_lint! { /// struct Point { /// x: f32, /// y: f32, + /// z: f32, /// } /// /// impl fmt::Display for Point { /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - /// let Self { x, y } = self; - /// write!(f, "({x}, {y})") + /// let Self { x, y, z } = self; + /// write!(f, "({x}, {y}, {z})") /// } /// } /// ``` @@ -59,7 +68,19 @@ declare_clippy_lint! { "accessing all fields of a struct individually instead of destructuring" } -declare_lint_pass!(UseDestructuring => [USE_DESTRUCTURING]); +impl_lint_pass!(UseDestructuring => [USE_DESTRUCTURING]); + +pub struct UseDestructuring { + min_fields: u64, +} + +impl UseDestructuring { + pub fn new(conf: &'static Conf) -> Self { + Self { + min_fields: conf.use_destructuring_min_fields, + } + } +} /// Tracks field accesses on a single local variable within a function body. struct LocalInfo<'tcx> { @@ -148,75 +169,77 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { if non_field_uses.contains(local_hir_id) || mutated_locals.contains(local_hir_id) { continue; } - check_local(cx, *local_hir_id, info); + self.lint_local(cx, *local_hir_id, info); } } } -fn check_local<'tcx>(cx: &LateContext<'tcx>, local_hir_id: HirId, info: &LocalInfo<'tcx>) { - let ty::Adt(adt_def, _) = info.ty.kind() else { - return; - }; +impl UseDestructuring { + fn lint_local<'tcx>(&self, cx: &LateContext<'tcx>, local_hir_id: HirId, info: &LocalInfo<'tcx>) { + let ty::Adt(adt_def, _) = info.ty.kind() else { + return; + }; - if !adt_def.is_struct() { - return; - } + if !adt_def.is_struct() { + return; + } - let variant = adt_def.non_enum_variant(); + let variant = adt_def.non_enum_variant(); - if variant.is_field_list_non_exhaustive() { - return; - } + if variant.is_field_list_non_exhaustive() { + return; + } - let struct_fields = &variant.fields; + let struct_fields = &variant.fields; - if struct_fields.len() < 2 { - return; - } + if (struct_fields.len() as u64) < self.min_fields { + return; + } - // Check that ALL fields are accessed - // TODO(emilk): future improvement is to emit this lint if the _majority_ - // of fields are accessed. This could catch bugs, where the writer forgot about a field! - // But the cutoff for this needs to be configurable in `clippy.toml` - if info.accessed_fields.len() != struct_fields.len() { - return; - } + // Check that ALL fields are accessed + // TODO(emilk): future improvement is to emit this lint if the _majority_ + // of fields are accessed. This could catch bugs, where the writer forgot about a field! + // But the cutoff for this needs to be configurable in `clippy.toml` + if info.accessed_fields.len() != struct_fields.len() { + return; + } - if !struct_fields.iter().all(|f| info.accessed_fields.contains(&f.name)) { - return; - } + if !struct_fields.iter().all(|f| info.accessed_fields.contains(&f.name)) { + return; + } - // Build the suggestion string - let type_name = cx.tcx.item_name(adt_def.did()); - - let var_name = if let Node::Pat(pat) = cx.tcx.hir_node(local_hir_id) - && let Some(ident) = pat.simple_ident() - { - ident.name.to_string() - } else { - "..".to_string() - }; - - let is_tuple_struct = variant.ctor.is_some(); - let help_msg = if is_tuple_struct { - let binding_names: Vec = (0..struct_fields.len()).map(|i| format!("field_{i}")).collect(); - let bindings_str = binding_names.join(", "); - format!("consider using `let {type_name}({bindings_str}) = {var_name};`") - } else { - let field_names: Vec<&str> = struct_fields.iter().map(|f| f.name.as_str()).collect(); - let fields_str = field_names.join(", "); - format!("consider using `let {type_name} {{ {fields_str} }} = {var_name};`") - }; - - span_lint_and_help( - cx, - USE_DESTRUCTURING, - info.first_field_access_span, - format!( - "all {} fields of `{type_name}` are accessed individually; consider destructuring", - struct_fields.len() - ), - None, - help_msg, - ); + // Build the suggestion string + let type_name = cx.tcx.item_name(adt_def.did()); + + let var_name = if let Node::Pat(pat) = cx.tcx.hir_node(local_hir_id) + && let Some(ident) = pat.simple_ident() + { + ident.name.to_string() + } else { + "..".to_string() + }; + + let is_tuple_struct = variant.ctor.is_some(); + let help_msg = if is_tuple_struct { + let binding_names: Vec = (0..struct_fields.len()).map(|i| format!("field_{i}")).collect(); + let bindings_str = binding_names.join(", "); + format!("consider using `let {type_name}({bindings_str}) = {var_name};`") + } else { + let field_names: Vec<&str> = struct_fields.iter().map(|f| f.name.as_str()).collect(); + let fields_str = field_names.join(", "); + format!("consider using `let {type_name} {{ {fields_str} }} = {var_name};`") + }; + + span_lint_and_help( + cx, + USE_DESTRUCTURING, + info.first_field_access_span, + format!( + "all {} fields of `{type_name}` are accessed individually; consider destructuring", + struct_fields.len() + ), + None, + help_msg, + ); + } } diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 6bb3db8db67f..642517ccf376 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -90,6 +90,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect unnecessary-box-size unreadable-literal-lint-fractions upper-case-acronyms-aggressive + use-destructuring-min-fields vec-box-size-threshold verbose-bit-mask-threshold warn-on-all-wildcard-imports @@ -191,6 +192,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect unnecessary-box-size unreadable-literal-lint-fractions upper-case-acronyms-aggressive + use-destructuring-min-fields vec-box-size-threshold verbose-bit-mask-threshold warn-on-all-wildcard-imports @@ -292,6 +294,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni unnecessary-box-size unreadable-literal-lint-fractions upper-case-acronyms-aggressive + use-destructuring-min-fields vec-box-size-threshold verbose-bit-mask-threshold warn-on-all-wildcard-imports diff --git a/tests/ui/use_destructuring.rs b/tests/ui/use_destructuring.rs index 1edc8dc0e2e3..18e6718f297b 100644 --- a/tests/ui/use_destructuring.rs +++ b/tests/ui/use_destructuring.rs @@ -3,7 +3,7 @@ use std::fmt; -// ============ SHOULD LINT ============ +// ============ SHOULD LINT (3+ fields, default threshold) ============ struct Vec3 { x: f32, @@ -31,44 +31,50 @@ impl fmt::Display for Vec3 { } } -struct Pair { - a: i32, - b: i32, +// All fields accessed on a function parameter +fn sum_vec3(v: Vec3) -> f32 { + v.x + v.y + v.z + //~^ use_destructuring } -impl fmt::Display for Pair { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "({}, {})", self.a, self.b) +// All fields accessed on a local variable +fn use_local() -> f32 { + let v = Vec3 { x: 1.0, y: 2.0, z: 3.0 }; + v.x + v.y + v.z + //~^ use_destructuring +} + +// Tuple struct with 3 fields +struct TupleStruct3(i32, i32, i32); + +impl TupleStruct3 { + fn sum(self) -> i32 { + self.0 + self.1 + self.2 //~^ use_destructuring } } -// All fields accessed on a function parameter -fn add_pair(p: Pair) -> i32 { - p.a + p.b - //~^ use_destructuring +// ============ SHOULD NOT LINT ============ + +// --- Only 2 fields (below default threshold of 3) --- +struct Pair { + a: i32, + b: i32, } -// All fields accessed on a local variable -fn use_local() -> i32 { - let p = Pair { a: 1, b: 2 }; +fn add_pair(p: Pair) -> i32 { p.a + p.b - //~^ use_destructuring } -// Tuple struct -struct TupleStruct(i32, i32); +struct TupleStruct2(i32, i32); -impl TupleStruct { +impl TupleStruct2 { fn sum(self) -> i32 { self.0 + self.1 - //~^ use_destructuring } } -// ============ SHOULD NOT LINT ============ - -// --- Only one field (fewer than 2) --- +// --- Only one field --- struct Single { x: f32, } @@ -109,18 +115,17 @@ impl Pair { } } -// --- Tuple struct (should still lint) --- - // --- Non-exhaustive struct --- #[non_exhaustive] struct NonExhaustive { x: i32, y: i32, + z: i32, } impl NonExhaustive { fn sum(self) -> i32 { - self.x + self.y + self.x + self.y + self.z } } diff --git a/tests/ui/use_destructuring.stderr b/tests/ui/use_destructuring.stderr index 0b9238307107..34b2f4dad6e8 100644 --- a/tests/ui/use_destructuring.stderr +++ b/tests/ui/use_destructuring.stderr @@ -24,37 +24,29 @@ LL | write!(f, "({}, {}, {})", self.x, self.y, self.z) | = help: consider using `let Vec3 { x, y, z } = self;` -error: all 2 fields of `Pair` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:41:31 - | -LL | write!(f, "({}, {})", self.a, self.b) - | ^^^^^^ - | - = help: consider using `let Pair { a, b } = self;` - -error: all 2 fields of `Pair` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:48:5 +error: all 3 fields of `Vec3` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:36:5 | -LL | p.a + p.b +LL | v.x + v.y + v.z | ^^^ | - = help: consider using `let Pair { a, b } = p;` + = help: consider using `let Vec3 { x, y, z } = v;` -error: all 2 fields of `Pair` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:55:5 +error: all 3 fields of `Vec3` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:43:5 | -LL | p.a + p.b +LL | v.x + v.y + v.z | ^^^ | - = help: consider using `let Pair { a, b } = p;` + = help: consider using `let Vec3 { x, y, z } = v;` -error: all 2 fields of `TupleStruct` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:64:9 +error: all 3 fields of `TupleStruct3` are accessed individually; consider destructuring + --> tests/ui/use_destructuring.rs:52:9 | -LL | self.0 + self.1 +LL | self.0 + self.1 + self.2 | ^^^^^^ | - = help: consider using `let TupleStruct(field_0, field_1) = self;` + = help: consider using `let TupleStruct3(field_0, field_1, field_2) = self;` -error: aborting due to 7 previous errors +error: aborting due to 6 previous errors From 5d5c08d5b8766b41e0d2b2f06b643b87312b908a Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 4 Apr 2026 13:06:19 +0200 Subject: [PATCH 3/6] Do not suggest destructuring if a name collision would be the result --- clippy_lints/src/use_destructuring.rs | 40 ++++++++++++++++++++++++--- tests/ui/use_destructuring.rs | 6 ++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/use_destructuring.rs b/clippy_lints/src/use_destructuring.rs index 9ff593911512..c455ed3cb450 100644 --- a/clippy_lints/src/use_destructuring.rs +++ b/clippy_lints/src/use_destructuring.rs @@ -115,6 +115,14 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { // Locals that have field mutations (e.g. self.x = 5) let mut mutated_locals: FxHashSet = FxHashSet::default(); + // All local binding names in the function (for collision detection) + let mut local_names: FxHashMap = FxHashMap::default(); + for param in body.params { + if let Some(ident) = param.pat.simple_ident() { + local_names.insert(param.pat.hir_id, ident.name); + } + } + for_each_expr_without_closures(body.value, |expr| { match expr.kind { ExprKind::Field(base, ident) => { @@ -148,8 +156,14 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { ExprKind::Path(QPath::Resolved(_, path)) => { // A direct use of a local variable (not as part of field access) if let Res::Local(local_hir_id) = path.res { - // Check if the parent expression is a field access on this local. - // If so, this is already handled above — not a "non-field use". + // Record the local's binding name for collision detection + if let Node::Pat(pat) = cx.tcx.hir_node(local_hir_id) + && let Some(ident) = pat.simple_ident() + { + local_names.insert(local_hir_id, ident.name); + } + + // Check if parent is a field access — if not, it's a non-field use let is_field_base = matches!( cx.tcx.parent_hir_node(expr.hir_id), Node::Expr(parent_expr) if matches!(parent_expr.kind, ExprKind::Field(_, _)) @@ -164,18 +178,31 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { ControlFlow::::Continue(()) }); + // Collect all local names except the ones we're considering for destructuring #[expect(rustc::potential_query_instability)] // order-independent for (local_hir_id, info) in &locals { if non_field_uses.contains(local_hir_id) || mutated_locals.contains(local_hir_id) { continue; } - self.lint_local(cx, *local_hir_id, info); + // Names of OTHER locals (not the one being destructured) + let other_names: FxHashSet = local_names + .iter() + .filter(|(id, _)| *id != local_hir_id) + .map(|(_, name)| *name) + .collect(); + self.lint_local(cx, *local_hir_id, info, &other_names); } } } impl UseDestructuring { - fn lint_local<'tcx>(&self, cx: &LateContext<'tcx>, local_hir_id: HirId, info: &LocalInfo<'tcx>) { + fn lint_local<'tcx>( + &self, + cx: &LateContext<'tcx>, + local_hir_id: HirId, + info: &LocalInfo<'tcx>, + local_names: &FxHashSet, + ) { let ty::Adt(adt_def, _) = info.ty.kind() else { return; }; @@ -208,6 +235,11 @@ impl UseDestructuring { return; } + // Skip if any field name would collide with an existing local binding + if struct_fields.iter().any(|f| local_names.contains(&f.name)) { + return; + } + // Build the suggestion string let type_name = cx.tcx.item_name(adt_def.did()); diff --git a/tests/ui/use_destructuring.rs b/tests/ui/use_destructuring.rs index 18e6718f297b..9068e10b87a4 100644 --- a/tests/ui/use_destructuring.rs +++ b/tests/ui/use_destructuring.rs @@ -158,3 +158,9 @@ impl Triple { self.a + self.a } } + +// --- Field name would collide with an existing local binding --- +fn name_collision(v: Vec3) -> f32 { + let x = 42.0_f32; + x + v.x + v.y + v.z +} From c3fdb2da2a9b2a399f520bac98b179f16f3870ab Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 4 Apr 2026 15:44:23 +0200 Subject: [PATCH 4/6] use-destructoring-scope --- CHANGELOG.md | 1 + book/src/lint_configuration.md | 12 +++++ clippy_config/src/conf.rs | 9 +++- clippy_config/src/types.rs | 13 +++++ clippy_lints/src/use_destructuring.rs | 50 ++++++++++++++++--- .../toml_unknown_key/conf_unknown_key.stderr | 3 ++ 6 files changed, 80 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18d64ab8b27c..0616c967dcb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7412,6 +7412,7 @@ Released 2018-09-13 [`unreadable-literal-lint-fractions`]: https://doc.rust-lang.org/clippy/lint_configuration.html#unreadable-literal-lint-fractions [`upper-case-acronyms-aggressive`]: https://doc.rust-lang.org/clippy/lint_configuration.html#upper-case-acronyms-aggressive [`use-destructuring-min-fields`]: https://doc.rust-lang.org/clippy/lint_configuration.html#use-destructuring-min-fields +[`use-destructuring-scope`]: https://doc.rust-lang.org/clippy/lint_configuration.html#use-destructuring-scope [`vec-box-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#vec-box-size-threshold [`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold [`warn-on-all-wildcard-imports`]: https://doc.rust-lang.org/clippy/lint_configuration.html#warn-on-all-wildcard-imports diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 557a60c5d1ed..d909b3c72d3a 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -1195,6 +1195,18 @@ The minimum number of struct fields for the `use_destructuring` lint to trigger. * [`use_destructuring`](https://rust-lang.github.io/rust-clippy/master/index.html#use_destructuring) +## `use-destructuring-scope` +Which struct types the `use_destructuring` lint applies to: +`"self"` (only `Self`), `"crate"` (types from the current crate), +`"workspace"` (types from the current workspace), or `"*"` (all types). + +**Default Value:** `"crate"` + +--- +**Affected lints:** +* [`use_destructuring`](https://rust-lang.github.io/rust-clippy/master/index.html#use_destructuring) + + ## `vec-box-size-threshold` The size of the boxed type in bytes, where boxing in a `Vec` is allowed diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index c15499bd0a4a..6e071fe8a452 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -1,7 +1,7 @@ use crate::ClippyConfiguration; use crate::types::{ - DisallowedPath, DisallowedPathWithoutReplacement, InherentImplLintScope, MacroMatcher, MatchLintBehaviour, - PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering, SourceItemOrderingCategory, + DestructuringScope, DisallowedPath, DisallowedPathWithoutReplacement, InherentImplLintScope, MacroMatcher, + MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering, SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds, SourceItemOrderingWithinModuleItemGroupings, }; @@ -909,6 +909,11 @@ define_Conf! { /// The minimum number of struct fields for the `use_destructuring` lint to trigger. #[lints(use_destructuring)] use_destructuring_min_fields: u64 = 3, + /// Which struct types the `use_destructuring` lint applies to: + /// `"self"` (only `Self`), `"crate"` (types from the current crate), + /// `"workspace"` (types from the current workspace), or `"*"` (all types). + #[lints(use_destructuring)] + use_destructuring_scope: DestructuringScope = DestructuringScope::Crate, /// The size of the boxed type in bytes, where boxing in a `Vec` is allowed #[lints(vec_box)] vec_box_size_threshold: u64 = 4096, diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index 8d9326a904b1..17dc21ccab85 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -699,6 +699,19 @@ pub enum PubUnderscoreFieldsBehaviour { AllPubFields, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum DestructuringScope { + /// Only suggest destructuring for `Self` (i.e. inside an impl block for the type). + #[serde(rename = "self")] + SelfOnly, + /// Only suggest destructuring for types defined in the current crate. + Crate, + /// Suggest destructuring for all types, including external ones. + #[serde(rename = "*")] + All, +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum InherentImplLintScope { diff --git a/clippy_lints/src/use_destructuring.rs b/clippy_lints/src/use_destructuring.rs index c455ed3cb450..ee82fa623056 100644 --- a/clippy_lints/src/use_destructuring.rs +++ b/clippy_lints/src/use_destructuring.rs @@ -1,6 +1,7 @@ use std::ops::ControlFlow; use clippy_config::Conf; +use clippy_config::types::DestructuringScope; use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::visitors::for_each_expr_without_closures; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -28,6 +29,9 @@ declare_clippy_lint! { /// /// - `use-destructuring-min-fields`: The minimum number of struct fields /// required for the lint to trigger (default: `3`). + /// - `use-destructuring-scope`: Which types to lint — `"self"` (only `Self` + /// in impl blocks), `"crate"` (types from the current crate, the default), + /// `"workspace"`, or `"*"` (all types including external). /// /// ### Example /// ```no_run @@ -72,12 +76,14 @@ impl_lint_pass!(UseDestructuring => [USE_DESTRUCTURING]); pub struct UseDestructuring { min_fields: u64, + scope: DestructuringScope, } impl UseDestructuring { pub fn new(conf: &'static Conf) -> Self { Self { min_fields: conf.use_destructuring_min_fields, + scope: conf.use_destructuring_scope, } } } @@ -102,7 +108,7 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { _decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, _span: Span, - _def_id: LocalDefId, + fn_def_id: LocalDefId, ) { let typeck_results = cx.typeck_results(); @@ -190,20 +196,48 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { .filter(|(id, _)| *id != local_hir_id) .map(|(_, name)| *name) .collect(); - self.lint_local(cx, *local_hir_id, info, &other_names); + self.lint_local(cx, *local_hir_id, info, &other_names, fn_def_id); } } } impl UseDestructuring { + /// Check if the struct type is within the configured scope. + fn is_in_scope(&self, cx: &LateContext<'_>, struct_did: rustc_span::def_id::DefId, fn_def_id: LocalDefId) -> bool { + match self.scope { + DestructuringScope::All => true, + DestructuringScope::Crate => struct_did.is_local(), + DestructuringScope::SelfOnly => { + // Check if the struct is the Self type of the enclosing impl block. + // Walk up from the function to find the parent impl. + let parent_did = cx.tcx.local_parent(fn_def_id); + if let Node::Item(item) = cx.tcx.hir_node_by_def_id(parent_did) + && let rustc_hir::ItemKind::Impl(_) = item.kind + { + let self_ty = cx.tcx.type_of(parent_did).skip_binder(); + if let ty::Adt(self_adt, _) = self_ty.kind() { + return self_adt.did() == struct_did; + } + } + false + }, + } + } + fn lint_local<'tcx>( &self, cx: &LateContext<'tcx>, local_hir_id: HirId, info: &LocalInfo<'tcx>, local_names: &FxHashSet, + fn_def_id: LocalDefId, ) { - let ty::Adt(adt_def, _) = info.ty.kind() else { + let LocalInfo { + ty, + accessed_fields, + first_field_access_span, + } = info; + let ty::Adt(adt_def, _) = ty.kind() else { return; }; @@ -211,6 +245,10 @@ impl UseDestructuring { return; } + if !self.is_in_scope(cx, adt_def.did(), fn_def_id) { + return; + } + let variant = adt_def.non_enum_variant(); if variant.is_field_list_non_exhaustive() { @@ -227,11 +265,11 @@ impl UseDestructuring { // TODO(emilk): future improvement is to emit this lint if the _majority_ // of fields are accessed. This could catch bugs, where the writer forgot about a field! // But the cutoff for this needs to be configurable in `clippy.toml` - if info.accessed_fields.len() != struct_fields.len() { + if accessed_fields.len() != struct_fields.len() { return; } - if !struct_fields.iter().all(|f| info.accessed_fields.contains(&f.name)) { + if !struct_fields.iter().all(|f| accessed_fields.contains(&f.name)) { return; } @@ -265,7 +303,7 @@ impl UseDestructuring { span_lint_and_help( cx, USE_DESTRUCTURING, - info.first_field_access_span, + *first_field_access_span, format!( "all {} fields of `{type_name}` are accessed individually; consider destructuring", struct_fields.len() diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 642517ccf376..3206501004dd 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -91,6 +91,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect unreadable-literal-lint-fractions upper-case-acronyms-aggressive use-destructuring-min-fields + use-destructuring-scope vec-box-size-threshold verbose-bit-mask-threshold warn-on-all-wildcard-imports @@ -193,6 +194,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect unreadable-literal-lint-fractions upper-case-acronyms-aggressive use-destructuring-min-fields + use-destructuring-scope vec-box-size-threshold verbose-bit-mask-threshold warn-on-all-wildcard-imports @@ -295,6 +297,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni unreadable-literal-lint-fractions upper-case-acronyms-aggressive use-destructuring-min-fields + use-destructuring-scope vec-box-size-threshold verbose-bit-mask-threshold warn-on-all-wildcard-imports From 5bd6a2ba9a47dd6fc5c2b8a9263bdfe84385ce7c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 4 Apr 2026 16:24:01 +0200 Subject: [PATCH 5/6] Configure `self` or `Self` --- book/src/lint_configuration.md | 6 ++--- clippy_config/src/conf.rs | 6 ++--- clippy_config/src/types.rs | 10 +++++--- clippy_lints/src/use_destructuring.rs | 34 ++++++++++++++++++++------- tests/ui/use_destructuring.rs | 6 ++--- tests/ui/use_destructuring.stderr | 20 ++-------------- 6 files changed, 42 insertions(+), 40 deletions(-) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index d909b3c72d3a..5221b0ceec73 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -1197,10 +1197,10 @@ The minimum number of struct fields for the `use_destructuring` lint to trigger. ## `use-destructuring-scope` Which struct types the `use_destructuring` lint applies to: -`"self"` (only `Self`), `"crate"` (types from the current crate), -`"workspace"` (types from the current workspace), or `"*"` (all types). +`"self"` (only the `self` parameter), `"Self"` (any variable whose type +is `Self`), `"crate"` (types from the current crate), or `"*"` (all types). -**Default Value:** `"crate"` +**Default Value:** `"self"` --- **Affected lints:** diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 6e071fe8a452..8112da550d4d 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -910,10 +910,10 @@ define_Conf! { #[lints(use_destructuring)] use_destructuring_min_fields: u64 = 3, /// Which struct types the `use_destructuring` lint applies to: - /// `"self"` (only `Self`), `"crate"` (types from the current crate), - /// `"workspace"` (types from the current workspace), or `"*"` (all types). + /// `"self"` (only the `self` parameter), `"Self"` (any variable whose type + /// is `Self`), `"crate"` (types from the current crate), or `"*"` (all types). #[lints(use_destructuring)] - use_destructuring_scope: DestructuringScope = DestructuringScope::Crate, + use_destructuring_scope: DestructuringScope = DestructuringScope::SelfBinding, /// The size of the boxed type in bytes, where boxing in a `Vec` is allowed #[lints(vec_box)] vec_box_size_threshold: u64 = 4096, diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index 17dc21ccab85..e56e29761fab 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -702,10 +702,14 @@ pub enum PubUnderscoreFieldsBehaviour { #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(rename_all = "lowercase")] pub enum DestructuringScope { - /// Only suggest destructuring for `Self` (i.e. inside an impl block for the type). + /// Only suggest destructuring for the `self` parameter binding. #[serde(rename = "self")] - SelfOnly, - /// Only suggest destructuring for types defined in the current crate. + SelfBinding, + /// Suggest destructuring for any variable whose type is the `Self` type + /// of the enclosing impl block. + #[serde(rename = "Self")] + SelfType, + /// Suggest destructuring for types defined in the current crate. Crate, /// Suggest destructuring for all types, including external ones. #[serde(rename = "*")] diff --git a/clippy_lints/src/use_destructuring.rs b/clippy_lints/src/use_destructuring.rs index ee82fa623056..461de8e64cb4 100644 --- a/clippy_lints/src/use_destructuring.rs +++ b/clippy_lints/src/use_destructuring.rs @@ -29,9 +29,10 @@ declare_clippy_lint! { /// /// - `use-destructuring-min-fields`: The minimum number of struct fields /// required for the lint to trigger (default: `3`). - /// - `use-destructuring-scope`: Which types to lint — `"self"` (only `Self` - /// in impl blocks), `"crate"` (types from the current crate, the default), - /// `"workspace"`, or `"*"` (all types including external). + /// - `use-destructuring-scope`: Which types to lint — `"self"` (only the + /// `self` parameter), `"Self"` (any variable whose type is `Self` in + /// an impl block), `"crate"` (types from the current crate, the default), + /// or `"*"` (all types including external). Default: `"self"`. /// /// ### Example /// ```no_run @@ -112,7 +113,14 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { ) { let typeck_results = cx.typeck_results(); - // Map from local HirId -> info about field accesses on that local + // HirId of the `self` parameter, if this is a method + let self_hir_id = body + .params + .first() + .and_then(|p| p.pat.simple_ident()) + .filter(|ident| ident.name == rustc_span::symbol::kw::SelfLower) + .map(|_| body.params[0].pat.hir_id); + let mut locals: FxHashMap> = FxHashMap::default(); // Locals that are used in non-field-access contexts (e.g. passed to a function) @@ -196,20 +204,27 @@ impl<'tcx> LateLintPass<'tcx> for UseDestructuring { .filter(|(id, _)| *id != local_hir_id) .map(|(_, name)| *name) .collect(); - self.lint_local(cx, *local_hir_id, info, &other_names, fn_def_id); + let is_self = self_hir_id == Some(*local_hir_id); + self.lint_local(cx, *local_hir_id, info, &other_names, fn_def_id, is_self); } } } impl UseDestructuring { /// Check if the struct type is within the configured scope. - fn is_in_scope(&self, cx: &LateContext<'_>, struct_did: rustc_span::def_id::DefId, fn_def_id: LocalDefId) -> bool { + fn is_in_scope( + &self, + cx: &LateContext<'_>, + struct_did: rustc_span::def_id::DefId, + fn_def_id: LocalDefId, + is_self: bool, + ) -> bool { match self.scope { DestructuringScope::All => true, DestructuringScope::Crate => struct_did.is_local(), - DestructuringScope::SelfOnly => { + DestructuringScope::SelfBinding => is_self, + DestructuringScope::SelfType => { // Check if the struct is the Self type of the enclosing impl block. - // Walk up from the function to find the parent impl. let parent_did = cx.tcx.local_parent(fn_def_id); if let Node::Item(item) = cx.tcx.hir_node_by_def_id(parent_did) && let rustc_hir::ItemKind::Impl(_) = item.kind @@ -231,6 +246,7 @@ impl UseDestructuring { info: &LocalInfo<'tcx>, local_names: &FxHashSet, fn_def_id: LocalDefId, + is_self: bool, ) { let LocalInfo { ty, @@ -245,7 +261,7 @@ impl UseDestructuring { return; } - if !self.is_in_scope(cx, adt_def.did(), fn_def_id) { + if !self.is_in_scope(cx, adt_def.did(), fn_def_id, is_self) { return; } diff --git a/tests/ui/use_destructuring.rs b/tests/ui/use_destructuring.rs index 9068e10b87a4..39b9aaaa0185 100644 --- a/tests/ui/use_destructuring.rs +++ b/tests/ui/use_destructuring.rs @@ -31,17 +31,15 @@ impl fmt::Display for Vec3 { } } -// All fields accessed on a function parameter +// --- Function parameter (not self, so not linted with default scope="self") --- fn sum_vec3(v: Vec3) -> f32 { v.x + v.y + v.z - //~^ use_destructuring } -// All fields accessed on a local variable +// --- Local variable (not self, so not linted with default scope="self") --- fn use_local() -> f32 { let v = Vec3 { x: 1.0, y: 2.0, z: 3.0 }; v.x + v.y + v.z - //~^ use_destructuring } // Tuple struct with 3 fields diff --git a/tests/ui/use_destructuring.stderr b/tests/ui/use_destructuring.stderr index 34b2f4dad6e8..5a38d3406335 100644 --- a/tests/ui/use_destructuring.stderr +++ b/tests/ui/use_destructuring.stderr @@ -24,29 +24,13 @@ LL | write!(f, "({}, {}, {})", self.x, self.y, self.z) | = help: consider using `let Vec3 { x, y, z } = self;` -error: all 3 fields of `Vec3` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:36:5 - | -LL | v.x + v.y + v.z - | ^^^ - | - = help: consider using `let Vec3 { x, y, z } = v;` - -error: all 3 fields of `Vec3` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:43:5 - | -LL | v.x + v.y + v.z - | ^^^ - | - = help: consider using `let Vec3 { x, y, z } = v;` - error: all 3 fields of `TupleStruct3` are accessed individually; consider destructuring - --> tests/ui/use_destructuring.rs:52:9 + --> tests/ui/use_destructuring.rs:50:9 | LL | self.0 + self.1 + self.2 | ^^^^^^ | = help: consider using `let TupleStruct3(field_0, field_1, field_2) = self;` -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors From 12e7dabdb45c2147ad6a340fa94e7d784ca2f9fc Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Sat, 4 Apr 2026 16:43:17 +0200 Subject: [PATCH 6/6] Apply the lint to clippy (dogfood) --- clippy_dev/src/parse.rs | 27 +++++------ clippy_lints/src/excessive_nesting.rs | 7 ++- .../src/loops/while_immutable_condition.rs | 9 ++-- clippy_lints/src/manual_clamp.rs | 7 +-- clippy_lints/src/matches/match_same_arms.rs | 20 +++++---- .../src/methods/bind_instead_of_map.rs | 11 +++-- clippy_lints/src/non_std_lazy_statics.rs | 13 ++++-- clippy_lints/src/only_used_in_recursion.rs | 16 ++++--- clippy_lints/src/operators/mod.rs | 31 +++++++------ clippy_lints/src/pass_by_ref_or_value.rs | 17 ++++--- clippy_lints/src/trait_bounds.rs | 22 ++++++--- clippy_lints/src/vec_init_then_push.rs | 28 ++++++++---- clippy_utils/src/consts.rs | 44 ++++++++++-------- clippy_utils/src/mir/mod.rs | 9 +++- declare_clippy_lint/src/lib.rs | 45 ++++++++++--------- rustc_tools_util/src/lib.rs | 18 +++++--- 16 files changed, 192 insertions(+), 132 deletions(-) diff --git a/clippy_dev/src/parse.rs b/clippy_dev/src/parse.rs index d51fb25552f9..2a2a8ec58ff8 100644 --- a/clippy_dev/src/parse.rs +++ b/clippy_dev/src/parse.rs @@ -294,6 +294,11 @@ impl<'cx> ParseCxImpl<'cx> { Bang, OpenParen, CaptureDocLines, CaptureIdent, CaptureOptLifetimeArg, FatArrow, OpenBracket, ]; + let Self { + str_buf, + arena, + str_list_buf, + } = self; let mut cursor = Cursor::new(contents); let mut captures = [Capture::EMPTY; 3]; while let Some(mac_name) = cursor.find_any_ident() { @@ -304,9 +309,9 @@ impl<'cx> ParseCxImpl<'cx> { assert!( data.lints .insert( - self.str_buf.alloc_ascii_lower(self.arena, cursor.get_text(captures[0])), + str_buf.alloc_ascii_lower(arena, cursor.get_text(captures[0])), Lint::Active(ActiveLint { - group: self.arena.alloc_str(cursor.get_text(captures[1])), + group: arena.alloc_str(cursor.get_text(captures[1])), module, path: path.into(), declaration_range: mac_name.pos..cursor.pos(), @@ -323,21 +328,17 @@ impl<'cx> ParseCxImpl<'cx> { }; let docs = match cursor.get_text(captures[0]) { "" => "", - x => self.arena.alloc_str(x), + x => arena.alloc_str(x), }; - let name = self.arena.alloc_str(cursor.get_text(captures[1])); + let name = arena.alloc_str(cursor.get_text(captures[1])); let lt = cursor.get_text(captures[2]); - let lt = if lt.is_empty() { - None - } else { - Some(self.arena.alloc_str(lt)) - }; + let lt = if lt.is_empty() { None } else { Some(arena.alloc_str(lt)) }; - let lints = self.str_list_buf.with(|buf| { + let lints = str_list_buf.with(|buf| { // Parses a comma separated list of paths and converts each path // to a string with whitespace removed. while !cursor.match_pat(CloseBracket) { - buf.push(self.str_buf.with(|buf| { + buf.push(str_buf.with(|buf| { if cursor.match_pat(DoubleColon) { buf.push_str("::"); } @@ -348,7 +349,7 @@ impl<'cx> ParseCxImpl<'cx> { let capture = cursor.capture_ident()?; buf.push_str(cursor.get_text(capture)); } - Some(self.arena.alloc_str(buf)) + Some(arena.alloc_str(buf)) })?); if !cursor.match_pat(Comma) { @@ -364,7 +365,7 @@ impl<'cx> ParseCxImpl<'cx> { &[] } else { buf.sort_unstable(); - &*self.arena.alloc_slice(buf) + &*arena.alloc_slice(buf) }) }); diff --git a/clippy_lints/src/excessive_nesting.rs b/clippy_lints/src/excessive_nesting.rs index 58d00933a785..114762268a11 100644 --- a/clippy_lints/src/excessive_nesting.rs +++ b/clippy_lints/src/excessive_nesting.rs @@ -125,10 +125,9 @@ struct NestingVisitor<'conf, 'cx> { impl NestingVisitor<'_, '_> { fn check_indent(&mut self, span: Span, id: NodeId) -> bool { - if self.nest_level > self.conf.excessive_nesting_threshold - && !span.in_external_macro(self.cx.sess().source_map()) - { - self.conf.nodes.insert(id); + let Self { nest_level, conf, cx } = self; + if *nest_level > conf.excessive_nesting_threshold && !span.in_external_macro(cx.sess().source_map()) { + conf.nodes.insert(id); return true; } diff --git a/clippy_lints/src/loops/while_immutable_condition.rs b/clippy_lints/src/loops/while_immutable_condition.rs index 7da4fa76e2c7..823efa10e99a 100644 --- a/clippy_lints/src/loops/while_immutable_condition.rs +++ b/clippy_lints/src/loops/while_immutable_condition.rs @@ -83,16 +83,17 @@ struct VarCollectorVisitor<'a, 'tcx> { impl<'tcx> VarCollectorVisitor<'_, 'tcx> { fn insert_def_id(&mut self, ex: &'tcx Expr<'_>) { + let Self { cx, ids, def_ids } = self; if let ExprKind::Path(ref qpath) = ex.kind && let QPath::Resolved(None, _) = *qpath { - match self.cx.qpath_res(qpath, ex.hir_id) { + match cx.qpath_res(qpath, ex.hir_id) { Res::Local(hir_id) => { - self.ids.insert(hir_id); + ids.insert(hir_id); }, Res::Def(DefKind::Static { .. }, def_id) => { - let mutable = self.cx.tcx.is_mutable_static(def_id); - self.def_ids.insert(def_id, mutable); + let mutable = cx.tcx.is_mutable_static(def_id); + def_ids.insert(def_id, mutable); }, _ => {}, } diff --git a/clippy_lints/src/manual_clamp.rs b/clippy_lints/src/manual_clamp.rs index c8fba3e5a7bc..d848c4aea3f1 100644 --- a/clippy_lints/src/manual_clamp.rs +++ b/clippy_lints/src/manual_clamp.rs @@ -595,16 +595,17 @@ impl<'tcx> BinaryOp<'tcx> { } fn flip(&self) -> Self { + let Self { op, left, right } = self; Self { - op: match self.op { + op: match *op { BinOpKind::Le => BinOpKind::Ge, BinOpKind::Lt => BinOpKind::Gt, BinOpKind::Ge => BinOpKind::Le, BinOpKind::Gt => BinOpKind::Lt, other => other, }, - left: self.right, - right: self.left, + left: *right, + right: *left, } } } diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index a8312a04f36f..c33361aa682b 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -211,22 +211,24 @@ struct PatRange { } impl PatRange { fn contains(&self, x: u128) -> bool { - x >= self.start - && match self.bounds { - RangeEnd::Included => x <= self.end, - RangeEnd::Excluded => x < self.end, + let Self { start, end, bounds } = self; + x >= *start + && match bounds { + RangeEnd::Included => x <= *end, + RangeEnd::Excluded => x < *end, } } fn overlaps(&self, other: &Self) -> bool { // Note: Empty ranges are impossible, so this is correct even though it would return true if an // empty exclusive range were to reside within an inclusive range. - (match self.bounds { - RangeEnd::Included => self.end >= other.start, - RangeEnd::Excluded => self.end > other.start, + let Self { start, end, bounds } = *self; + (match bounds { + RangeEnd::Included => end >= other.start, + RangeEnd::Excluded => end > other.start, } && match other.bounds { - RangeEnd::Included => self.start <= other.end, - RangeEnd::Excluded => self.start < other.end, + RangeEnd::Included => start <= other.end, + RangeEnd::Excluded => start < other.end, }) } } diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs index f8520c23ea50..7dded9ebe2b9 100644 --- a/clippy_lints/src/methods/bind_instead_of_map.rs +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -71,14 +71,19 @@ impl BindInsteadOfMap { } fn lint_msg(&self, cx: &LateContext<'_>) -> Option { - let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?; + let Self { + variant_lang_item, + bad_method_name, + good_method_name, + } = self; + let variant_id = cx.tcx.lang_items().get(*variant_lang_item)?; let item_id = cx.tcx.parent(variant_id); Some(format!( "using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`", cx.tcx.item_name(item_id), - self.bad_method_name, + bad_method_name, cx.tcx.item_name(variant_id), - self.good_method_name, + good_method_name, )) } diff --git a/clippy_lints/src/non_std_lazy_statics.rs b/clippy_lints/src/non_std_lazy_statics.rs index 487eaecf1889..085942adaea3 100644 --- a/clippy_lints/src/non_std_lazy_statics.rs +++ b/clippy_lints/src/non_std_lazy_statics.rs @@ -218,12 +218,17 @@ impl LazyInfo { } fn lint(&self, cx: &LateContext<'_>, sugg_map: &FxIndexMap>) { + let Self { + ty_span_no_args, + item_hir_id, + calls_span_and_id, + } = self; // Applicability might get adjusted to `Unspecified` later if any calls // in `calls_span_and_id` are not replaceable judging by the `sugg_map`. let mut app = Applicability::MachineApplicable; - let mut suggs = vec![(self.ty_span_no_args, "std::sync::LazyLock".to_string())]; + let mut suggs = vec![(*ty_span_no_args, "std::sync::LazyLock".to_string())]; - for (span, def_id) in &self.calls_span_and_id { + for (span, def_id) in calls_span_and_id { let maybe_sugg = sugg_map.get(def_id).cloned().flatten(); if let Some(sugg) = maybe_sugg { suggs.push((*span, sugg)); @@ -236,8 +241,8 @@ impl LazyInfo { span_lint_hir_and_then( cx, NON_STD_LAZY_STATICS, - self.item_hir_id, - self.ty_span_no_args, + *item_hir_id, + *ty_span_no_args, "this type has been superseded by `LazyLock` in the standard library", |diag| { diag.multipart_suggestion("use `std::sync::LazyLock` instead", suggs, app); diff --git a/clippy_lints/src/only_used_in_recursion.rs b/clippy_lints/src/only_used_in_recursion.rs index 6c45964da0da..0ae042f7d124 100644 --- a/clippy_lints/src/only_used_in_recursion.rs +++ b/clippy_lints/src/only_used_in_recursion.rs @@ -227,10 +227,11 @@ struct Params { } impl Params { fn insert(&mut self, param: Param, id: HirId) { - let idx = self.params.len(); - self.by_id.insert(id, idx); - self.by_fn.insert((param.fn_id, param.idx), idx); - self.params.push(param); + let Self { params, by_id, by_fn } = self; + let idx = params.len(); + by_id.insert(id, idx); + by_fn.insert((param.fn_id, param.idx), idx); + params.push(param); } fn remove_by_id(&mut self, id: HirId) { @@ -252,9 +253,10 @@ impl Params { } fn clear(&mut self) { - self.params.clear(); - self.by_id.clear(); - self.by_fn.clear(); + let Self { params, by_id, by_fn } = self; + params.clear(); + by_id.clear(); + by_fn.clear(); } /// Sets the `apply_lint` flag on each parameter. diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index b4519d654722..f8db53a89618 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -1012,6 +1012,12 @@ impl Operators { impl<'tcx> LateLintPass<'tcx> for Operators { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + let Self { + arithmetic_context, + verbose_bit_mask_threshold, + modulo_arithmetic_allow_comparison_to_zero, + msrv, + } = self; eq_op::check_assert(cx, e); match e.kind { ExprKind::Binary(op, lhs, rhs) => { @@ -1025,13 +1031,13 @@ impl<'tcx> LateLintPass<'tcx> for Operators { identity_op::check(cx, e, op.node, lhs, rhs); invalid_upcast_comparisons::check(cx, op.node, lhs, rhs, e.span); needless_bitwise_bool::check(cx, e, op.node, lhs, rhs); - manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv); - manual_is_multiple_of::check(cx, e, op.node, lhs, rhs, self.msrv); + manual_midpoint::check(cx, e, op.node, lhs, rhs, *msrv); + manual_is_multiple_of::check(cx, e, op.node, lhs, rhs, *msrv); decimal_bitwise_operands::check(cx, op.node, lhs, rhs); } - self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs); + arithmetic_context.check_binary(cx, e, op.node, lhs, rhs); bit_mask::check(cx, e, op.node, lhs, rhs); - verbose_bit_mask::check(cx, e, op.node, lhs, rhs, self.verbose_bit_mask_threshold); + verbose_bit_mask::check(cx, e, op.node, lhs, rhs, *verbose_bit_mask_threshold); double_comparison::check(cx, op.node, lhs, rhs, e.span); const_comparisons::check(cx, op, lhs, rhs, e.span); duration_subsec::check(cx, e, op.node, lhs, rhs); @@ -1041,34 +1047,27 @@ impl<'tcx> LateLintPass<'tcx> for Operators { cmp_owned::check(cx, e, op.node, lhs, rhs); float_cmp::check(cx, e, op.node, lhs, rhs); modulo_one::check(cx, e, op.node, rhs); - modulo_arithmetic::check( - cx, - e, - op.node, - lhs, - rhs, - self.modulo_arithmetic_allow_comparison_to_zero, - ); - manual_div_ceil::check(cx, e, op.node, lhs, rhs, self.msrv); + modulo_arithmetic::check(cx, e, op.node, lhs, rhs, *modulo_arithmetic_allow_comparison_to_zero); + manual_div_ceil::check(cx, e, op.node, lhs, rhs, *msrv); }, ExprKind::AssignOp(op, lhs, rhs) => { let bin_op = op.node.into(); if !e.span.from_expansion() { decimal_bitwise_operands::check(cx, bin_op, lhs, rhs); } - self.arithmetic_context.check_binary(cx, e, bin_op, lhs, rhs); + arithmetic_context.check_binary(cx, e, bin_op, lhs, rhs); misrefactored_assign_op::check(cx, e, bin_op, lhs, rhs); modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false); }, ExprKind::Assign(lhs, rhs, _) => { - assign_op_pattern::check(cx, e, lhs, rhs, self.msrv); + assign_op_pattern::check(cx, e, lhs, rhs, *msrv); self_assignment::check(cx, e, lhs, rhs); }, ExprKind::Unary(op, arg) => { #[expect(clippy::collapsible_match)] if op == UnOp::Neg { - self.arithmetic_context.check_negate(cx, e, arg); + arithmetic_context.check_negate(cx, e, arg); } }, _ => (), diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs index 6cb57b1d3b36..7cea867ef470 100644 --- a/clippy_lints/src/pass_by_ref_or_value.rs +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -126,7 +126,12 @@ impl PassByRefOrValue { } fn check_poly_fn(&self, cx: &LateContext<'_>, def_id: LocalDefId, decl: &FnDecl<'_>, span: Option) { - if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) { + let Self { + ref_min_size, + value_max_size, + avoid_breaking_exported_api, + } = *self; + if avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) { return; } @@ -170,7 +175,7 @@ impl PassByRefOrValue { let ty = cx.tcx.instantiate_bound_regions_with_erased(fn_sig.rebind(ty)); if is_copy(cx, ty) && let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()) - && size <= self.ref_min_size + && size <= ref_min_size && let hir::TyKind::Ref(_, MutTy { ty: decl_ty, .. }) = input.kind { if let Some(typeck) = cx.maybe_typeck_results() @@ -196,8 +201,7 @@ impl PassByRefOrValue { TRIVIALLY_COPY_PASS_BY_REF, input.span, format!( - "this argument ({size} byte) is passed by reference, but would be more efficient if passed by value (limit: {} byte)", - self.ref_min_size + "this argument ({size} byte) is passed by reference, but would be more efficient if passed by value (limit: {ref_min_size} byte)", ), "consider passing by value instead", value_type, @@ -219,15 +223,14 @@ impl PassByRefOrValue { if is_copy(cx, ty) && !is_self_ty(input) && let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()) - && size > self.value_max_size + && size > value_max_size { span_lint_and_sugg( cx, LARGE_TYPES_PASSED_BY_VALUE, input.span, format!( - "this argument ({size} byte) is passed by value, but might be more efficient if passed by reference (limit: {} byte)", - self.value_max_size + "this argument ({size} byte) is passed by value, but might be more efficient if passed by reference (limit: {value_max_size} byte)", ), "consider passing by reference instead", format!("&{}", snippet(cx, input.span, "_")), diff --git a/clippy_lints/src/trait_bounds.rs b/clippy_lints/src/trait_bounds.rs index e7a785289f39..1842c4e9d0a3 100644 --- a/clippy_lints/src/trait_bounds.rs +++ b/clippy_lints/src/trait_bounds.rs @@ -379,18 +379,28 @@ struct ComparableTraitRef<'a, 'tcx> { impl PartialEq for ComparableTraitRef<'_, '_> { fn eq(&self, other: &Self) -> bool { - SpanlessEq::eq_modifiers(self.modifiers, other.modifiers) - && SpanlessEq::new(self.cx) + let Self { + cx, + trait_ref, + modifiers, + } = *self; + SpanlessEq::eq_modifiers(modifiers, other.modifiers) + && SpanlessEq::new(cx) .paths_by_resolution() - .eq_path(self.trait_ref.path, other.trait_ref.path) + .eq_path(trait_ref.path, other.trait_ref.path) } } impl Eq for ComparableTraitRef<'_, '_> {} impl Hash for ComparableTraitRef<'_, '_> { fn hash(&self, state: &mut H) { - let mut s = SpanlessHash::new(self.cx).paths_by_resolution(); - s.hash_path(self.trait_ref.path); - s.hash_modifiers(self.modifiers); + let Self { + cx, + trait_ref, + modifiers, + } = self; + let mut s = SpanlessHash::new(cx).paths_by_resolution(); + s.hash_path(trait_ref.path); + s.hash_modifiers(*modifiers); state.write_u64(s.finish()); } } diff --git a/clippy_lints/src/vec_init_then_push.rs b/clippy_lints/src/vec_init_then_push.rs index 5d074208c029..5ae95bb12797 100644 --- a/clippy_lints/src/vec_init_then_push.rs +++ b/clippy_lints/src/vec_init_then_push.rs @@ -63,16 +63,26 @@ struct VecPushSearcher { } impl VecPushSearcher { fn display_err(&self, cx: &LateContext<'_>) { - let required_pushes_before_extension = match self.init { - _ if self.found == 0 => return, - VecInitKind::WithConstCapacity(x) if x > self.found => return, + let Self { + local_id, + init, + lhs_is_let, + let_ty_span, + name, + err_span, + found, + last_push_expr, + } = *self; + let required_pushes_before_extension = match init { + _ if found == 0 => return, + VecInitKind::WithConstCapacity(x) if x > found => return, VecInitKind::WithConstCapacity(x) => x, VecInitKind::WithExprCapacity(_) => return, _ => 3, }; let mut needs_mut = false; - let res = for_each_local_use_after_expr(cx, self.local_id, self.last_push_expr, |e| { + let res = for_each_local_use_after_expr(cx, local_id, last_push_expr, |e| { let Some(parent) = get_parent_expr(cx, e) else { return ControlFlow::Continue(()); }; @@ -119,11 +129,11 @@ impl VecPushSearcher { }); // Avoid allocating small `Vec`s when they'll be extended right after. - if res == ControlFlow::Break(true) && self.found <= required_pushes_before_extension { + if res == ControlFlow::Break(true) && found <= required_pushes_before_extension { return; } - let mut s = if self.lhs_is_let { + let mut s = if lhs_is_let { String::from("let ") } else { String::new() @@ -131,8 +141,8 @@ impl VecPushSearcher { if needs_mut { s.push_str("mut "); } - s.push_str(self.name.as_str()); - if let Some(span) = self.let_ty_span { + s.push_str(name.as_str()); + if let Some(span) = let_ty_span { s.push_str(": "); s.push_str(&snippet(cx, span, "_")); } @@ -141,7 +151,7 @@ impl VecPushSearcher { span_lint_and_sugg( cx, VEC_INIT_THEN_PUSH, - self.err_span, + err_span, "calls to `push` immediately after creation", "consider using the `vec![]` macro", s, diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 51db6b6b64c7..2f58cdb57533 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -745,17 +745,24 @@ impl<'tcx> ConstEvalCtxt<'tcx> { /// Lookup a possibly constant expression from an `ExprKind::Path` and apply a function on it. #[expect(clippy::too_many_lines)] fn fetch_path(&self, qpath: &QPath<'_>, id: HirId) -> Option { + let Self { + tcx, + typing_env, + typeck, + source, + ctxt, + } = self; // Resolve the path to a constant and check if that constant is known to // not change based on the target. // // This should be replaced with an attribute at some point. let did = match *qpath { QPath::Resolved(None, path) - if path.span.ctxt() == self.ctxt.get() - && path.segments.iter().all(|s| self.ctxt.get() == s.ident.span.ctxt()) + if path.span.ctxt() == ctxt.get() + && path.segments.iter().all(|s| ctxt.get() == s.ident.span.ctxt()) && let Res::Def(DefKind::Const { .. }, did) = path.res && (matches!( - self.tcx.get_diagnostic_name(did), + tcx.get_diagnostic_name(did), Some( sym::f32_legacy_const_digits | sym::f32_legacy_const_epsilon @@ -807,9 +814,9 @@ impl<'tcx> ConstEvalCtxt<'tcx> { | sym::i64_legacy_const_max | sym::i128_legacy_const_max ) - ) || self.tcx.opt_parent(did).is_some_and(|parent| { + ) || tcx.opt_parent(did).is_some_and(|parent| { matches!( - parent.opt_diag_name(&self.tcx), + parent.opt_diag_name(tcx), Some( sym::f16_consts_mod | sym::f32_consts_mod | sym::f64_consts_mod | sym::f128_consts_mod ) @@ -837,12 +844,12 @@ impl<'tcx> ConstEvalCtxt<'tcx> { | sym::f64 | sym::char ) || (ty_name.ident.name == sym::usize && const_name.ident.name == sym::MIN)) - && const_name.ident.span.ctxt() == self.ctxt.get() - && ty.span.ctxt() == self.ctxt.get() - && ty_name.ident.span.ctxt() == self.ctxt.get() + && const_name.ident.span.ctxt() == ctxt.get() + && ty.span.ctxt() == ctxt.get() + && ty_name.ident.span.ctxt() == ctxt.get() && matches!(ty_path.res, Res::PrimTy(_)) - && let Some((DefKind::AssocConst { .. }, did)) = self.typeck.type_dependent_def(id) - && self.tcx.inherent_impl_of_assoc(did).is_some() => + && let Some((DefKind::AssocConst { .. }, did)) = typeck.type_dependent_def(id) + && tcx.inherent_impl_of_assoc(did).is_some() => { did }, @@ -852,21 +859,20 @@ impl<'tcx> ConstEvalCtxt<'tcx> { _ if let Res::Def( DefKind::Const { is_type_const: false } | DefKind::AssocConst { is_type_const: false }, did, - ) = self.typeck.qpath_res(qpath, id) => + ) = typeck.qpath_res(qpath, id) => { - self.source.set(ConstantSource::NonLocal); + source.set(ConstantSource::NonLocal); did }, _ => return None, }; - self.tcx - .const_eval_resolve( - self.typing_env, - mir::UnevaluatedConst::new(did, self.typeck.node_args(id)), - qpath.span(), - ) - .ok() + tcx.const_eval_resolve( + *typing_env, + mir::UnevaluatedConst::new(did, typeck.node_args(id)), + qpath.span(), + ) + .ok() } fn index(&self, lhs: &'_ Expr<'_>, index: &'_ Expr<'_>) -> Option { diff --git a/clippy_utils/src/mir/mod.rs b/clippy_utils/src/mir/mod.rs index 40c553d1c9e6..2891edbb8cfc 100644 --- a/clippy_utils/src/mir/mod.rs +++ b/clippy_utils/src/mir/mod.rs @@ -66,13 +66,18 @@ struct V<'a, const N: usize> { impl<'tcx, const N: usize> Visitor<'tcx> for V<'_, N> { fn visit_place(&mut self, place: &Place<'tcx>, ctx: PlaceContext, loc: Location) { - if loc.block == self.location.block && loc.statement_index <= self.location.statement_index { + let Self { + locals, + location, + results, + } = self; + if loc.block == location.block && loc.statement_index <= location.statement_index { return; } let local = place.local; - for (self_local, result) in iter::zip(self.locals, &mut self.results) { + for (self_local, result) in iter::zip(*locals, &mut *results) { if local == *self_local { if !matches!( ctx, diff --git a/declare_clippy_lint/src/lib.rs b/declare_clippy_lint/src/lib.rs index f7d9c64bfbd0..58018156cb76 100644 --- a/declare_clippy_lint/src/lib.rs +++ b/declare_clippy_lint/src/lib.rs @@ -47,27 +47,30 @@ impl LintListBuilder { } pub fn register(self, store: &mut LintStore) { - store.register_lints(&self.lints); - store.register_group(true, "clippy::all", Some("clippy_all"), self.all); - store.register_group(true, "clippy::cargo", Some("clippy_cargo"), self.cargo); - store.register_group(true, "clippy::complexity", Some("clippy_complexity"), self.complexity); - store.register_group( - true, - "clippy::correctness", - Some("clippy_correctness"), - self.correctness, - ); - store.register_group(true, "clippy::nursery", Some("clippy_nursery"), self.nursery); - store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), self.pedantic); - store.register_group(true, "clippy::perf", Some("clippy_perf"), self.perf); - store.register_group( - true, - "clippy::restriction", - Some("clippy_restriction"), - self.restriction, - ); - store.register_group(true, "clippy::style", Some("clippy_style"), self.style); - store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), self.suspicious); + let Self { + lints, + all, + cargo, + complexity, + correctness, + nursery, + pedantic, + perf, + restriction, + style, + suspicious, + } = self; + store.register_lints(&lints); + store.register_group(true, "clippy::all", Some("clippy_all"), all); + store.register_group(true, "clippy::cargo", Some("clippy_cargo"), cargo); + store.register_group(true, "clippy::complexity", Some("clippy_complexity"), complexity); + store.register_group(true, "clippy::correctness", Some("clippy_correctness"), correctness); + store.register_group(true, "clippy::nursery", Some("clippy_nursery"), nursery); + store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), pedantic); + store.register_group(true, "clippy::perf", Some("clippy_perf"), perf); + store.register_group(true, "clippy::restriction", Some("clippy_restriction"), restriction); + store.register_group(true, "clippy::style", Some("clippy_style"), style); + store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), suspicious); } } diff --git a/rustc_tools_util/src/lib.rs b/rustc_tools_util/src/lib.rs index 3b7d2d4085d9..fa14dcd84a11 100644 --- a/rustc_tools_util/src/lib.rs +++ b/rustc_tools_util/src/lib.rs @@ -85,18 +85,26 @@ impl std::fmt::Display for VersionInfo { impl std::fmt::Debug for VersionInfo { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let Self { + major, + minor, + patch, + host_compiler, + commit_hash, + commit_date, + crate_name, + } = self; write!( f, - "VersionInfo {{ crate_name: \"{}\", major: {}, minor: {}, patch: {}", - self.crate_name, self.major, self.minor, self.patch, + "VersionInfo {{ crate_name: \"{crate_name}\", major: {major}, minor: {minor}, patch: {patch}", )?; - if let Some(ref commit_hash) = self.commit_hash { + if let Some(commit_hash) = commit_hash { write!(f, ", commit_hash: \"{}\"", commit_hash.trim())?; } - if let Some(ref commit_date) = self.commit_date { + if let Some(commit_date) = commit_date { write!(f, ", commit_date: \"{}\"", commit_date.trim())?; } - if let Some(ref host_compiler) = self.host_compiler { + if let Some(host_compiler) = host_compiler { write!(f, ", host_compiler: \"{}\"", host_compiler.trim())?; }