Skip to content

Commit f2ad2d8

Browse files
committed
wip: skip rules that do not have any relevant node types
1 parent f6b3a21 commit f2ad2d8

File tree

12 files changed

+6610
-25
lines changed

12 files changed

+6610
-25
lines changed

Cargo.lock

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/oxc_linter/src/lib.rs

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use std::{path::Path, rc::Rc, sync::Arc};
55

66
use oxc_allocator::Allocator;
7+
use oxc_ast::ast_kind::AST_TYPE_MAX;
78
use oxc_semantic::{AstNode, Semantic};
89

910
#[cfg(all(feature = "oxlint2", not(feature = "disable_oxlint2")))]
@@ -25,6 +26,7 @@ mod module_graph_visitor;
2526
mod module_record;
2627
mod options;
2728
mod rule;
29+
mod rule_runner_impls;
2830
mod service;
2931
mod tsgolint;
3032
mod utils;
@@ -56,7 +58,7 @@ pub use crate::{
5658
module_record::ModuleRecord,
5759
options::LintOptions,
5860
options::{AllowWarnDeny, InvalidFilterKind, LintFilter, LintFilterKind},
59-
rule::{RuleCategory, RuleFixMeta, RuleMeta},
61+
rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleRunner},
6062
service::{LintService, LintServiceOptions, RuntimeFileSystem},
6163
tsgolint::TsGoLintState,
6264
utils::{read_to_arena_str, read_to_string},
@@ -167,7 +169,23 @@ impl Linter {
167169
// Collect rules into a Vec so that we can iterate over the rules multiple times
168170
let rules = rules.collect::<Vec<_>>();
169171

172+
// TODO: How should we preallocate this?
173+
let mut rules_by_ast_type = vec![Vec::new(); AST_TYPE_MAX as usize + 1];
174+
// TODO: compute needed capacity
175+
let mut rules_any_ast_type = Vec::with_capacity(1024);
176+
170177
for (rule, ctx) in &rules {
178+
if rule.should_run(&ctx_host) {
179+
let (ast_types, all_types) = rule.types_info();
180+
if all_types {
181+
rules_any_ast_type.push((rule, ctx));
182+
} else {
183+
for ty in ast_types {
184+
rules_by_ast_type[ty as usize].push((rule, ctx));
185+
}
186+
}
187+
}
188+
171189
rule.run_once(ctx);
172190
}
173191

@@ -177,8 +195,12 @@ impl Linter {
177195
}
178196
}
179197

198+
// Run rules on nodes
180199
for node in semantic.nodes() {
181-
for (rule, ctx) in &rules {
200+
for (rule, ctx) in &rules_by_ast_type[node.kind().ty() as usize] {
201+
rule.run(node, ctx);
202+
}
203+
for (rule, ctx) in &rules_any_ast_type {
182204
rule.run(node, ctx);
183205
}
184206
}
@@ -198,8 +220,17 @@ impl Linter {
198220
rule.run_on_symbol(symbol, ctx);
199221
}
200222

201-
for node in semantic.nodes() {
202-
rule.run(node, ctx);
223+
let (ast_types, all_types) = rule.types_info();
224+
if all_types {
225+
for node in semantic.nodes() {
226+
rule.run(node, ctx)
227+
}
228+
} else {
229+
for node in semantic.nodes() {
230+
if ast_types.has(node.kind().ty()) {
231+
rule.run(node, ctx);
232+
}
233+
}
203234
}
204235

205236
if should_run_on_jest_node {

crates/oxc_linter/src/rule.rs

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::{fmt, hash::Hash};
55
use schemars::{JsonSchema, SchemaGenerator, schema::Schema};
66
use serde::{Deserialize, Serialize};
77

8-
use oxc_semantic::SymbolId;
8+
use oxc_semantic::{AstTypesBitset, SymbolId};
99

1010
use crate::{
1111
AstNode, FixKind,
@@ -66,6 +66,16 @@ pub trait Rule: Sized + Default + fmt::Debug {
6666
}
6767
}
6868

69+
pub trait RuleRunner: Rule {
70+
/// `AstType`s that rule acts on
71+
const NODE_TYPES: &AstTypesBitset;
72+
/// `true` if codegen can't figure out what node types rule acts on
73+
const ANY_NODE_TYPE: bool;
74+
75+
fn types_info(&self) -> (&'static AstTypesBitset, bool) {
76+
(Self::NODE_TYPES, Self::ANY_NODE_TYPE)
77+
}
78+
}
6979
pub trait RuleMeta {
7080
const NAME: &'static str;
7181

@@ -279,6 +289,8 @@ impl From<RuleFixMeta> for FixKind {
279289

280290
#[cfg(test)]
281291
mod test {
292+
use crate::{RuleMeta, RuleRunner};
293+
282294
use super::RuleCategory;
283295

284296
#[test]
@@ -322,4 +334,83 @@ mod test {
322334
assert_eq!(de, RuleCategory::try_from(input).unwrap(), "{input}");
323335
}
324336
}
337+
338+
#[test]
339+
fn test_rule_runner_impls() {
340+
use crate::rules::*;
341+
use oxc_ast::AstType::*;
342+
343+
// The RuleRunner code is automatically generated by the `oxc_linter_codegen` crate.
344+
// This is set of manually verified test cases to ensure that the generated code
345+
// is working as expected and is not skipping rules for nodes that actually should be linted.
346+
assert_rule_runs_on_node_types(&eslint::no_debugger::NoDebugger, &[DebuggerStatement]);
347+
assert_rule_runs_on_node_types(&eslint::no_with::NoWith, &[WithStatement]);
348+
assert_rule_runs_on_node_types(
349+
&eslint::curly::Curly::default(),
350+
&[
351+
IfStatement,
352+
ForStatement,
353+
ForInStatement,
354+
ForOfStatement,
355+
WhileStatement,
356+
DoWhileStatement,
357+
],
358+
);
359+
assert_rule_runs_on_node_types(
360+
&eslint::default_case::DefaultCase::default(),
361+
&[SwitchStatement],
362+
);
363+
assert_rule_runs_on_node_types(
364+
&import::no_mutable_exports::NoMutableExports,
365+
&[ExportNamedDeclaration, ExportDefaultDeclaration],
366+
);
367+
assert_rule_runs_on_node_types(
368+
&jest::prefer_jest_mocked::PreferJestMocked,
369+
&[TSAsExpression, TSTypeAssertion],
370+
);
371+
assert_rule_runs_on_node_types(
372+
&jsx_a11y::anchor_is_valid::AnchorIsValid::default(),
373+
&[JSXElement],
374+
);
375+
assert_rule_runs_on_node_types(
376+
&nextjs::no_head_element::NoHeadElement,
377+
&[JSXOpeningElement],
378+
);
379+
assert_rule_runs_on_node_types(
380+
&oxc::bad_bitwise_operator::BadBitwiseOperator,
381+
&[BinaryExpression, AssignmentExpression],
382+
);
383+
assert_rule_runs_on_node_types(
384+
&promise::prefer_await_to_callbacks::PreferAwaitToCallbacks,
385+
&[CallExpression, Function, ArrowFunctionExpression],
386+
);
387+
assert_rule_runs_on_node_types(
388+
&react::button_has_type::ButtonHasType::default(),
389+
&[JSXOpeningElement, CallExpression],
390+
);
391+
assert_rule_runs_on_node_types(
392+
&typescript::ban_types::BanTypes,
393+
&[TSTypeReference, TSTypeLiteral],
394+
);
395+
assert_rule_runs_on_node_types(
396+
&unicorn::explicit_length_check::ExplicitLengthCheck::default(),
397+
&[StaticMemberExpression],
398+
);
399+
}
400+
401+
fn assert_rule_runs_on_node_types<R: RuleMeta + RuleRunner>(
402+
rule: &R,
403+
node_types: &[oxc_ast::AstType],
404+
) {
405+
let (types, _) = rule.types_info();
406+
assert!(!R::ANY_NODE_TYPE, "{} should not have ANY_NODE_TYPE set to true", R::NAME);
407+
for node_type in node_types {
408+
assert!(
409+
types.has(*node_type),
410+
"{}: missing {:?} in its NODE_TYPES (this means it will incorrectly skip nodes it needs to lint)",
411+
R::NAME,
412+
node_type
413+
);
414+
}
415+
}
325416
}

0 commit comments

Comments
 (0)