Skip to content

refactor(linter): migrate JS-only visitors to VisitJs#24530

Merged
graphite-app[bot] merged 1 commit into
mainfrom
visit-js-migration
Jul 15, 2026
Merged

refactor(linter): migrate JS-only visitors to VisitJs#24530
graphite-app[bot] merged 1 commit into
mainfrom
visit-js-migration

Conversation

@Boshen

@Boshen Boshen commented Jul 15, 2026

Copy link
Copy Markdown
Member

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).

@github-actions github-actions Bot added A-linter Area - Linter A-parser Area - Parser A-semantic Area - Semantic A-cli Area - CLI A-minifier Area - Minifier A-ast Area - AST A-transformer Area - Transformer / Transpiler A-ast-tools Area - AST tools A-formatter Area - Formatter labels Jul 15, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 66 skipped benchmarks1


Comparing visit-js-migration (df6c460) with main (91541dd)2

Open in CodSpeed

Footnotes

  1. 66 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (4279eca) during the generation of this report, so 91541dd was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Boshen
Boshen force-pushed the visit-js-migration branch from 7040cb8 to ff20b51 Compare July 15, 2026 02:22
@Boshen
Boshen force-pushed the visit-js-migration branch from ff20b51 to df6c460 Compare July 15, 2026 03:18
@Boshen Boshen changed the title refactor(linter,transformer,minifier,react_compiler): migrate JS-only visitors to VisitJs refactor(linter): migrate JS-only visitors from Visit to VisitJs Jul 15, 2026
@Boshen Boshen changed the title refactor(linter): migrate JS-only visitors from Visit to VisitJs refactor(linter): switch JS-only visitors from Visit to VisitJs Jul 15, 2026
@oxc-guard

oxc-guard Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@Boshen Boshen added the run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it label Jul 15, 2026
@oxc-guard

oxc-guard Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@oxc-guard oxc-guard Bot removed the run-monitor-oxc Add to a PR to dispatch oxc-project/monitor-oxc CI against it label Jul 15, 2026
@Boshen Boshen changed the title refactor(linter): switch JS-only visitors from Visit to VisitJs refactor(linter): migrate JS-only visitors to VisitJs Jul 15, 2026
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 camc314 added the 0-merge Merge with Graphite Merge Queue label Jul 15, 2026

camc314 commented Jul 15, 2026

Copy link
Copy Markdown
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
graphite-app Bot force-pushed the visit-js-migration branch from df6c460 to 161d2f1 Compare July 15, 2026 10:08
@graphite-app
graphite-app Bot merged commit 161d2f1 into main Jul 15, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 15, 2026
@graphite-app
graphite-app Bot deleted the visit-js-migration branch July 15, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools A-cli Area - CLI A-formatter Area - Formatter A-linter Area - Linter A-minifier Area - Minifier A-parser Area - Parser A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants