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/vitest): implement prefer-each #5203

Merged
merged 4 commits into from
Aug 26, 2024
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 @@ -458,6 +458,7 @@ mod promise {
mod vitest {
pub mod no_conditional_tests;
pub mod no_import_node_test;
pub mod prefer_each;
pub mod prefer_to_be_falsy;
pub mod prefer_to_be_truthy;
pub mod require_local_test_context_for_concurrent_snapshots;
Expand Down Expand Up @@ -874,6 +875,7 @@ oxc_macros::declare_all_lint_rules! {
promise::no_return_in_finally,
promise::prefer_await_to_then,
vitest::no_import_node_test,
vitest::prefer_each,
vitest::prefer_to_be_falsy,
vitest::prefer_to_be_truthy,
vitest::no_conditional_tests,
Expand Down
283 changes: 283 additions & 0 deletions crates/oxc_linter/src/rules/vitest/prefer_each.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,283 @@
use std::collections::HashSet;

use oxc_ast::AstKind;
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{AstNode, AstNodeId};
use oxc_span::{GetSpan, Span};

use crate::{
context::LintContext,
rule::Rule,
utils::{parse_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode},
};

fn use_prefer_each(span: Span, fn_name: &str) -> OxcDiagnostic {
OxcDiagnostic::warn("Enforce using `each` rather than manual loops")
.with_help(format!("Prefer using `{fn_name}.each` rather than a manual loop."))
.with_label(span)
}

#[inline]
fn is_in_test(ctx: &LintContext<'_>, id: AstNodeId) -> bool {
ctx.nodes().iter_parents(id).any(|node| {
let AstKind::CallExpression(ancestor_call_expr) = node.kind() else { return false };
let Some(ancestor_member_expr) = ancestor_call_expr.callee.as_member_expression() else {
return false;
};
let Some(id) = ancestor_member_expr.object().get_identifier_reference() else {
return false;
};

matches!(JestFnKind::from(id.name.as_str()), JestFnKind::General(JestGeneralFnKind::Test))
})
}

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

declare_oxc_lint!(
/// ### What it does
/// This rule enforces using `each` rather than manual loops.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// for (const item of items) {
/// describe(item, () => {
/// expect(item).toBe('foo')
/// })
/// }
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// describe.each(items)('item', (item) => {
/// expect(item).toBe('foo')
/// })
/// ```
PreferEach,
style,
);

impl Rule for PreferEach {
fn run_once(&self, ctx: &LintContext<'_>) {
let mut skip = HashSet::<AstNodeId>::new();
ctx.nodes().iter().for_each(|node| {
Self::run(node, ctx, &mut skip);
});
}
}

impl PreferEach {
fn run<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>, skip: &mut HashSet<AstNodeId>) {
let kind = node.kind();

let AstKind::CallExpression(call_expr) = kind else { return };

let Some(vitest_fn_call) =
parse_jest_fn_call(call_expr, &PossibleJestNode { node, original: None }, ctx)
else {
return;
};

if !matches!(
vitest_fn_call.kind(),
JestFnKind::General(
JestGeneralFnKind::Describe | JestGeneralFnKind::Hook | JestGeneralFnKind::Test
)
) {
return;
}

for parent_node in ctx.nodes().iter_parents(node.id()).skip(1) {
match parent_node.kind() {
AstKind::CallExpression(_) => {
return;
}
AstKind::ForStatement(_)
| AstKind::ForInStatement(_)
| AstKind::ForOfStatement(_) => {
if skip.contains(&parent_node.id()) || is_in_test(ctx, parent_node.id()) {
return;
}

let fn_name = if matches!(
vitest_fn_call.kind(),
JestFnKind::General(JestGeneralFnKind::Test)
) {
"it"
} else {
"describe"
};

let span = match parent_node.kind() {
AstKind::ForStatement(statement) => {
Span::new(statement.span.start, statement.body.span().start)
}
AstKind::ForInStatement(statement) => {
Span::new(statement.span.start, statement.body.span().start)
}
AstKind::ForOfStatement(statement) => {
Span::new(statement.span.start, statement.body.span().start)
}
_ => unreachable!(),
};

skip.insert(parent_node.id());
ctx.diagnostic(use_prefer_each(span, fn_name));

break;
}
_ => {}
}
}
}
}

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

let pass = vec![
r#"it("is true", () => { expect(true).toBe(false) });"#,
r#"it.each(getNumbers())("only returns numbers that are greater than seven", number => {
expect(number).toBeGreaterThan(7);
});"#,
r#"it("returns numbers that are greater than five", function () {
for (const number of getNumbers()) {
expect(number).toBeGreaterThan(5);
}
});"#,
r#"it("returns things that are less than ten", function () {
for (const thing in things) {
expect(thing).toBeLessThan(10);
}
});"#,
r#"it("only returns numbers that are greater than seven", function () {
const numbers = getNumbers();

for (let i = 0; i < numbers.length; i++) {
expect(numbers[i]).toBeGreaterThan(7);
}
});"#,
];

let fail = vec![
" for (const [input, expected] of data) {
it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}",
" for (const [input, expected] of data) {
describe(`when the input is ${input}`, () => {
it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
});
}",
"for (const [input, expected] of data) {
describe(`when the input is ${input}`, () => {
it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
});
}

for (const [input, expected] of data) {
it.skip(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}",
"for (const [input, expected] of data) {
it.skip(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}",
"it('is true', () => {
expect(true).toBe(false);
});

for (const [input, expected] of data) {
it.skip(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}",
" for (const [input, expected] of data) {
it.skip(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}

it('is true', () => {
expect(true).toBe(false);
});",
" it('is true', () => {
expect(true).toBe(false);
});

for (const [input, expected] of data) {
it.skip(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}

it('is true', () => {
expect(true).toBe(false);
});",
"for (const [input, expected] of data) {
it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});

it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}",
"for (const [input, expected] of data) {
it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}

for (const [input, expected] of data) {
it(`results in ${expected}`, () => {
expect(fn(input)).toBe(expected)
});
}",
"for (const [input, expected] of data) {
beforeEach(() => setupSomething(input));

test(`results in ${expected}`, () => {
expect(doSomething()).toBe(expected)
});
}",
r#"
for (const [input, expected] of data) {
it("only returns numbers that are greater than seven", function () {
const numbers = getNumbers(input);

for (let i = 0; i < numbers.length; i++) {
expect(numbers[i]).toBeGreaterThan(7);
}
});
}
"#,
r#"
for (const [input, expected] of data) {
beforeEach(() => setupSomething(input));

it("only returns numbers that are greater than seven", function () {
const numbers = getNumbers();

for (let i = 0; i < numbers.length; i++) {
expect(numbers[i]).toBeGreaterThan(7);
}
});
}
"#,
];

Tester::new(PreferEach::NAME, pass, fail).test_and_snapshot();
}
Loading