Skip to content

Commit

Permalink
refactor(linter): use run_on_jest_node for existing lint rules (#6722)
Browse files Browse the repository at this point in the history
- closes #6038

Migrates all simple iterations over Jest nodes to use the `run_on_jest_node` method. Rules which require more custom data (such as getting nodes in order or storing data about counts, duplicates, etc.) have not been migrated.

Some simple benchmarking shows that this is ~5% faster on `vscode` when the Jest/Vitest rules are NOT enabled (due to being able to skip them now). And it is up to 8% on `vscode` when the Jest/Vitest plugins are enabled.

```
hyperfine -i -N --warmup 3 --runs 30 './oxlint-jest-iter --deny=all --silent ./vscode' './oxlint-main --deny=all --silent ./vscode'
Benchmark 1: ./oxlint-jest-iter --deny=all --silent ./vscode
  Time (mean ± σ):      2.348 s ±  0.104 s    [User: 16.922 s, System: 0.641 s]
  Range (min … max):    2.141 s …  2.544 s    30 runs

Benchmark 2: ./oxlint-main --deny=all --silent ./vscode
  Time (mean ± σ):      2.476 s ±  0.042 s    [User: 17.768 s, System: 0.668 s]
  Range (min … max):    2.430 s …  2.598 s    30 runs

Summary
  ./oxlint-jest-iter --deny=all --silent ./vscode ran
    1.05 ± 0.05 times faster than ./oxlint-main --deny=all --silent ./vscode
```

```
hyperfine -i -N --warmup 3 --runs 30 './oxlint-jest-iter --deny=all --jest-plugin --vitest-plugin --silent ./vscode' './oxlint-main --deny=all --jest-plugin --vitest-plugin --silent ./vscode'
Benchmark 1: ./oxlint-jest-iter --deny=all --jest-plugin --vitest-plugin --silent ./vscode
  Time (mean ± σ):      2.728 s ±  0.118 s    [User: 19.782 s, System: 0.786 s]
  Range (min … max):    2.580 s …  3.078 s    30 runs

Benchmark 2: ./oxlint-main --deny=all --jest-plugin --vitest-plugin --silent ./vscode
  Time (mean ± σ):      2.939 s ±  0.051 s    [User: 21.259 s, System: 0.859 s]
  Range (min … max):    2.848 s …  3.064 s    30 runs

Summary
  ./oxlint-jest-iter --deny=all --jest-plugin --vitest-plugin --silent ./vscode ran
    1.08 ± 0.05 times faster than ./oxlint-main --deny=all --jest-plugin --vitest-plugin --silent ./vscode
```
  • Loading branch information
camchenry committed Oct 21, 2024
1 parent 97195ec commit d6609e9
Show file tree
Hide file tree
Showing 35 changed files with 243 additions and 213 deletions.
4 changes: 2 additions & 2 deletions apps/oxlint/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ mod test {
let args = &[
"-c",
"fixtures/eslintrc_vitest_replace/eslintrc.json",
"fixtures/eslintrc_vitest_replace/foo.js",
"fixtures/eslintrc_vitest_replace/foo.test.js",
];
let result = test(args);
assert_eq!(result.number_of_files, 1);
Expand All @@ -549,7 +549,7 @@ mod test {
"--vitest-plugin",
"-c",
"fixtures/eslintrc_vitest_replace/eslintrc.json",
"fixtures/eslintrc_vitest_replace/foo.js",
"fixtures/eslintrc_vitest_replace/foo.test.js",
];
let result = test(args);
assert_eq!(result.number_of_files, 1);
Expand Down
13 changes: 7 additions & 6 deletions crates/oxc_linter/src/rules/jest/expect_expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, get_node_name, is_type_of_jest_fn_call, JestFnKind,
JestGeneralFnKind, PossibleJestNode,
get_node_name, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode,
},
};

Expand Down Expand Up @@ -107,10 +106,12 @@ impl Rule for ExpectExpect {
}))
}

fn run_once(&self, ctx: &LintContext) {
for possible_jest_node in &collect_possible_jest_call_node(ctx) {
run(self, possible_jest_node, ctx);
}
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
run(self, jest_node, ctx);
}
}

Expand Down
12 changes: 7 additions & 5 deletions crates/oxc_linter/src/rules/jest/no_alias_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{collect_possible_jest_call_node, parse_expect_jest_fn_call, PossibleJestNode},
utils::{parse_expect_jest_fn_call, PossibleJestNode},
};

fn no_alias_methods_diagnostic(x1: &str, x2: &str, span3: Span) -> OxcDiagnostic {
Expand Down Expand Up @@ -59,10 +59,12 @@ declare_oxc_lint!(
);

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

Expand Down
14 changes: 8 additions & 6 deletions crates/oxc_linter/src/rules/jest/no_conditional_expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, is_type_of_jest_fn_call, parse_expect_jest_fn_call,
JestFnKind, JestGeneralFnKind, PossibleJestNode,
is_type_of_jest_fn_call, parse_expect_jest_fn_call, JestFnKind, JestGeneralFnKind,
PossibleJestNode,
},
};

Expand Down Expand Up @@ -70,10 +70,12 @@ declare_oxc_lint!(
struct InConditional(bool);

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

Expand Down
14 changes: 8 additions & 6 deletions crates/oxc_linter/src/rules/jest/no_disabled_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, parse_general_jest_fn_call, JestFnKind, JestGeneralFnKind,
ParsedGeneralJestFnCall, PossibleJestNode,
parse_general_jest_fn_call, JestFnKind, JestGeneralFnKind, ParsedGeneralJestFnCall,
PossibleJestNode,
},
};

Expand Down Expand Up @@ -90,10 +90,12 @@ impl Message {
}

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

Expand Down
13 changes: 7 additions & 6 deletions crates/oxc_linter/src/rules/jest/no_done_callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, get_node_name, parse_general_jest_fn_call, JestFnKind,
JestGeneralFnKind, PossibleJestNode,
get_node_name, parse_general_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode,
},
};

Expand Down Expand Up @@ -77,10 +76,12 @@ declare_oxc_lint!(
);

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

Expand Down
14 changes: 8 additions & 6 deletions crates/oxc_linter/src/rules/jest/no_focused_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, parse_general_jest_fn_call, JestFnKind, JestGeneralFnKind,
MemberExpressionElement, ParsedGeneralJestFnCall, PossibleJestNode,
parse_general_jest_fn_call, JestFnKind, JestGeneralFnKind, MemberExpressionElement,
ParsedGeneralJestFnCall, PossibleJestNode,
},
};

Expand Down Expand Up @@ -66,10 +66,12 @@ declare_oxc_lint!(
);

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

Expand Down
15 changes: 7 additions & 8 deletions crates/oxc_linter/src/rules/jest/no_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ use oxc_span::{CompactStr, GetSpan, Span};
use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind,
PossibleJestNode,
},
utils::{is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode},
};

fn unexpected_hook_diagonsitc(span: Span) -> OxcDiagnostic {
Expand Down Expand Up @@ -96,10 +93,12 @@ impl Rule for NoHooks {
Self(Box::new(NoHooksConfig { allow }))
}

fn run_once(&self, ctx: &LintContext) {
for possible_jest_node in collect_possible_jest_call_node(ctx) {
self.run(&possible_jest_node, ctx);
}
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
self.run(jest_node, ctx);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{collect_possible_jest_call_node, parse_expect_jest_fn_call, PossibleJestNode},
utils::{parse_expect_jest_fn_call, PossibleJestNode},
};

fn no_interpolation_in_snapshots_diagnostic(span: Span) -> OxcDiagnostic {
Expand Down Expand Up @@ -55,10 +55,12 @@ declare_oxc_lint!(
);

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

Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_linter/src/rules/jest/no_large_snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_hash::FxHashMap;
use crate::{
context::LintContext,
rule::Rule,
utils::{collect_possible_jest_call_node, parse_expect_jest_fn_call, PossibleJestNode},
utils::{iter_possible_jest_call_node, parse_expect_jest_fn_call, PossibleJestNode},
};

// TODO: re-word diagnostic messages
Expand Down Expand Up @@ -167,8 +167,8 @@ impl Rule for NoLargeSnapshots {
}
}
} else {
for possible_jest_node in &collect_possible_jest_call_node(ctx) {
self.run(possible_jest_node, ctx);
for possible_jest_node in iter_possible_jest_call_node(ctx.semantic()) {
self.run(&possible_jest_node, ctx);
}
}
}
Expand Down
15 changes: 7 additions & 8 deletions crates/oxc_linter/src/rules/jest/no_restricted_jest_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ use rustc_hash::FxHashMap;
use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind,
PossibleJestNode,
},
utils::{is_type_of_jest_fn_call, JestFnKind, JestGeneralFnKind, PossibleJestNode},
};

fn restricted_jest_method(x0: &str, span1: Span) -> OxcDiagnostic {
Expand Down Expand Up @@ -76,10 +73,12 @@ impl Rule for NoRestrictedJestMethods {
}))
}

fn run_once(&self, ctx: &LintContext) {
for possible_jest_node in &collect_possible_jest_call_node(ctx) {
self.run(possible_jest_node, ctx);
}
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
self.run(jest_node, ctx);
}
}

Expand Down
14 changes: 8 additions & 6 deletions crates/oxc_linter/src/rules/jest/no_restricted_matchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, is_type_of_jest_fn_call, parse_expect_jest_fn_call,
JestFnKind, KnownMemberExpressionProperty, PossibleJestNode,
is_type_of_jest_fn_call, parse_expect_jest_fn_call, JestFnKind,
KnownMemberExpressionProperty, PossibleJestNode,
},
};

Expand Down Expand Up @@ -90,10 +90,12 @@ impl Rule for NoRestrictedMatchers {
}))
}

fn run_once(&self, ctx: &LintContext<'_>) {
for possible_jest_node in &collect_possible_jest_call_node(ctx) {
self.run(possible_jest_node, ctx);
}
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
self.run(jest_node, ctx);
}
}

Expand Down
14 changes: 8 additions & 6 deletions crates/oxc_linter/src/rules/jest/no_test_prefixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{
context::LintContext,
rule::Rule,
utils::{
collect_possible_jest_call_node, parse_general_jest_fn_call, JestGeneralFnKind,
KnownMemberExpressionProperty, ParsedGeneralJestFnCall, PossibleJestNode,
parse_general_jest_fn_call, JestGeneralFnKind, KnownMemberExpressionProperty,
ParsedGeneralJestFnCall, PossibleJestNode,
},
};

Expand Down Expand Up @@ -58,10 +58,12 @@ declare_oxc_lint!(
);

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

Expand Down
16 changes: 7 additions & 9 deletions crates/oxc_linter/src/rules/jest/no_untyped_mock_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{
context::LintContext,
rule::Rule,
utils::{collect_possible_jest_call_node, PossibleJestNode},
};
use crate::{context::LintContext, rule::Rule, utils::PossibleJestNode};

fn add_type_parameter_to_module_mock_diagnostic(x0: &str, span1: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(
Expand Down Expand Up @@ -92,10 +88,12 @@ declare_oxc_lint!(
);

impl Rule for NoUntypedMockFactory {
fn run_once(&self, ctx: &LintContext<'_>) {
for possible_jest_node in &collect_possible_jest_call_node(ctx) {
Self::run(possible_jest_node, ctx);
}
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
Self::run(jest_node, ctx);
}

fn should_run(&self, ctx: &crate::context::ContextHost) -> bool {
Expand Down
12 changes: 7 additions & 5 deletions crates/oxc_linter/src/rules/jest/prefer_called_with.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_span::Span;
use crate::{
context::LintContext,
rule::Rule,
utils::{collect_possible_jest_call_node, parse_expect_jest_fn_call, PossibleJestNode},
utils::{parse_expect_jest_fn_call, PossibleJestNode},
};

fn use_to_be_called_with(span: Span) -> OxcDiagnostic {
Expand Down Expand Up @@ -48,10 +48,12 @@ declare_oxc_lint!(
);

impl Rule for PreferCalledWith {
fn run_once(&self, ctx: &LintContext<'_>) {
for possible_jest_node in &collect_possible_jest_call_node(ctx) {
Self::run(possible_jest_node, ctx);
}
fn run_on_jest_node<'a, 'c>(
&self,
jest_node: &PossibleJestNode<'a, 'c>,
ctx: &'c LintContext<'a>,
) {
Self::run(jest_node, ctx);
}
}

Expand Down
Loading

0 comments on commit d6609e9

Please sign in to comment.