linter: duplicate work in Jest/Vitest rules
This is a big one. I haven't taken the time to write this down until now.
Problem
Most (if not all) eslint-plugin-jest rules use collect_possible_jest_call_node in a run_once block, then pass those nodes to an internal "run" implementation. For example,
// crates/oxc_linter/src/rules/jest/no_hooks.rs
impl Rule for NoHooks {
fn from_configuration(value: serde_json::Value) -> Self {
let allow = value
.get(0)
.and_then(|config| config.get("allow"))
.and_then(serde_json::Value::as_array)
.map(|v| {
v.iter().filter_map(serde_json::Value::as_str).map(ToString::to_string).collect()
})
.unwrap_or_default();
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);
}
}
}
This is problematic. Each jest rule repeats the same work, which wastes cycles. To make things worse, collect_possible_jest_call_node allocates nodes it discovers onto the heap.
(Possible) Solution
- Update the
Ruletrait, adding arun_on_jest_nodemethod. - Update
Linter::runto usecollect_possible_jest_call_nodeand pass a reference to them torule.run_on_jest_node - Remove all
run_onceimpls from jest/vitest rules.
Acceptance Criteria
Lintershould skip jest node collection if neither the jest or vitest plugins are enabled- Same as above, but non-test files should also be skipped, regardless if jest/vitest plugins are enabled or not
Extra Credit
Bonus points if you can refactor collect_possible_jest_node to return an impl Iterator instead of a Vec.
CC: @camchenry, who has actively been making performance improvements to the linter.
I attempted to do this, but I had an absolute heck of a time getting the lifetimes to work out properly. I'm not experienced enough with Rust yet to know exactly where the mistake is, so help appreciated: https://github.com/oxc-project/oxc/pull/6049
Closing this as done with https://github.com/oxc-project/oxc/pull/6722, we even got the extra credit 😄