Repository navigation
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
`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
m0g3r
force-pushed
the
fix/2231-warn-dropped-lint-rules
branch
from
September 28, 2026 13:01
af44ac9 to
a6f1298
Compare
fengmk2
pushed a commit
that referenced
this pull request
Oct 15, 2026
<!-- claude-projects-attribution: {"github_login":"Brooooooklyn"} -->
_Requested by **Brooklyn** · [project
thread](https://claude.ai/code/project/chan_018s9RxRdtH424j2EhXdzV3N?thread=cmsg_018s9RxRdtH424j2EhXdzV3NQxbeR8Pt7qaqSQwoq9rEFo)_
<!-- routine:prune-tests:duplicate-shell-and-helper-tests -->
Before: `vp_global_cli` and `vp_pm_cli` carried Rust unit tests that
could not fail when the product broke: an `assert!(true)` in
`create.rs`, four `env use` shell-detection tests that repeat
`commands::shell` tests for code `env use` only imports, and three tests
of test-only helpers. `upgrade_check.rs` had 20 hand-unrolled lookup
tests.
After: those 8 tests are gone and the 20 lookup tests are 2 table-driven
tests that keep every row and comment, with the failing row named in the
assertion message. Only test code changes: every hunk is inside a
`#[cfg(test)]` module or the `#[cfg(test)]`-only
`resolution::test_utils` module.
How: each candidate was defended by mutating product code with the most
realistic regression only it would catch, running the crate's tests, and
reverting. No deleted test failed alone on any mutation.
### Totals
| | Before (origin/main c04bcf6) | After |
|---|---|---|
| Files touched | – | 5 |
| `vp_global_cli` cases collected (Linux) | 502 (+2 ignored) | 479 (+2
ignored) |
| `vp_pm_cli` cases collected | 923 (+2 ignored) | 920 (+2 ignored) |
- Cases deleted: **8** of 1,425 collected in the two touched crates.
Repo-wide: about 2,432 `#[test]`/`#[tokio::test]` attributes in tracked
Rust (static count) and 1,492 static `it`/`test` calls in 101 JS spec
files (not collected; Vitest needs a full `packages/core` build).
- Cases folded: **20 into 2**, every row and comment kept.
Runner-collected total: −26.
- Net lines: **−159** (+42 / −201).
- Runtime saved: about 0.03 s (the 8 deleted cases each report 0.000 s;
the 20 folded cases summed 0.028 s against 0.011 s for the 2
replacements).
- Test-binary time on this shared machine: `vp_global_cli` 3.13 s → 3.32
s, `vp_pm_cli` 27.81 s → 29.06 s (noise).
### Per file
1. **`crates/vp_global_cli/src/commands/create.rs` ›
`test_create_command_module_exists`**
- Class: tautology (`assert!(true)`).
- Best defense: M1 (delegate `"new"` instead of `"create"`) passes all
502 tests, so this test cannot fail. The whole `mod tests` goes.
- Coverage gap (pre-existing, unchanged by this PR): no unit test covers
`vp create` delegation. It may be covered by the PTY snapshot suite,
which was not run.
2. **`crates/vp_global_cli/src/commands/env/use.rs` ›
`test_detect_shell_vp_shell_{powershell,fish,nu}`,
`test_detect_shell_posix_default`**
- Class: duplicates of stronger tests. `detect_shell` /
`Shell::from_str` live in `commands/shell.rs`; `env use` only imports
them.
- Still covered: `fish` is byte-identical to
`commands::shell::tests::test_detect_shell_vp_shell_fish`; `nu` to
`test_detect_shell_vp_shell_explicit`; lowercase `powershell` by
`test_shell_from_str` and `test_detect_shell_vp_shell_case_insensitive`;
`posix_default` by `test_detect_shell_defaults_without_vp_shell`.
- Best defenses M2–M5: each failed the nominee together with at least
one `commands::shell` test.
- The `use vp_shared::env_vars;` import is now `#[cfg(windows)]`, since
only the three Windows-only tests in the module still use it.
3. **`crates/vp_pm_cli/src/resolution/test_utils.rs` ›
`parses_args_without_a_program_name`,
`parses_subcommands_without_a_program_name`**
- Class: test tooling testing test tooling.
- Best defense: M7 fails 122 tests and M8 fails 17; every resolution
parser test exercises these helpers.
4. **`crates/vp_pm_cli/src/resolution/commands/list.rs` ›
`list_args_helper_keeps_pattern`** (and its test-only `list_args`
helper, which had no other caller)
- Class: test tooling testing test tooling; it only checked struct
construction.
- Best defenses: M9 fails only
`test_list_with_pattern_and_pass_through_args`; M10 fails
`test_parser_accepts_short_dev_and_global_flags` and
`cli::tests::projects_managed_global_commands`. The nominee passes both.
### Folds (`crates/vp_global_cli/src/upgrade_check.rs`)
- The 9 `is_newer_version_*` tests → `is_newer_version_compares_semver`
(12 rows, original comments kept, one comment added for the `0.0.0`
dev-build row).
- The 11 `should_run_for_*` / `should_not_run_for_*` tests →
`should_run_for_command_skips_excluded_and_machine_readable_commands`
(11 rows).
- M11, M12 and M13 still fail the folded tables.
### Kept after defense — do not re-nominate
None: no nominee survived its defense.
### Case counts
| file | before | after | delta | expected delta |
|---|---|---|---|---|
| vp_global_cli `commands/create.rs` | 1 | 0 | −1 | −1 |
| vp_global_cli `commands/env/use.rs` (Linux) | 16 | 12 | −4 | −4 |
| vp_global_cli `upgrade_check.rs` | 36 | 18 | −18 | −18 (20 folded into
2) |
| vp_pm_cli `resolution/commands/list.rs` | 45 | 44 | −1 | −1 |
| vp_pm_cli `resolution/test_utils.rs` | 2 | 0 | −2 | −2 |
| **Crates total** | 1,425 | 1,399 | −26 | −26 |
**0 new failures** (0 failed and 4 ignored, before and after).
### Mutations
Each was temporary and reverted (`git status` clean afterwards).
1. M1 `create.rs:15`: delegate `"new"` instead of `"create"`. Failed:
none.
2. M2 `shell.rs:30`: `"fish"` → `"fishshell"`. Failed: `use::…_fish`,
`shell::test_shell_from_str`, `shell::…_fish`.
3. M3 `shell.rs:31`: `"nu" | "nushell"` → `"nushell"`. Failed:
`use::…_nu`, `shell::…_explicit`, `shell::test_shell_from_str`.
4. M4 `shell.rs:32`: drop `"powershell"`. Failed: `use::…_powershell`,
`shell::…_case_insensitive`, `shell::test_shell_from_str`.
5. M5 `shell.rs:54`: default always `Cmd`. Failed:
`use::…_posix_default`, `shell::…_defaults_without_vp_shell`,
`shell::…_invalid_vp_shell_falls_back_to_default`.
6. M6 `shell.rs:49`: upper-case the value. Failed: none (equivalent
mutation; `from_str` lowercases).
7. M7 `test_utils.rs:70`: `test_argv` no longer prepends a program name.
Failed: 122 tests, including both nominees.
8. M8 `test_utils.rs:57`: `parse_subcommand` gets raw args. Failed: 17
tests (`config`, `dist_tag`, `owner`, `stage`, `token`, plus the
nominee).
9. M9 `list.rs:78-80`: pnpm drops the pattern. Failed:
`test_list_with_pattern_and_pass_through_args` only.
10. M10 `list.rs:11`: `pattern` becomes `#[arg(long)]`. Failed:
`test_parser_accepts_short_dev_and_global_flags`,
`cli::tests::projects_managed_global_commands`.
11. M11 `upgrade_check.rs:170` (after the fold): `>` → `>=`. Failed:
`is_newer_version_compares_semver`.
12. M12 `upgrade_check.rs:338` (after the fold): `Upgrade` removed from
the excluded commands. Failed:
`should_run_for_command_skips_excluded_and_machine_readable_commands`.
13. M13 `upgrade_check.rs:165` (after the fold): the `0.0.0` early
return removed. Failed: `is_newer_version_compares_semver`.
### What ran and what did not
- Ran, at origin/main and on this branch: `cargo test -p vp_global_cli`
and `cargo test -p vp_pm_cli` with `--report-time`; `cargo clippy -p
vp_global_cli -p vp_pm_cli --all-targets --all-features -- --deny
warnings` with the `just lint` allow-list; `cargo fmt --all -- --check`;
`git diff --check`. All pass.
- Not run: the full workspace `just test` / `just lint` / `just check`;
Windows targets (none installed here, so the `#[cfg(windows)]` import
gate was reviewed but not compiled; Windows CI will compile it); the
JS/Vitest suite; the PTY snapshot suite.
### Excluded pull requests
55 open PRs touch 737 files between them; none of the 5 files here is
among them. #3055 #3052 #3051 #3049 #3047 #3046 #3044 #3040 #3038 #3037
#3020 #3018 #3014 #3012 #3009 #3008 #3000 #2986 #2981 #2966 #2936 #2922
#2900 #2863 #2862 #2810 #2792 #2740 #2721 #2699 #2695 #2574 #2557 #2552
#2549 #2543 #2542 #2541 #2483 #2475 #2463 #2462 #2461 #2456 #2428 #2418
#2357 #2267 #2125 #1958 #1783 #1782 #1614 #1517 #1180.
### Age check (cutoff: 30 days, 2026-09-14)
First-added dates from `git log -S`, followed across the `vite_*` →
`vp_*` crate rename.
| Test | First added |
|---|---|
| `test_create_command_module_exists` | 2026-02-10 (a29ac83, #570) |
| `test_detect_shell_posix_default` | 2026-02-07 (194c0dc, #524) |
| `test_detect_shell_vp_shell_{powershell,fish,nu}` | 2026-05-24
(53ae721, #1658) |
| `test_utils` tests and `list_args_helper_keeps_pattern` | 2026-08-03
(c1574dd, #2140) |
| The 20 folded `upgrade_check` tests | 2026-03-30 (db75675) |
None was added for a bug or incident, and none is in a permission,
security or entitlement suite.
### Checks
| Check | Before | After |
|---|---|---|
| Tests `vp_global_cli` | 502 pass | 479 pass |
| Tests `vp_pm_cli` | 923 pass | 920 pass |
| Clippy (deny warnings) | pass | pass |
| rustfmt | – | pass |
| `git diff --check` | – | clean |
To revert: `git revert 687935d`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf
---
_Generated by [Claude
Code](https://claude.ai/code/session_01PxVsmvB9xfZKfVnUonY8kf)_
Co-authored-by: Claude <noreply@anthropic.com>
This branch has not been deployed
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