The goal here is hopefully to make our CI a bit greener.
9/10 times, the failure of windows napi CI is due to memory failing to allocate. This PR retries the test, (hopefully) making it less flaky).
We should remove this once https://github.com/oxc-project/oxc/issues/19395 is fixed
In JS plugins tests, it's sometimes required to ensure that files are linted in deterministic order.
For example, #17207 fixes a bug where some internal global state was not reset correctly after an error is thrown during visiting the AST. This affects what happens when linting the *next* file.
In order to write a test for this, and produces deterministic output in the snapshot file, the order files are linted in needs to be deterministic.
This PR provides this determinism by:
1. Adding a `force_deterministic_order` cargo feature which is enabled in test build.
2. When this feature is enabled *and* Oxlint is run with `--threads 1`, file paths are sorted before linting.
3. Adding an option for test fixtures `singleThread`. When `true`, the fixture is run with `--threads 1`.
Aim is to only enable deterministic ordering in test fixtures which *need* determinism, so that most test fixtures are still testing Oxlint running as it does usually for a user.
Tests in the same file typically run in sequence. we can improve the performance by running these in parallel:
https://vitest.dev/guide/parallelism.html#test-parallelism
- Before (sequential): ~5.2s total, e2e.test.ts ~5000ms
- After (concurrent): ~1.50s total, e2e.test.ts ~1200ms
~3.5x faster
#15921 added a debug build for Oxlint, and various PRs since have tweaked it. However, I now realize there's little point in generating *both* a release build and debug build.
Just create one build, and use env var `DEBUG` to control whether it has debug assertions included in it or not.
`pnpm run build` produces release build with no debug assertions, `pnpm run build-test` includes the asserts. I've verified that both do what they're meant to.
Generate a debug build, in addition to the normal release build.
In the debug build, `DEBUG` global is defined as `true` and `assertIsNonNull` does a runtime assertion. In release build, `DEBUG === false` and minifier removes debug-only code.
Run the tests in debug mode.
Debug build is generated in a separate directory from `dist`, so is *not* included in NPM package.
Remove the need to list every fixture in `e2e.test.ts` file by automatically running tests on every fixture in `test/fixtures` directory.
Running Oxlint with `--fix` and running fixtures with ESLint is now controlled by adding an `options.json` file to the fixture directory contining e.g. `{ "fix": true }` or `{ "eslint": true }`.
Now we can add a new fixture just by creating a new directory in `fixtures`.
Rename the directories containing files to lint in the nested config tests from `nested` to `files`, to match the rest, and only lint the `files` directories.
I don't think this has any effect on the operation of the tests, but makes it easier to lint these directories as part of the repo (#15887).
This also removes the need for `overrideFiles` property in `testFixture` function's options, so remove it.
Implement `sourceCode.isSpaceBetween`. Implementation is not quite correct, because we don't have tokens yet. But it's better than no implementation at all. We can correct it once we have tokens.
The reason it's not correct is explained in comment on `isSpaceBetween` implementation, and the incorrect behavior is demonstrated in the test fixture.
Fix#15325.
Some ESLint rules create a wrapped copy of `context`, in order to patch `report` and pass that wrapped context object to another rule.
Some rules also rely on unspecified details of ESLint's implementation - that `context` is formed of 2 layers - top layer being `RuleContext` (`id`, `report` etc), and bottom layer being `FileContext` (`filename`, `sourceText` etc).
This usage was broken in our implementation because:
1. We collapsed the 2 layers into 1.
2. We used a `Context` class which stored its internal data in a private property. When `context.report` was called with a different `this`, it failed because the private property couldn't be accessed from an object which isn't an instance of the class.
Fix this problem by:
1. Splitting `Context` into 2 layers like ESLint does.
2. Not using classes or private properties.
3. No methods/getters on context objects use `this`.
We now have:
* Separate `Context` object per rule, containing properties related to the rule - a plain object which uses data from closure scope.
* All rule `Context`s inherit from the same singleton `FILE_CONTEXT` object, which provides getters for properties related to the file (same for all rules).
Tests adapted from #15326.
When replacing absolute paths in fixture snapshots, replace paths anywhere in the output, rather than only in certain specified patterns.
Additionally, introduce `<fixtures>` and `<fixture>` replacements to shorten paths in snapshots.
```diff
- File path: <root>/apps/oxlint/test/fixtures/fixture_name/files/1.js
+ File path: <fixture>/files/1.js
- File path: <root>/apps/oxlint/test/fixtures
+ File path: <fixtures>
```
This allows removing the `relativePath` helper function from some test plugins, outputting paths (e.g. `cwd`) anywhere, and generally will avoid problems with snapshots being system-dependent, and dependent on directory layout of fixtures.
Also, amend ESLint `stdout` output in snapshots to align all rule names, to reduce churn in snapshots, and avoid mismatches between local and CI (presumably due to differences in terminal width).
```diff
- filename: <fixture>/files/1.js define-plugin-plugin/create-once
- filename: <fixture>/files/long_filename.js define-plugin-plugin/create-once
- filename: <fixture>/files/very_long_filename.js define-plugin-plugin/create-once
+ filename: <fixture>/files/1.js define-plugin-plugin/create-once
+ filename: <fixture>/files/long_filename.js define-plugin-plugin/create-once
+ filename: <fixture>/files/very_long_filename.js define-plugin-plugin/create-once
```
## Summary
- Changed `sourceCode.parserServices` from throwing an error to
returning an empty object `{}`
- Aligns with ESLint specification for parsers that don't implement
parser services
- Prevents crashes in ESLint plugins such as
`eslint-plugin-better-tailwindcss`.
## Motivation
According to the [ESLint custom parser
documentation](https://eslint.org/docs/latest/extend/custom-parsers#parseforeslint-return-object):
> services can contain any parser-dependent services (such as type
checkers for nodes). The value of the services property is available to
rules as context.sourceCode.parserServices. **Default is an empty
object.**
The current implementation throws an error when `parserServices` is
accessed, which breaks plugins that use defensive checks like:
```typescript
if(typeof ctx.sourceCode.parserServices?.defineTemplateBodyVisitor === "function"){
// ...
}
```
Plugin authors defensively handle:
- ✅ `parserServices` being undefined
- ✅ `defineTemplateBodyVisitor` not existing
- ❌ `parserServices` getter throwing an error (unexpected)
## Solution
Return an empty object `{}` for unimplemented parser services, which:
1. Matches ESLint's documented default behavior
2. Allows plugins' defensive checks to work correctly
3. Provides correct semantics for "not implemented"
## Test Plan
- Existing tests should continue to pass
- Plugins using defensive checks like the example above will no longer
crash
## Related
Fixes the compatibility issue with plugins like
[eslint-plugin-better-tailwindcss](https://github.com/schoero/eslint-plugin-better-tailwindcss/blob/a90cc3f547098a4293d7b0b7998bf6774f717cd2/src/utils/rule.ts#L140)
---------
Signed-off-by: magic-akari <akari.ccino@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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>
Part of #14564. Corrects start and end offsets to accommodate two byte
characters. Conversion of UTF-8 indices to UTF-16 takes place on the
rust side for performance.
Part of #14564. Implement the remaining `SourceCode` APIs related to
comments (`getCommentsBefore`, `getCommentsAfter`, `getCommentsInside`,
`commentsExistBetween`).
---------
Co-authored-by: overlookmotel <theoverlookmotel@gmail.com>
Implement support for selectors DSL in Oxlint JS plugins.
The implementation is based on ESLint's implementation. Like ESLint, it uses [esquery](https://www.npmjs.com/package/esquery) as the underlying selector parser and matcher.
One detail is not implemented. ESLint ensures that visit functions are called in order of their "specificity" with the most specific called first. This implementation just calls them in the order they're defined. This is a shortcoming and will need to be fixed in future, but I imagine that in practice visitation order makes no difference for the vast majority of rules.
I've made what I hope are some optimizations versus ESLint's version. Notably:
1. Making simple selectors which unconditionally visit a subset of nodes (e.g. `:matches(FunctionDeclaration, FunctionExpression)` bypass the selector match process - as it'd always return `true` anyway.
2. Faster implementations of union and intersection.
Extend support for placeholder interpolation to normal `message` (without `messageId`).
```js
context.report({
message: 'Do not use {{ type }}',
data: { type: node.type },
node,
});
```
Avoid deserializing the AST if it's not required.
It's possible that all rules return an empty visitor, or that all `createOnce` rules return `false` from their `before` hook. In those cases, the AST does not need to be walked, so AST doesn't need to be deserialized. And the source text doesn't need to be decoded from buffer either.
These cases probably aren't so common, but not completely uncommon either.
However, source text and AST are both accessible before traversal via `context.sourceCode`. Enable the usage of those APIs in `create` method / `before` hook by decoding source text / deserializing AST on demand when those APIs are used.
Add support for ESLint's `SourceCode` API. At present the only method on `SourceCode` is `getText`. More methods will be added in further PRs to follow.
Raise an error if a JS plugin has same name as a built-in plugin e.g. `import`. It'd be impossible to know whether `import/rule-name` was referring to the built-in rule, or the JS plugin's rule of same name.
Make tests for Oxlint more robust by specifying the current NodeJS executable to run `oxlint` with. I was hitting problems locally with a node version manager resulting in running the tests in an older NodeJS version.
Remove `--js-plugins` CLI option. JS plugins are now enabled by default if config contains a `jsPlugins` array.
In place of the CLI option, print a warning when user uses JS plugins:
```
WARNING: JS plugins are experimental and not subject to semver.
Breaking changes are possible while JS plugins support is under development.
```
Pure refactor. Remove `"ignorePatterns": ["**/*", "!files/**"]` from `.oxlintrc.json` in every test fixture, by running `oxlint` with `oxlint --js-plugins files`.
Pure refactor of tests. Standardize the directory structure of all test fixtures.
* Move files to be linted into `files` subdirectory.
* Move plugin to a file called `plugin.js` (except 1 fixture which has multiple plugins).
* Change `ignorePatterns` in `.oxlintrc.json` config files to whitelist files in `files` dir, instead of blacklisting the plugin file.
Export a method `definePlugin` from `oxlint` package, to go with `defineRule`. Used as follows:
```js
import { definePlugin, defineRule } from 'oxlint';
const myRule = defineRule({
create(context) { /* rule impl */ },
});
export default definePlugin({
meta: { name: "my-plugin" },
rules: {
'my-rule': myRule,
},
});
```
At present this method doesn't have much utility, as we don't export TS types for it yet, but it's part of the intended API, so may as well add it now.
renovate