From 15bc29fc6500d5191ff6db1cc2672e9c20403744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Sun, 21 Jun 2026 13:43:23 +0200 Subject: [PATCH] feat: lints for redundant field names and needless struct updates --- crates/diagnostics/src/lint.rs | 14 ++ .../passes/lints/ast_walk/checks/helpers.rs | 25 +++ .../src/passes/lints/ast_walk/checks/mod.rs | 4 + .../lints/ast_walk/checks/needless_update.rs | 72 +++++++ .../ast_walk/checks/redundant_field_names.rs | 64 ++++++ .../passes/src/passes/lints/ast_walk/mod.rs | 30 +-- tests/ui/lint/mod.rs | 204 +++++++++++++++++- tests/ui/lint/snapshots/needless_update.snap | 12 ++ .../needless_update_enum_variant.snap | 12 ++ .../lint/snapshots/redundant_field_names.snap | 12 ++ .../redundant_field_names_enum_variant.snap | 12 ++ 11 files changed, 446 insertions(+), 15 deletions(-) create mode 100644 crates/passes/src/passes/lints/ast_walk/checks/needless_update.rs create mode 100644 crates/passes/src/passes/lints/ast_walk/checks/redundant_field_names.rs create mode 100644 tests/ui/lint/snapshots/needless_update.snap create mode 100644 tests/ui/lint/snapshots/needless_update_enum_variant.snap create mode 100644 tests/ui/lint/snapshots/redundant_field_names.snap create mode 100644 tests/ui/lint/snapshots/redundant_field_names_enum_variant.snap diff --git a/crates/diagnostics/src/lint.rs b/crates/diagnostics/src/lint.rs index 65a0673b..03cd7ef0 100644 --- a/crates/diagnostics/src/lint.rs +++ b/crates/diagnostics/src/lint.rs @@ -241,6 +241,20 @@ pub fn replaceable_with_zero_fill(span: &Span, kept: &str, struct_name: &str) -> )) } +pub fn redundant_field_names(span: &Span, name: &str) -> LisetteDiagnostic { + LisetteDiagnostic::info("Redundant field name") + .with_lint_code("redundant_field_names") + .with_span_label(span, "can be simpler") + .with_help(format!("Replace `{name}: {name}` with `{name}`")) +} + +pub fn needless_update(span: &Span, base: &str) -> LisetteDiagnostic { + LisetteDiagnostic::info("Needless struct update") + .with_lint_code("needless_update") + .with_span_label(span, "has no effect") + .with_help(format!("Remove `..{base}`")) +} + pub fn double_negation(span: &Span, is_bool: bool) -> LisetteDiagnostic { let (code, msg) = if is_bool { ("double_bool_negation", "Double boolean negation") diff --git a/crates/passes/src/passes/lints/ast_walk/checks/helpers.rs b/crates/passes/src/passes/lints/ast_walk/checks/helpers.rs index 8cebb655..432a7f4c 100644 --- a/crates/passes/src/passes/lints/ast_walk/checks/helpers.rs +++ b/crates/passes/src/passes/lints/ast_walk/checks/helpers.rs @@ -1,6 +1,9 @@ +use ecow::EcoString; +use rustc_hash::FxHashSet as HashSet; use syntax::ast::{ BinaryOperator, Expression, FormatStringPart, Literal, Pattern, Span, UnaryOperator, }; +use syntax::program::DefinitionBody; use syntax::types::{SimpleKind, Type, unqualified_name}; use crate::passes::walk::visit_ast; @@ -10,6 +13,28 @@ pub(super) use crate::passes::comparison::{ expressions_equivalent, flip_comparison, is_side_effect_free, signed_integer_literal, }; +pub(super) fn struct_field_names( + store: &Store, + ty: &Type, + name: &str, +) -> Option> { + let Type::Nominal { id, .. } = ty.strip_refs() else { + return None; + }; + let def = store.get_definition(id.as_str())?; + match &def.body { + DefinitionBody::Struct { fields, .. } => { + Some(fields.iter().map(|f| f.name.clone()).collect()) + } + DefinitionBody::Enum { variants, .. } => { + let variant_name = unqualified_name(name); + let variant = variants.iter().find(|v| v.name == variant_name)?; + Some(variant.fields.iter().map(|f| f.name.clone()).collect()) + } + _ => None, + } +} + pub(super) fn is_float_operand(store: &Store, expression: &Expression) -> bool { let resolved = store.deep_resolve_alias(&expression.get_type()); if let Some(kind) = resolved.underlying_simple_kind() { diff --git a/crates/passes/src/passes/lints/ast_walk/checks/mod.rs b/crates/passes/src/passes/lints/ast_walk/checks/mod.rs index 503e0085..4e20d755 100644 --- a/crates/passes/src/passes/lints/ast_walk/checks/mod.rs +++ b/crates/passes/src/passes/lints/ast_walk/checks/mod.rs @@ -58,6 +58,7 @@ mod naming; mod needless_bool_assign; mod needless_match; mod needless_question_mark; +mod needless_update; mod neg_multiply; mod negated_equality; mod negated_logical_operand; @@ -68,6 +69,7 @@ mod redundant_closure; mod redundant_closure_call; mod redundant_comparison; mod redundant_else; +mod redundant_field_names; mod redundant_guards; mod redundant_operation; mod redundant_pattern_matching; @@ -158,6 +160,7 @@ pub use naming::{check_expression_naming, check_pattern_naming}; pub use needless_bool_assign::check_needless_bool_assign; pub use needless_match::check_needless_match; pub use needless_question_mark::check_needless_question_mark; +pub use needless_update::check_needless_update; pub use neg_multiply::check_neg_multiply; pub use negated_equality::check_negated_equality; pub use negated_logical_operand::check_negated_logical_operand; @@ -168,6 +171,7 @@ pub use redundant_closure::check_redundant_closure; pub use redundant_closure_call::check_redundant_closure_call; pub use redundant_comparison::check_redundant_comparison; pub use redundant_else::check_redundant_else; +pub use redundant_field_names::check_redundant_field_names; pub use redundant_guards::check_redundant_guards; pub use redundant_operation::check_redundant_operation; pub use redundant_pattern_matching::check_redundant_pattern_matching; diff --git a/crates/passes/src/passes/lints/ast_walk/checks/needless_update.rs b/crates/passes/src/passes/lints/ast_walk/checks/needless_update.rs new file mode 100644 index 00000000..f1789fd3 --- /dev/null +++ b/crates/passes/src/passes/lints/ast_walk/checks/needless_update.rs @@ -0,0 +1,72 @@ +use rustc_hash::FxHashSet as HashSet; +use syntax::ast::{Expression, Span, StructSpread}; +use syntax::types::Type; + +use super::helpers::{is_side_effect_free, struct_field_names}; +use crate::passes::walk::NodeCtx; + +pub fn check_needless_update(expression: &Expression, ctx: &NodeCtx) { + let Expression::StructCall { + name, + field_assignments, + spread, + ty, + .. + } = expression + else { + return; + }; + + let StructSpread::From(base) = spread else { + return; + }; + if !is_side_effect_free(base) { + return; + } + if is_go_imported(ty) { + return; + } + if !same_named_type(&base.get_type(), ty) { + return; + } + + let Some(fields) = struct_field_names(ctx.store, ty, name) else { + return; + }; + let assigned: HashSet<&str> = field_assignments.iter().map(|f| f.name.as_str()).collect(); + if !fields.iter().all(|f| assigned.contains(f.as_str())) { + return; + } + + let base_span = base.get_span(); + let span = spread_span(ctx.source, base_span); + let base_text = ctx + .source + .get(base_span.byte_offset as usize..base_span.end() as usize) + .unwrap_or(""); + ctx.sink + .push(diagnostics::lint::needless_update(&span, base_text)); +} + +fn is_go_imported(ty: &Type) -> bool { + matches!(ty.strip_refs(), Type::Nominal { id, .. } if id.as_str().starts_with("go:")) +} + +fn same_named_type(a: &Type, b: &Type) -> bool { + matches!( + (a.strip_refs(), b.strip_refs()), + ( + Type::Nominal { id: ai, params: ap, .. }, + Type::Nominal { id: bi, params: bp, .. }, + ) if ai == bi && ap == bp + ) +} + +fn spread_span(source: &str, base_span: Span) -> Span { + let start = source + .get(..base_span.byte_offset as usize) + .and_then(|prefix| prefix.rfind("..")) + .map(|pos| pos as u32) + .unwrap_or(base_span.byte_offset); + Span::new(base_span.file_id, start, base_span.end() - start) +} diff --git a/crates/passes/src/passes/lints/ast_walk/checks/redundant_field_names.rs b/crates/passes/src/passes/lints/ast_walk/checks/redundant_field_names.rs new file mode 100644 index 00000000..2824889c --- /dev/null +++ b/crates/passes/src/passes/lints/ast_walk/checks/redundant_field_names.rs @@ -0,0 +1,64 @@ +use syntax::ast::{Expression, Span, StructFieldAssignment}; + +use super::helpers::struct_field_names; +use crate::passes::walk::NodeCtx; + +pub fn check_redundant_field_names(expression: &Expression, ctx: &NodeCtx) { + let Expression::StructCall { + name, + field_assignments, + ty, + .. + } = expression + else { + return; + }; + + if !field_assignments + .iter() + .any(|a| redundant_field(a).is_some()) + { + return; + } + + let Some(fields) = struct_field_names(ctx.store, ty, name) else { + return; + }; + + for assignment in field_assignments { + if !fields.contains(&assignment.name) { + continue; + } + let Some(value_span) = redundant_field(assignment) else { + continue; + }; + let span = assignment.name_span.merge(value_span); + ctx.sink.push(diagnostics::lint::redundant_field_names( + &span, + &assignment.name, + )); + } +} + +fn redundant_field(assignment: &StructFieldAssignment) -> Option { + let Expression::Identifier { + value, + span, + binding_id, + qualified, + .. + } = assignment.value.as_ref() + else { + return None; + }; + if value != &assignment.name { + return None; + } + if binding_id.is_none() && qualified.is_none() { + return None; + } + if span.byte_offset == assignment.name_span.byte_offset { + return None; + } + Some(*span) +} diff --git a/crates/passes/src/passes/lints/ast_walk/mod.rs b/crates/passes/src/passes/lints/ast_walk/mod.rs index 5a240f34..76f1f527 100644 --- a/crates/passes/src/passes/lints/ast_walk/mod.rs +++ b/crates/passes/src/passes/lints/ast_walk/mod.rs @@ -37,21 +37,21 @@ use checks::{ check_map_unwrap_or, check_match_as_if_let, check_match_literal_collection, check_match_on_bool, check_match_same_arms, check_match_single_binding, check_misrefactored_assign_op, check_needless_bool_assign, check_needless_match, - check_needless_question_mark, check_neg_multiply, check_negated_equality, - check_negated_logical_operand, check_non_negative_comparison, check_or_fn_call, - check_out_of_domain_value, check_pattern_naming, check_redundant_closure, + check_needless_question_mark, check_needless_update, check_neg_multiply, + check_negated_equality, check_negated_logical_operand, check_non_negative_comparison, + check_or_fn_call, check_out_of_domain_value, check_pattern_naming, check_redundant_closure, check_redundant_closure_call, check_redundant_comparison, check_redundant_else, - check_redundant_guards, check_redundant_operation, check_redundant_pattern_matching, - check_redundant_rebinding, check_redundant_slice_bounds, check_redundant_sprintf, - check_regexp_in_loop, check_replaceable_with_zero_fill, check_rest_only_slice_pattern, - check_self_assignment, check_self_comparison, check_single_arm_select, - check_single_element_loop, check_type_limit_comparison, check_uninterpolated_fstring, - check_unnecessary_bool, check_unnecessary_first_then_check, check_unnecessary_lazy_evaluations, - check_unnecessary_map_on_constructor, check_unnecessary_min_or_max, - check_unnecessary_range_loop, check_unnecessary_raw_string_expression, - check_unnecessary_raw_string_pattern, check_unnecessary_return, check_unsigned_comparison, - check_verbose_failure_propagation, check_waitgroup_add_in_task, check_while_let_loop, - check_wildcard_in_or_patterns, + check_redundant_field_names, check_redundant_guards, check_redundant_operation, + check_redundant_pattern_matching, check_redundant_rebinding, check_redundant_slice_bounds, + check_redundant_sprintf, check_regexp_in_loop, check_replaceable_with_zero_fill, + check_rest_only_slice_pattern, check_self_assignment, check_self_comparison, + check_single_arm_select, check_single_element_loop, check_type_limit_comparison, + check_uninterpolated_fstring, check_unnecessary_bool, check_unnecessary_first_then_check, + check_unnecessary_lazy_evaluations, check_unnecessary_map_on_constructor, + check_unnecessary_min_or_max, check_unnecessary_range_loop, + check_unnecessary_raw_string_expression, check_unnecessary_raw_string_pattern, + check_unnecessary_return, check_unsigned_comparison, check_verbose_failure_propagation, + check_waitgroup_add_in_task, check_while_let_loop, check_wildcard_in_or_patterns, }; static LINT_CHECKS: LazyLock = LazyLock::new(|| { @@ -156,6 +156,8 @@ static LINT_CHECKS: LazyLock = LazyLock::new(|| { &[Struct, Enum, TypeAlias, Interface, Function], ), (check_replaceable_with_zero_fill, &[StructCall]), + (check_redundant_field_names, &[StructCall]), + (check_needless_update, &[StructCall]), (check_lost_query_mutation, &[Call]), (check_lost_cancel, &[Let]), (check_redundant_rebinding, &[Let]), diff --git a/tests/ui/lint/mod.rs b/tests/ui/lint/mod.rs index 0e3c5a22..cec8a09d 100644 --- a/tests/ui/lint/mod.rs +++ b/tests/ui/lint/mod.rs @@ -7554,7 +7554,7 @@ struct Vec2 { x: int, y: int } impl Vec2 { fn new(x: int, y: int) -> Vec2 { - Vec2 { x: x, y: y } + Vec2 { x, y } } fn length_squared(self: Vec2) -> int { @@ -18910,3 +18910,205 @@ fn main() { "# ); } + +#[test] +fn redundant_field_names() { + assert_lint_snapshot!( + r#" +struct Point { x: int, y: int } + +fn read(p: Point) -> int { p.x + p.y } + +fn make(x: int) -> Point { + Point { x: x, y: 0 } +} + +fn main() { + let _ = read(make(5)) +} +"# + ); +} + +#[test] +fn redundant_field_names_enum_variant() { + assert_lint_snapshot!( + r#" +enum Action { + Move { x: int, y: int }, + Stop, +} + +fn build(x: int) -> Action { + Action.Move { x: x, y: 0 } +} + +fn main() { + let _ = match build(5) { + Action.Move { x, y } => x + y, + Action.Stop => 0, + } +} +"# + ); +} + +#[test] +fn redundant_field_names_shorthand_no_warning() { + assert_no_lint_warnings!( + r#" +struct Point { x: int, y: int } + +fn read(p: Point) -> int { p.x + p.y } + +fn make(x: int, y: int) -> Point { + Point { x, y } +} + +fn main() { + let _ = read(make(1, 2)) +} +"# + ); +} + +#[test] +fn redundant_field_names_different_name_no_warning() { + assert_no_lint_warnings!( + r#" +struct Point { x: int, y: int } + +fn read(p: Point) -> int { p.x + p.y } + +fn make(a: int, b: int) -> Point { + Point { x: a, y: b } +} + +fn main() { + let _ = read(make(1, 2)) +} +"# + ); +} + +#[test] +fn redundant_field_names_parenthesized_no_warning() { + assert_no_lint_warnings!( + r#" +struct Point { x: int, y: int } + +fn read(p: Point) -> int { p.x + p.y } + +fn make(x: int) -> Point { + Point { x: (x), y: 0 } +} + +fn main() { + let _ = read(make(5)) +} +"# + ); +} + +#[test] +fn needless_update() { + assert_lint_snapshot!( + r#" +struct Config { debug: bool, port: int } + +fn read(c: Config) -> int { if c.debug { c.port } else { 0 } } + +fn rebuild(base: Config) -> Config { + Config { debug: true, port: 80, ..base } +} + +fn main() { + let _ = read(rebuild(Config { debug: false, port: 1 })) +} +"# + ); +} + +#[test] +fn needless_update_enum_variant() { + assert_lint_snapshot!( + r#" +enum Shape { + Rect { w: int, h: int }, + Dot, +} + +fn resize(base: Shape) -> Shape { + Shape.Rect { w: 5, h: 6, ..base } +} + +fn main() { + let _ = match resize(Shape.Rect { w: 1, h: 2 }) { + Shape.Rect { w, h } => w + h, + Shape.Dot => 0, + } +} +"# + ); +} + +#[test] +fn needless_update_partial_spread_no_warning() { + assert_no_lint_warnings!( + r#" +struct Config { debug: bool, port: int } + +fn read(c: Config) -> int { if c.debug { c.port } else { 0 } } + +fn rebuild(base: Config) -> Config { + Config { debug: true, ..base } +} + +fn main() { + let _ = read(rebuild(Config { debug: false, port: 1 })) +} +"# + ); +} + +#[test] +fn needless_update_side_effecting_base_no_warning() { + assert_no_lint_warnings!( + r#" +struct Config { debug: bool, port: int } + +fn read(c: Config) -> int { if c.debug { c.port } else { 0 } } + +fn fresh() -> Config { + Config { debug: false, port: 1 } +} + +fn rebuild() -> Config { + Config { debug: true, port: 80, ..fresh() } +} + +fn main() { + let _ = read(rebuild()) +} +"# + ); +} + +#[test] +fn needless_update_zero_fill_no_warning() { + assert_no_lint_warnings!( + r#" +struct Config { debug: bool, port: int } + +fn read(c: Config) -> int { if c.debug { c.port } else { 0 } } + +fn rebuild() -> Config { + Config { debug: true, port: 80, .. } +} + +fn main() { + let _ = read(rebuild()) +} +"# + ); +} diff --git a/tests/ui/lint/snapshots/needless_update.snap b/tests/ui/lint/snapshots/needless_update.snap new file mode 100644 index 00000000..9af3c975 --- /dev/null +++ b/tests/ui/lint/snapshots/needless_update.snap @@ -0,0 +1,12 @@ +--- +source: tests/ui/lint/mod.rs +--- + [info] Needless struct update + ╭─[test.lis:7:35] + 6 │ fn rebuild(base: Config) -> Config { + 7 │ Config { debug: true, port: 80, ..base } + · ───┬── + · ╰── has no effect + 8 │ } + ╰──── + help: Remove `..base` · code: [lint.needless_update] diff --git a/tests/ui/lint/snapshots/needless_update_enum_variant.snap b/tests/ui/lint/snapshots/needless_update_enum_variant.snap new file mode 100644 index 00000000..566c7a4b --- /dev/null +++ b/tests/ui/lint/snapshots/needless_update_enum_variant.snap @@ -0,0 +1,12 @@ +--- +source: tests/ui/lint/mod.rs +--- + [info] Needless struct update + ╭─[test.lis:8:28] + 7 │ fn resize(base: Shape) -> Shape { + 8 │ Shape.Rect { w: 5, h: 6, ..base } + · ───┬── + · ╰── has no effect + 9 │ } + ╰──── + help: Remove `..base` · code: [lint.needless_update] diff --git a/tests/ui/lint/snapshots/redundant_field_names.snap b/tests/ui/lint/snapshots/redundant_field_names.snap new file mode 100644 index 00000000..1afc0414 --- /dev/null +++ b/tests/ui/lint/snapshots/redundant_field_names.snap @@ -0,0 +1,12 @@ +--- +source: tests/ui/lint/mod.rs +--- + [info] Redundant field name + ╭─[test.lis:7:11] + 6 │ fn make(x: int) -> Point { + 7 │ Point { x: x, y: 0 } + · ──┬─ + · ╰── can be simpler + 8 │ } + ╰──── + help: Replace `x: x` with `x` · code: [lint.redundant_field_names] diff --git a/tests/ui/lint/snapshots/redundant_field_names_enum_variant.snap b/tests/ui/lint/snapshots/redundant_field_names_enum_variant.snap new file mode 100644 index 00000000..b06d3fa0 --- /dev/null +++ b/tests/ui/lint/snapshots/redundant_field_names_enum_variant.snap @@ -0,0 +1,12 @@ +--- +source: tests/ui/lint/mod.rs +--- + [info] Redundant field name + ╭─[test.lis:8:17] + 7 │ fn build(x: int) -> Action { + 8 │ Action.Move { x: x, y: 0 } + · ──┬─ + · ╰── can be simpler + 9 │ } + ╰──── + help: Replace `x: x` with `x` · code: [lint.redundant_field_names]