## Summary
- reject JS plugins whose normalized names collide
- keep registered plugin names isolated per LSP workspace
- add a CLI regression fixture covering two plugins that expose the same rule name
Fixes#25164
This PR is prompted by comments from @Sysix on Discord:
> Found one weird bug, where `createWorkspace` is called before the old workspace instance is cleared. TBH I do not know how to fix this (or if this is `vscode-test-cli` problem).
I am also unclear how to fix this. So this PR just disables destroying workspaces for now.
It'd be ideal if we did remove `Workspace` objects to free up memory, but it's not *so* important for 2 reasons:
1. Usually workspace destruction only happens when you reload workspace, so it'll get discarded within seconds when `createWorkspace` over-writes it anyway.
2. Most of the data in the workspace is persisted anyway in NodeJS module cache, so destroying workspace doesn't free that much memory.
IMO it's better to hold onto a little memory unnecessarily (and it will be only a little) than to risk LSP getting broken in the common scenario of reloading a workspace.
Note: This could happen even when user is not using workspaces in their project. A non-workspace project is treated as a project with a single workspace.
Fixes#18802.
When a workspace is reloaded, previously:
* `destroyWorkspace` sets `currentWorkspaceUri` to `null`.
* `createWorkspace` sets `currentWorkspaceUri` back to the URL of the workspace (same value as before).
* Next call to `loadPlugin` checks if `currentWorkspaceUri === workspaceUri`.
* The two match, so it doesn't perform a workspace switch. Global state (including `registeredRules`) stays as it was before (`registeredRules` is not empty).
* `loadPlugin` adds rules to `registeredRules` but **without removing the old entries first**.
* Panic in `register_plugin` because number of registered rules on JS side and on Rust side are out of sync.
Fix this by setting `currentWorkspaceUri` to `null` in `createWorkspace`. On next call to `loadPlugin`, it forces a workspace switch, which replaces `registeredRules` with an empty array before rules start being added to it again.
Workspaces support (#17809) added overhead on JS side of hashmap lookups. Every call to `lintFile` has to look up details (e.g. registered rules, rule options) from hashmaps keyed by workspace URI.
### Perf
Take a different approach which is optimized for common case of there being only 1 workspace:
* At any time there's 1 "current workspace".
* Store rules, options, and CWD in top-level vars (back to how it was before #17809).
* Switch workspaces only when required. Switching reassigns the values in these top level vars.
Before:
* Get rules, options, and CWD from hashmaps keyed by workspace URI.
After:
* Quick check "is the current workspace the one we need?".
* If so, do nothing - the top-level vars `registeredRules`, `allOptions`, and `cwd` elready contain the right settings for the desired workspace.
* If current workspace is not the right one, look up the details for desired workspace in `workspaces` hash map and write them into the top-level vars.
In CLI, `workspaceUri` passed to `lintFile` etc is `null`, so the quick "in the right workspace?" check is particularly cheap - just `null === null`. So this reduces the overhead that workspace support adds to CLI to just that 1 instruction.
Add of these functions are entirely synchronous, so there's no possibility of a race where 1 call switches out the values in the top-level vars, while another call is busy using them.
### CLI: No call to `createWorkspace`
CLI now doesn't need to call `createWorkspace` at all. The "are we in right workspace?" check always succeeds, so it's irrelevant whether a workspace has been created or not - `workspaces` hashmap is never accessed.
So workspaces is now an LSP-only thing.
### Bundle
This PR also removes all imports from other files from `src-js/workspace/index.ts`. It now only imports `debugAssert`, which is optimized out in release build., leaving no imports at all.
Previously, the imports from other files were causing most of the JS plugins code to get loaded on first call to `createWorkspace`, which broke the lazy loading optimization - Oxlint CLI and LSP were both loading all the JS plugins code eagerly even if the user doesn't use JS plugins.
With this change, the lazy loading optimization is restored. This is visible in the conformance snapshot change in this PR.
Pure refactor. Remove these 2 types which were both just aliases for `string`.
In Rust type wrappers can be useful (e.g. `ScopeId`) but in TS they don't add type safety, and IMO make the code less readable.
## The problem
### 1. Bug with plugins outside of workspace directory
There was a bug in workspaces implementation. `loadPlugin` was assuming that the plugin file is within the workspace, but that's not necessarily the case. e.g.:
* Workspace 1: `/path/to/project/packages/foo`
* Workspace 2: `/path/to/project/packages/bar`
* JS plugin: `/path/to/project/node_modules/eslint-plugin-whatever`
This problem arises because `loadPlugin` is determining which workspace to load a plugin for by search for the workspace whose root directory contains the plugin file.
### 2. Performance
Every call to `loadPlugin`, `setupRuleConfigs`, or `lintFile` was first searching through all workspaces to find the right one (`getResponsibleWorkspace`).
This is really expensive when there are lots of workspaces, and quite expensive even when there's only 1 because iterating hashsets is relatively slow.
## This PR
This PR separates out CWD and workspace identifiers into 2 different concerns.
* Each workspace has an ID which is unique.
* Each workspace has a CWD (path to root directory as a file path).
The workspace ID is currently the workspace's root directory as a `file://...` URL. Using that as it's guaranteed unique, even for paths which are not valid UTF-8 (`Path::to_string_lossy` can produce the same `&str` for different paths). But ID it doesn't have to be the URI - we could change it later to an integer ID if we want.
Rust-side code passes the workspace ID to every call to `loadPlugin`, `setupRuleConfigs`, or `lintFile`.
In CLI, it just passes `null` as the ID. There's only ever 1 workspace in CLI, so no point in the expense of passing a `String` on every call.
These functions look up the CWD from the workspace ID in the `cwds` map.
Separating these 2 concerns solves both problems listed above:
1. Plugins can be outside of the workspace's CWD (see added test).
2. `getResponsibleWorkspace` is removed, solving the perf issue.
Note: The multiple hashmap lookups on each call to `lintFile` remain a perf drain. This is addressed in a later PR in this stack.
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 https://github.com/oxc-project/oxc/pull/17853
camc314