diff --git a/crates/oxc_linter/src/rules/eslint/no_useless_constructor.rs b/crates/oxc_linter/src/rules/eslint/no_useless_constructor.rs index 98b639bfb053c..7e29a739f6ed5 100644 --- a/crates/oxc_linter/src/rules/eslint/no_useless_constructor.rs +++ b/crates/oxc_linter/src/rules/eslint/no_useless_constructor.rs @@ -1,7 +1,7 @@ use oxc_ast::{ AstKind, ast::{ - Argument, BindingPattern, CallExpression, Expression, FormalParameter, FormalParameterRest, + Argument, BindingPattern, CallExpression, Expression, FormalParameterRest, FormalParameters, FunctionBody, MethodDefinition, Statement, TSAccessibility, }, }; @@ -112,19 +112,23 @@ impl Rule for NoUselessConstructor { let Some(body) = &constructor.value.body else { return; }; - // allow `private private constructor() {}`. However, `public private - // constructor() {}` is the same as `constructor() {}` and so is not allowed. - if constructor.accessibility.is_some_and(|access| { - matches!(access, TSAccessibility::Private | TSAccessibility::Protected) - }) { - return; - } let class = ctx.nodes().ancestors(node.id()).find_map(|parent| parent.kind().as_class()); debug_assert!(class.is_some(), "Found a constructor outside of a class definition"); let Some(class) = class else { return; }; + + match &constructor.accessibility { + Some(TSAccessibility::Private | TSAccessibility::Protected) => { + return; + } + Some(TSAccessibility::Public) if class.super_class.is_some() => { + return; + } + _ => {} + } + if class.declare { return; } @@ -149,7 +153,13 @@ fn lint_empty_constructor<'a>( // allow constructors with parameter properties since they actually declare // class members - if constructor.value.params.items.iter().any(FormalParameter::has_modifier) { + if constructor + .value + .params + .items + .iter() + .any(|param| param.has_modifier() || !param.decorators.is_empty()) + { return; } @@ -172,6 +182,7 @@ fn lint_redundant_super_call<'a>( if is_only_simple_params(params) && !is_overriding(params) + && !has_decorated_params(params) && (is_spread_arguments(super_args) || is_passing_through(params, super_args)) { ctx.diagnostic_with_fix( @@ -185,6 +196,10 @@ fn is_overriding(params: &FormalParameters) -> bool { params.items.iter().any(|param| param.r#override) } +fn has_decorated_params(params: &FormalParameters) -> bool { + params.items.iter().any(|param| !param.decorators.is_empty()) +} + /// Check if a function body only contains a single `super()` call. Ignores directives. /// /// Returns the call expression if the body contains a single `super()` call, otherwise [`None`]. @@ -324,6 +339,205 @@ fn test() { // ", ]; + let pass_typescript = vec![ + "class A {}", + " + class A { + constructor() { + doSomething(); + } + } + ", + " + class A extends B { + constructor() {} + } + ", + " + class A extends B { + constructor() { + super('foo'); + } + } + ", + " + class A extends B { + constructor(foo, bar) { + super(foo, bar, 1); + } + } + ", + " + class A extends B { + constructor() { + super(); + doSomething(); + } + } + ", + " + class A extends B { + constructor(...args) { + super(...args); + doSomething(); + } + } + ", + " + class A { + dummyMethod() { + doSomething(); + } + } + ", + " + class A extends B.C { + constructor() { + super(foo); + } + } + ", + " + class A extends B.C { + constructor([a, b, c]) { + super(...arguments); + } + } + ", + " + class A extends B.C { + constructor(a = f()) { + super(...arguments); + } + } + ", + " + class A extends B { + constructor(a, b, c) { + super(a, b); + } + } + ", + " + class A extends B { + constructor(foo, bar) { + super(foo); + } + } + ", + " + class A extends B { + constructor(test) { + super(); + } + } + ", + " + class A extends B { + constructor() { + foo; + } + } + ", + " + class A extends B { + constructor(foo, bar) { + super(bar); + } + } + ", + " + declare class A { + constructor(); + } + ", + " + class A { + constructor(); + } + ", + " + abstract class A { + constructor(); + } + ", + " + class A { + constructor(private name: string) {} + } + ", + " + class A { + constructor(public name: string) {} + } + ", + " + class A { + constructor(protected name: string) {} + } + ", + " + class A { + private constructor() {} + } + ", + " + class A { + protected constructor() {} + } + ", + " + class A extends B { + public constructor() {} + } + ", + " + class A extends B { + protected constructor(foo, bar) { + super(bar); + } + } + ", + " + class A extends B { + private constructor(foo, bar) { + super(bar); + } + } + ", + " + class A extends B { + public constructor(foo) { + super(foo); + } + } + ", + " + class A extends B { + public constructor(foo) {} + } + ", + " + class A { + constructor(foo); + } + ", + " + class A extends Object { + constructor(@Foo foo: string) { + super(foo); + } + } + ", + " + class A extends Object { + constructor(foo: string, @Bar() bar) { + super(foo, bar); + } + } + ", + ]; + let fail = vec![ "class A { constructor(){} }", "class A { 'constructor'(){} }", @@ -343,6 +557,68 @@ fn test() { "class A { public constructor(){} }", ]; + let fail_typescript = vec![ + " + class A { + constructor() {} + } + ", + " + class A extends B { + constructor() { + super(); + } + } + ", + " + class A extends B { + constructor(foo) { + super(foo); + } + } + ", + " + class A extends B { + constructor(foo, bar) { + super(foo, bar); + } + } + ", + " + class A extends B { + constructor(...args) { + super(...args); + } + } + ", + " + class A extends B.C { + constructor() { + super(...arguments); + } + } + ", + " + class A extends B { + constructor(a, b, ...c) { + super(...arguments); + } + } + ", + " + class A extends B { + constructor(a, b, ...c) { + super(a, b, ...c); + } + } + ", + " + class A { + public constructor() {} + } + ", + ]; + let fix = vec![ ("class A { constructor(){} }", "class A { }"), ( @@ -351,6 +627,9 @@ fn test() { ), ]; + let pass = [pass, pass_typescript].concat(); + let fail = [fail, fail_typescript].concat(); + Tester::new(NoUselessConstructor::NAME, NoUselessConstructor::PLUGIN, pass, fail) .expect_fix(fix) .test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/eslint_no_useless_constructor.snap b/crates/oxc_linter/src/snapshots/eslint_no_useless_constructor.snap index d74b69116e938..cfb4f86190942 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_useless_constructor.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_useless_constructor.snap @@ -105,3 +105,119 @@ source: crates/oxc_linter/src/tester.rs · ────────────────────── ╰──── help: Remove the constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Empty constructors are unnecessary + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A { + 3 │ constructor() {} + · ──────────────── + 4 │ } + ╰──── + help: Remove the constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B { + 3 │ constructor() { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(); + · ───┬─── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B { + 3 │ constructor(foo) { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(foo); + · ─────┬──── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B { + 3 │ constructor(foo, bar) { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(foo, bar); + · ───────┬─────── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B { + 3 │ constructor(...args) { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(...args); + · ───────┬────── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B.C { + 3 │ constructor() { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(...arguments); + · ─────────┬───────── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B { + 3 │ constructor(a, b, ...c) { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(...arguments); + · ─────────┬───────── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Redundant super call in constructor + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A extends B { + 3 │ constructor(a, b, ...c) { + · ─────┬───── + · ╰── This constructor is unnecessary, + 4 │ super(a, b, ...c); + · ────────┬──────── + · ╰── because it only passes arguments through to the superclass + 5 │ } + ╰──── + help: Subclasses automatically use the constructor of their superclass, making this redundant. + Remove this constructor or add code to it. + + ⚠ eslint(no-useless-constructor): Empty constructors are unnecessary + ╭─[no_useless_constructor.tsx:3:6] + 2 │ class A { + 3 │ public constructor() {} + · ─────────────────────── + 4 │ } + ╰──── + help: Remove the constructor or add code to it.