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

perf(linter): re-use possible jest nodes across rules #6049

Closed
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
10 changes: 10 additions & 0 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use context::ContextHost;
use options::LintOptions;
use oxc_diagnostics::Error;
use oxc_semantic::{AstNode, Semantic};
use utils::{collect_possible_jest_call_node, PossibleJestNode};

pub use crate::{
builder::LinterBuilder,
Expand Down Expand Up @@ -149,6 +150,15 @@ impl Linter {
}
}

if self.options.framework_hints.contains(FrameworkFlags::Jest) {
let jest_nodes = collect_possible_jest_call_node(semantic);
for node in &jest_nodes {
for (rule, ctx) in &rules {
rule.run_on_jest_node(node, ctx);
}
}
}

ctx_host.take_diagnostics()
}

Expand Down
6 changes: 6 additions & 0 deletions crates/oxc_linter/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};

use crate::{
context::{ContextHost, LintContext},
utils::PossibleJestNode,
AllowWarnDeny, AstNode, FixKind, RuleEnum,
};

Expand All @@ -35,6 +36,11 @@ pub trait Rule: Sized + Default + fmt::Debug {
#[inline]
fn run_once(&self, ctx: &LintContext) {}

#[expect(unused_variables)]
#[inline]
fn run_on_jest_node<'a>(&self, jest_node: &'a PossibleJestNode<'a, 'a>, ctx: &LintContext<'a>) {
}

/// Check if a rule should be run at all.
///
/// You usually do not need to implement this function. If you do, use it to
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_linter/src/rules/jest/expect_expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,8 @@ 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>(&self, jest_node: &'a PossibleJestNode<'a, 'a>, ctx: &LintContext<'a>) {
run(self, jest_node, ctx);
}
}

Expand Down
21 changes: 10 additions & 11 deletions crates/oxc_linter/src/utils/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use oxc_ast::{
},
AstKind,
};
use oxc_semantic::{AstNode, ReferenceId};
use oxc_semantic::{AstNode, ReferenceId, Semantic};
use phf::phf_set;

use crate::LintContext;
Expand Down Expand Up @@ -138,6 +138,7 @@ pub fn parse_expect_jest_fn_call<'a>(
None
}

#[derive(Debug, Clone, Copy)]
pub struct PossibleJestNode<'a, 'b> {
pub node: &'b AstNode<'a>,
pub original: Option<&'a str>, // if this node is imported from 'jest/globals', this field will be Some(original_name), otherwise None
Expand All @@ -146,7 +147,7 @@ pub struct PossibleJestNode<'a, 'b> {
/// Collect all possible Jest fn Call Expression,
/// for `expect(1).toBe(1)`, the result will be a collection of node `expect(1)` and node `expect(1).toBe(1)`.
pub fn collect_possible_jest_call_node<'a, 'b>(
ctx: &'b LintContext<'a>,
semantic: &'b Semantic<'a>,
) -> Vec<PossibleJestNode<'a, 'b>> {
// Some people may write codes like below, we need lookup imported test function and global test function.
// ```
Expand All @@ -156,13 +157,13 @@ pub fn collect_possible_jest_call_node<'a, 'b>(
// expect(1 + 2).toEqual(3);
// });
// ```
let mut reference_id_with_original_list = collect_ids_referenced_to_import(ctx);
let mut reference_id_with_original_list = collect_ids_referenced_to_import(semantic);
if JEST_METHOD_NAMES
.iter()
.any(|name| ctx.scopes().root_unresolved_references().contains_key(*name))
.any(|name| semantic.scopes().root_unresolved_references().contains_key(*name))
{
reference_id_with_original_list.extend(
collect_ids_referenced_to_global(ctx)
collect_ids_referenced_to_global(semantic)
// set the original of global test function to None
.map(|id| (id, None)),
);
Expand All @@ -171,9 +172,9 @@ pub fn collect_possible_jest_call_node<'a, 'b>(
// get the longest valid chain of Jest Call Expression
reference_id_with_original_list.iter().fold(vec![], |mut acc, id_with_original| {
let (reference_id, original) = id_with_original;
let mut id = ctx.symbols().get_reference(*reference_id).node_id();
let mut id = semantic.symbols().get_reference(*reference_id).node_id();
loop {
let parent = ctx.nodes().parent_node(id);
let parent = semantic.nodes().parent_node(id);
if let Some(parent) = parent {
let parent_kind = parent.kind();
if matches!(parent_kind, AstKind::CallExpression(_)) {
Expand All @@ -196,9 +197,7 @@ pub fn collect_possible_jest_call_node<'a, 'b>(
})
}

fn collect_ids_referenced_to_import<'a>(
ctx: &LintContext<'a>,
) -> Vec<(ReferenceId, Option<&'a str>)> {
fn collect_ids_referenced_to_import<'a>(ctx: &Semantic<'a>) -> Vec<(ReferenceId, Option<&'a str>)> {
ctx.symbols()
.resolved_references
.iter_enumerated()
Expand Down Expand Up @@ -242,7 +241,7 @@ fn find_original_name<'a>(import_decl: &'a ImportDeclaration<'a>, name: &str) ->
}

fn collect_ids_referenced_to_global<'c>(
ctx: &'c LintContext,
ctx: &'c Semantic,
) -> impl Iterator<Item = ReferenceId> + 'c {
ctx.scopes()
.root_unresolved_references()
Expand Down
8 changes: 7 additions & 1 deletion crates/oxc_macros/src/declare_all_lint_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream {
let expanded = quote! {
#(pub use self::#use_stmts::#struct_names;)*

use crate::{context::{ContextHost, LintContext}, rule::{Rule, RuleCategory, RuleFixMeta, RuleMeta}, AstNode};
use crate::{context::{ContextHost, LintContext}, rule::{Rule, RuleCategory, RuleFixMeta, RuleMeta}, utils::PossibleJestNode, AstNode};
use oxc_semantic::SymbolId;

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -134,6 +134,12 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream {
}
}

pub(super) fn run_on_jest_node<'a>(&self, jest_node: &'a PossibleJestNode<'a, 'a>, ctx: &LintContext<'a>) {
match self {
#(Self::#struct_names(rule) => rule.run_on_jest_node(jest_node, ctx)),*
}
}

pub(super) fn should_run(&self, ctx: &ContextHost) -> bool {
match self {
#(Self::#struct_names(rule) => rule.should_run(ctx)),*
Expand Down
Loading