fix(linter): align lsp extends default plugins#22788
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns the oxlint LSP config-loading behavior with the CLI when resolving extends, ensuring the default plugin set is materialized consistently so nested/child workspace roots don’t accidentally lose default plugins (e.g., typescript) during extends merges.
Changes:
- Introduces a shared
materialize_default_pluginshelper and uses it in both CLI and LSP root-config loading paths beforeextendsresolution. - Updates CLI plugin initialization to use the shared helper (preserving existing behavior, but centralizing it).
- Adds an LSP regression fixture + snapshot test covering issue #22758 (missing TypeScript diagnostics under nested/extended configs).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/oxlint/src/config_loader.rs | Adds shared helper to materialize default plugins on root configs before further resolution. |
| apps/oxlint/src/lint.rs | Uses the shared helper in the CLI flow prior to applying CLI plugin overrides. |
| apps/oxlint/src/lsp/server_linter.rs | Uses the shared helper in the LSP flow and adds a regression test for #22758. |
| apps/oxlint/src/lsp/snapshots/fixtures_lsp_issue_22758_apps_api@app__auth__guard__firebase.ts.snap | New snapshot asserting the expected TypeScript diagnostic + code actions in LSP mode. |
| apps/oxlint/fixtures/lsp/issue_22758/apps/api/app/auth/guard/firebase.ts | Fixture TS file that should trigger a TypeScript rule diagnostic. |
| apps/oxlint/fixtures/lsp/issue_22758/.oxlintrc.json | Fixture parent config that explicitly sets plugins to reproduce the plugin-loss scenario. |
| apps/oxlint/fixtures/lsp/issue_22758/apps/api/.oxlintrc.json | Fixture child config extending the parent to validate correct plugin materialization via extends. |
Sysix
left a comment
There was a problem hiding this comment.
So every root config has Some(plugins)?
Maybe we can refactor it later into the root config loader mechanisms, good enough for me now 👍
Thank you
|
I can confirm that this fixes the issue. (tested on https://github.com/kerwanp/oxc-linter-lsp-repro) WIthout fix❯ node ./lsp-repro.mjs
LSP returned 0 diagnostic(s):WIth fix❯ node ./lsp-repro.mjs
LSP returned 2 diagnostic(s):
typescript(consistent-type-imports): All imports in the declaration are only used as types. Use `import type`.
typescript(no-wrapper-object-types): Do not use wrapper object types. |
yeah this is annoying it works like this - it should be refactored to make a bit more sense.
Thanks! |
Merge activity
|
Fixes #22758. The LSP path was not materializing the default plugin set before resolving a root config with `extends`, while the CLI path already did that before building the config. That meant a child config launched as the LSP workspace root could extend a parent config with an explicit `plugins` array and accidentally lose default plugins like `typescript`, even though `oxlint` CLI and `--print-config` showed those plugins as active. This moves the default-plugin materialization into a shared helper used by both CLI and LSP config loading, so both paths resolve the same effective root config before `extends` are applied. I added a regression fixture for the reported nested config shape and snapshot coverage for the missing TypeScript diagnostic.
167b185 to
1599f11
Compare
# Oxlint ### 🚀 Features - e4b1f46 linter/typescript: Implement `method-signature-style` rule (#22679) (Mikhail Baev) - bc462ca linter/vue: Implement no-reserved-component-names rule (#22741) (bab) - ef9e751 linter/vue: Implement component-definition-name-casing rule (#22818) (bab) - d67f51a linter/vue: Implement require-prop-type-constructor rule (#22708) (bab) - 1444f82 linter/promise/spec-only: Add `Promise.try` to `Promise` static methods (#22812) (Ben Saufley) - 8422e8b linter/jsdoc: Implement `require-yields-description` rule (#22805) (Mikhail Baev) - fe93f97 linter/eslint: Implement `prefer-named-capture-group` rule (#22759) (Sebastian Poxhofer) - 1a7798b linter: Add suggestion for `unicorn/no-new-array` (#22682) (Sysix) ### 🐛 Bug Fixes - 760a9f9 linter: Report errors when writing to the filesystem (#22881) (camc314) - e5a2748 linter: Avoid no-unreachable false positive after conditional loop (#22869) (camc314) - 39d92d6 linter/arrow-body-style: Preserve comments within function (#22854) (Sysix) - 3d13e29 parser: Reject `declare` in an already-ambient context (TS1038) (#22850) (Boshen) - 5152854 parser: Reject statements in ambient contexts (TS1036) (#22849) (Boshen) - 2eafea6 parser: Reject function implementations in ambient contexts (TS1183) (#22845) (Boshen) - c645615 parser: Reject incompatible class member modifiers (#22843) (Boshen) - 4a1ca4a linter/export: Detect duplicate explicit exports (#22798) (camc314) - 0a9a735 linter/no-loop-func: Allow safe let closures (#22811) (camc314) - 1599f11 linter: Align lsp extends default plugins (#22788) (camc314) - db32ec9 linter/no-accumulating-spread: Use loop as primary span (#22800) (camc314) - 33ec6b4 linter/consistent-test-it: Avoid adjacent describe leakage (#22796) (camc314) - 2606069 linter/no-array-sort: Unwrap parenthesized sort args (#22794) (camc314) - 9f2f709 linter/no-array-sort: Skip non compare fn sort arguments (#22752) (Gaurav Dubey) - 27268a0 linter/no-else-return: Preserve statement boundary in fixer (#22687) (camc314) - d9cb6d8 linter/no-empty-function: Allow functions callbacks with `allow: functions` (#22764) (camc314) - a40a314 linter/no-shadow-restricted-names: Ignore enum members (#22762) (camc314) - 82366d9 linter/no-cond-assign: Align ternary handling (#22761) (camc314) ### 📚 Documentation - 5e113ba linter: Add license notices for ported ESLint plugins (#22768) (Boshen) # Oxfmt ### 🚀 Features - d75cbbf oxfmt: Format `parser:json` files by `oxc_formatter_json` (#22709) (leaysgur) - 49db054 formatter_json: Implement `oxc_formatter_json` (json variant only) (#22641) (leaysgur) - 9c71f2e ast, codegen, formatter: Add `WithClauseKeyword::as_str` helper and use it (#22791) (camc314) ### 🐛 Bug Fixes - d3cdd62 oxfmt: Skip formatting for whitespace-only file (#22780) (leaysgur) - 23f0cc8 formatter: Don't move comments inside variable declaration in for in loop (#22776) (leaysgur) - f200c40 formatter: Don't move comments inside variable declaration in for of loop (#22773) (Leonabcd123) ### 📚 Documentation - 845f393 oxfmt,formatter,formatter_json,formatter_core: Add/update AGENTS.md (#22873) (leaysgur)
…t config (#22903) Follow up from #22896 where we respected the parent (omitted) default plugins. This PR does the same for the child config, example: ``` { "extends": ["./other-config.json"], } ``` Will now enable the default plugins too. This is not detected by the root config, where we always have a plugin vec. See #22788 (review) fixes #22867
Fixes #22758.
The LSP path was not materializing the default plugin set before resolving a root config with
extends, while the CLI path already did that before building the config. That meant a child config launched as the LSP workspace root could extend a parent config with an explicitpluginsarray and accidentally lose default plugins liketypescript, even thoughoxlintCLI and--print-configshowed those plugins as active.This moves the default-plugin materialization into a shared helper used by both CLI and LSP config loading, so both paths resolve the same effective root config before
extendsare applied.I added a regression fixture for the reported nested config shape and snapshot coverage for the missing TypeScript diagnostic.