refactor(oxlint): drive hybrid routing from generated capabilities - #656
Conversation
Replace the hand-maintained `oxlintRuleMapping` table with a resolver
backed by generated capability data. The old table had drifted: five
rules were still mapped despite no longer being enabled anywhere in the
preset.
Layout: everything a generator writes now lives in `src/generated/`, and
the hand-written routing modules move to `src/oxlint/{adapters,routing}.ts`,
leaving `src/rules/` to mean only "rule maps". Both root lint configs
ignore the generated directory.
Payload: the generated artifacts hold rule names only. Oxlint names,
plugin aliases and package specifiers are derived via `jsPluginAdapterFor`,
and the human-facing metadata (categories, fix status, doc links) stays in
`src/oxlint/typegen.d.ts` where an editor can show it. This module loads on
every ESLint startup, so the runtime data went 719 KB -> 145 KB.
Two behavioural fixes:
- The drop path read the sampled compatibility view while the emit path
read the resolver, so a rule the preset enables only under an unsampled
option stayed in ESLint while oxlint also ran it. `test.jest.extended`
hit this and double-reported all five `jest-extended/*` rules. Both
paths now ask the resolver, so they agree by construction.
- Off-only entries were skipped wholesale, dropping the explicit `"off"`
the preset relies on to keep deliberately disabled native rules down.
Harmless at defaults, but a consumer setting `categories` got them back.
The skip now applies only to jsPlugin routes, which are the ones that
would load a plugin to carry a disable.
Also fixes `flawless/toml-sort-keys` being routed to oxlint, which cannot
lint TOML; matches the other non-JS flawless rules by name shape so a new
`toml-*` rule cannot repeat it.
The generator no longer depends on the host Node version (`nodeMajor` is
pinned through a shared `GENERATOR_NODE_MAJOR`), covers `jest.extended`,
and shares `requiresTypeChecking` with the type-aware split instead of
re-narrowing the metadata. `scripts/audit-native-parity.ts` asked for a
type-aware run in a tsconfig-less scratch dir and could never complete;
it now runs and reports.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ Deploy Preview for beamish-daffodil-b0f61d ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (14)
WalkthroughThe PR replaces static Oxlint mapping with generated capability data and resolver-based routing, updates hybrid ESLint/Oxlint handoff logic, relocates generated snapshots, and expands routing, parity, fixture, and documentation coverage. ChangesOxlint routing and generation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ESLintConfig
participant OxlintResolver
participant RuleSplitter
participant OxlintEngine
ESLintConfig->>OxlintResolver: resolve rule route
OxlintResolver->>RuleSplitter: provide target and translation
RuleSplitter->>OxlintEngine: emit native/jsPlugin rules
ESLintConfig->>OxlintEngine: remove rules handed to Oxlint
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/typegen-oxlint.ts`:
- Around line 507-520: Update the effectivePresetRuleNames construction around
rawRuleStates so every raw-enabled rule is added as well as raw-disabled rules,
excluding only LEGACY_PRESET_DISABLES and the intentional oxfmt/oxfmt removal.
Preserve the existing effectiveVariant handling and ensure rules scoped to
patterns such as GLOB_TYPE_TESTS remain represented in the routing compatibility
view.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: df9c7f3f-1660-4f8f-8fc4-79d94817feca
⛔ Files ignored due to path filters (6)
src/generated/oxlint-capabilities.tsis excluded by!**/generated/**src/generated/oxlint-native.tsis excluded by!**/generated/**src/generated/stylistic.tsis excluded by!**/generated/**src/generated/type-aware.tsis excluded by!**/generated/**test/__snapshots__/oxlint-fixtures.spec.ts.snapis excluded by!**/*.snaptest/__snapshots__/oxlint.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (35)
CLAUDE.mddocs/oxlint.mdeslint.config.tsoxlint.config.tsproject-words.txtscripts/audit-native-parity.tsscripts/config-factories.tsscripts/stylisticgen.tsscripts/typeawaregen.tsscripts/typegen-oxlint.tssrc/eslint/configs/e18e.tssrc/eslint/factory.tssrc/eslint/oxlint-drop.tssrc/eslint/type-aware-split.tssrc/eslint/types.tssrc/oxlint/adapters.tssrc/oxlint/configs/oxfmt.tssrc/oxlint/configs/typescript.tssrc/oxlint/index.tssrc/oxlint/override-diagnostics.tssrc/oxlint/routing.tssrc/oxlint/utils.tssrc/rules/javascript.tssrc/rules/oxlint-mapping.tssrc/rules/oxlint-native-generated.tssrc/rules/typescript.tstest/no-duplicate-imports.spec.tstest/oxlint-fixtures.spec.tstest/oxlint-helpers.tstest/oxlint-native-only.spec.tstest/oxlint-parity.spec.tstest/oxlint-routing.spec.tstest/oxlint-run.tstest/oxlint.spec.tstest/stylistic-rule-names.spec.ts
💤 Files with no reviewable changes (3)
- src/rules/oxlint-native-generated.ts
- src/eslint/configs/e18e.ts
- src/rules/oxlint-mapping.ts
| // Legacy compatibility disables name rules no current ESLint implementation | ||
| // owns. They remain useful as raw preset relaxations, but are not capabilities | ||
| // and must not enter the generated ownership view. | ||
| const LEGACY_PRESET_DISABLES = new Set(["no-new-symbol", "ts/no-dupe-class-members"]); | ||
| const presetRuleNames = new Set(effectiveEnabledRuleNames); | ||
| // Formatting has its own hybrid hand-off and is intentionally outside the | ||
| // rule resolver's public compatibility view. | ||
| presetRuleNames.delete("oxfmt/oxfmt"); | ||
| for (const [rule, enabled] of rawRuleStates) { | ||
| if (!enabled && !LEGACY_PRESET_DISABLES.has(rule)) { | ||
| presetRuleNames.add(rule); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'effectivePresetRuleNames|enabledPresetRuleNames' --type=tsRepository: christopher-buss/eslint-config
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Changed/mentioned files:"
git diff --name-only HEAD~1..HEAD 2>/dev/null || git diff --name-only 2>/dev/null || true
echo
echo "Locate typegen file and relevant symbols:"
fd -a 'typegen-oxlint\.ts|config-factories\.ts|configs/test\.ts|routing\.ts|native.*parity.*ts' . | sed 's#^\./##'
echo
echo "Search preset-related symbols:"
rg -n 'effectivePresetRuleNames|enabledPresetRuleNames|presetRuleNames|presetEnabledRuleNames|effectiveEnabledRuleNames|unknownFamilies|GLOB_TYPE_TESTS|typeTestRules' .Repository: christopher-buss/eslint-config
Length of output: 4792
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "scripts/typegen-oxlint.ts relevant section:"
sed -n '400,575p' scripts/typegen-oxlint.ts | cat -n
echo
echo "src/generated/oxlint-capabilities.ts line ranges around generated exports:"
sed -n '1930,1955p;2745,2770p' src/generated/oxlint-capabilities.ts | cat -n
echo
echo "scripts/audit-native-parity.ts consumers:"
sed -n '1,150p' scripts/audit-native-parity.ts | cat -n
echo
echo "src/oxlint/routing.ts consumers:"
sed -n '1,35p;270,310p' src/oxlint/routing.ts | cat -n
echo
echo "src/oxlint/utils.ts relevant area:"
sed -n '305,335p' src/oxlint/utils.ts | cat -n
echo
echo "test.ts config/rules relevant sections:"
sed -n '1,240p' src/eslint/configs/test.ts | cat -n
echo
echo "globs exports:"
sed -n '90,125p' src/globs.ts | cat -n
echo
echo "Check typeTestRules contents:"
rg -n -C3 'function typeTestRules|const typeTestRules|export function typeTestRules|typeTestRules\(' src/rules/test.ts scripts/typegen-oxlint.ts src/oxlint/configs/test.tsRepository: christopher-buss/eslint-config
Length of output: 28651
🌐 Web query:
TypeScript tsconfig composite project types rootDir outDir type roots typecheck source files
💡 Result:
In TypeScript, the "composite" configuration (enabled via "composite": true) allows a project to function as part of a larger, multi-project build orchestrated by tsc --build [1][2]. Enabling this flag enforces specific constraints designed to ensure that build outputs are stable and discoverable by other projects [3][1]. Key requirements and behaviors for composite projects include: 1. Declaration Generation: You must enable "declaration": true [3][4]. This is necessary because downstream projects consume the declarations (.d.ts files) rather than the source code of the referenced project [5][6]. 2. rootDir and outDir Configuration: - When "composite": true is set, the default for "rootDir" changes to the directory containing the tsconfig.json file (if not explicitly set otherwise) [3][7]. - It is standard practice to explicitly set "rootDir" (often to "src") and "outDir" (often to "dist") to ensure a stable, predictable output structure [5][6][2]. - TypeScript enforces that all files intended for emission must reside within the directory specified by "rootDir" [8]. 3. Input File Specification: All implementation files must be explicitly included via the "include" pattern or the "files" array [3][4]. Implicit file discovery is disabled; if a file is not matched by these configurations, TypeScript will report an error [3][9]. 4. Build Orchestration: - Composite projects are built using tsc --build (or tsc -b) [1][2]. This mode acts as a build orchestrator that computes a dependency graph and only rebuilds projects when necessary [1][6]. - A common pattern is to create a "solution" tsconfig.json file at the root of a monorepo. This file typically has "files": [] and a "references" array pointing to the individual project configuration files [1][9][2]. This root file serves only to define the build graph and does not contain source files itself [5][2]. Regarding "typeRoots," while it is not strictly constrained by the composite flag, it remains a standard compiler option [7]. In a composite/monorepo environment, ensure that any custom type declarations are properly resolved across project boundaries, often by using path aliases or workspace-level configuration if using a monorepo tool [10]. To verify your setup, ensure all referenced projects have "composite": true and that you are using tsc --build to trigger the compilation process rather than standard tsc [6][2].
Citations:
- 1: https://www.typescriptlang.org/docs/handbook/project-references
- 2: https://www.stanza.dev/courses/typescript-production/monorepos-references/typescript-production-project-references
- 3: https://www.typescriptlang.org/tsconfig/composite.html
- 4: https://github.com/microsoft/TypeScript-Handbook/blob/543d2dff/pages/Project%20References.md
- 5: https://www.resumelens.org/blog/typescript/typescript-monorepo-project-references
- 6: https://typescript.page/typescript-project-references-guide
- 7: https://www.typescriptlang.org/tsconfig/
- 8: https://www.typescriptlang.org/tsconfig/rootDir.html
- 9: https://github.com/microsoft/TypeScript-Website/blob/v2/packages/documentation/copy/en/project-config/Project%20References.md
- 10: https://nx.dev/blog/typescript-project-references
Don’t drop raw-enabled rules from effectivePresetRuleNames.
enabledPresetRuleNames keeps every raw-enabled rule, but effectivePresetRuleNames only adds raw-named rules when they are disabled unless they were already captured by effectiveVariant paths. Add the enabled ones too so a rule scoped to a glob like GLOB_TYPE_TESTS doesn’t appear in the “enabled somewhere” artifact while being omitted from the routing compatibility view.
🐛 Proposed fix
for (const [rule, enabled] of rawRuleStates) {
- if (!enabled && !LEGACY_PRESET_DISABLES.has(rule)) {
+ if (!LEGACY_PRESET_DISABLES.has(rule)) {
presetRuleNames.add(rule);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Legacy compatibility disables name rules no current ESLint implementation | |
| // owns. They remain useful as raw preset relaxations, but are not capabilities | |
| // and must not enter the generated ownership view. | |
| const LEGACY_PRESET_DISABLES = new Set(["no-new-symbol", "ts/no-dupe-class-members"]); | |
| const presetRuleNames = new Set(effectiveEnabledRuleNames); | |
| // Formatting has its own hybrid hand-off and is intentionally outside the | |
| // rule resolver's public compatibility view. | |
| presetRuleNames.delete("oxfmt/oxfmt"); | |
| for (const [rule, enabled] of rawRuleStates) { | |
| if (!enabled && !LEGACY_PRESET_DISABLES.has(rule)) { | |
| presetRuleNames.add(rule); | |
| } | |
| } | |
| // Legacy compatibility disables name rules no current ESLint implementation | |
| // owns. They remain useful as raw preset relaxations, but are not capabilities | |
| // and must not enter the generated ownership view. | |
| const LEGACY_PRESET_DISABLES = new Set(["no-new-symbol", "ts/no-dupe-class-members"]); | |
| const presetRuleNames = new Set(effectiveEnabledRuleNames); | |
| // Formatting has its own hybrid hand-off and is intentionally outside the | |
| // rule resolver's public compatibility view. | |
| presetRuleNames.delete("oxfmt/oxfmt"); | |
| for (const [rule, enabled] of rawRuleStates) { | |
| if (!LEGACY_PRESET_DISABLES.has(rule)) { | |
| presetRuleNames.add(rule); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/typegen-oxlint.ts` around lines 507 - 520, Update the
effectivePresetRuleNames construction around rawRuleStates so every raw-enabled
rule is added as well as raw-disabled rules, excluding only
LEGACY_PRESET_DISABLES and the intentional oxfmt/oxfmt removal. Preserve the
existing effectiveVariant handling and ensure rules scoped to patterns such as
GLOB_TYPE_TESTS remain represented in the routing compatibility view.
Follow-ups from the cleanup review. The generated capability data (~145 KB of Sets) was reachable from `src/eslint/factory.ts` through a static import chain, so every plain ESLint run parsed it even though nothing reads it with `oxlint: false`. Two links to cut: `oxlint-drop.ts` is now imported dynamically inside the `oxlintMode !== "off"` branch that already guards its only use, and the two hand-written Sets the type-aware split wanted moved to `adapters.ts`, which has no generated imports. Verified with a module-resolution hook: importing the factory now loads neither generated module; `oxlint: true` loads them on demand. `oxlintRuleMapping` was also built at module scope — ~800 resolves for a view the drop path never consults. The build moves behind `buildOxlintRuleMapping`, called once from `src/oxlint/index.ts` (which the ESLint factory never loads) for the public export, and memoized for the preset-scoped predicates. Those predicates are renamed to say what they are: `isPresetRuleOxlintCovered` and `isPresetRuleJsPlugin` read the sampled view, while `resolveOxlintRule` beside them is total, and nothing in the old names said so. That ambiguity had already bitten — `src/oxlint/configs/typescript.ts` gated *emission* on the sampled `isTsgolintRule`, the same resolver-vs-view asymmetry the drop path was just fixed to avoid. It now uses the resolver-backed `runsInTsgolint`. Also: - `isPruningViewConfig` is shared between the generator and the test helper instead of two hand-rolled name-prefix filters. They were not actually the same predicate: the generator skipped `isentinel/markdown/disables` too, and the helper's comment wrongly claimed they matched. - The vitest sampling variant re-derived four effective configs identical to the jest variant's; it now samples only the paths where the two differ. Generated output is byte-identical, ~360 ms off every `pnpm gen`. - `typeawaregen.ts` -> `type-aware-gen.ts` and `stylisticgen.ts` -> `stylistic-gen.ts`, so `typeaware` and `stylisticgen` come back out of the spell-check dictionary rather than being pinned there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
What
Replaces the hand-maintained
oxlintRuleMappingtable (~1100 lines, 742 entries) with a resolver backed by generated capability data, and fixes two behavioural defects found while doing it.The old table had genuinely drifted —
unicorn/no-loop-iterable-mutation,unicorn/isolated-functions,unicorn/consistent-compound-words,sonar/no-trivial-assertionsandsonar/no-use-of-empty-return-valuewere all still mapped despite no longer being enabled anywhere in the preset. DerivingtypeAwarefrommeta.docs.requiresTypeCheckinginstead of a hand-list means a dependency bump now produces a reviewable capability diff rather than silent drift.Layout
Everything a generator writes moves to
src/generated/(suffix dropped — the directory carries the meaning); hand-written routing moves tosrc/oxlint/{adapters,routing}.ts, leavingsrc/rules/to mean only "rule maps". Both root lint configs ignore the generated directory.src/rules/oxlint-native-generated.tssrc/generated/oxlint-native.tssrc/rules/oxlint-capabilities-generated.tssrc/generated/oxlint-capabilities.tssrc/rules/stylistic-generated.tssrc/generated/stylistic.tssrc/rules/type-aware-generated.tssrc/generated/type-aware.tssrc/rules/oxlint-mapping.tssrc/oxlint/routing.tsPublic API is unaffected —
package.jsonexports point at bundles andsrc/oxlint/index.tsre-exports by name.Payload
The generated artifacts now hold rule names only. Oxlint names, plugin aliases and package specifiers are derived via
jsPluginAdapterFor; the human-facing metadata (categories, fix status, doc links) stays insrc/oxlint/typegen.d.tswhere an editor can show it.documentationUrlwas being shipped twice — once as runtime data nothing read, once as@seeJSDoc.This module loads on every ESLint startup, so it matters: 719 KB → 145 KB.
A test asserts every generated rule name has an adapter, which is the invariant that lets the derivation replace the stored fields.
Two behavioural fixes
Hybrid double-reporting. The drop path read the sampled compatibility view while the emit path read the resolver. A rule the preset enables only under an unsampled option stayed in ESLint while oxlint also ran it —
test: { jest: { extended: true } }double-reported all fivejest-extended/*rules. Both paths now ask the resolver, so they agree by construction. Newoxlint hybrid hand-offsuite intest/oxlint-parity.spec.tsreproduces the failure and guards it.categoriesopt-in. Off-only entries were skipped wholesale, dropping the explicit"off"the preset relies on to keep deliberately disabled native rules down. Harmless at defaults (DEFAULT_CATEGORIESis all-off), but a consumer settingcategories: { correctness: "error" }got nativeno-unused-vars,getter-returnandno-setter-returnback against the preset's intent. The skip now applies only to jsPlugin routes — the ones that would load a plugin just to carry a disable.require-awaitis pinned again, matching pre-change behaviour.Also fixed along the way
flawless/toml-sort-keyswas being routed to oxlint, which cannot lint TOML. Making the drop path total exposed it. Non-JS flawless rules are now matched by name shape so a newtoml-*rule cannot repeat it.pnpm gen(e18e/prefer-get-or-insertis gated onnodeMajor). Pinned through a sharedGENERATOR_NODE_MAJOR.scripts/audit-native-parity.tsasked the oxlint factory for a type-aware run inside a tsconfig-less scratch dir, so tsgolint died before the first diagnostic — it could never complete. Now runs, and reports 5 pre-existing native divergences worth a separate look:jsdoc/require-param,jsdoc/require-returns,max-lines,no-console,unicorn/prefer-includes.staysInEslint(public) had lost its per-rule detail and the oxc#23290 link; restored.excludedFromOxlint,hasNativeOxlintRule, an unreachablepreference === "native"branch, and three generated fields nothing read.Verification
pnpm typecheckcleanpnpm lintclean (oxc + fast + typed passes)pnpm genbyte-stable across consecutive runs (idempotent and deterministic)Notes for review
style/*rules the oxfmt layer disables are now dropped from ESLint in hybrid mode. Behaviour-neutral — both engines disable them — but it movedtest/oxlint.spec.ts's "every dropped rule must be enabled in oxlint" invariant, which was measuring raw config unions. Rather than weaken it,formatterDisabledRulesexcludes rules theisentinel/oxfmt/*layer switches off, using the same discriminatorscripts/typegen-oxlint.tsalready uses.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation