fix(lsp): fix workspace worker selection for nested and similar-named workspaces#17853
fix(lsp): fix workspace worker selection for nested and similar-named workspaces#17853
Conversation
- Add helper method `find_worker_for_uri` that finds the most specific worker by path length - Update all worker lookups in backend.rs to use the new helper - Add comprehensive tests for nested workspaces and similar-named workspaces - Fix workspace option matching to use exact URI comparison Co-authored-by: Sysix <3897725+Sysix@users.noreply.github.com>
|
@copilot Remove |
- Inline the starts_with check directly into find_worker_for_uri - Remove is_responsible_for_uri method as it was only doing a simple check - Remove unit tests that tested is_responsible_for_uri directly - Integration tests still validate the functionality end-to-end Co-authored-by: Sysix <3897725+Sysix@users.noreply.github.com>
Done in 74c1c61. Removed |
|
@copilot do not write integration tests, unit tests are enough. Write them inside |
…r_uri - Remove integration tests from tests.rs - Add 5 unit tests in backend.rs module - Optimize find_worker_for_uri to avoid repeated to_file_path() calls - Use filter_map with tuple to compute path length once per worker Co-authored-by: Sysix <3897725+Sysix@users.noreply.github.com>
Done in 0682bc0. Replaced integration tests with 5 unit tests in |
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect workspace worker selection in the LSP server when multiple workspaces exist, particularly for nested workspaces (e.g., workspace and workspace/deeper) or similarly-named workspaces (e.g., workspace and workspace-2). The previous implementation used .find() which returned the first matching worker rather than the most specific one.
Changes:
- Added
find_worker_for_urihelper function that selects workers based on longest matching path instead of first match - Removed
is_responsible_for_urimethod fromWorkspaceWorkerand centralized the logic - Updated 11 worker lookup locations across LSP operations to use the new helper or exact URI comparison where appropriate
- Added comprehensive unit tests covering nested workspaces, similar-named workspaces, and edge cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/oxc_language_server/src/worker.rs | Removed is_responsible_for_uri method and its test, centralizing workspace matching logic |
| crates/oxc_language_server/src/backend.rs | Added find_worker_for_uri helper with longest-path matching; updated 11 call sites to use new helper or exact URI comparison; added 5 comprehensive unit tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Alexander S. <sysix@sysix-coding.de>
# Oxlint ### 🚀 Features - 9699a1b linter/prefer-global-this: Add suggestion (#17873) (Mikhail Baev) - 51c2815 linter/no-invalid-regexp: Add labels and help text to flag diagnostics (#17865) (camchenry) - 344d77d linter/no-ex-assign: Improve diagnostic with more detail (#17864) (camchenry) - 7d280e0 linter: Add fixer for `unicorn/no-useless-error-capture-stack-trace` rule (#17839) (Mikhail Baev) - af1d0e3 linter/prefer-optional-chain: Add rule (#17831) (camc314) - e3c4108 vscode: Add more supported languages to extension (#17812) (Alexander Lichter) - 4a46678 vscode: Activate extension on more languages (#17717) (Sysix) - b1298fc vscode: Sync formatter with supported files (#17615) (Alexander Lichter) - c7f0848 linte/rno-required-prop-with-default: Implement suggestion (#17747) (Minsu Lee) - 0e8127e linter/vue: Implement no-lifecycle-after-await (#17701) (yefan) - 3567304 linter/vitest: Implement `consistent-each-for` (#17601) (Said Atrahouch) - 883e156 linter: Add fixer for `unicorn/no-useless-collection-argument` rule (#17594) (Mikhail Baev) - 4eb335c linter/vitest: Implemented prefer-called-once (#17674) (Said Atrahouch) - 2bd2d5a linter/vitest: Implement hoisted-apis-on-top (#17658) (Said Atrahouch) - cfb2bcc linter/vue: Implement no-arrow-functions-in-watch (#17672) (yefan) - a68208a linter/eslint-plugin-vitest: Implements `prefer-describe-function-title` (#17677) (Said Atrahouch) - efa029f linter/vitest: Implement no-unneeded-async-expect-function (#17494) (Minsu Lee) ### 🐛 Bug Fixes - 49cf66e lsp: Fix workspace worker selection for nested and similar-named workspaces (#17853) (Copilot) - 84f4f3c linter: Add doc url for tsgolint diagnostics (#17879) (Sysix) - 76c903f linter/consistent-indexed-object-style: Skip fixing default exported interface (#17874) (Copilot) - 7e87d16 linter/tabindex-no-positive: Improve diagnostic phrasing (#17849) (connorshea) - 28f9fba vscode: Fix nested search for binaries (#17832) (Sysix) - 8ca2cd2 linter: Move jsx-a11y/no-static-element-interactions rule to nursery. (#17818) (connorshea) - dc9fdd6 linter/consistent-indexed-object-style: Re-port test cases and fix some bugs (#17802) (camc314) - 7bbd880 linter: Update prefer-destructuring rule metadata (#17642) (Hamir Mahal) - 3c45185 linter/consistent-indexed-object-style: False positive with circular reference (#17789) (heygsc) - bd186b4 vscode: Search for `oxlint` and `oxfmt` in every workspace directory (#17760) (Sysix) - 3e0dff7 linter/no-hooks: Add punctuation to diagnostic message (#17751) (camc314) - 6ae21f9 linter/prefer-called-once: Avoid panic on trailing comma (#17735) (Said Atrahouch) - 32c3901 oxlint: Do not panic on invalid `no-unused-vars` configuration (#17719) (Sysix) - 59a6228 parser: Detect TS1363 error for type-only imports with mixed default and named/namespace bindings (#17712) (Copilot) ### ⚡ Performance - f87a1e2 linter: Check for giving reserved plugin name before calling `load_plugin` on napi side (#17841) (Sysix) ### 📚 Documentation - a2b3a24 linter/no-caller: Improve docs and diagnostic for rule. (#17890) (connorshea) - aa48247 linter/no-unsafe-finally: Improve rule docs. (#17891) (connorshea) - 1b0bdee linter: Tweak docs for no-useless-constructor and hoisted-apis-on-top (#17888) (connorshea) - 8f24fa9 vscode: Remove mention of a built-in server (#17836) (Sysix) - e81a306 linter: Update the tsconfig flag mention for the import plugin. (#17778) (connorshea) # Oxfmt ### 🚀 Features - 539b350 formatter/sort_imports: Update `NODE_BUILTINS` modules (#17771) (nilptr) - 2e03ebf oxfmt/lsp: Use `SourceFormatter` to support non-JS files and napi features (#17655) (leaysgur) - 623f7eb oxfmt/sort_package_json: Use `options.sort_scripts` (#17740) (leaysgur) - 86c0168 oxfmt/sort_package_json: Handle `oxfmtrc.sort_scripts` option (#17738) (leaysgur) - 256636a oxfmt/lsp: Add `.editorconfig` to `get_watcher_patterns` (#17694) (leaysgur) - 3f3db39 oxfmt/lsp: Use `ConfigResolver` to align with CLI (#17654) (leaysgur) ### 🐛 Bug Fixes - fdd1e1e formatter: Don't wrap parenthesis for type assertion when it's an declaration of export default (#17878) (Dunqing) - f0813ad formatter: Incorrect type annotation check for short argument (#17877) (Dunqing) - 9e89389 formatter/tailwindcss: Nested class string doesn't respect `singleQuote: true` (#17838) (Dunqing) - e2f534c formatter/sort_imports: Handle alignable comment with JsLabels (#17791) (leaysgur) - f0cedd4 formatter/tailwindcss: Class name is broken after sorting when its contains single quotes with `singleQuote: true` (#17790) (Dunqing) - 1864142 oxfmt/tailwindcss: Bundle `prettier/plugins/*` (#17782) (leaysgur) - 3a9d43b oxfmt: Ignore explicit positional path which is ignored by directory (#17732) (leaysgur) - 0563217 formatter: Classes will be stripped out when both `experimentalTailwindcss` and `experimentalSortImports` are enabled (#17726) (Dunqing) ### ⚡ Performance - d1bc514 formatter: Optimize RegExpLiteral formatting to avoid heap allocations (#17797) (Dunqing) ### 📚 Documentation - 62b7a01 formatter: Clarify `experimentalTailwindcss` configuration comments (#17898) (Dunqing) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
…stroyWorkspace` (#17809) This PR introduces workspace isolation for the oxlint NAPI plugin system, adding `createWorkspace` and `destroyWorkspace` functions to manage multi workspace support for the language server. This does not fully solve the integration, but should be a good start for all future work. One example is the buffer for multiple calls at the same time (?). Added a new workspace isolation layer that uses the current working directory (cwd) as the workspace identifier: - `createWorkspace(cwd: string)`: Initializes a new isolated workspace for plugins and configurations - `destroyWorkspace(cwd: string)`: Cleans up workspace resources, unloading all associated plugins and options - `getResponsibleWorkspace(filePath: string)`: Determines which workspace is responsible for a given file path, reflecting the changes in #17853
The language server incorrectly routed files to workspace workers when multiple workspaces existed. With
[workspace, workspace/deeper], files in the nested workspace could be handled by the parent worker because the code used.find()which returns the first match rather than the most specific one.Changes
find_worker_for_urihelper that selects the worker with the longest matching path instead of first matchis_responsible_for_urimethodImplementation
For nested workspaces like
/workspaceand/workspace/deeper, both match files in the deeper path viastarts_with, but the helper now selects the longer (more specific) path. The implementation usesfilter_mapto compute path lengths once per worker, avoiding redundantto_file_path()calls for better performance.Tests
Added unit tests in
backend.rs:workspacevsworkspace/deeper)workspacevsworkspace-2)Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.