refactor(linter): migrate JS-only visitors to VisitJs#24530
Merged
Conversation
Boshen
requested review from
Dunqing,
camc314,
leaysgur and
overlookmotel
as code owners
July 15, 2026 02:07
Merging this PR will not alter performance
Comparing Footnotes
|
Boshen
force-pushed
the
visit-js-migration
branch
from
July 15, 2026 02:22
7040cb8 to
ff20b51
Compare
Boshen
force-pushed
the
visit-js-migration
branch
from
July 15, 2026 03:18
ff20b51 to
df6c460
Compare
Contributor
Contributor
Monitor OxcCommit:
|
graphite-app Bot
pushed a commit
that referenced
this pull request
Jul 15, 2026
…s from Visit to VisitJs (#24532) Migrate the transform-pipeline `Visit` implementors to `VisitJs` (#24499): `oxc_transformer` (3), `oxc_minifier` (4), `oxc_react_compiler` (1), plus the parser regex example and the coverage driver. Part of #24317; linter half in #24530. Behavior is unchanged. Together with #24530: `oxlint` −129.4 KiB (−0.94%) stripped, `oxc_transform` napi −16.2 KiB (−0.46%). ## Why each visitor can switch A visitor is safe on `VisitJs` when its hooks cannot observe anything inside the pruned type grammar — either the hooked node kinds are value-space-only, or the AST it walks cannot contain type nodes at that pipeline stage. | Visitor | File | Why equivalent | |---|---|---| | `PrivateInExpressionDetector` | `crates/oxc_transformer/src/decorator/legacy/mod.rs` | Hooks `visit_private_in_expression` (+`visit_decorators`); `#x in y` is runtime-only and cannot occur in type grammar, even inside decorator expressions carrying casts. | | `BindingMover` | `crates/oxc_transformer/src/es2017/async_to_generator.rs` | Its formal-parameter overrides visit only pattern + initializer (`BindingPattern` carries no type field — annotations are sibling fields it never touches); runs in the exit phase after TS strip. | | `UsedInJSXBindingsCollector` | `crates/oxc_transformer/src/jsx/refresh.rs` | Runs on un-stripped TSX, but its hooks (JSX-shaped calls, `visit_jsx_opening_element`, declarations/imports) cannot fire inside type grammar; its hand-written `visit_ts_type_annotation` skip-stub is deleted — `VisitJs` prunes that natively. | | `Finder` | `crates/oxc_react_compiler/src/react_compiler/entrypoint/program.rs` | Matches functions by `scope_id`; real JS function scopes never live in pure type grammar, and `VisitJs` still walks the places one could hide (enum initializers, namespace bodies, cast inner expressions). | | `KeepVar` | `crates/oxc_minifier/src/keep_var.rs` | Hooks `visit_statement`/`visit_variable_declaration`; statements and `var` declarations don't exist in type space, and namespace/enum bodies (which can contain them) are still walked by `VisitJs`. | | `DirectEvalFlagCheck` | `crates/oxc_minifier/src/peephole/mod.rs` | Debug-only; hooks `visit_call_expression` only — call expressions cannot occur in valid type grammar. | | `LiveDirectEvalCollector` | `crates/oxc_minifier/src/peephole/mod.rs` | Same single call-expression hook; type-space-immune regardless of input. | | `TestVisitor` | `crates/oxc_minifier/tests/ecmascript/prop_name.rs` | Test helper parsing `SourceType::default()` (plain JS); single non-recursing `visit_object_expression` hook. | | `RegularExpressionVisitor` | `crates/oxc_parser/examples/regular_expression.rs` | Finds `RegExpLiteral` / `new RegExp(...)`; neither node kind can occur in type grammar. | | `CheckASTNodes` | `tasks/coverage/src/driver.rs` | Its only active override is `visit_reg_exp_literal` (regex print idempotency); `RegExpLiteral` cannot occur in type space. (The commented-out `visit_span` full-AST check would need `Visit` if ever revived.) | Left on `Visit`: the minifier's incremental-scoping bookkeeping (`DropDiff`, `OverPruneCheck`, `LiveRefCollector`) — dropping `let x: Foo = …` must unmark the type-space `Foo` reference, which the `peephole::remove_unused_declaration` tests verify (they fail under `VisitJs`; confirmed and reverted during this work). `ChildScopeCollector` also stays: TS type nodes (mapped/conditional/function types) carry real `scope_id`s it must collect. --- 🤖 This PR was written with the assistance of Claude Code (Claude Fable 5).
camc314
approved these changes
Jul 15, 2026
Contributor
Merge activity
|
Migrate 33 JS-only `Visit` implementors in `oxc_linter` (plus the 3 in `tasks/rulegen`) to `VisitJs` (#24499). Part of #24317; transformer/minifier half in #24532. Behavior is unchanged — all 1169 rule tests pass with identical snapshots. Together with #24532: `oxlint` −129.4 KiB (−0.94%) stripped, `oxc_transform` napi −16.2 KiB (−0.46%). ## Why each visitor can switch A visitor is safe on `VisitJs` when its hooks cannot observe anything inside the pruned type grammar. The three ways that holds here: the hooked node kinds are value-space-only (`return`, `throw`, `await`, calls, real functions — none exist inside `TSType`), the visitor already skipped type space via hand-written `visit_ts_*` stubs, or its type inspection is by direct field access rather than walking. | Visitor | File (`crates/oxc_linter/src/`) | Why equivalent | |---|---|---| | `ReturnStatementFinder` | `rules/eslint/array_callback_return/return_checker.rs` | Hooks only `visit_return_statement`; `return` cannot occur in type grammar. | | `ComplexityVisitor` | `rules/eslint/complexity.rs` | All scoring hooks are value-space; the one explicit walk into property type annotations is deleted — valid type grammar contains no complexity-scoring constructs (no `?:`/`&&`/`?.`/default params). | | `StatementCounter` | `rules/eslint/max_statements.rs` | Counts statements via function/class/static-block body hooks; type declarations contain no statements. | | `YieldBeforeLoopExitFinder` | `rules/eslint/no_constant_condition.rs` | Hooks only `visit_yield_expression`; `yield` cannot occur in type grammar. | | `LoopFunctionCollector` | `rules/eslint/no_loop_func.rs` | Hooks `visit_function`/`visit_arrow_function_expression`; those nodes never appear in type grammar (`TSFunctionType` is a distinct node). | | `ReturnStatementFinder` | `rules/eslint/no_promise_executor_return.rs` | `return`-statement hook only. | | `ThrowFinder` | `rules/eslint/preserve_caught_error.rs` | Hooks only `visit_throw_statement`; `throw` cannot occur in type grammar. | | `AwaitFinder` | `rules/eslint/require_await.rs` | Hooks `visit_await_expression`/`visit_for_of_statement`; `await`/`for await` are runtime-only. | | `RequireCallScanner` | `rules/import/newline_after_import.rs` | Matches `require(...)` calls; calls cannot occur in valid type grammar. | | `ResolveFinder` | `rules/promise/no_multiple_resolved.rs` | `leave_node` records only value-space kinds (new/import/yield/member); its `visit_call_expression` doesn't descend. | | `ReturnWrapFinder` | `rules/promise/no_return_wrap.rs` | `return`-statement hook only. | | `AssertionVisitor` | `rules/shared/jest_vitest/expect_expect.rs` | Call/if/block hooks hunting assertion calls; none can fire in type space. | | `HookScanner`, `BodyScanner` | `rules/shared/jest_vitest/prefer_expect_assertions.rs` | Call + loop/function-body hooks; `expect.hasAssertions()` calls and loops cannot occur in types. | | `CallLikeExpressionVisitor` | `rules/shared/jest_vitest/prefer_mock_return_shorthand.rs` | call/new/tagged-template/import hooks — value-space only. (Sibling `IdentifierCollectorVisitor` stays on `Visit`: it collects identifiers, which do leak through type space.) | | `PromiseExpectScanner` | `rules/shared/jest_vitest/valid_expect_in_promise.rs` | call/await hooks. (Sibling `IdentifierFinder` stays on `Visit` for the same reason as above.) | | `AwaitDetector`, `ExposeAfterAwaitVisitor` | `rules/vue/no_expose_after_await.rs` | await/call hooks. | | `LifecycleAfterAwaitVisitor` | `rules/vue/no_lifecycle_after_await.rs` | await/call hooks. | | `WatchAfterAwaitVisitor` | `rules/vue/no_watch_after_await.rs` | await/expression-statement hooks. | | `ReturnFinder` | `rules/vue/require_direct_export.rs` | `return`-statement hook only. | | `ReturnVisitor` | `rules/vue/return_in_emits_validator.rs` | function/arrow depth tracking + return hook; none fire for `TSFunctionType`. | | `DefineOptionsChecker` | `rules/vue/valid_define_options.rs` | Matches `defineOptions()` calls; reads `type_arguments` by field access, not by walking. | | `LocalReferenceChecker` | `rules/vue/valid_define_options.rs` | Already skipped type space via a hand-written `visit_ts_type` stub ("`as X`/`typeof str` are statically resolvable") — `VisitJs` subsumes it; stub deleted. | | `ExhaustiveDepsVisitor` | `rules/react/exhaustive_deps.rs` | Already skipped type space via 4 noop `visit_ts_*` stubs (deleted). The `visit_ts_type_query` stub shows `typeof x` deps were deliberately not collected — `VisitJs` preserves exactly that. | | `ComponentFinder` | `rules/react/no_multi_comp.rs` | Component-defining hooks (class/function/call/object-property/assignment) are all value-space kinds. | | `MayThrowBeforeHook` | `rules/react/rules_of_hooks.rs` | `enter_node` treats only Call/New/Member kinds as may-throw; type annotations are erased at runtime and cannot throw. | | `ConstructorAssignmentCollector` | `rules/typescript/class_literal_property_style.rs` | Assignment-expression hook; assignments cannot occur in type grammar. | | `ReturnStatementChecker` | `rules/typescript/explicit_function_return_type.rs` | `return`-statement hook only. | | `ExplicitTypesChecker` | `rules/typescript/explicit_module_boundary_types.rs` | Inspects types by direct field access (`.return_type.is_none()`, `is_const_type_reference()`), never by walking; its 3 cast overrides survive on `VisitJs`. The deleted walk into `new` type args could only produce spurious "missing argument type" reports from function-typed type arguments. | | `AssignmentVisitor` | `rules/typescript/no_unnecessary_parameter_property_assignment.rs` | Assignment hook; RHS casts are unwrapped via `get_inner_expression`, not walker descent. | | `JsxFinder` | `utils/react.rs` | JSX/createElement hooks; JSX cannot occur in type grammar. All consumers pass value-space function bodies. | | `ThisExpressionFinder` | `utils/this_expression.rs` | The `this` type is `TSThisType`, which never fires `visit_this_expression`; the only leak path is a `typeof this.X` qualified name, invalid at the entry scopes its two consumers scan, and nested functions are already cut off by the `visit_function` stub. | | `TestCase`, `State`, `RuleConfig` | `tasks/rulegen/src/main.rs` | Extract value-space data (test `code` strings, schema object literals) via manual recursion/structural matching; `as`/`satisfies` wrappers are unwrapped with `get_inner_expression`, and `VisitJs` still walks those. | Left on `Visit` (results would change): `consistent_function_scoping`, `branches_sharing_code`, `prefer_arrow_callback`, `object_shorthand`, `class_methods_use_this`, the two jest identifier collectors above — their identifier/`this` hooks fire inside type space today (`typeof this`, type references) and that input feeds reports or autofixes. Also `no_loop_func`'s `NestedFunctionFinder`/`UnsafeReferenceFinder`, which share a `V: Visit<'a>` generic driver with one of them. --- 🤖 This PR was written with the assistance of Claude Code (Claude Fable 5).
graphite-app
Bot
force-pushed
the
visit-js-migration
branch
from
July 15, 2026 10:08
df6c460 to
161d2f1
Compare
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrate 33 JS-only
Visitimplementors inoxc_linter(plus the 3 intasks/rulegen) toVisitJs(#24499). Part of #24317; transformer/minifier half in #24532. Behavior is unchanged — all 1169 rule tests pass with identical snapshots.Together with #24532:
oxlint−129.4 KiB (−0.94%) stripped,oxc_transformnapi −16.2 KiB (−0.46%).Why each visitor can switch
A visitor is safe on
VisitJswhen its hooks cannot observe anything inside the pruned type grammar. The three ways that holds here: the hooked node kinds are value-space-only (return,throw,await, calls, real functions — none exist insideTSType), the visitor already skipped type space via hand-writtenvisit_ts_*stubs, or its type inspection is by direct field access rather than walking.crates/oxc_linter/src/)ReturnStatementFinderrules/eslint/array_callback_return/return_checker.rsvisit_return_statement;returncannot occur in type grammar.ComplexityVisitorrules/eslint/complexity.rs?:/&&/?./default params).StatementCounterrules/eslint/max_statements.rsYieldBeforeLoopExitFinderrules/eslint/no_constant_condition.rsvisit_yield_expression;yieldcannot occur in type grammar.LoopFunctionCollectorrules/eslint/no_loop_func.rsvisit_function/visit_arrow_function_expression; those nodes never appear in type grammar (TSFunctionTypeis a distinct node).ReturnStatementFinderrules/eslint/no_promise_executor_return.rsreturn-statement hook only.ThrowFinderrules/eslint/preserve_caught_error.rsvisit_throw_statement;throwcannot occur in type grammar.AwaitFinderrules/eslint/require_await.rsvisit_await_expression/visit_for_of_statement;await/for awaitare runtime-only.RequireCallScannerrules/import/newline_after_import.rsrequire(...)calls; calls cannot occur in valid type grammar.ResolveFinderrules/promise/no_multiple_resolved.rsleave_noderecords only value-space kinds (new/import/yield/member); itsvisit_call_expressiondoesn't descend.ReturnWrapFinderrules/promise/no_return_wrap.rsreturn-statement hook only.AssertionVisitorrules/shared/jest_vitest/expect_expect.rsHookScanner,BodyScannerrules/shared/jest_vitest/prefer_expect_assertions.rsexpect.hasAssertions()calls and loops cannot occur in types.CallLikeExpressionVisitorrules/shared/jest_vitest/prefer_mock_return_shorthand.rsIdentifierCollectorVisitorstays onVisit: it collects identifiers, which do leak through type space.)PromiseExpectScannerrules/shared/jest_vitest/valid_expect_in_promise.rsIdentifierFinderstays onVisitfor the same reason as above.)AwaitDetector,ExposeAfterAwaitVisitorrules/vue/no_expose_after_await.rsLifecycleAfterAwaitVisitorrules/vue/no_lifecycle_after_await.rsWatchAfterAwaitVisitorrules/vue/no_watch_after_await.rsReturnFinderrules/vue/require_direct_export.rsreturn-statement hook only.ReturnVisitorrules/vue/return_in_emits_validator.rsTSFunctionType.DefineOptionsCheckerrules/vue/valid_define_options.rsdefineOptions()calls; readstype_argumentsby field access, not by walking.LocalReferenceCheckerrules/vue/valid_define_options.rsvisit_ts_typestub ("as X/typeof strare statically resolvable") —VisitJssubsumes it; stub deleted.ExhaustiveDepsVisitorrules/react/exhaustive_deps.rsvisit_ts_*stubs (deleted). Thevisit_ts_type_querystub showstypeof xdeps were deliberately not collected —VisitJspreserves exactly that.ComponentFinderrules/react/no_multi_comp.rsMayThrowBeforeHookrules/react/rules_of_hooks.rsenter_nodetreats only Call/New/Member kinds as may-throw; type annotations are erased at runtime and cannot throw.ConstructorAssignmentCollectorrules/typescript/class_literal_property_style.rsReturnStatementCheckerrules/typescript/explicit_function_return_type.rsreturn-statement hook only.ExplicitTypesCheckerrules/typescript/explicit_module_boundary_types.rs.return_type.is_none(),is_const_type_reference()), never by walking; its 3 cast overrides survive onVisitJs. The deleted walk intonewtype args could only produce spurious "missing argument type" reports from function-typed type arguments.AssignmentVisitorrules/typescript/no_unnecessary_parameter_property_assignment.rsget_inner_expression, not walker descent.JsxFinderutils/react.rsThisExpressionFinderutils/this_expression.rsthistype isTSThisType, which never firesvisit_this_expression; the only leak path is atypeof this.Xqualified name, invalid at the entry scopes its two consumers scan, and nested functions are already cut off by thevisit_functionstub.TestCase,State,RuleConfigtasks/rulegen/src/main.rscodestrings, schema object literals) via manual recursion/structural matching;as/satisfieswrappers are unwrapped withget_inner_expression, andVisitJsstill walks those.Left on
Visit(results would change):consistent_function_scoping,branches_sharing_code,prefer_arrow_callback,object_shorthand,class_methods_use_this, the two jest identifier collectors above — their identifier/thishooks fire inside type space today (typeof this, type references) and that input feeds reports or autofixes. Alsono_loop_func'sNestedFunctionFinder/UnsafeReferenceFinder, which share aV: Visit<'a>generic driver with one of them.🤖 This PR was written with the assistance of Claude Code (Claude Fable 5).