fix(migrate): report lint rules dropped during oxlint sanitization - #2542
Open
m0g3r wants to merge 1 commit into
Open
fix(migrate): report lint rules dropped during oxlint sanitization#2542m0g3r wants to merge 1 commit into
m0g3r wants to merge 1 commit into
Conversation
`sanitizeMigratedOxlintConfig` removes rules whose namespace no surviving
plugin contributes. That removal is necessary — Oxlint refuses to start on a
rule naming a plugin it cannot resolve ("Plugin 'x' not found") — but it
happened silently, while dropped plugins and jsPlugins already warned.
A local jsPlugin makes this visible. Its real namespace comes from the
plugin's `meta.name`, which cannot be derived from a path specifier such as
`./lint/kumo.js`, so every `kumo/*` rule was filtered out of the migrated
config with no output at all.
Collect the removed rule keys and warn with them, pointing at the object
form (`{ name, specifier }`) that lets a user name the namespace explicitly.
Refs voidzero-dev#2231
✅ Deploy Preview for viteplus-preview canceled.
|
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.
Refs #2231. This fixes the "silently" half of that issue and deliberately leaves the namespace-resolution half open — see What this leaves for #2231 below.
Problem
When
vp migratemerges.oxlintrc.jsonintovite.config.ts,sanitizeMigratedOxlintConfigremoves every rule whose namespace no surviving plugin contributes. DroppedjsPluginsand droppedpluginseach produce a warning; dropped rules produced nothing at all.The case #2231 reports makes it concrete. A local plugin is kept by the sanitizer (
partitionJsPluginspasses./-style specifiers through, because Oxlint resolves them itself), butderiveJsPluginNamespacehas only the path to work with and returns./lint/kumo.jsverbatim. No rule key can ever match that, so everykumo/*rule is filtered out of the migrated config and the user is told nothing.Why the rules still have to be dropped
I checked what Oxlint actually does before assuming the filtering could simply be relaxed, using the bundled oxlint 1.79.0 on a scratch project.
A rule under a namespace no plugin backs is not a warning — it stops Oxlint from starting at all:
So keeping unattributable rules would trade a silent config loss for
vp lintfailing outright on the whole project. Dropping them is the right behaviour; doing it without a word is not.Two other things that same probe established, both of which shape the fix:
meta.name— withmeta: { name: 'kumo' },kumo/no-fooresolves and fires (a.js:1:7: error kumo(no-foo): ...). The migrator has no way to know that from the path alone.jsPluginsobject form is a working escape hatch, andnamesets the namespace:{ "name": "kumo", "specifier": "./lint/kumo.js" }resolves identically, and{ "name": "renamed", ... }re-namespaces the same plugin torenamed/no-foo. The migrator already handles this form correctly (jsPluginsToNamespacesreadsentry.name), so it is a real remedy to point users at.Changes
filterRulesAgainstNamespacesrecords every key it removes into a caller-supplied set.sanitizeMigratedOxlintConfigaccumulates those keys across the base config and every override, then warns once, naming the rules and pointing at the{ name, specifier }form.Behaviour is otherwise unchanged: the same rules are dropped as before, and nothing new is kept.
What this leaves for #2231
The issue suggests resolving the real namespace by importing the plugin and reading
meta.name, which is exactly what Oxlint does. I have not done that here, because it is a design decision I do not think a contributor should make unilaterally:sanitizeMigratedOxlintConfigand its callermergeViteConfigFilesare synchronous, soimport()means making that chain async.vp migrateexecute arbitrary user plugin code, immediately after the ESLint cleanup has removed packages that plugin may well import.Alternatives that avoid both — a synchronous
requireof the plugin, statically extractingmeta.name, or having the migrator rewrite local jsPlugins into the object form so the namespace is recorded explicitly — each trade differently. Happy to implement whichever you prefer, here or as a follow-up. In the meantime this at least means the rules no longer vanish silently.Testing
Three tests added next to the existing sanitizer tests in
packages/cli/src/migration/__tests__/migrator.spec.ts: the #2231 local-plugin case, an override-rules case, and a negative case asserting no warning when nothing is dropped.Verified in both directions — with
migrator/eslint.tsreverted tomainand the tests kept, the two positive tests fail (AssertionError: expected undefined to be defined); with the change applied they pass. The negative test passes either way, as it should.Checks actually run on this branch:
vitest run packages/cli/src/migration— 411 passed across 10 files (408 onmainhere, plus these 3)vitest run(full unit suite) — 1025 passed, 1 skipped; 7 pre-existing snapshot failures inpackages/prompts/src/__tests__/render.spec.tsthat also fail on an unmodified tree in this environmentvp lint --type-aware --type-check— no diagnostics in either touched filevp fmt --check— both touched files cleanOne caveat on the lint run, in case anyone reproduces it:
pnpm buildgenerates an untrackedpackages/cli/src/migration/versions.ts, which in a local build carries only{ vite, vitest }. While it is present, type-checking reports two errors inmigrator/eslint.ts:139-141(an unused@ts-expect-errorand a missingversions.oxlint) that have nothing to do with this change — that code deliberately resolvesversions.jsfromdist/at runtime. With that generated file removed, both errors disappear and the only remaining diagnostics are 19 in the unbuiltdocs/workspace.Not run: the PTY snapshot suite and ecosystem e2e. This changes no CLI command output beyond adding a migration warning, which is reported through
MigrationReportand asserted directly in the tests.AI assistance
Claude Opus 5 wrote the implementation, the tests, the oxlint probes, and this description. The change is agent-authored and has not had a separate human review. Every result quoted above is from an actual run on this branch, not an estimate.
Generated by Claude Code