Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): eslint-plugin-jest: no-test-return-statement #1979

Merged
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
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ mod jest {
pub mod no_mocks_import;
pub mod no_standalone_expect;
pub mod no_test_prefixes;
pub mod no_test_return_statement;
pub mod prefer_todo;
pub mod valid_describe_callback;
pub mod valid_expect;
Expand Down Expand Up @@ -396,6 +397,7 @@ oxc_macros::declare_all_lint_rules! {
jest::no_mocks_import,
jest::no_standalone_expect,
jest::no_test_prefixes,
jest::no_test_return_statement,
jest::prefer_todo,
jest::valid_describe_callback,
jest::valid_expect,
Expand Down
240 changes: 240 additions & 0 deletions crates/oxc_linter/src/rules/jest/no_test_return_statement.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
use crate::{
context::LintContext,
rule::Rule,
utils::{is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode},
};

use oxc_allocator::Box as OBox;
use oxc_ast::{
ast::{Argument, CallExpression, Expression, FunctionBody, Statement},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_semantic::AstNode;
use oxc_span::{GetSpan, Span};

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value")]
#[diagnostic(severity(warning))]
pub struct NoTestReturnStatementDiagnostic(#[label] pub Span);

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

declare_oxc_lint!(
/// ### What it does
///
/// Disallow explicitly returning from tests.
///
/// ### Why is this bad?
///
/// Tests in Jest should be void and not return values.
/// If you are returning Promises then you should update the test to use
/// `async/await`.
///
/// ### Example
/// ```javascript
/// ```
NoTestReturnStatement,
style,
);

impl Rule for NoTestReturnStatement {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.kind() {
AstKind::CallExpression(call_expr) => {
check_call_expression(call_expr, node, ctx);
}
AstKind::Function(fn_decl) => {
let Some(func_body) = &fn_decl.body else {
return;
};
check_test_return_statement(func_body, ctx);
}
_ => (),
}
}
}

fn check_call_expression<'a>(
call_expr: &'a CallExpression<'a>,
node: &AstNode<'a>,
ctx: &LintContext<'a>,
) {
if !is_type_of_jest_fn_call(
call_expr,
&PossibleJestNode { node, original: None },
ctx,
&[JestFnKind::General(JestGeneralFnKind::Test)],
) {
return;
}

for argument in &call_expr.arguments {
let Argument::Expression(arg_expr) = argument else {
continue;
};
match arg_expr {
Expression::ArrowExpression(arrow_expr) => {
check_test_return_statement(&arrow_expr.body, ctx);
}
Expression::FunctionExpression(func_expr) => {
let Some(func_body) = &func_expr.body else {
continue;
};
check_test_return_statement(func_body, ctx);
}
_ => continue,
}
}
}

fn check_test_return_statement<'a>(func_body: &OBox<'_, FunctionBody<'a>>, ctx: &LintContext<'a>) {
let Some(return_stmt) =
func_body.statements.iter().find(|stmt| matches!(stmt, Statement::ReturnStatement(_)))
else {
return;
};

let Statement::ReturnStatement(stmt) = return_stmt else {
return;
};
let Some(Expression::CallExpression(call_expr)) = &stmt.argument else {
return;
};
let Expression::MemberExpression(mem_expr) = &call_expr.callee else {
return;
};
let Expression::CallExpression(mem_call_expr) = mem_expr.object() else {
return;
};
let Expression::Identifier(ident) = &mem_call_expr.callee else {
return;
};

if ident.name != "expect" {
return;
}

ctx.diagnostic(NoTestReturnStatementDiagnostic(Span {
start: return_stmt.span().start,
end: call_expr.span.start - 1,
mysteryven marked this conversation as resolved.
Show resolved Hide resolved
}));
}

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

let pass = vec![
("test('noop', () => {});", None),
("test('one', () => expect(1).toBe(1));", None),
("test('empty')", None),
(
"
test('one', () => {
expect(1).toBe(1);
});
",
None,
),
(
"
it('one', function () {
expect(1).toBe(1);
});
",
None,
),
(
"
it('one', myTest);
function myTest() {
expect(1).toBe(1);
}
",
None,
),
(
"
it('one', () => expect(1).toBe(1));
function myHelper() {}
",
None,
),
];

let fail = vec![
(
"
test('one', () => {
return expect(1).toBe(1);
});
",
None,
),
(
"
it('one', function () {
return expect(1).toBe(1);
});
",
None,
),
(
"
it.skip('one', function () {
return expect(1).toBe(1);
});
",
None,
),
(
"
it.each``('one', function () {
return expect(1).toBe(1);
});
",
None,
),
(
"
it.each()('one', function () {
return expect(1).toBe(1);
});
",
None,
),
(
"
it.only.each``('one', function () {
return expect(1).toBe(1);
});
",
None,
),
(
"
it.only.each()('one', function () {
return expect(1).toBe(1);
});
",
None,
),
(
"
it('one', myTest);
function myTest () {
return expect(1).toBe(1);
}
",
None,
),
];

Tester::new(NoTestReturnStatement::NAME, pass, fail).with_jest_plugin(true).test_and_snapshot();
}
118 changes: 118 additions & 0 deletions crates/oxc_linter/src/snapshots/no_test_return_statement.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
---
source: crates/oxc_linter/src/tester.rs
assertion_line: 144
expression: no_test_return_statement
---
⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ test('one', () => {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.skip('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.skip('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.each``('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.each``('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.each()('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.each()('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.only.each``('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.only.each``('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.only.each()('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:2:1]
2 │ it.only.each()('one', function () {
3 │ return expect(1).toBe(1);
· ──────
4 │ });
╰────

⚠ eslint-plugin-jest(no-test-return-statement): Jest tests should not return a value
╭─[no_test_return_statement.tsx:3:1]
3 │ function myTest () {
4 │ return expect(1).toBe(1);
· ──────
5 │ }
╰────