feat(linter): add eslint/no-useless-constructor (#3594)

Co-authored-by: Boshen <boshenc@gmail.com>

.typos.toml

@@ -29,5 +29,5 @@ seeked    = "seeked"
 labeledby = "labeledby"
 
 [default.extend-identifiers]
-IIFEs = "IIFEs"
+IIFEs      = "IIFEs"
 allowIIFEs = "allowIIFEs"

crates/oxc_allocator/src/arena.rs

@@ -69,6 +69,12 @@ impl<'alloc, T: ?Sized> ops::DerefMut for Box<'alloc, T> {
     }
 }
 
+impl<'alloc, T: ?Sized> AsRef<T> for Box<'alloc, T> {
+    fn as_ref(&self) -> &T {
+        self
+    }
+}
+
 impl<'alloc, T: ?Sized + Debug> Debug for Box<'alloc, T> {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         self.deref().fmt(f)

crates/oxc_ast/src/ast/js.rs

@@ -2453,6 +2453,14 @@ impl<'a> FormalParameters<'a> {
     pub fn parameters_count(&self) -> usize {
         self.items.len() + self.rest.as_ref().map_or(0, |_| 1)
     }
+
+    /// Iterates over all bound parameters, including rest parameters.
+    pub fn iter_bindings(&self) -> impl Iterator<Item = &BindingPattern<'a>> + '_ {
+        self.items
+            .iter()
+            .map(|param| &param.pattern)
+            .chain(self.rest.iter().map(|rest| &rest.argument))
+    }
 }
 
 #[visited_node]
@@ -2651,14 +2659,42 @@ impl<'a> Class<'a> {
         }
     }
 
+    /// `true` if this [`Class`] is an expression.
+    ///
+    /// For example,
+    /// ```ts
+    /// var Foo = class { /* ... */ }
+    /// ```
     pub fn is_expression(&self) -> bool {
         self.r#type == ClassType::ClassExpression
     }
 
+    /// `true` if this [`Class`] is a declaration statement.
+    ///
+    /// For example,
+    /// ```ts
+    /// class Foo {
+    ///   // ...
+    /// }
+    /// ```
+    ///
+    /// Not to be confused with [`Class::is_declare`].
     pub fn is_declaration(&self) -> bool {
         self.r#type == ClassType::ClassDeclaration
     }
 
+    /// `true` if this [`Class`] is being within a typescript declaration file
+    /// or `declare` statement.
+    ///
+    /// For example,
+    /// ```ts
+    /// declare global {
+    ///   declare class Foo {
+    ///    // ...
+    ///   }
+    /// }
+    ///
+    /// Not to be confused with [`Class::is_declaration`].
     pub fn is_declare(&self) -> bool {
         self.modifiers.contains(ModifierKind::Declare)
     }

crates/oxc_linter/src/rules.rs

@@ -102,6 +102,7 @@ mod eslint {
     pub mod no_unused_private_class_members;
     pub mod no_useless_catch;
     pub mod no_useless_concat;
+    pub mod no_useless_constructor;
     pub mod no_useless_escape;
     pub mod no_useless_rename;
     pub mod no_var;
@@ -491,6 +492,7 @@ oxc_macros::declare_all_lint_rules! {
     eslint::no_useless_escape,
     eslint::no_useless_rename,
     eslint::no_useless_concat,
+    eslint::no_useless_constructor,
     eslint::no_var,
     eslint::no_void,
     eslint::no_with,

crates/oxc_linter/src/rules/eslint/no_useless_constructor.rs

@@ -0,0 +1,274 @@
+use oxc_ast::{
+    ast::{
+        Argument, BindingPattern, BindingPatternKind, BindingRestElement, CallExpression,
+        Expression, FormalParameters, FunctionBody, MethodDefinition, Statement,
+    },
+    AstKind,
+};
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::Span;
+
+use crate::{context::LintContext, rule::Rule, AstNode};
+
+fn no_empty_constructor(constructor_span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn("eslint(no-useless-constructor): Empty constructors are unnecessary")
+        .with_labels([constructor_span.into()])
+        .with_help("Remove the constructor or add code to it.")
+}
+fn no_redundant_super_call(constructor_span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn("eslint(no-useless-constructor): Redundant super call in constructor")
+        .with_labels([constructor_span.into()])
+        .with_help("Constructors of subclasses invoke super() automatically if they are empty. Remove this constructor or add code to it.")
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct NoUselessConstructor;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Disallow unnecessary constructors
+    ///
+    /// This rule flags class constructors that can be safely removed without
+    /// changing how the class works.
+    ///
+    /// ES2015 provides a default class constructor if one is not specified. As
+    /// such, it is unnecessary to provide an empty constructor or one that
+    /// simply delegates into its parent class, as in the following examples:
+    ///
+    ///
+    /// ### Example
+    ///
+    /// Examples of **incorrect** code for this rule:
+    /// ```javascript
+    /// class A {
+    ///     constructor () {
+    ///     }
+    /// }
+    ///
+    /// class B extends A {
+    ///     constructor (...args) {
+    ///       super(...args);
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// Examples of **correct** code for this rule:
+    /// ```javascript
+    /// class A { }
+    ///
+    /// class B {
+    ///     constructor () {
+    ///         doSomething();
+    ///     }
+    /// }
+    ///
+    /// class C extends A {
+    ///     constructor() {
+    ///         super('foo');
+    ///     }
+    /// }
+    ///
+    /// class D extends A {
+    ///     constructor() {
+    ///         super();
+    ///         doSomething();
+    ///     }
+    /// }
+    ///```
+    NoUselessConstructor,
+    suspicious,
+);
+
+impl Rule for NoUselessConstructor {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::MethodDefinition(constructor) = node.kind() else {
+            return;
+        };
+        if !constructor.kind.is_constructor() {
+            return;
+        }
+        let Some(body) = &constructor.value.body else {
+            return;
+        };
+
+        let class = ctx
+            .nodes()
+            .iter_parents(node.id())
+            .skip(1)
+            .find(|parent| matches!(parent.kind(), AstKind::Class(_)));
+        debug_assert!(class.is_some(), "Found a constructor outside of a class definition");
+        let Some(class_node) = class else {
+            return;
+        };
+        let AstKind::Class(class) = class_node.kind() else { unreachable!() };
+        if class.is_declare() {
+            return;
+        }
+
+        if class.super_class.is_none() {
+            lint_empty_constructor(ctx, constructor, body);
+        } else {
+            lint_redundant_super_call(ctx, constructor, body);
+        }
+    }
+}
+
+fn lint_empty_constructor<'a>(
+    ctx: &LintContext<'a>,
+    constructor: &MethodDefinition<'a>,
+    body: &FunctionBody<'a>,
+) {
+    if !body.statements.is_empty() {
+        return;
+    }
+    ctx.diagnostic_with_fix(no_empty_constructor(constructor.span), |fixer| {
+        fixer.delete_range(constructor.span)
+    });
+}
+
+fn lint_redundant_super_call<'a>(
+    ctx: &LintContext<'a>,
+    constructor: &MethodDefinition<'a>,
+    body: &FunctionBody<'a>,
+) {
+    let Some(super_call) = is_single_super_call(body) else {
+        return;
+    };
+
+    let params = &*constructor.value.params;
+    let super_args = &super_call.arguments;
+
+    if is_only_simple_params(params)
+        && (is_spread_arguments(super_args) || is_passing_through(params, super_args))
+    {
+        ctx.diagnostic_with_fix(no_redundant_super_call(constructor.span), |fixer| {
+            fixer.delete_range(constructor.span)
+        });
+    }
+}
+/// 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`].
+fn is_single_super_call<'f, 'a: 'f>(body: &'f FunctionBody<'a>) -> Option<&'f CallExpression<'a>> {
+    if body.statements.len() != 1 {
+        return None;
+    }
+    let Statement::ExpressionStatement(expr) = &body.statements[0] else { return None };
+    let Expression::CallExpression(call) = &expr.expression else { return None };
+
+    matches!(call.callee, Expression::Super(_)).then(|| call.as_ref())
+}
+
+/// Returns `false` if any parameter is an array/object unpacking binding or an
+/// assignment pattern.
+fn is_only_simple_params(params: &FormalParameters) -> bool {
+    params.iter_bindings().all(|param| param.kind.is_binding_identifier())
+}
+
+fn is_spread_arguments(super_args: &[Argument<'_>]) -> bool {
+    super_args.len() == 1 && super_args[0].is_spread()
+}
+
+fn is_passing_through<'a>(
+    constructor_params: &FormalParameters<'a>,
+    super_args: &[Argument<'a>],
+) -> bool {
+    if constructor_params.parameters_count() != super_args.len() {
+        return false;
+    }
+    if let Some(rest) = &constructor_params.rest {
+        let all_but_last = super_args.iter().take(super_args.len() - 1);
+        let Some(last_arg) = super_args.iter().next_back() else { return false };
+        constructor_params
+            .items
+            .iter()
+            .zip(all_but_last)
+            .all(|(param, arg)| is_matching_identifier_pair(&param.pattern, arg))
+            && is_matching_rest_spread_pair(rest, last_arg)
+    } else {
+        constructor_params
+            .iter_bindings()
+            .zip(super_args)
+            .all(|(param, arg)| is_matching_identifier_pair(param, arg))
+    }
+}
+
+fn is_matching_identifier_pair<'a>(param: &BindingPattern<'a>, arg: &Argument<'a>) -> bool {
+    match (&param.kind, arg) {
+        (BindingPatternKind::BindingIdentifier(param), Argument::Identifier(arg)) => {
+            param.name == arg.name
+        }
+        _ => false,
+    }
+}
+fn is_matching_rest_spread_pair<'a>(rest: &BindingRestElement<'a>, arg: &Argument<'a>) -> bool {
+    match (&rest.argument.kind, arg) {
+        (BindingPatternKind::BindingIdentifier(param), Argument::SpreadElement(spread)) => {
+            matches!(&spread.argument, Expression::Identifier(ident) if param.name == ident.name)
+        }
+        _ => false,
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = 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(options: any); }", // {                "parser": require("../../fixtures/parsers/typescript-parsers/declare-class")            }
+    ];
+
+    let fail = vec![
+        "class A { constructor(){} }",
+        "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); } }",
+    ];
+
+    let fix = vec![
+        ("class A { constructor(){} }", "class A {  }"),
+        (
+            r"
+class A extends B {
+    constructor() {
+        super();
+    }
+    foo() {
+        bar();
+    }
+}",
+            r"
+class A extends B {
+    
+    foo() {
+        bar();
+    }
+}",
+        ),
+    ];
+
+    Tester::new(NoUselessConstructor::NAME, pass, fail).expect_fix(fix).test_and_snapshot();
+}