Fix an incompatibility with `eslint-plugin-jsdoc`. It relies on properties of `context` being values rather than getters:
https://github.com/gajus/eslint-plugin-jsdoc/blob/26276d428631a2625575607cba7875dab6797091/src/buildForbidRuleDefinition.js#L86-L103
When `options` property is a getter, this code throws an error because it attempts to define a property with both `value` and `get` in its descriptor.
This PR replaces the `id` and `options` getters with value properties.
The getters were there to prevent the `Context` object from being mutated by rule code, and allow the object to be frozen, as it is in ESLint. But each rule gets its own `Context` object, so if it chooses to mutate it, it only harms itself, and can't impact other rules/plugins. In ESLint, trying to alter the frozen `Context` object would be a hard error, so it's doubtful that any ESLint plugins do this anyway.
This is no real downside to this change for us, so we might as well make the change, and fix `eslint-plugin-jsdoc` and any other plugins which may depend on ESLint's behavior here.
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
Fixes#15631.
#16930 implemented options validation against `rule.meta.schema`, for the purpose of supporting default options added by schema. But it didn't actually do anything if validation failed.
Implement validation of options against schema, and pass any errors back to Rust to print.
ESLint has 2 different ways to define default options for a rule - `rule.meta.defaultOptions`, or `rule.meta.schema` containing a schema which defines default values.
We previously supported the former, but not the latter. This PR implements support for defining defaults via `rule.meta.schema`.
We compile the schema and apply it to options on JS side, using [`ajv`](https://www.npmjs.com/package/ajv), which is what ESLint uses. But this process only happens once before the start of linting, not once for every file, so the perf hit should be small.
This also prepares the way for validating options against the schema (#15631).
Previously we had a pattern of destructuring properties of global objects into top-level vars. e.g.:
```js
const { keys: ObjectKeys, hasOwn } = Object,
{ isArray } = Array;
```
Doing this is more performant, but it makes the code harder to read.
Instead, write code normally, and use a TSDown plugin to convert:
```js
const keys = Object.keys(obj);
```
to:
```js
import { ObjectKeys } from "../utils/globals.ts";
const keys = ObjectKeys(obj);
```
Defining all the destructured globals in a single file (`utils/globals.ts`) is advantageous for bundle size. If the TSDown plugin created top level `const ObjectKeys = Object.keys;` statements in each file, then you can end up with this in bundle as minifier doesn't de-duplicate:
```js
const ObjectKeys = Object.keys;
const ObjectKeys$1 = Object.keys;
const ObjectKeys$2 = Object.keys;
```
The globals we use have to be manually added to `utils/globals.ts`, but TSDown plugin warns when this needs to be done, and doesn't generate a broken build in the meantime.
This plugin is run in debug builds too, so that the tests will catch any bugs.
Throw an error if `rule.meta.defaultOptions` isn't JSON-serializable. This makes using `deepFreezeJsonArray` on it safe - can't lead to infinite loop if `defaultOptions` is self-referential.
`RuleTester` call `setOptions` to set up rule options, same as linter does. This makes sure the rule tester's behavior matches actual linter exactly.
It also prepares the way for applying defaults from `rule.meta.schema` (#16930).
Fix for an edge case.
Code for merging options was assuming that `defaultOptions` is a plain object with standard prototype, but that assumption doesn't actually hold. Alter the merging logic to ignore properties on prototype of objects in `defaultOptions`.
Closes#15630.
* #16170 added support for default options defined in the rule.
* #16215 added support for options defined in configs.
* #16217 implemented the logic for merging the 2 but only applied it in `RuleTester`.
Bring these elements together, and support merging default options from rule into options provided in config.
- Implements #14825
- Rebased #15822. Refactored. Pruned the slop. Tests passing.
### Big changes from the original approach
- [x] Options IDs are stored in their own arrays. This was laid out in
#16158. The original approach was to store `ruleId`-`optionId` pairs as
tuples in an array - several heap allocated objects and excessive
pointer indirection.
- [x] The rust code populates the options by calling a new JS callback
named `setupConfigs()`. I expect we would use this callback for more
plugin related data, including settings which are currently implemented
in a hacky way.
- [ ] Investigate a refactor to avoid requiring a mutable reference to
`ExternalPluginStore` everywhere.
<details>
<summary>
#### Smaller Changes
</summary>
- Removed the extra fixture ("working version"). There were no
meaningful changes compared to the first fixture. Probably slop.
</details>
---------
Co-authored-by: Peter Wagenet <peter@wagenet.us>
Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
Implement function for merging rule options from config with the rule's default options.
This function is not currently used, as we don't have a way to get options from config over from Rust to JS yet, but I've implemented it now, as it's also needed for the rule tester (#16206).
A step towards #15630. Where a rule provides default options, pass them to `create` function as `context.options`.
We don't yet have full options support (#14825), so this is incomplete. Once user can provide options for rules in config, those options need to be merged with default options.
Initially, we should probably do that merging on JS side (borrowing ESLint's code), and then later on port it to Rust.
Preparatory work for supporting rule options.
Store options for all rules in an `allOptions` array. Currently this array only contains a single value for empty options, but in future we'll populate it with options extracted from configs, sent over from Rust side.
Am adding this now, before full implementation (#14825), as it's required for rule tester.
Boshen