Skip to content
Open
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: 1 addition & 1 deletion ..._vitest_replace__eslintrc.json [email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ working directory:
help: Remove the appending `.skip`

Found 1 warning and 1 error.
Finished in <variable>ms on 1 file with 102 rules using 1 threads.
Finished in <variable>ms on 1 file with 103 rules using 1 threads.
----------
CLI result: LintFoundErrors
----------
5 changes: 5 additions & 0 deletions crates/oxc_linter/src/generated/rule_runner_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4015,6 +4015,11 @@ impl RuleRunner for crate::rules::vitest::require_local_test_context_for_concurr
const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::RunOnJestNode;
}

impl RuleRunner for crate::rules::vitest::warn_todo::WarnTodo {
const NODE_TYPES: Option<&AstTypesBitset> = None;
const RUN_FUNCTIONS: RuleRunFunctionsImplemented = RuleRunFunctionsImplemented::RunOnJestNode;
}

impl RuleRunner for crate::rules::vue::define_emits_declaration::DefineEmitsDeclaration {
const NODE_TYPES: Option<&AstTypesBitset> =
Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
Expand Down
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ pub(crate) mod vitest {
pub mod prefer_to_be_object;
pub mod prefer_to_be_truthy;
pub mod require_local_test_context_for_concurrent_snapshots;
pub mod warn_todo;
}

pub(crate) mod node {
Expand Down Expand Up @@ -1329,6 +1330,7 @@ oxc_macros::declare_all_lint_rules! {
vitest::prefer_to_be_object,
vitest::prefer_to_be_truthy,
vitest::require_local_test_context_for_concurrent_snapshots,
vitest::warn_todo,
vue::define_emits_declaration,
vue::define_props_declaration,
vue::define_props_destructuring,
Expand Down
149 changes: 149 additions & 0 deletions crates/oxc_linter/src/rules/vitest/warn_todo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
use oxc_ast::AstKind;
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

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

fn warn_todo_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("The use of `.todo` is not recommended.")
.with_help("Write an actual test and remove the `.todo` modifier before pushing/merging your changes.")
.with_label(span)
}

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

declare_oxc_lint!(
/// ### What it does
///
/// This rule triggers warnings when `.todo` is used in `describe`, `it`, or `test` functions.
/// It is recommended to use this with your CI pipeline to annotate PR diffs.
///
/// ### Why is this bad?
///
/// The test that you push should be completed, any pending/"TODO" code should not be committed.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// describe.todo('foo', () => {})
/// it.todo('foo', () => {})
/// test.todo('foo', () => {})
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// describe([])('foo', () => {})
/// it([])('foo', () => {})
/// test([])('foo', () => {})
/// ```
WarnTodo,
vitest,
correctness,
);

impl Rule for WarnTodo {
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
WarnTodo::run(jest_node, ctx);
}
}

impl WarnTodo {
fn run<'a>(possible_jest_node: &PossibleJestNode<'a, '_>, ctx: &LintContext<'a>) {
let node = possible_jest_node.node;
if let AstKind::CallExpression(call_expr) = node.kind() {
if !is_type_of_jest_fn_call(
call_expr,
possible_jest_node,
ctx,
&[
JestFnKind::General(JestGeneralFnKind::Describe),
JestFnKind::General(JestGeneralFnKind::Test),
],
) {
return;
}

let Some(parsed_vi_fn_call) =
parse_general_jest_fn_call(call_expr, possible_jest_node, ctx)
else {
return;
};

let warn_modifier =
parsed_vi_fn_call.members.iter().find(|member| member.is_name_equal("todo"));

if let Some(modifier) = warn_modifier {
ctx.diagnostic(warn_todo_diagnostic(modifier.span));
}
}
}
}

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

/*
* Currently the responsible to set what frameworks are active or not is not `with_vitest_plugin` or oxlint config.
* The code that set what test framewors are active is ContextHost::sniff_for_frameworks, and the current detection lead to a
* a false negative. To detect if the current source code belongs to vitest is based if a `vitest` import exist, if not, assumes
* we are on a possible jest test. On top of that, the method `frameworks::is_jestlike_file` most of the times is going to be true, at least in
* our current situation. So this lead that the ContextHost can have jest and vitest active **at same time**.
*
* This detection isn't compatible on how `parse_general_jest_fn_call` handle if a node is valid or not. To make it simple:
*
* - Jest file: ctx.frameworks().is_jest() is true && ctx.frameworks().is_vitest() is false
* - Vitest file: ctx.frameworks().is_jest() is true && ctx.frameworks().is_vitest is true
*
* And if you are dealing with non compatible modifiers or methods, that only exists in vitest, it will fail as in jest doesn't exist.
*
* In case of dealing with syntax that only exists in vitest, add an import of `vitest` to force the ContextHost to detect we are dealing with vitest.
* This probably will allow reuse allow of the methods that rely on this false negative detection.
*/
macro_rules! vitest_context {
($test: literal) => {
concat!("import * as vi from 'vitest'\n\n", $test)
};
}

let pass = vec![
(vitest_context!("describe('foo', function () {})"), None),
(vitest_context!("it('foo', function () {})"), None),
(vitest_context!("it.concurrent('foo', function () {})"), None),
(vitest_context!("test('foo', function () {})"), None),
(vitest_context!("test.concurrent('foo', function () {})"), None),
(vitest_context!("describe.only('foo', function () {})"), None),
(vitest_context!("it.only('foo', function () {})"), None),
(vitest_context!("it.each()('foo', function () {})"), None),
];

let fail = vec![
(vitest_context!("describe.todo('foo', function () {})"), None),
(vitest_context!("it.todo('foo', function () {})"), None),
(vitest_context!("test.todo('foo', function () {})"), None),
(vitest_context!("describe.todo.each([])('foo', function () {})"), None),
(vitest_context!("it.todo.each([])('foo', function () {})"), None),
(vitest_context!("test.todo.each([])('foo', function () {})"), None),
(vitest_context!("describe.only.todo('foo', function () {})"), None),
(vitest_context!("it.only.todo('foo', function () {})"), None),
(vitest_context!("test.only.todo('foo', function () {})"), None),
];

Tester::new(WarnTodo::NAME, WarnTodo::PLUGIN, pass, fail)
.with_vitest_plugin(true)
.test_and_snapshot();
}
74 changes: 74 additions & 0 deletions crates/oxc_linter/src/snapshots/vitest_warn_todo.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:10]
2 │
3 │ describe.todo('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:4]
2 │
3 │ it.todo('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:6]
2 │
3 │ test.todo('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:10]
2 │
3 │ describe.todo.each([])('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:4]
2 │
3 │ it.todo.each([])('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:6]
2 │
3 │ test.todo.each([])('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:15]
2 │
3 │ describe.only.todo('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:9]
2 │
3 │ it.only.todo('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.

⚠ eslint-plugin-vitest(warn-todo): The use of `.todo` is not recommended.
╭─[warn_todo.tsx:3:11]
2 │
3 │ test.only.todo('foo', function () {})
· ────
╰────
help: Write an actual test and remove the `.todo` modifier before pushing/merging your changes.
23 changes: 17 additions & 6 deletions crates/oxc_linter/src/utils/jest/parse_jest_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,23 @@ pub fn parse_jest_fn_call<'a>(
let mut call_chains = Vec::from([Cow::Borrowed(name)]);
call_chains.extend(members.iter().filter_map(KnownMemberExpressionProperty::name));

if ctx.frameworks().is_jest() && !is_valid_jest_call(&call_chains) {
return None;
}

if ctx.frameworks().is_vitest() && !is_valid_vitest_call(&call_chains) {
return None;
match (ctx.frameworks().is_jest(), ctx.frameworks().is_vitest()) {
(true, true) => {
if !is_valid_jest_call(&call_chains) && !is_valid_vitest_call(&call_chains) {
return None;
}
}
(true, false) => {
if !is_valid_jest_call(&call_chains) {
return None;
}
}
(false, true) => {
if !is_valid_vitest_call(&call_chains) {
return None;
}
}
(false, false) => {}
}

return Some(ParsedJestFnCall::GeneralJest(ParsedGeneralJestFnCall {
Expand Down
Loading