For historical reasons, `oxc_span` crate was re-exporting string types which now live in `oxc_str` crate.
Remove these re-exports, and import these types directly from `oxc_str`.
Main motivation is just that it makes sense, but it should also have a minor positive effect on compile times - `oxc_span` and `oxc_str` crates can now be compiled in parallel.
## Summary
Add `property_write_side_effects` option to `TreeShakeOptions` (default `true`). When set to `false`, property assignments like `A.from = () => {}` are considered side-effect-free, enabling the minifier to drop declarations and fresh value bindings with property assignments when unused.
### Examples
```js
// Function declaration + property assignments → eliminated
function A() {}
A.from = () => {}
A.to = () => {}
// Output: (empty)
// Class declaration + property assignment → eliminated
class A {}
A.foo = 1
// Output: (empty)
// Const variable with arrow function → eliminated
const A = () => {}
A.foo = 1
// Output: (empty)
// Const variable with object literal → eliminated
const B = {}
B.foo = 1
// Output: (empty)
// Const variable with function expression → eliminated
const C = function() {}
C.foo = 1
// Output: (empty)
```
### How it works
1. Wires the existing `property_write_side_effects` from `oxc_ecmascript`'s `MayHaveSideEffectsContext` through the minifier's compress options
2. For member expression assignments, checks `may_have_side_effects()` then `is_member_assign_to_unused_binding()`
3. `is_member_assign_to_unused_binding` verifies:
- Single-level member expression only (`A.foo`, not `a.b.c`)
- All references to the symbol have the `MemberWriteTarget` flag (from parent PR)
- Symbol creates a fresh value (`is_fresh_value`) and is not exported
4. Fixed-point loop handles cascading: first pass drops assignments, `exit_program` cleans up references, second pass drops declarations
### Fresh value detection
A symbol is considered "fresh" (cannot alias another binding and property writes are side-effect-free) if it is:
- A **function declaration**
- A **class declaration** without static setters, static accessor properties, or static property definitions with values
- A variable initialized with an **array/arrow function/function expression** literal
- A variable initialized with an **object literal** without setter/getter properties or property values containing setters/getters
- A variable initialized with a **class expression** (same rules as class declarations above)
Non-fresh values (e.g. `const b = a`) are conservatively preserved to prevent breaking aliased references.
#### Setter/getter detection (addresses review feedback)
Following how esbuild, Terser, Rollup, and SWC handle setter cases — all tools preserve property writes when setters are visible in the AST:
- **Static setter methods**: `class A { static set foo(v) { ... } }` → not fresh
- **Static accessor properties**: `class A { static accessor foo = 0 }` → not fresh (auto-generates getter+setter)
- **Static property definitions with values**: `class A { static b = 0 }` → not fresh (matches SWC's conservative approach)
- **Static property values containing setters/getters**: `class A { static b = { set x(v) { ... } } }` → not fresh
- **Object literal setters/getters**: `{ set foo(v) { ... } }` or `{ get foo() { ... } }` → not fresh
- **Nested setters/getters in object property values**: `{ bar: { set x(v) { ... } } }` → not fresh
- **Non-static setters are fine**: `class A { set foo(v) { ... } }` → fresh (instance setters don't affect class-level property writes)
### Limitations
**Only single-level member expressions are supported** (`A.foo`, not `a.b.c`). Deep nested assignments like `a.b.c = 1` are always preserved because intermediate properties could alias external objects:
```js
const exported = {};
const a = { b: exported };
a.b.c = 1; // writes to exported.c — unsafe to drop!
export { exported };
```
Supporting deeper nesting would require full object graph tracking (like Rollup's `ObjectEntity` model), which is a much larger effort for marginal gain.
### Safety guards
Addresses concerns raised in [PR #20730 review](https://github.com/oxc-project/oxc/pull/20730#pullrequestreview-4011376657) and [comment](https://github.com/oxc-project/oxc/pull/20730#issuecomment-4131710117):
- **Aliases**: `const b = a; b.add = 1; export { a }` → kept (not fresh, could mutate exported `a` through alias)
- **Chained access**: `const b = { a }; b.a.add = 1; export { a }` → kept (only single-level member expressions allowed)
- **Exported symbols**: `export function A() {} A.foo = 1;` → kept (observable by importers)
- **Other reads**: `function A() {} A.foo = 1; console.log(A)` → kept (symbol has non-member-write references)
- **Side-effectful RHS**: `function A() {} A.foo = sideEffect()` → kept (`may_have_side_effects` returns true)
- **Globals**: `globalObj.foo = 1` → kept (no local binding)
- **Script mode top-level**: top-level bindings in script mode are kept (they're globals visible to other scripts)
- **Setters**: `class A { static set foo(v) { ... } } A.foo = 1` → kept (setter triggers side effects)
- **Static properties**: `class A { static b = 0 } A.b = 1` → kept (matches SWC's conservative approach)
### Comparison with other tools
| Tool | Approach | Option needed? |
|------|----------|---------------|
| **Rollup** | Structural aliasing analysis — tracks object identity through `LocalVariable.isReassigned` + `ObjectEntity` setter detection. Drops `A.foo = 1` automatically when `A` is a local unreassigned object with no setters. | No (always active) |
| **Rolldown** | Adds explicit [`treeshake.propertyWriteSideEffects`](https://rolldown.rs/reference/InputOptions.treeshake#propertywritesideeffects) option (default `'always'`). Not in upstream Rollup. | Yes |
| **Terser** | Checks `is_constant_expression` on the base object after unwrapping property chain. If the base is constant within scope, drops the assignment. | No (always active, but less precise) |
| **esbuild** | All property assignments always preserved. Rationale: `Object.defineProperty` could install prototype setters. Only escape hatch is `/* @__PURE__ */` IIFE wrapping. | No option available |
| **SWC** | `may_have_side_effects` returns `true` for all assignments unconditionally. Classes with static property definitions are conservatively preserved. | No option available |
| **Oxc (this PR)** | Fresh value tracking (`is_fresh_value`) + `MemberWriteTarget` reference flags + explicit option. Closest to Rollup's aliasing analysis but gated behind an option like Rolldown. | Yes (`property_write_side_effects: false`) |
Closes#14207
## Test plan
- [x] 20 test cases covering all safety scenarios including setter/getter edge cases
- [x] All minifier tests pass (476/476)
- [x] No minsize regressions
- [x] Clippy clean
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Small perf optimization to raw transfer. When calculating a `u32` offset, shift first then add (`(pos >> 2) + 4`) instead of adding first (`(pos + 16) >> 2`). If a function contains multiple `pos >> 2` calculations, V8 may be able to combine them into a single calculation. Previously it couldn't prove that `pos` is a multiple of 4, so couldn't make this optimization.
Continuation of #21132. All uses of the `Uint32Array` view of the buffer in raw transfer code have been removed in preceding PRs. So we can now remove it entirely.
Continuation of #21132. Use `Int32Array` view of the buffer for all code in lazy deserialization. Lazy deserialization is not currently in use, but keeping it in line with the main raw transfer deserializer allows removing the `Unint32Array` view of the buffer entirely in #21140 without breaking the lazy deserializer tests.
Small perf optimization. When deserializing tokens in `oxc-parser`, use only the `Int32Array` view of the buffer, instead of both `Int32Array` and `Uint8Array`. Less values to keep in registers, and `int32` doesn't need to be obtained with a property lookup on each call.
Continuation of #21132. Get span `start` and `end` from the `Int32Array` view of the buffer, instead of the `Uint32Array`view. This allows V8 to statically see that they are SMIs, and skip type checks.
Utilize the `#[estree(raw_deser_inline)]` attribute introduced in #21134 to mark all custom deserializers which do not depend on `parent` as inline-able.
This reduces deserializer code size, as well as being more performant in some cases, by avoiding write boundary checks.
Use `int32` (`Int32Array`) instead of `uint32`(`Uint32Array`) for getting data from buffer in raw transfer deserializer. The buffer is 2 GiB in size, so all offsets within the buffer can be represented as a `u31` which can be stored in an `i32` without any loss, and with no risk of being interpreted as negative numbers.
Same as in #21129, the advantage of fetching offsets from an `Int32Array` is that V8 statically knows the value can be stored in an SMI, its native integer type, without any range checks.
`>>` is cheaper than `>>>` because `>>` produces a 32-bit _signed_ integer which is V8's native number type (SMI). `>>>` produces a 32-bit _unsigned_ integer, which needs to be boxed and stored on the heap.
Source text in raw transfer is limited to 1 GiB, and therefore source offsets, number of lines, number of tokens, and number of comments after all less than `1 << 30`. Therefore even the sum of 2 of them cannot reach `1 << 31` (the maximum positive integer which can be stored as a positive signed 31-bit int. Therefore it's safe to use `>>` in these binary loops.
Refacfor. In a strange pattern, we were injecting `getNodeLoc` into `deserialize.js` module on every call to `deserializeProgramOnly`. It's a static function, so just import it.
There's no need to defer clearing vars containing buffers and source text in raw transfer deserializer module until end of linting. It was a left-over from when this module contained code which lazily deserialized comments, but that's now done elsewhere.
Clear these vars as soon as deserialization is complete. Buffer and `sourceText` are still held in vars in other modules until linting completes, but `sourceTextLatin` (introduced in #21021) can be freed immediately.
Optimize raw transfer string decoding.
Replace `TextDecoder("utf8")` with `Buffer.prototype.utf8Slice` to decode Rust UTF-8 strings to JS strings.
Benchmarks show this is an average 5% speed-up in `deserializeStr`, though the reason why is unclear. It skips various checks, and avoids creating a temporary `Uint8Array` (which is required with `TextDecoder.decode`) which trims 60 bytes of the size of the assembly created for `deserializeStr`. But that's not enough to explain the speed-up 🤷. Anyway, take the win.
Improve perf of deserializing strings in raw transfer. This PR combines several optimizations, which have been tested and benchmarked in https://github.com/overlookmotel/oxc-raw-str-bench. This PR implements the version "latin-slice-onebyte64" from that repo, which is the current winner.
String deserialization is the main bottleneck in raw transfer, so speeding it up will likely make a large impact on deserialization overall.
This work follows on from #20834 which produced a major speed-up in many files by making files which contain some non-ASCII characters take the fast path of slicing `sourceText` more often.
This PR tackles the remainder - speeding up the fallback path where the fast path can't be taken.
## Optimizations
The optimizations in this PR are:
### Latin1
When source is not 100% ASCII, decode source text from buffer as Latin1.
A Latin1-decoded string represents each UTF-8 byte as a single Latin1 character, so it can be indexed into using UTF-8 offsets.
So when we can't slice the string from `sourceText` because the UTF-8 and UTF-16 offsets differ (after any non-ASCII character), loop through the string's bytes and check if they're all ASCII. If they are, the string can be sliced from `sourceTextLatin` instead, with the original UTF-8 offsets.
This is way faster than calling `textDecoder.decode`, as it avoids a call into C++. [Benchmarks show](https://github.com/overlookmotel/oxc-raw-str-bench/blob/4f96275efa9a35d5d27615abb27f21a137149cc0/README.md#apply30-vs-latin-vs-latin-source64) speed up of 55% on average, and up to 70% on some files.
### Latin1 decoding method
It turns out that `new TextDecoder("latin1").decode(arr)` doesn't actually decode to Latin1!
Per the WHATWG Encoding Standard, "latin1" is mapped to "windows-1252".
The result is that with `TextDecoder("latin1")`:
1. `decode` is quite complicated, requiring a 2-pass scan of the bytes to determine if they're all ASCII, followed by a 2nd pass to do the actual `windows-1252` decoding. If the string *does* contain any non-ASCII characters (which it always does in our usecase), NodeJS implements the decoding in JS, not native code. Slow.
2. `decode` produces a 2-byte-per-char string (`TWO_BYTE` in V8), which takes more memory, and is slower for all operations on it e.g. string comparison, hashing for use as an object key etc.
Instead, use `Buffer.prototype.latin1Slice` which:
1. Does a pure Latin1 decode, which is just a single `memcpy` call.
2. Produces a 1-byte-per-char string (`ONE_BYTE` in V8).
`latin1Slice` involves a call into C++, but we only do it once per file, so this cost is tiny in context of deserializing the whole AST.
### Latin1 string slicing
In the fast path, slice from the Latin1-decoded string, instead of `sourceText`. In the fast path, we know that all bytes of source comprising the string are ASCII, so no further checks are required.
This makes no difference on benchmarks for `deserializeStr` itself, but it may have beneficial effects downstream for code (e.g. lint rules) which access strings in the AST, e.g. `Identifier` names.
Because Latin1-decoded source text is `ONE_BYTE`-encoded, slices of it are too. In comparison, slices of `sourceText` may be `ONE_BYTE` or `TWO_BYTE`. If a file's source is pure ASCII, it'll be `ONE_BYTE`, if source contains any non-ASCII characters, it'll be `TWO_BYTE`. Files in a repo will likely be a mix of both, which makes strings returned from `deserializeStr` and placed in the AST a mix too. This in turn makes functions (e.g. lint rule visitors) polymorphic. V8 cannot optimize them as aggressively as if they see only `ONE_BYTE` strings.
We cannot make sure that all strings returned by `deserializeStr` are `ONE_BYTE`. Some string may contain non-ASCII characters, and they *have* to be represented in `TWO_BYTE` form. But we can minimize it - now only strings which *themselves* contain non-ASCII characters are `TWO_BYTE`, whereas before they would be if the source text as a whole contains a single non-ASCII byte.
Code which accesses `Identifier` names, for example will exclusively see `ONE_BYTE` strings and will be more heavily optimized, because Unicode `Identifier`s are rarer than hen's teeth in real-world code.
### Remove string-concatenation loop
Previously strings which are outside of source text were assembled byte-by-byte in a loop via concatenation.
Instead, check that all the bytes are ASCII first, copy them into an array and pass that array to `String.fromCharCode` with `fromCharCode.apply(null, array)`.
To avoid allocating a fresh array every time, hold a stock of arrays for all string lengths that this path can require, and reuse them.
This is a variation on the approach that #20883 took, but without the massive switch. This produces much tighter assembly, and avoids regressing the fast path due making `deserializeStr` a very large function.
Despite the complexity, and multiple operations, [this is up to 3x faster](https://github.com/overlookmotel/oxc-raw-str-bench/blob/4f96275efa9a35d5d27615abb27f21a137149cc0/README.md#apply30-vs-switch30) than the switch approach, and gives an average 30% speed-up.
### Increase native call threshold
The above optimizations make the slow path much faster. This shifts the tipping point at which it's faster to make a native call to `TextDecoder.decode` from 9 bytes to 64 bytes. Most strings now avoid the native call and stay in JS code which is heavily optimized by Turbofan.
The tipping point of 64 is something of a guesstimate. Benchmarking shows its in the right ballpark, but we could finesse it, and probably squeeze out another couple of %.
## Credit
The Latin1 string technique was cooked up by @joshuaisaact in https://github.com/overlookmotel/oxc-raw-str-bench/pull/1. All credit to him for this masterstroke which cracks the whole problem!
Tiny optimization to raw transfer string deserialization. Initialize vars which contain integers as `0`. This may help V8 optimize usage of these variables as they always have SMI type.
The effect, if there is one, is too small to register on benchamarks. But it's certainly not a regression, and may be a tiny gain, so why not?
Follow-on after #20834.
Simplify the branch condition in `deserializeStr` for detemining if can take the fast path of just slicing `sourceText`. There's no need to check `sourceIsAscii`, just compare the offset to `firstNonAsciiPos` (the position in buffer of first non-ASCII byte in source code). When source is 100% ASCII, `firstNonAsciiPos = sourceEndPos`, so `pos < firstNonAsciiPos` passes for all positions in source.
The implementation is different for parser and for Oxlint, as the source text sits in a different location in buffer - at the start in parser, at the end in Oxlint - but the principle is the same in both.
[Benchmarking](https://github.com/overlookmotel/oxc-raw-str-bench) showed this speeds up `deserializeStr` by a small percentage.
Small optimization to raw transfer string deserialization. Use `String.fromCharCode`instead of `String.fromCodePoint`. [Benchmarking](https://github.com/overlookmotel/oxc-raw-str-bench) showed it's slightly faster, as it's simpler - doesn't need to handle astral code points.
`raw` field of `NumericLiteral`, `StringLiteral`, `BigIntLiteral`, `RegExpLiteral`, and `JSXText` are slices of source text.
So in raw transfer deserializer, skip going through `deserializeStr`, which can be slow when source contains any non-ASCII characters. Instead just slice the string from the source text directly with `sourceText.slice(start, end)`.
String decoding is the slowest part of raw transfer, so this should be a significant speed gain.
**AI Disclosure:** Developed with Claude Code (Opus). The winning
approach came out of an automated experiment loop
([auto-claude](https://github.com/joshuaisaact/auto-claude)) — I was
looking for a tight feedback loop to test the tool on and the `TODO:
Find best switch-over point` comment in `deserializeStr` caught my eye.
20 experiments, keep-or-revert on each (all 20 summarized in an
expandable section at the bottom). All code reviewed and understood.
Happy to close this if it's not useful or doesn't meet the bar.
## Why
I was profiling the raw transfer deserialization path (`node --prof` on
`checker.ts`) and noticed `StringAdd_CheckNone` at 13.7% of time — the
single hottest function. It comes from the byte-by-byte `out +=
fromCodePoint(c)` loop in `deserializeStr` when `sourceIsAscii` is
false.
The thing is, `sourceIsAscii` is false for almost everything. All 5 NAPI
bench fixtures are non-ASCII. `checker.ts` has literally one Bengali
character at position 2.1M out of 2.9M. That one character disables the
fast `substr` path for all ~148K strings.
## What
Two changes to the generator
(`tasks/ast_tools/src/generators/raw_transfer.rs`) — that's the only
file with real changes. The 9 generated JS files in the diff are the
mechanical output of `cargo run -p oxc_ast_tools`.
**1. `firstNonAsciiPos` scan at init** — On non-ASCII sources, find the
first non-ASCII byte once upfront. Strings ending before that position
can still use `sourceText.substr()` since byte offsets equal char
offsets in the ASCII prefix. For `checker.ts` this covers 73% of the
file, for `pdf.mjs` 98%.
**2. Lower TextDecoder threshold from 50 to 9** — The existing TODO
asked for the right switch-over point. Experimentally, 9 is the sweet
spot: `TextDecoder` beats the `fromCodePoint` concat loop for strings of
10+ bytes, and the concat loop is still faster for very short strings
where the native call overhead dominates.
Benchmarked on the `complicated()` test set (5 rounds of 30 iters,
dropping round 1 for JIT warmup):
```
Before After
checker.ts 26.7ms 13.0ms -51%
cal.com.tsx 15.4ms 9.1ms -41%
antd.js 53.8ms 44.7ms -17%
pdf.mjs 3.8ms 4.3ms noise
binder.ts 0.7ms 0.5ms noise
Total 100.5ms 71.6ms -29%
```
Also verified across 15 files — the 10 non-ASCII files above plus 5
ASCII files (`react.development.js`, `binder.ts`, `moment.js`,
`jquery.js`, `vue.js`). ASCII files are unchanged (our code only touches
the non-ASCII path). -16% across non-ASCII files, no regressions.
## References
- Addresses the `TODO: Find best switch-over point` in
`STR_DESERIALIZER_BODY`
- Related to perf goals in #19918
<details>
<summary>I appreciate this is already a bloated PR description (sorry
@overlookmotel) but given the fairly unusual approach I thought you
might want to see a very short summary of all 20 experiments Claude
clauded through:</summary>
The loop works like this: edit code, benchmark, keep if faster, `git
reset --hard` if not. Metric is total deserialization time across the
benchmark corpus.
**Baseline: 97.7ms** (5-file corpus, all non-ASCII)
| # | Idea | Result | Verdict |
|---|------|--------|---------|
| 1 | Always use TextDecoder, delete the loop entirely | 99.2ms |
Revert. TextDecoder's ~78ns fixed overhead kills short strings. |
| 2 | Lower TextDecoder threshold from 50 to 10 (ts.js only) | 89.8ms |
**Keep.** First real win — moves 50% of strings off the concat loop. |
| 3 | Threshold 5 | 91.3ms | Revert. Too aggressive, too many short
strings go to TextDecoder. |
| 4 | Threshold 8 | 95.0ms | Revert. Worse than 10. |
| 5 | Threshold 12 | 91.2ms | Revert. Worse than 10. |
| 6 | Threshold 10 + unrolled `switch` on len for inline
`String.fromCharCode(uint8[pos], ...)` | 90.9ms | Revert. Switch
dispatch overhead eats the gain. |
| 7 | Threshold 10 + special fast path for len=1 | 94.8ms | Revert.
Extra branch hurts more than the 1-byte optimization helps. |
| 8 | Threshold 10 + accumulate char codes in array, single
`fromCharCode.apply` at end | 90.5ms | Revert. Array allocation
overhead. |
| 9 | Threshold 15 | 93.2ms | Revert. 10 is still the sweet spot. |
| 10 | Unrolled ASCII check for bytes 1-4, TextDecoder for 5+ | 94.7ms |
Revert. Branching overhead. |
| 11 | **Apply threshold 10 to js.js too** (had only been changing
ts.js) | 82.5ms | **Keep.** Facepalm moment — antd.js uses the JS
deserializer. |
| 12 | Threshold 9 in both files | 77.9ms | **Keep.** New best. |
| 13 | Threshold 7 | 79.0ms | Revert. 9 wins. |
| 14 | Threshold 11 | 84.8ms | Revert. 9 confirmed. |
| 15 | Replace `fromCodePoint` with `String.fromCharCode` in the loop |
82.5ms | Revert. V8 optimizes the pre-extracted `fromCodePoint` better.
|
| 16 | Various unrolled `fromCharCode` approaches for short strings | —
| Abandoned, too complex for marginal gain. |
| 17 | Always TextDecoder for non-source strings (remove loop) | 96.1ms
| Revert. Confirms the short-string loop IS valuable for 1-9 bytes. |
| 18 | `Buffer.from().toString()` instead of TextDecoder | 80.4ms |
Revert. TextDecoder is faster. |
| 19 | `firstNonAsciiPos` only (use substr before it, TextDecoder after,
no loop) | 84.9ms | Revert. checker.ts loved it (-34%) but antd.js hated
it (+20%) because its first non-ASCII byte is at 1.3%. |
| 20 | **`firstNonAsciiPos` + threshold 9 + keep the loop** | 76.4ms |
**Keep.** Best of both worlds — substr where possible, TextDecoder for
medium strings, loop for short. |
Three things I (Claude) learned:
- Experiment 11 was the biggest single win and it was just... applying
the change to the other file. Embarrassing.
- Every attempt to replace the `fromCodePoint` loop for short strings
(1-9 bytes) made things worse. The loop is genuinely good for that
range.
- `firstNonAsciiPos` only works when combined with the threshold change.
On its own it hurts files where non-ASCII appears early (antd.js,
cal.com.tsx).
</details>
Pure refactor.
Rename `Atom` to `Str`. The name "Atom" is a leftover from when we used to intern strings (years ago), and is misleading now.
Just call it `Str`. This follows the naming convention we have for arena-allocated versions of other types e.g. `Box`, `Vec`, `HashMap` - which we name the same as the native equivalents but with a lifetime (`Box<'a>` not `ArenaBox<'a>`, so `Str<'a>` not `ArenaStr<'a>`).
Rename all variables that contained "atom", and methods which relate to `Str`s to match the new type name.
Rename `as_atom` methods to `as_arena_str`. The longer name is required here to disambiguate from `as_str` methods which return a `&str`.
Some variables called "atom" still exist in `oxc_regular_expression` crate - that's a different meaning of "atom" - the meaning defined in regexp spec.
The diff of this PR is large but it's 100% renaming. No substantive or unrelated changes whatsoever.
Performance improvement to tokens and comments APIs.
## The problem
Previously, all tokens and comments methods would deserialize *all* tokens/comments into an array of `Token` / `Comment` / `Token | Comment` objects, and then binary search through those arrays to find the token(s) / comment(s) they're looking for.
This has 2 major disadvantages:
1. Files typically contain *a lot* of tokens (even more than the number of AST nodes). Deserializing them all is very costly (up to 30% of total Oxlint runtime when run with only a JS rule which just calls a tokens-related method).
2. The binary searches these methods do are quite expensive. Even in TurboFan-optimized code, accessing `token.start` involves getting pointer to the `Token` object from the `tokens` array, an "is this object a `Token`?" safety check, then reading the `start` field from the `Token` - all just to access a single `u32`, and that happens over and over.
## This PR's solution
Solve both these problems by making tokens and comments methods read `start` / `end` offsets directly from the buffers which contain the tokens/comments data.
This data is tightly packed in memory, and strongly typed (read from `Uint32Array`s), so getting `start` / `end` of a token requires no indirection and no type checks.
More importantly, it removes the need to deserialize all tokens / comments upfront. The desired token(s) are located, touching only the buffer, and then *only* the ones which need to be returned to rule code are deserialized into JS objects.
If a rule accesses `ast.tokens`, `ast.comments`, or `sourceCode.tokensAndComments` then all tokens / comments need to be deserialized, as they're all returned to the rule as an array - but that's unavoidable. This PR doesn't make that any cheaper, but it doesn't make it measurably more costly either.
But where no rule requires the full array of tokens / comments, and they only use token/comment search methods (e.g. `getFirstToken`, `getCommentsBefore`), a great deal of work will be saved. This covers the vast majority of rules.
## Implementation details
The main complication is the `includeComments` option to tokens methods. When `true`, search needs to be over a combined set of both tokens and comments.
When `includeComments: true` option is passed to a tokens method, a buffer is created containing data about all tokens and comments, interleaved in source code order. This buffer can then be used for binary search in tokens methods.
Whether each token / comment has been deserialized already or not is tracked by a "deserialized" flag in the tokens/comments buffers. Each token / comment in the buffer is 16 bytes. This flag lives in byte 15. For tokens, this byte is always already 0 in the buffer when it arrives from Rust side. For comments, we manually set `comment.content = CommentContent::None;` for every comment on Rust side. `comment.content` is positioned at byte 15 in the `Comment` struct, and `CommentContent::None` is stored as 0.
## Possible future improvements
### SoA storage
Binary search operates only on `start` field of tokens / comments, which are 16 bytes apart in the buffer. It would be more efficient if tokens were stored in struct-of-arrays (SoA) style so all `start` values were tightly packed together. This would reduce CPU cache misses in the hot loops of binary searches.
### Pre-compute tokens-and-comments buffer on Rust side
The buffer containing tokens and comments, required to support `includeComments: true`, is currently generated on JS side (but lazily). We could move that to Rust side, which would be faster. However, it might be redundant work in many cases because the buffer is only required if a rule uses `includeComments: true`.
We could alternatively keep the laziness optimization, by calling back into Rust to build the buffer on demand - but JS-Rust calls have a cost too. Maybe communicating via `Atomics` would be faster than an actual function call?
If we had a way to share buffers with WASM, optimal solution might be to generate the buffer lazily (as now) but in WASM, which would be faster for this kind of pure number-crunching, but without the overhead of calling into Rust.
Alter the code in `ast_tools` which re-orders struct fields, to make `NodeId` always occupy a consistent "slot" in every AST node - at byte position 8, just after `Span`.
This will be a sizeable gain once we start utilizing the `NodeId` fields more. Because, for example, every variant of `Expression` has its `NodeId` stored in same location as all the other variants, `Expression::node_id()` is just 1 operation - a pointer read - rather than a nest of branches, or a lookup table.
Also alter the algorithm for ordering struct fields to fill in the 4-byte gap after `NodeId` field with other field(s), to avoid excess padding.
The new algorithm also prioritizes keeping fields in definition order as much as possible, rather than sorting them strictly in order of size and alignment. This is mildly advantageous because field definition order is the order the AST is walked in, so it avoids bouncing between cache lines while iterating through the fields of a struct when visiting the AST.
No types change size in the process. Fields remain packed to keep type sizes the minimum they can be - they just change order.
Refactor. Generate assertions for type sizes in the order which they are in memory. This makes it easier to see how these types are laid out in memory.
One annoyance with `ast_tools`, our codegen, has been that when you want it to generate code from types in some file, you had to manually add the file paths to a list in `ast_tools` itself.
This PR removes that list and instead finds crates in the monorepo which depend on `oxc_ast_macros` crate, via `cargo metadata`, and searches all files in those crates for types with `#[ast]` attributes etc - types that it need to act on.
There is no longer a list to keep updated, you just mark types `#[ast]` and the codegen will find them.
This produces some churn in generated files. None of the generated code changes, it just gets re-ordered in some places, due to the different order that files get found in now. This order is deterministic, and it should not change again.
Belated follow-on after #15925.
`VariableDeclarator` doesn't need a whole-struct ESTree converter. Switch to a converter just for the `id` field, to reduce custom conversion code.
Apply the same optimization as #19978 to comments - hold a pool of `Comment` objects, and re-use those objects rather than creating new objects each time.
Same as with `Token`s, `loc` property is a getter which calculates `loc` lazily, and caches it in a private property.
Comments-only change. Add JSDoc comments to the constants related to raw transfer in generated files `constants.js` / `constants.ts`. There are now a lot of constants, and it wasn't clear what they all mean.
## Summary
close: #13067
Part of https://github.com/oxc-project/oxc/issues/19673
Depends on #20217.
Port known-globals lists from Rolldown to Oxc's side-effect detection:
- **Global identifiers** (Math, Array, console, DOM classes, etc.) are now considered side-effect-free to access via `is_known_global_identifier`, matching Rolldown's `GLOBAL_IDENT` set
- **Property reads** like `Math.PI`, `console.log`, `Object.keys` are considered side-effect-free via `is_known_global_property`
- **3-level chains** like `Object.prototype.hasOwnProperty` are supported via `is_known_global_property_deep`
## Test plan
- [x] Added tests for known global identifiers (Math, Array, Object, console, etc.)
- [x] Added tests for known global property reads (Math.PI, Object.keys, etc.)
- [x] Added tests for 3-level chains (Object.prototype.hasOwnProperty, etc.)
- [x] Existing minifier test expectations updated for improved side-effect detection
- [x] All existing tests pass
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Remove `noUnusedLocals` option from `tsconfig.json` files. It's redundant, as Oxlint `no-unused-vars` covers this for us.
It's an annoyance to have `noUnusedLocals` enabled, as in VS Code TypeScript complains about unused vars, even if they're prefixed with `_`.
Removing this option allows simplifying type tests added in #20066. Only reason these types had to be exported was to stop TS complaining that they were unused.
overlookmotel