Follow-on after #19092.
That PR added 1 to offsets of fixes before sending them to Rust side, to support offset `-1` (legal when fix is to remove a BOM).
Upon reflection, I think that solution is too complicated and confusing. Instead, represent `start` and `end` offsets of fixes as `i64`s in `JsFix`. This supports `-1` as an offset, and also the full range of valid `u32` values. Check on Rust side that the values are in range before converting to `u32`s.
This also makes for better error messages for invalid offsets - no more inexplicable "Failed to deserialize JSON returned by `lintFile`" errors.
`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.
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.
Refactor. Simplify the type used to represent a fix when sending from JS to Rust.
Previously we avoided creating new objects by using the same fix object returned by `fix` function, and so were locked to ESLint's design.
That worked, but the code was excessively complicated. Fixes should be fairly rare, so this optimization is on a cold path - it's not worth it.
Simplify implementation by converting `Fix`es to a simpler type (`FixReport`) before sending to Rust.
Implement support for suggestions.
Implementation exactly matches ESLint's.
The tests demonstrate that:
* `--fix-suggestions` applies suggestions.
* `--fix` does *not* apply suggestions.
* When a rule provides multiple suggestions, only the 1st is applied.
I've also confirmed that suggestions show up correctly in VSCode (once language server supports JS plugins).
Pure refactor. Split `getFixes` into 2 functions. New function `getFixesFromFixFn` calls a fix function and processes the fixes it returns. `getFixesFromFixFn` will also be used for getting the fixes for suggestions in a later PR.
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.
Use `const` where variables are not reassigned instead of `let`. Preparation for enabling ESLint's `prefer-const` rule (running as an Oxlint JS plugin) in our linting setup.
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()`).
Pure refactor. Split up `plugins/utils.ts` into 3 files in `utils` directory.
Most importantly, assertion functions are by themselves in an `asserts.ts` file. This paves the way for the next PR (#15946).
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.
Add `range` field to AST nodes, using the support for `range` in raw transfer added in #14319.
Use `range` field rather than `start` + `end` in nodes passed to `context.report` and fixer methods - for closer alignment with ESLint.
Small optimization. Store if a rule is fixable as a single `isFixable` boolean, instead of putting the whole `RuleMeta` object in `Context`.
This is more efficient on memory, and a boolean is faster to check as it's stored inline in the object, as opposed to strings which are stored as a reference to another object on the heap.
`oxlint` package export TS types. `defineRule` and `definePlugin` now give you type-safety and intellisense.
Writing plugins in TS is only supported on versions of NodeJS which support type-stripping. Have not added a library (e.g. Jiti) to support older NodeJS versions.
Probably some of the types are wrong. But I think this is a decent start.
"Are the types wrong" (added in #14177) is reporting an error in `index.d.ts`:
```ts
import { VisitorObject as Visitor } from "../dist/generated/visit/visitor.js";
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
because the file in `dist/generated/visit` is actually called `visitor.d.ts` (`.d.ts`, not `.js`).
I'm not sure if that's a mistake in Rolldown's bundling, "Are the types wrong" being too strict, or a genuine problem.
But regardless, TS seems to pick up the types from `visitor.d.ts`, so I think we can probably merge this and look into / fix it later.
Fix types for `fix` function. Allow it to return `undefined`, so a pattern where bail out unable to fix is legal:
```js
const rule = {
create(context) {
return {
Identifier(node) {
context.report({
message: 'No identifiers!',
node,
fix(fixer) {
if (node.name === 'foo') return; // Can't fix 'foo' for some reason
return fixer.remove(node);
},
});
},
};
},
};
```
Another faulty type I found. `NodeOrToken` type was too restrictive. The `node` argument passed to fixer methods just needs to have `start` and `end` properties.
overlookmotel