Large change to how raw transfer stores metadata about the allocations it uses.
Previously the two metadata structures `RawTransferMetadata` and `FixedSizeAllocator` after the chunk's `ChunkFooter`.
This had a few problems:
1. It was confusing and unwieldy - a recipe for bugs.
2. `ChunkFooter`'s memory was exposed on JS side - a bit unsafe as altering bytes in this region could easily trigger UB.
3. It entwined `Arena` (which is just the thing that allocates) with the details of exactly _what_ raw transfer allocates.
4. Imposed annoying alignment requirements, because `ChunkFooter` must be aligned on 16, and so anything after it must ensure it doesn't break that invariant.
Old layout:
```
WHOLE BLOCK - aligned on 4 GiB
<-----------------------------------------------------> Allocated block (`BLOCK_SIZE` bytes)
ALLOCATOR
<-----------------------------------------> `Allocator` chunk (`CHUNK_SIZE` bytes)
<----> `ChunkFooter` (aligned on 16)
<-----------------------------------> `Allocator` chunk data storage (for AST)
(`ACTIVE_SIZE` bytes)
METADATA
<----> `RawTransferMetadata`
<----> `FixedSizeAllocatorMetadata`
BUFFER SENT TO JS
<-----------------------------------------------> Buffer sent to JS (`BUFFER_SIZE` bytes)
```
This PR moves `RawTransferMetadata` and `FixedSizeAllocatorMetadata` into the chunk itself. New layout:
```
WHOLE BLOCK - size 2 GiB - 16, aligned on 4 GiB
<-----------------------------------------------------> Allocated block (`BLOCK_SIZE` bytes)
ARENA
<-----------------------------------------------------> Chunk (fills whole block)
<--------------------------------------> Allocatable region for AST (`ACTIVE_SIZE` bytes)
<---> `RawTransferMetadata`
<---> `FixedSizeAllocatorMetadata`
<---> `ChunkFooter` (aligned on 16, last in block)
BUFFER SENT TO JS
<-------------------------------------------> Buffer sent to JS (`BUFFER_SIZE` bytes)
```
`FixedSizeAllocatorMetadata` and `ChunkFooter` are no longer in the region which is shared with JS side. As far as `Arena` is concerned, they're now just some data (like any other data) which is allocated in the arena.
Also:
- Introduce more consistency to the naming of constants which specify the size and position of these various data structures in the arena.
- Add more const assertions to ensure everything is laid out and aligned as it should be.
Oxlint's behavior when applying two fixes which are directly adjacent to each other is different from ESLint.
* ESLint considers adjacent fixes to be overlapping and only applies the 1st.
* Oxlint considers adjacent fixes NOT to be overlapping and applies them both.
Alter this behavior in `RuleTester` to follow ESLint's logic when `eslintCompat` option is used. This fixes the rest of the failing conformance tests which were broken by #19091.
This PR does not alter the logic for Oxlint itself. I think we should consider doing that, but it needs discussion, so I'll open a separate issue for it.
Fix an edge case bug where Oxlint was unable to handle fixes with with offset `-1`. Offset `-1` is used in files which contain a BOM to represent "before the BOM".
ESLint's `unicode-bom` rule produces a fix `{ range: [-1, 0], text: "" }` to remove BOM from a file with such a BOM.
`-1` can't be represented as a `u32`, so we have to take a rather annoying workaround - add 1 to offsets before sending them to Rust.
An alternative would have been to make `start` and `end` `i64`s so they can represent `-1`, but this would seem wasteful since we only need to represent -1, not any other negative value. So adding 1 to offsets is cheaper.
This fixes 2 ESLint conformance tests.
`RuleTester` test that fixes provided by the rule match what's expected in the test case - that code after applying fixes matches the test case's `output` property.
This reveals some bugs - 12 ESLint + 62 `eslint-plugin-stylistic` conformance tests are failing after this PR. These bugs are fixed in later PRs in this stack.
## 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
Previously we had to store source text at start of buffers sent to JS via raw transfer.
#18376 made changes to how raw transfer deserializer handles strings, in order to support files containing a BOM. Building on that, we're now able to remove the requirement that source text be at start of the buffer entirely.
This PR changes the deserializer used in Oxlint JS plugins to accept source text being anywhere in the buffer, *as long as no other strings are after it*. In practice this just means that the source text must be allocated before anything else, which is easy to satisfy.
Now the source text can be allocated with just the usual safe `allocator.alloc_str(source_text)` method.
This change removes a ton of dodgy workarounds and unsafe code we used previously to get source text at the start of buffer. It makes the code less labyrinthine and far less likely a slip up can inadvertently introduce UB.
Note: In `napi/parser`, source text still *is* at start of the buffer, as that's simpler and more efficient when the source text is written into the buffer on JS side. This change only affects Oxlint.
#18089 added support in parser for `commonjs` source type.
Extend this support to the NAPI packages - accept `sourceType: 'commonjs'` in options for `oxc-parser` and `oxc-transform`.
`setupConfigs` is not very descriptive - it is unclear if this is referring to oxlint configs, or rule configs, or something else.
This commit simply renames it to be explicit (`setupRuleConfigs`).
Please see the PR ontop of this for more info.
A couple of changes to how plugin names are dealt with:
### Package names
Don't use package name if the specifier used to load the plugin is a relative path.
If your project is called `super-server`, and you load a plugin from `../plugins/no-foo.js`, that plugin shouldn't be called `super-server` - that's the name of the project, not the plugin.
Now it throws an error, telling you to give the plugin a name.
### Ensure `context.id` matches the plugin name
Previously `context.id` might not match the actual plugin name (it could include e.g. `eslint-plugin-` prefix, for example). Now it always matches.
This required re-implementing `normalize_plugin_name` on JS side too.
### Tests
Add a bunch of tests for every combination I could imagine of `plugin.meta.name`, package name, and alias. Those tests make sure `context.id` is same as the plugin name on Rust side.
### Incomplete
There's 2 things remaining to get naming completely correct:
1. Support plugin names with slashes in them e.g. `@scope/eslint-plugin-name` (#14557).
2. It's possible for the same plugin to have different names in different places, because you can alias the same plugin twice with different names in a config, and an override. Plugin names (in `context.id`, and in `// oxlint-disable` comments) should reflect the plugin name *as it is in the config which enabled that rule*.
I'm leaving those 2 for now.
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.
Add an option to `RuleTester` to ignore non-fatal parsing errors.
```js
const tester = new RuleTester({
languageOptions: {
sourceType: "module",
parserOptions: {
ignoreNonFatalErrors: true,
},
},
});
tester.run("my-rule", rule, {
valid: [
// This is invalid code, but it gets a pass
"function f(x, x) {}",
],
invalid: [],
});
```
This will be useful in conformance tests, because ESLint's tests contain invalid syntax in many of their test cases!
Add options to the parser used in `RuleTester`. Currently `RuleTester` doesn't actually use these options, but this prepares the way for other changes to `RuleTester` in future PRs.
The extra types for `Comment` etc in `bindings.d.ts` are just the result of using `oxc_napi` crate for its `get_source_type` function. We don't use those types, but they're also not exposed externally, so it doesn't matter.
- This PR implements a naive, inefficient solution to #16359.
- To establish current and expected behavior, this PR serializes
rust-resolved and -overridden globals once for every file.
## Future work
- [ ] Add test for globals from the `env` field. Implement support if
not currently supported.
- [ ] Optimization: we want to serialize `globals` for all files only
once at the start of the lint run.
## AI Usage
- This PR was drafted by Claude Sonnet 4.5 in Plan + Agent mode, then
manually reviewed and iterated on.
---------
Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
We expect the common case to be that linting does not report any errors - because if there are errors, user will generally fix them!
Make a fast path for this common case - just return `null` from `lintFile`, skipping JSON serialization on JS side, and deserialization on Rust side.
- 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>
So far, settings have only been for use for the first party rules
written in Rust. Now that we support JS plugins, we need to provide
settings in the JS API.
- Fixes#14649
- [x] `OxlintSettings` now contains the full source settings in the new
`.json` field, in addition to the 5 well known fields used by internal
rust plugins.
- [x] Settings are serialized to the JS side as arguments of the
`lint_file` callback.
- [x] Overrides may also contain settings, which is handled.
- [x] Merging of settings when the config is extended.
- Direct commits from maintainers welcome.
#### Future Work
- Optimizing settings serialization frequency.
---------
Co-authored-by: Cameron Clark <cameron.clark@hey.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
fixes#14150
related: #14112, #14071
This seems to be the only reliable way to get arguments across different runtimes.
when running with deno, the args from `std::env::args_os()` look like:
```
[
"/Users/cameron/.deno/bin/deno",
"run",
"--ext=js",
"-A",
"/Users/cameron/github/camc314/oxlint-repros/issue-14150/node_modules/.deno/oxlint@1.18.0/node_modules/oxlint/bin/oxlint",
".",
]
```
however `process.argv` looks like:
```
[
"oxlint",
"/Users/cameron/github/camc314/oxlint-repros/issue-14150/node_modules/.deno/oxlint@1.18.0/node_modules/oxlint/bin/oxlint",
"."
]
```
so skipping the first two means that we end up with incorrect args that we're trying to parts.
There are (potentially) other solutions e.g. search for one of the args ending in`oxlint`, or other methods of trying to guess.
However, this feels like the most reliable solution.
Convert `oxlint` to a NAPI package.
This PR:
1. Moves files from `napi/oxlint` into `apps/oxlint`.
2. Converts `oxlint` into a NAPI package.
3. Alters the [release workflow](https://github.com/oxc-project/oxc/pull/13723/files#diff-80cb497c859bbaf7756da6ced2bca3d31933affc96102983eee1ad7d4bcc60fd) and [script](https://github.com/oxc-project/oxc/pull/13723/files#diff-4c4b7045c68805aeeae7c0af94cf7cf7487cb0403496da974a6a23accdc457d0) for building `oxlint` packages.
Most of necessary code modifications and refactoring was done in previous PRs (#13701, #13712, #13722, #13745), so this PR only includes changes necessary for packaging as NAPI.
I've done my best to update release workflow, but am mostly "flying blind" without ability to test it, so very likely there are mistakes.
A few notes and oddities:
* I've kept the `oxlint` NPM package as CommonJS. We could probably switch to ESM, but I thought it better to minimize the changes in this PR.
* NAPI-RS creates `.node` files named with a `-msvc` postfix for Windows. The scripts here remove that postfix so that filename of `.node` files and NPM package names match for all platforms, for simplicity.
* `tasks/website` has a dependency on `oxlint` as a normal library. This resulted in linker errors, so have had to put all the NAPI stuff behind `napi` cargo feature. Tests for `tasks/website` are run separately in CI, to avoid feature unification turning on the `napi` feature for `website`.
I've made a few comments below for the parts which aren't fairly self-explanatory.
camc314