#26076 updated Oxlint JS plugin conformance snapshots, revealing that one ESLint Stylistic test was failing.
Turns out the cause was #25154 which (correctly) made it illegal for a `TSIndexSignature` to contain more than one parameter. The code in this test case is invalid. So just skip the test.
Conformance snapshots had got out of sync. At some point 1 test for ESLint Stylistic's `no-comma` rule has got broken (unclear when or how).
This PR doesn't attempt to fix the bug, but just makes sure the snapshot matches reality.
Fix the init script which initializes submodules for Oxlint JS plugins
conformance tests.
The script was broken by upgrade to PNPM 11 (#22224), which made
installing packages which have install scripts a hard error.
Fix by telling PNPM not to run packages' install scripts, but not to
raise an error about it either.
This also simplifies the way we track the conformance versions a bit, by centralizing the info into `repos.json`.
I know #20607 will conflict with this one, so feel free to merge whichever of these two you prefer, and then I'll resolve the conflicts afterward.
AI Disclosure: Generated with help from Claude Code.
AI Disclosure: Used Claude Code w/ Opus 4.6 for troubleshooting, basic
setup, and handling the weirdness of the snapshot tests. Manually
reviewed and verified the changes, iterated on it a good amount to make
sure the test failures that exist are actually valid, and that the
correct logic is used to validate fixers and failures are as-expected.
This one was a mess to figure out due to the complexity of the snapshot
testing system. I am fairly confident in this changeset and its accuracy
after I've iterated on it a good amount now with Claude and my own
testing.
There are a handful of failures/differences, however they appear to be
either actual differences, or due to unsupported features in the
snapshot testing system:
- There are some that use `// eslint-disable-line` which the snapshot
testing system doesn't support as far as I can tell
- There are some that use `"sourceType": "script"` and `/* exported
varname */`, which Oxlint also does not support. I assume we can skip
these?
- "Don't conflict even if using the rules together." failures are due to
not using the RuleTester in those tests and attempting to test multiple
rules together, should probably be skipped?
One difference here that appears to be a genuine bug in the JS plugins
code is the failures on `sort-flags`? I was able to reproduce one of the
failures (I only tested one manually) locally by running oxlint with the
plugin, and the `/\w/yvsimg` regex is not being fixed to sort the flags
by Oxlint. It is being fixed correctly by ESLint though, so it's
definitely a bug in our code. Seemingly, Oxlint doesn't recognize
`/\w/yvsimg` as a violation of the `regexp/sort-flags` rule at all. I've
created https://github.com/oxc-project/oxc/issues/20609 regarding this
issue and potentially fixing it.
See the plugin source code here:
https://github.com/ota-meshi/eslint-plugin-regexp/tree/788787a7e9e820c40321fe2f1095d00d4a486866
Result:
```
### Rules
| Status | Count | % |
| ----------------- | ----- | ------ |
| Total rules | 82 | 100.0% |
| Fully passing | 78 | 95.1% |
| Partially passing | 4 | 4.9% |
| Fully failing | 0 | 0.0% |
| Load errors | 0 | 0.0% |
| No tests run | 0 | 0.0% |
### Tests
| Status | Count | % |
| ----------- | ----- | ------ |
| Total tests | 2370 | 100.0% |
| Passing | 2349 | 99.1% |
| Failing | 9 | 0.4% |
| Skipped | 12 | 0.5% |
```
---------
Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
- Tried to keep it as simple as I could, basically copy-pasted the e18e
code and then had Claude help me with parsing issues I ran into. The
`stories.js` files that are used for these tests are actually JSX/TSX
and so I needed to force Oxlint to use the TypeScript/TSX parser by
overriding the file name. I am unsure why Storybook does this or if
there's a simpler fix, but it works for now.
- `--conditions=code` in the conformance command is seemingly necessary
for storybook's eslint plugin to be loaded, I am admittedly unsure if
that causes any problems for the other plugins but I was able to run all
of them (excluding reactHooks) fine locally after the change with no
diffs in the snaps, so ¯\\\_(ツ)_/¯. reactHooks wasn't working for me
before this change either, so I don't think that's a problem with my
change.
- The `no-uninstalled-addons` rule is skipped because it relies on
reading package.json files and that seems to be not-possible, as best as
I can tell. Or at least, I didn't feel it worthwhile to try to figure
out how to make it work after solving for the parser issues on the other
rules.
See the storybook plugin source code here:
https://github.com/storybookjs/storybook/tree/1ba2d0721a9b5e50dcb3974bae7f40d10d71c043/code/lib/eslint-plugin
AI Disclosure: As noted above, used Claude for troubleshooting/fixing
parser config issues.
Result:
```
### Rules
| Status | Count | % |
| ----------------- | ----- | ------ |
| Total rules | 16 | 100.0% |
| Fully passing | 16 | 100.0% |
| Partially passing | 0 | 0.0% |
| Fully failing | 0 | 0.0% |
| Load errors | 0 | 0.0% |
| No tests run | 0 | 0.0% |
### Tests
| Status | Count | % |
| ----------- | ----- | ------ |
| Total tests | 161 | 100.0% |
| Passing | 141 | 87.6% |
| Failing | 0 | 0.0% |
| Skipped | 20 | 12.4% |
```
(note that one of the rules is skipped, so I'd think the "No tests run"
should be 1?)
---------
Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
Prompted by #20262. Fix an omission in docs for conformance tests, and explain how to filter tests while working on adding a plugin to conformance suite.
Builds on #19497. Use the tokens generated by Oxc's parser in linter plugins, instead of running TS parser to tokenize source.
This is a sizeable perf gain, and also allows us to remove TS parser from `oxlint`'s bundle (#19531).
Additionally, our parser is more accurate than TypeScript - we handle HTML comments and space before slash in closing JSX elements (`< /div>`) - so this fixes a few conformance tests.
Refactor of conformance tests.
Sometimes a custom parser is a named export of a module, not the default export. Support this case in the conformance runner, and simplify the E18E conformance tests setup by using it.
Conformance tests need to mock some modules to replace ESLint / TSESLint's `RuleTester` with our own. Prevent errors during mocking when the module is transpiled from ESM and so the property to replace is a getter.
This supports #19184.
Conformance tests need to mock some modules to replace ESLint / TS-ESLint's `RuleTester` with our own, or to mock TS-ESLint parser.
Make this easier to do when then the module to be mocked is imported by test files via a chain of other imports by taking a `via` parameter. Example of usage in a doc comment added in this PR.
This supports #19184.
`RuleTester` test that suggestions provided by the rule match what's expected in the test case - that code after applying each suggestion matches the test case's `output` property, and that suggestions' descriptions also match those specified in test cases.
Skip one conformance test for ESLint's `prefer-regex-literals` rule where the suggestion description in Oxlint is different from ESLint, but in a completely cosmetic way. There's no need to match it exactly. That apart, no change to conformance test results, which strongly suggests that Oxlint is treating suggestions correctly.
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.
Add a `recursive` option to `RuleTester` which makes multiple rounds of fixes rather than a single round.
* `recursive: false` = no extra rounds of fixes.
* `recursive: 2` = 2 extra rounds.
* `recursive: true` = 10 extra rounds.
Default is `false`, which is ESLint's `RuleTester`'s behavior.
If a round produces no further fixes, the loop exits, so `recursive: 10` really means *up to* 10 rounds.
`eslint-plugin-stylistic` uses `eslint-vitest-rule-tester` for its tests, which offers this `recursive` option. Adding support for this option to our `RuleTester` fixes 12 failing conformance tests.
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.
Remove re-exports of `definePlugin`, `defineRule` and `RuleTester` from main `index` entry point of `oxlint` package.
We want this to be cheap:
```js
import { defineConfig } from "oxlint";
```
But with the deprecated exports, this would eagerly load ~20,000 lines of JS plugins-related code. See `index.js` in `dist` directory before and after this PR in release build. It was the import from `./lint.js` which was massive.
Personally, I don't think we need to keep the deprecated exports, for 3 reasons:
1. We have very clearly said that JS plugins are experimental and that breaking changes are very possible.
2. Given how broken our types are at present, probably few people were using `definePlugin` and `defineRule` anyway!
3. The fix for users who do get broken is trivial:
```diff
- import { definePlugin } from "oxlint";
+ import { definePlugin } from "@oxlint/plugins";
```
Part of #18610.
Our intent is to move `RuleTester` to a separate package `@oxlint/plugins-dev`.
So rename the current entry point which exports `RuleTester` from `oxlint/rule-tester` to `oxlint/plugins-dev`.
Part of #18610.
Our intent is to move `definePlugin`, `defineRule`, and `eslintCompatPlugin` to a separate package `@oxlint/plugins` (plural to avoid confusion with naming convention for plugins `oxlint-plugin-xxx`).
So rename the current entry point which exports these functions from `oxlint/plugin` to `oxlint/plugins` (plural).
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.
Add a `cwd` option to `RuleTester`.
Use this option in conformance tests to set `cwd` to the test files directory. This fixes the 1 test which #17809 broke.
Add `eslint-plugin-sonarjs` to conformance tests.
This was largely motivated by trying to track down the CFG malfunctions reported in #18555. But there are none to be found!
99.7% pass rate. The last 0.3% (14 tests) is rules which use `context.languageOptions.parser.parse`, which we haven't implemented yet.
Failing test cases for rules which are type-aware are skipped (385 tests), as we cannot support them.
`eslint-plugin-sonarjs` sets `filename` property of test cases to an absolute path. Potentially other plugins do this too.
In snapshots, convert to a path relative to repo root, so that snapshot contents will be the same on any machine it's run on. Also conform to unix-style `/`s.
Conformance tester was identifying known parsers specified by test cases by comparing the `parser` object to known values. Instead, compare `parser.parseForESLint` functions.
This produces an accurate match even when the test case object is deep cloned which results in the `parser` object being a new (unknown) object. But the `parseForESLint` function stays the same after cloning.
`eslint-plugin-sonarjs` does this kind of deep cloning.
When mocking modules containing `RuleTester`, don't replace the whole module, just alter the `RuleTester` property within it. e.g. when mocking the `RuleTester` export of `eslint` package, leave all package's other exports unchanged.
Catch errors in `describe(() => { ... })` blocks and turn them into test cases. This results in a failing test rather than the whole test file failing to load.
In conformance tester, process errors for when a test case was not run with the correct `RuleTester` in the same way as other errors. This allows skipping them if this behavior is expected (e.g. unit tests which don't use `RuleTester`).
In ES3, directives didn't exist yet, and therefore `"use strict"` is treated as a standard string literal, rather than a directive.
In conformance tests, when test case specifies ES3, walk the AST and set `directive` property of all `ExpressionStatement`s to `null`. This transforms any directives in AST into plain string literals.
Combined with #18632 which fixed `"use strict"` processing in TS-ESLint, this means there are 8 ESLint test cases we no longer have to skip. More importantly, we're less likely to run into erroneous test failures when testing other plugins, which are laborious to diagnose.
This is technically breaking (only for types), but custom plugins are experimental.
The ruletester/defineRule/definePlugin are re-exported but with a `@deprecated` tag which I think is a good enough trade off
I want to avoid users bringing in a ton of custom plugin code when doing `import { defineConfig } from 'oxlint'`
Add `eslint-plugin-stylistic` to conformance tests.
99.96% pass rate. The remaining 7 failing tests are not easily fixable:
#### `jsx-props-no-multi-spaces` rule
1 failing test
The rule clones nodes with `Object.assign({}, node)`, and then uses the `loc` property of the cloned node. Because `loc` is a getter on *prototype* of AST nodes, `loc` property is not present on the cloned node.
This bug may have real-world impact. We'll need to make `loc` a real property of node objects to fix it (#10307). We originally had `loc` getter on the node objects themselves, but that caused a 3x slowdown.
#### `jsx-tag-spacing` rule
6 failing tests
TypeScript parser (which we use for generating tokens) cannot parse closing JSX tags with whitespace before the `/` - e.g. `<App>text< /App>`.
TS-ESLint parser also fails to parse this [AST explorer](https://ast-explorer.dev/#eNpFyzEKwzAMBdCrGM0p3UModOkROnkRRgUH1xaWUlqC716pHbJJ7/+/Q4EZVnyhpJ5ZYQI20A/TH04kJVf3ZL5cmS+P1pZw9su0me6xhhCB0hPv1CW3GmE2KKgkGmE68huhbp3EC7+ZB6u8/de+kdOIdcD4AgUpMYU=), so this bug is unlikely to have real-world impact.
overlookmotel