Skip to content

Commit

Permalink
feat(linter): eslint/no-constructor-return (#3321)
Browse files Browse the repository at this point in the history
  • Loading branch information
woai3c authored May 30, 2024
1 parent 0cdb45a commit 4c17bc6
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 0 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ mod eslint {
pub mod no_const_assign;
pub mod no_constant_binary_expression;
pub mod no_constant_condition;
pub mod no_constructor_return;
pub mod no_continue;
pub mod no_control_regex;
pub mod no_debugger;
Expand Down Expand Up @@ -493,6 +494,7 @@ oxc_macros::declare_all_lint_rules! {
eslint::no_new_native_nonconstructor,
eslint::no_restricted_globals,
eslint::prefer_exponentiation_operator,
eslint::no_constructor_return,
typescript::adjacent_overload_signatures,
typescript::array_type,
typescript::ban_ts_comment,
Expand Down
110 changes: 110 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_constructor_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use oxc_ast::{
ast::{MethodDefinition, MethodDefinitionKind},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::AstNodeId;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode};

fn no_constructor_return_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(
"eslint(no-constructor-return): Unexpected return statement in constructor.",
)
.with_labels([span.into()])
}

#[derive(Debug, Default, Clone)]
pub struct NoConstructorReturn;

declare_oxc_lint!(
/// ### What it does
///
/// Disallow returning value from constructor
///
/// ### Why is this bad?
///
/// In JavaScript, returning a value in the constructor of a class may be a mistake.
/// Forbidding this pattern prevents mistakes resulting from unfamiliarity with the language or a copy-paste error.
///
/// ### Example
/// Bad:
/// ```rust
/// class C {
/// constructor() { return 42; }
/// }
/// ```
///
/// Good:
/// ```rust
/// class C {
/// constructor() { this.value = 42; }
/// }
/// ```
NoConstructorReturn,
correctness
);

impl Rule for NoConstructorReturn {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::ReturnStatement(ret) = node.kind() else { return };
if ret.argument.is_none() {
return;
}

if is_definitely_in_constructor(ctx, node.id()) {
ctx.diagnostic(no_constructor_return_diagnostic(ret.span));
}
}
}

fn is_constructor(node: &AstNode<'_>) -> bool {
matches!(
node.kind(),
AstKind::MethodDefinition(MethodDefinition { kind: MethodDefinitionKind::Constructor, .. })
)
}

fn is_definitely_in_constructor(ctx: &LintContext, node_id: AstNodeId) -> bool {
ctx.nodes()
.ancestors(node_id)
.map(|id| ctx.nodes().get_node(id))
.skip_while(|node| !node.kind().is_function_like())
.nth(1)
.is_some_and(is_constructor)
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
"function fn() { return }",
"function fn(kumiko) { if (kumiko) { return kumiko } }",
"const fn = function () { return }",
"const fn = function () { if (kumiko) { return kumiko } }",
"const fn = () => { return }",
"const fn = () => { if (kumiko) { return kumiko } }",
"return 'Kumiko Oumae'",
"class C { }",
"class C { constructor() {} }",
"class C { constructor() { let v } }",
"class C { method() { return '' } }",
"class C { get value() { return '' } }",
"class C { constructor(a) { if (!a) { return } else { a() } } }",
"class C { constructor() { function fn() { return true } } }",
"class C { constructor() { this.fn = function () { return true } } }",
"class C { constructor() { this.fn = () => { return true } } }",
"class C { constructor() { return } }",
"class C { constructor() { { return } } }",
];

let fail = vec![
"class C { constructor() { return '' } }",
"class C { constructor(a) { if (!a) { return '' } else { a() } } }",
];

Tester::new(NoConstructorReturn::NAME, pass, fail).test_and_snapshot();
}
15 changes: 15 additions & 0 deletions crates/oxc_linter/src/snapshots/no_constructor_return.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: crates/oxc_linter/src/tester.rs
expression: no_constructor_return
---
eslint(no-constructor-return): Unexpected return statement in constructor.
╭─[no_constructor_return.tsx:1:27]
1class C { constructor() { return '' } }
· ─────────
╰────

eslint(no-constructor-return): Unexpected return statement in constructor.
╭─[no_constructor_return.tsx:1:38]
1class C { constructor(a) { if (!a) { return '' } else { a() } } }
· ─────────
╰────

0 comments on commit 4c17bc6

Please sign in to comment.