wip: skip rules that do not have any relevant node types

Author: camchenry

.cargo/config.toml

@@ -8,6 +8,7 @@ benchmark = "bench -p oxc_benchmark"
 minsize = "run -p oxc_minsize --profile coverage --"
 allocs = "run -p oxc_track_memory_allocations --profile coverage --"
 rule = "run -p rulegen"
+lintgen = "run -p oxc_linter_codegen"
 
 # Build oxlint in release mode
 oxlint = "build --release -p oxlint --bin oxlint --features allocator"

.github/workflows/ci.yml

@@ -353,3 +353,30 @@ jobs:
           cargo run -p oxc_ast_tools
           git diff --exit-code ||
           (echo 'AST changes caused the "generated" code to get outdated. Have you forgotten to run the `just ast` command and/or commit generated codes?' && exit 1)
+
+  lintgen:
+    name: Linter changes
+    runs-on: ubuntu-latest
+    steps:
+      - uses: taiki-e/checkout-action@b13d20b7cda4e2f325ef19895128f7ff735c0b3d # v1.3.1
+      - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
+        id: filter
+        with:
+          filters: |
+            src:
+              - '.github/workflows/ci.yml'
+              - 'crates/oxc_linter/**'
+              - 'tasks/linter_codegen/**'
+
+      - uses: oxc-project/setup-rust@cd82e1efec7fef815e2c23d296756f31c7cdc03d # v1.0.0
+        if: steps.filter.outputs.src == 'true'
+        with:
+          cache-key: lintgen
+          save-cache: ${{ github.ref_name == 'main' }}
+
+      - name: Check linter changes
+        if: steps.filter.outputs.src == 'true'
+        run: |
+          cargo lintgen
+          git diff --exit-code ||
+          (echo 'Linter codegen has changed. Run the `cargo lintgen` command to update the linter code generated and commit it.' && exit 1)

Cargo.lock

@@ -4,6 +4,7 @@
 use std::{path::Path, rc::Rc, sync::Arc};
 
 use oxc_allocator::Allocator;
+use oxc_ast::ast_kind::AST_TYPE_MAX;
 use oxc_semantic::{AstNode, Semantic};
 
 #[cfg(all(feature = "oxlint2", not(feature = "disable_oxlint2")))]
@@ -25,6 +26,7 @@ mod module_graph_visitor;
 mod module_record;
 mod options;
 mod rule;
+mod rule_runner_impls;
 mod service;
 mod tsgolint;
 mod utils;
@@ -56,7 +58,7 @@ pub use crate::{
     module_record::ModuleRecord,
     options::LintOptions,
     options::{AllowWarnDeny, InvalidFilterKind, LintFilter, LintFilterKind},
-    rule::{RuleCategory, RuleFixMeta, RuleMeta},
+    rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleRunner},
     service::{LintService, LintServiceOptions, RuntimeFileSystem},
     tsgolint::TsGoLintState,
     utils::{read_to_arena_str, read_to_string},
@@ -167,7 +169,28 @@ impl Linter {
             // Collect rules into a Vec so that we can iterate over the rules multiple times
             let rules = rules.collect::<Vec<_>>();
 
+            // TODO: It seems like there is probably a more intelligent way to preallocate space here. This will
+            // likely incur quite a few unnecessary reallocs currently. We theoretically could compute this at
+            // compile-time since we know all of the rules and their AST node type information ahead of time.
+            let mut rules_by_ast_type = vec![Vec::new(); AST_TYPE_MAX as usize + 1];
+            // TODO: Compute needed capacity. This is a slight overestimate as not 100% of rules will need to run on all
+            // node types, but it at least guarantees we won't need to realloc.
+            let mut rules_any_ast_type = Vec::with_capacity(rules.len());
+
             for (rule, ctx) in &rules {
+                // Collect node type information for rules. In large files, benchmarking showed it was worth
+                // collecting rules into buckets by AST node type to avoid iterating over all rules for each node.
+                if rule.should_run(&ctx_host) {
+                    let (ast_types, all_types) = rule.types_info();
+                    if all_types {
+                        rules_any_ast_type.push((rule, ctx));
+                    } else {
+                        for ty in ast_types {
+                            rules_by_ast_type[ty as usize].push((rule, ctx));
+                        }
+                    }
+                }
+
                 rule.run_once(ctx);
             }
 
@@ -177,8 +200,12 @@ impl Linter {
                 }
             }
 
+            // Run rules on nodes
             for node in semantic.nodes() {
-                for (rule, ctx) in &rules {
+                for (rule, ctx) in &rules_by_ast_type[node.kind().ty() as usize] {
+                    rule.run(node, ctx);
+                }
+                for (rule, ctx) in &rules_any_ast_type {
                     rule.run(node, ctx);
                 }
             }
@@ -198,8 +225,19 @@ impl Linter {
                     rule.run_on_symbol(symbol, ctx);
                 }
 
-                for node in semantic.nodes() {
-                    rule.run(node, ctx);
+                // For smaller files, benchmarking showed it was faster to iterate over all rules and just check the
+                // node types as we go, rather than pre-bucketing rules by AST node type and doing extra allocations.
+                let (ast_types, all_types) = rule.types_info();
+                if all_types {
+                    for node in semantic.nodes() {
+                        rule.run(node, ctx);
+                    }
+                } else {
+                    for node in semantic.nodes() {
+                        if ast_types.has(node.kind().ty()) {
+                            rule.run(node, ctx);
+                        }
+                    }
                 }
 
                 if should_run_on_jest_node {

crates/oxc_linter/src/lib.rs

@@ -5,7 +5,7 @@ use std::{fmt, hash::Hash};
 use schemars::{JsonSchema, SchemaGenerator, schema::Schema};
 use serde::{Deserialize, Serialize};
 
-use oxc_semantic::SymbolId;
+use oxc_semantic::{AstTypesBitset, SymbolId};
 
 use crate::{
     AstNode, FixKind,
@@ -66,6 +66,16 @@ pub trait Rule: Sized + Default + fmt::Debug {
     }
 }
 
+pub trait RuleRunner: Rule {
+    /// `AstType`s that rule acts on
+    const NODE_TYPES: &AstTypesBitset;
+    /// `true` if codegen can't figure out what node types rule acts on
+    const ANY_NODE_TYPE: bool;
+
+    fn types_info(&self) -> (&'static AstTypesBitset, bool) {
+        (Self::NODE_TYPES, Self::ANY_NODE_TYPE)
+    }
+}
 pub trait RuleMeta {
     const NAME: &'static str;
 
@@ -279,6 +289,8 @@ impl From<RuleFixMeta> for FixKind {
 
 #[cfg(test)]
 mod test {
+    use crate::{RuleMeta, RuleRunner};
+
     use super::RuleCategory;
 
     #[test]
@@ -322,4 +334,83 @@ mod test {
             assert_eq!(de, RuleCategory::try_from(input).unwrap(), "{input}");
         }
     }
+
+    #[test]
+    fn test_rule_runner_impls() {
+        use crate::rules::*;
+        use oxc_ast::AstType::*;
+
+        // The RuleRunner code is automatically generated by the `oxc_linter_codegen` crate.
+        // This is set of manually verified test cases to ensure that the generated code
+        // is working as expected and is not skipping rules for nodes that actually should be linted.
+        assert_rule_runs_on_node_types(&eslint::no_debugger::NoDebugger, &[DebuggerStatement]);
+        assert_rule_runs_on_node_types(&eslint::no_with::NoWith, &[WithStatement]);
+        assert_rule_runs_on_node_types(
+            &eslint::curly::Curly::default(),
+            &[
+                IfStatement,
+                ForStatement,
+                ForInStatement,
+                ForOfStatement,
+                WhileStatement,
+                DoWhileStatement,
+            ],
+        );
+        assert_rule_runs_on_node_types(
+            &eslint::default_case::DefaultCase::default(),
+            &[SwitchStatement],
+        );
+        assert_rule_runs_on_node_types(
+            &import::no_mutable_exports::NoMutableExports,
+            &[ExportNamedDeclaration, ExportDefaultDeclaration],
+        );
+        assert_rule_runs_on_node_types(
+            &jest::prefer_jest_mocked::PreferJestMocked,
+            &[TSAsExpression, TSTypeAssertion],
+        );
+        assert_rule_runs_on_node_types(
+            &jsx_a11y::anchor_is_valid::AnchorIsValid::default(),
+            &[JSXElement],
+        );
+        assert_rule_runs_on_node_types(
+            &nextjs::no_head_element::NoHeadElement,
+            &[JSXOpeningElement],
+        );
+        assert_rule_runs_on_node_types(
+            &oxc::bad_bitwise_operator::BadBitwiseOperator,
+            &[BinaryExpression, AssignmentExpression],
+        );
+        assert_rule_runs_on_node_types(
+            &promise::prefer_await_to_callbacks::PreferAwaitToCallbacks,
+            &[CallExpression, Function, ArrowFunctionExpression],
+        );
+        assert_rule_runs_on_node_types(
+            &react::button_has_type::ButtonHasType::default(),
+            &[JSXOpeningElement, CallExpression],
+        );
+        assert_rule_runs_on_node_types(
+            &typescript::ban_types::BanTypes,
+            &[TSTypeReference, TSTypeLiteral],
+        );
+        assert_rule_runs_on_node_types(
+            &unicorn::explicit_length_check::ExplicitLengthCheck::default(),
+            &[StaticMemberExpression],
+        );
+    }
+
+    fn assert_rule_runs_on_node_types<R: RuleMeta + RuleRunner>(
+        rule: &R,
+        node_types: &[oxc_ast::AstType],
+    ) {
+        let (types, _) = rule.types_info();
+        assert!(!R::ANY_NODE_TYPE, "{} should not have ANY_NODE_TYPE set to true", R::NAME);
+        for node_type in node_types {
+            assert!(
+                types.has(*node_type),
+                "{}: missing {:?} in its NODE_TYPES (this means it will incorrectly skip nodes it needs to lint)",
+                R::NAME,
+                node_type
+            );
+        }
+    }
 }