diff --git a/crates/oxc_linter/src/rules/oxc/no_redundant_constructor_init.rs b/crates/oxc_linter/src/rules/oxc/no_redundant_constructor_init.rs index 9c928c30afcc5..9d713f2a901e8 100644 --- a/crates/oxc_linter/src/rules/oxc/no_redundant_constructor_init.rs +++ b/crates/oxc_linter/src/rules/oxc/no_redundant_constructor_init.rs @@ -1,6 +1,6 @@ use oxc_ast::{ AstKind, - ast::{AssignmentTarget, Expression, MethodDefinitionKind, Statement}, + ast::{AssignmentTarget, Expression, MethodDefinitionKind}, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -58,53 +58,47 @@ impl Rule for NoRedundantConstructorInit { if method.kind != MethodDefinitionKind::Constructor { return; } - let public_members = method - .value - .params - .items - .iter() - .filter_map( - |param| if param.is_public() { param.pattern.get_identifier_name() } else { None }, - ) - .collect::>(); - if public_members.is_empty() { - return; - } - let Some(body) = method.value.body.as_ref() else { - return; - }; - body.statements.iter().for_each(|stmt| { - let Statement::ExpressionStatement(expr_stmt) = stmt else { - return; - }; - let Expression::AssignmentExpression(assignment_expr) = &expr_stmt.expression else { - return; + for param in &method.value.params.items { + if !param.is_public() { + continue; + } + let Some(ident) = param.pattern.get_binding_identifier() else { + continue; }; + for reference in ctx.symbol_references(ident.symbol_id()) { + // check if the param is being read which would indicate an assignment + if !reference.is_read() { + continue; + } - // check for assigning to this: this.x = ? - let AssignmentTarget::StaticMemberExpression(static_member_expr) = - &assignment_expr.left - else { - return; - }; - let Expression::ThisExpression(_this_expr) = &static_member_expr.object else { - return; - }; - let assignment_name = static_member_expr.property.name; + let Some(AstKind::AssignmentExpression(assignment_expr)) = + ctx.nodes().parent_kind(reference.node_id()) + else { + continue; + }; - // check both sides of assignment have the same name: this.x = x - let Expression::Identifier(assignment_target_ident) = &assignment_expr.right else { - return; - }; - if assignment_target_ident.name != assignment_name { - return; - } + // check for assigning to this: this.x = ? + let AssignmentTarget::StaticMemberExpression(static_member_expr) = + &assignment_expr.left + else { + continue; + }; + if !matches!(&static_member_expr.object, Expression::ThisExpression(_)) { + continue; + } + let assignment_name = static_member_expr.property.name; + + // check both sides of assignment have the same name: this.x = x + let Expression::Identifier(assignment_target_ident) = &assignment_expr.right else { + continue; + }; + if assignment_target_ident.name != assignment_name { + continue; + } - // check if this was a public param - if public_members.iter().any(|param| param == &assignment_name) { - ctx.diagnostic(no_redundant_constructor_init_diagnostic(expr_stmt.span)); + ctx.diagnostic(no_redundant_constructor_init_diagnostic(assignment_expr.span)); } - }); + } } } @@ -139,6 +133,29 @@ fn test() { } } ", + r" + class Foo { + constructor(public name: unknown) { + this.name = 'other'; + } + } + ", + r" + class Foo { + constructor(public name: unknown) { + this.name = name + 'edited'; + } + } + ", + r" + class Foo { + constructor(public name: unknown) { + if (maybeTrue) { + this.name = name + 'edited'; + } + } + } + ", ]; let fail = vec![ @@ -157,6 +174,26 @@ fn test() { } } ", + r" + class Foo { + constructor(public name: unknown) { + this.name = name; + this.name = name; + this.name = name; + } + } + ", + r" + class Foo { + constructor(public name: unknown) { + if (maybeTrue) { + this.name = name; + } else { + this.name = name + 'edited'; + } + } + } + ", ]; Tester::new(NoRedundantConstructorInit::NAME, NoRedundantConstructorInit::PLUGIN, pass, fail) diff --git a/crates/oxc_linter/src/snapshots/oxc_no_redundant_constructor_init.snap b/crates/oxc_linter/src/snapshots/oxc_no_redundant_constructor_init.snap index 4a71165b94d1d..397620a551d58 100644 --- a/crates/oxc_linter/src/snapshots/oxc_no_redundant_constructor_init.snap +++ b/crates/oxc_linter/src/snapshots/oxc_no_redundant_constructor_init.snap @@ -5,7 +5,7 @@ source: crates/oxc_linter/src/tester.rs ╭─[no_redundant_constructor_init.tsx:4:13] 3 │ constructor(public name: unknown) { 4 │ this.name = name; - · ───────────────── + · ──────────────── 5 │ } ╰──── help: Remove the explicit initialization @@ -14,7 +14,43 @@ source: crates/oxc_linter/src/tester.rs ╭─[no_redundant_constructor_init.tsx:5:13] 4 │ this.other = other; 5 │ this.name = name; - · ───────────────── + · ──────────────── 6 │ } ╰──── help: Remove the explicit initialization + + ⚠ oxc(no-redundant-constructor-init): Explicit initialization of public members is redundant + ╭─[no_redundant_constructor_init.tsx:4:13] + 3 │ constructor(public name: unknown) { + 4 │ this.name = name; + · ──────────────── + 5 │ this.name = name; + ╰──── + help: Remove the explicit initialization + + ⚠ oxc(no-redundant-constructor-init): Explicit initialization of public members is redundant + ╭─[no_redundant_constructor_init.tsx:5:13] + 4 │ this.name = name; + 5 │ this.name = name; + · ──────────────── + 6 │ this.name = name; + ╰──── + help: Remove the explicit initialization + + ⚠ oxc(no-redundant-constructor-init): Explicit initialization of public members is redundant + ╭─[no_redundant_constructor_init.tsx:6:13] + 5 │ this.name = name; + 6 │ this.name = name; + · ──────────────── + 7 │ } + ╰──── + help: Remove the explicit initialization + + ⚠ oxc(no-redundant-constructor-init): Explicit initialization of public members is redundant + ╭─[no_redundant_constructor_init.tsx:5:15] + 4 │ if (maybeTrue) { + 5 │ this.name = name; + · ──────────────── + 6 │ } else { + ╰──── + help: Remove the explicit initialization