Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 80 additions & 43 deletions crates/oxc_linter/src/rules/oxc/no_redundant_constructor_init.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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::<Vec<_>>();
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));
}
});
}
}
}

Expand Down Expand Up @@ -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![
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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