Fixes#20416 for Oxlint
- Same `VP_VERSION` gating for config discovery, LSP file watchers, and JS config loading
- Refactor JS config loader into `resolveJsConfig` / `resolveVitePlusConfig` with shared `importConfig` + `loadConfigs`
- Split test fixtures into `vite_config_*` (non VP mode) and `vite_plus_*` (VP mode)
- Add `env` field to fixture `options.json` for passing env vars to test runs
- Fix repo `oxlintrc.json` ignore glob (`*/files/**` → `**/files/**`) for nested fixture paths like monorepo usecase
Follow-on after #19675.
The type for `lint` function that NAPI-RS produces is incorrect. `Option::None` on Rust side is translated to `null`, never `undefined`. Check this with a debug assertion, instead of handling `undefined` at runtime (which is pointless, because it never is `undefined`).
Fixes#19265.
It looks like the cause of the "Resource temporarily unavailable" errors was that NodeJS sets stdin/stdout into non-blocking mode when run from a non-TTY. In non-blocking mode it's possible to fill the stdin / stdout buffers faster than they're consumed, which leads to the error.
The language server communicates with `oxlint` via piped stdin/stdout, and can exchange a lot of data via these channels.
Solution is to switch to blocking stdio when it's detected that Oxlint is not being run from a TTY.
Oxfmt also ran into the same problem, and has a similar workaround:
https://github.com/oxc-project/oxc/blob/87a6724f167740ca116e73bf3f611d94ec7645e0/apps/oxfmt/src-js/cli.ts#L16-L23
In the case of Oxlint-language server, traffic on both stdin and stdout can be voluminous, so I've made both channels blocking (where Oxfmt only makes `stdout` blocking). I don't know if this is entirely necessary, but it seems better to err on the safe side.
---
I've tested this PR with both the plugins which were confirmed to cause the problem in #19265. Both now work, and both enabled at the same time also works.
---
References:
* https://github.com/oxc-project/oxc/issues/17939
* https://github.com/napi-rs/napi-rs/issues/1630
Credit to @camc314 for joining the dots and figuring out that this weird problem was connected to this.
## 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
`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.
- 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>
Rename assertion functions to clarify that they're not "real" assertions which we can rely on for validating input, and are only for compile-time testing or debug assertions.
`typeAssertIs` is not a great name, but I couldn't come up with anything better. `debugAssertIs` would be not be accurate, as it'd imply that in debug builds in performs an actual assertion, which it doesn't.
In any case, we should replace `typeAssertIs` with specialized assertion functions which do actually perform runtime assertions in debug builds (e.g. `typeAssertIs<Fix>()` -> `debugAssertIsFix()`).
After #15946, we're now free to use `assertIsNonNull` in `cli.ts` without breaking dead code removal. So use it instead of unchecked `!` assertions. In debug build it becomes a real runtime assertion.
Adapted from #15868. Enable `strictNullChecks` in TSConfig for `apps/oxlint`.
Rather than using `!` all over the place, I've added a function `assertIsNonNull`, which performs the same function. Idea is that we can later on produce a debug build where `assertIsNonNull` does an actual runtime assertion (rather than just a TS `asserts`) that the value is not `null` or `undefined`. We can use the debug build in tests, and catch any logic errors.
Unfortunately, TypeScript doesn't seem able to track side effects, so there seems to be no way to tell it "after calling `initAst()`, `ast` is always non-null". Don't want to switch to a having a `getAst` function which returns a non-null value, as that'd introduce unnecessary temp vars, and function calls, on hot paths. So this seems to be the best we can do without incurring runtime cost.
`loadPlugin` is already async, so we don't need to use `require(esm)` for lazy-loading the JS plugins-related code. Use `import` instead.
This has 3 advantages:
1. Removes the hacky workaround of a separate entry point for TSDown, and hard-coded path in `require('./plugins.js')`, which is only correct if TSDown config remains unchanged.
2. TypeScript can "see into" the `import` -> better type safety.
3. Support versions of NodeJS which don't support `require(esm)`.
On the last one, it's not strictly necessary, as `oxlint` requires a version of NodeJS with `require(esm)` support:
https://github.com/oxc-project/oxc/blob/b622ef87adf1bbed1e76ba0f610b653acb93e72b/apps/oxlint/package.json#L19-L21
But if user *is* on an older version of NodeJS, JS plugins should now work.
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.
leaysgur