From 9fe0bfe8bcc2f2769ce73384d5f8dda876ba0bdb Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 25 Sep 2024 00:33:30 -0400 Subject: [PATCH] perf(linter): re-use possible jest nodes across rules --- crates/oxc_linter/src/lib.rs | 10 +++++++++ crates/oxc_linter/src/rule.rs | 6 ++++++ .../src/rules/jest/expect_expect.rs | 6 ++---- crates/oxc_linter/src/utils/jest.rs | 21 +++++++++---------- .../oxc_macros/src/declare_all_lint_rules.rs | 8 ++++++- 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index b75e629d59ba2..eefb13d9a5b3e 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -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, @@ -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() } diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index abd2b0b23a9c2..c48aecae2156e 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; use crate::{ context::{ContextHost, LintContext}, + utils::PossibleJestNode, AllowWarnDeny, AstNode, FixKind, RuleEnum, }; @@ -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 diff --git a/crates/oxc_linter/src/rules/jest/expect_expect.rs b/crates/oxc_linter/src/rules/jest/expect_expect.rs index f98d35b9a51b1..407cbca7c22fe 100644 --- a/crates/oxc_linter/src/rules/jest/expect_expect.rs +++ b/crates/oxc_linter/src/rules/jest/expect_expect.rs @@ -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); } } diff --git a/crates/oxc_linter/src/utils/jest.rs b/crates/oxc_linter/src/utils/jest.rs index 21d6c8c5d1e56..32ed36b2be89d 100644 --- a/crates/oxc_linter/src/utils/jest.rs +++ b/crates/oxc_linter/src/utils/jest.rs @@ -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; @@ -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 @@ -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> { // Some people may write codes like below, we need lookup imported test function and global test function. // ``` @@ -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)), ); @@ -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(_)) { @@ -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() @@ -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 + 'c { ctx.scopes() .root_unresolved_references() diff --git a/crates/oxc_macros/src/declare_all_lint_rules.rs b/crates/oxc_macros/src/declare_all_lint_rules.rs index b6e429b9f42dc..3f483f6cb0c04 100644 --- a/crates/oxc_macros/src/declare_all_lint_rules.rs +++ b/crates/oxc_macros/src/declare_all_lint_rules.rs @@ -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)] @@ -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)),*