Perf optimization to merging visit function when compiling AST visitors.
Previously we took an array of `VisitProp` objects, extracted the `fn` property from each into a temporary array, and then called the merger function using spread `merger(...visitFns)`.
Remove the temp array and the mapping entirely by just passing the arrays of `VisitProp` objects directly to the merger function, and destructuring it internally.
This is simpler and also more performant.
My concern with taking this approach originally was that the merged functions might capture the `VisitProp[]` passed in to outer function in its scope, which would bloat them. These functions are extremely hot - called 1,000s or 10,000s of times during AST visitation - so even a small slow-down in these functions would be measurable.
But analysis using V8 built-in functions (`%DebugPrint`) to inspect what these functions capture in their scopes reveals that this concern was unwarranted - they only capture the variables that they actually use, and do not capture to `VisitProp[]` arrays. So they are unchanged after this PR.
We hard-code merger functions for merging up to 5 AST node visitor functions. This seems a reasonable number as it's common for many rules to visit the same AST node types.
However, 5 seems excessive for CFG event handlers, as using the CFG is far less common. Reduce it to 3. In the rare case that merger functions for number of CFG event handlers above this number are required, they'll be dynamically generated.
Previously we had 2 functions specialized for generating merger functions for AST node visit functions and CFG event handler functions. Merge them into 1. This reduces the amount of code and makes it clearer.
When compiling a visitor, visit functions from different rules which act on the same AST node (e.g. `Identifier`) need to be merged into a single function.
Functions which merge up to 5 visit function are hard-coded. If need to merge 6 or more functions, new merger functions are generated dynamically.
Simplify this logic so there's only a single call to `createMerger`. After warm-up over first few files, the `while` loop will be skipped, and the `=== null` check will fail. Can't avoid 2 branches here, but both become highly predictable after warm-up.
Perf optimization to visitor compilation. Continuation of #20187.
Instead of populating the cache of `EnterExit` objects in `finalizeCompiledVisitor`, populate it in full at process startup.
The main gain is that it makes these objects, which are heavily accessed during AST walk, more likely to be grouped together in memory, reducing L1 cache misses, and consuming less L1 cache.
Perf optimization to visitor compilation.
Replace 3 x `Arrays` with `Uint8Array`s. These arrays only contain positive integers between 0 and 171, so `Uint8Array`s are more suitable. Each entry takes 1 byte instead of 8 bytes, and the entries are strictly typed so should help V8 to optimize.
Resetting these arrays is also cheaper, and will never result in allocations, which the original `Array`s likely did.
Perf optimization to visitor compilation.
Apply the same optimization as #20187 to the cache of `VisitProp` objects.
Instead of main loop in `addVisitorToCompiled` checking on each turn of the loop whether cache contains a `VisitProp` object it can reuse, instead pre-populate the cache to ensure it always does.
Perf optimization to visitor compilation.
Previously in the main loop for non-leaf visitors in `finalizeCompiledVisitor`, every turn of the loop had to check if cache contains an `EnterExit` object it can reuse, and to create a new one if not.
Instead, pre-populate the cache of `EnterExit` object *before* that main loop, to ensure there are always enough in the main loop.
After warmup over the first few files, the cache will be populated with enough `EnterExit` objects to service every file, and the "populate" loop will be skipped entirely.
This converts what was 1 branch per loop iteration to 1 highly predictable branch per file.
Perf optimization to visitor compilation.
Previously the flow for compiling and using a visitor was:
1. Call `initCompiledVisitor`.
2. Call `addVisitorToCompiled` with each visitor object.
3. Call `finalizeVisitor`.
4. Walk AST with compiled visitor.
`initCompiledVisitor` resets state after previous compilation.
Change this to:
1. Call `addVisitorToCompiled` with each visitor object.
2. Call `finalizeVisitor`.
3. Walk AST with compiled visitor.
4. Call `resetCompiledVisitor`.
The difference is that resetting state now happens at *end* of linting a file, rather than at the start of linting the *next* file.
The previous lifecycle design was motivated by needing to ensure state is reset even if an error is thrown during linting, but this case is now handled correctly by another means (#20166).
The reset-at-the-end lifecycle makes more sense.
It also has a perf benefit: Visit functions are freed earlier, so they can be garbage collected. This reduces the chance of ephemeral visit functions being pointlessly migrated to "old space" before they're deleted, which makes garbage collection cheaper. Also, they can now be garbage collected *between* linting files, while Rust side code is busy doing other tasks - rather than GC having to happen during linting on JS side, blocking main JS thread.
Perf optimization to visitor compilation.
Reset compiled visitor with a single `compiledVisitor.fill(null)` call, instead of a loop which writes `null` to each entry individually. V8 should optimize this to a single `memset` call.
Small refactor. Import `VisitorObject` from where it's actually defined, instead of the alias in `types.ts` which renames it to `Visitor`. We also have a different type called `Visitor` so using the alias is a bit confusing.
Fix#18555.
CFG event handlers receive multiple arguments (unlike standard AST node visit functions which only receive `node`). The code which merges multiple visitors into one was built on assumption that all visit functions take a single argument so merges 2 function like this:
```js
fn merge(visit1, visit2) {
return (node) => {
visit1(node);
visit2(node);
};
}
```
This works well for AST node visit functions, but not for CFG event handlers - `visit1` and `visit2` only get called with the first argument, and the remaining arguments are lost.
Fix this by adding a 2nd function for merging CFG event handlers, which passes through all arguments:
```js
fn mergeCfg(handle1, handle2) {
return (...args) => {
handle1(...args);
handle2(...args);
};
}
```
Implement CFG (or "code path analysis" as ESLint calls it).
This is the was the last remaining gap in our API.
The implementation is pretty slow, I imagine. I've pulled in ESLint's code for the actual control flow graph construction, and copied ESLint's code for the rest.
Fitting with how we combine enter and exit nodes for leaf nodes (`Identifier`s etc) required skipping creating "steps" for exiting leaf nodes, which should also be a perf gain, as they're so common. But that apart, I've made no effort to optimize it. There's plenty of scope for improving it. I've added comments with some starting points for doing that, and will open an issue in the hope someone would like to pick up on that.
I also cannot for the life of me figure out how to add the CFG event handlers (`onCodePathStart` etc) to `VisitorObject` type, without breaking everything. Me and Claude went at it for about an hour, and got nowhere. So I'm just leaving that for now.
Add error messages to all `debugAssert()` calls, for easier debugging.
When running Oxlint normally, we can easily figure out where errors are coming from stack traces, but in conformance test snapshot, line and column are removed from errors in the snapshot, so it's less easy.
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.
#16834 implemented sorting visit methods by specificity. Speed up the sorting by storing `specificity` as a single integer, rather than 2 separate properties `identifierCount` and `attributeCount`.
See comment in `selector.ts` for further explanation.
Note: I've checked that minifier const-folds and inlines all the consts in release build.
ESLint calls visitor functions in order of selector specificity, with the least specific selectors first.
Attribute count takes priority in calculation of specificity. A selector with 1 attribute is considered more specific than a selector with any number of identifiers, but no attributes.
e.g. these visit methods are called in the order they're defined:
```js
const rule = {
create(context) {
return {
// 0 identifiers, 0 attributes (least specific)
"*"(node) {},
// 1 identifier, 0 attributes
"Identifier"(node) {},
// 2 identifiers, 0 attributes
":matches(Identifier, Program)"(node) {},
// 1 identifier, 1 attribute (most specific)
"Identifier[name=foo]"(node) {},
// 0 identifiers, 0 attributes (least specific)
"*:exit"(node) {},
// 1 identifier, 0 attributes
"Identifier:exit"(node) {},
// 2 identifiers, 0 attributes (most specific)
":matches(Identifier, Program):exit"(node) {},
};
}
};
```
This PR implement ordering by specificity, to match ESLint.
Fixes 900 conformance tests for `indent` rule.
Pure refactor. Move TS type defs related to visitor compilation from `types.ts` into `visitor.ts`. Does not change the types in any way, only moves them.
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).
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.
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.
Pure refactor. Split code for adding a visitor function to compiled visitor into multiple functions. This PR does not alter the code in any way, just splits it up. This is a preparatory step for implementing selectors DSL.
Pure refactor. Generated files `constants.js` and `type_ids.js` are JS files, but they only define `const`s, so are also valid TS, without adding any type annotations. So rename them to `.ts` to allow TS to infer the types of their exports.
Pure refactor. Just rename generated `types.js` files to `type_ids.js`. This name better describes what the files contain, and it avoids naming clash with `types.d.ts` in `apps/oxlint/src-js/generated`.
Refactor. Include generated files `types.js` and `walk.js` in bundle, instead of externalizing them. It's only the deserializer code that we want to keep unbundled to make it easier to debug without source maps. `types.js` and `walk.js` aren't too complicated.
Small perf optimization. All the code in `apps/oxlint` loads methods of global object e.g. `Object`, `Array` into local vars, for faster access. Continue that convention in these files.
Now that `apps/oxlint` and `napi/parser` are both ESM packages (#13723 and #14042), we can use `.js` file extensions for all ESM files, rather than the mix of `.js` and `.mjs` file we had previously.
It's less confusing having to remember what's `.js` and what's `.mjs`, and avoids ugly workarounds like #14038 (`.d.mts`??).
Use eager deserialization instead of lazy deserialization in linter plugins, so AST custom rules receive is ESTree-compatible.
Unfortunately this reduces performance significantly, but we'll bring back lazy deserialization later on once we have it producing an ESTree-compatible AST.
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.
overlookmotel