Skip to content

Commit

Permalink
perf(linter): re-use possible jest nodes across rules
Browse files Browse the repository at this point in the history
  • Loading branch information
camchenry committed Oct 2, 2024
1 parent 585ccda commit 9fe0bfe
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 16 deletions.
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

0 comments on commit 9fe0bfe

Please sign in to comment.