Skip to content

Bun.Transpiler: add unstable_parse() to expose the internal AST - #36745

Open
robobun wants to merge 33 commits into
mainfrom
farm/2bc68b00/transpiler-unstable-parse
Open

robobun wants to merge 33 commits into
mainfrom
farm/2bc68b00/transpiler-unstable-parse

Conversation

@robobun

@robobun robobun commented Aug 1, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds Bun.Transpiler.prototype.unstable_parse(code, loaderOrOptions?), which parses JS/TS/JSX and returns Bun's internal AST as { buffer, root, visit }:

const t = new Bun.Transpiler({ loader: "ts" });
const { root, visit } = t.unstable_parse(`const x: number = 1 + 2;`);

root.stmts[0].decls[0].value.op;   // "bin_add" (reads three field slots from the buffer)
JSON.parse(JSON.stringify(root));  // materializes the full tree
visit({
  e_call(n) { console.log(n.target.name, n.args.length) },
  s_function() { return false },   // skip subtree
});

buffer is a self-describing binary tape (24-byte header + key-name table + 4-byte-aligned records with fixed 12-byte {keyId,type,lo,hi} field slots + deduped string pool). root is a lazy Proxy over it that decodes fields on access, so nothing is materialized until read. visit() linear-scans the tape dispatching on kind.

The output is explicitly not ESTree. Node kind strings are the snake_case tags bun_ast's strum::IntoStaticStr derives already emit (s_local, e_binary, b_identifier, ...), so there is no separate mapping table to keep in sync when the AST changes. The API is named, typed, and documented as unstable: the shape may change or be removed in any patch release.

How?

The AST is walked once in Rust (src/runtime/api/js_transpiler_ast.rs) to produce the tape, which the JS builtin (src/js/builtins/Transpiler.ts) wraps in DataView-backed lazy Proxies with toJSON/ownKeys/has/mutation-reject traps, plus visit(). The tape carries its own field-name table, so the JS reader needs no build-time schema. The walker covers all 33 Stmt, 47 Expr, and 4 Binding variants plus the shared G::* nodes, with StackCheck guards and a u32 tape-size guard.

Performance

typescript.js (8.72 MB), release build, 5-run median:

ms
scan() (parse only) 148
unstable_parse() 308
+ visit({ e_call(){} }) full walk 375
+ manual walk via JSON.parse(JSON.stringify(root)) 668
@swc/core parseSync 1288

Closes #2729
Closes #12896

How did you verify your code works?

  • bun bd test test/js/bun/transpiler/unstable-parse.test.ts (34 tests covering locals/functions/classes/destructuring/imports/optional-chain/templates/regex/try-catch/UTF-16 round-trip/non-finite numbers/loader override/parse errors/depth limit/Uint8Array/malformed UTF-8/empty-string pool/lone surrogates/large UTF-16/BOM/options-getter detach/JSON round-trip, plus proxy behavior and visit()).
  • Fail-before verified: USE_SYSTEM_BUN=1 bun test fails every test (unstable_parse is not a function).
  • Stress-tested against node_modules/typescript/lib/typescript.js (8.7 MB).
  • test/bundler/transpiler/transpiler.test.js and test/integration/bun-types/bun-types.test.ts still pass.

[review] gate passed · iteration 12 · 10 files touched

fails on main (without fix)
ASAN without fix: BUILD FAILED (no junit output)
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/transpiler/unstable-parse.test.ts
ninja: Entering directory `/workspace/bun/build/debug'
[1/189] gen ErrorCode+*.h
[2/189] gen JSEvent.lut.h
Generating /workspace/bun/build/debug/codegen/JSEvent.lut.h from /workspace/bun/src/jsc/bindings/webcore/JSEvent.cpp
[3/189] gen JSBuffer.lut.h
Generating /workspace/bun/build/debug/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[4/189] gen cpp.rs (cppbind)
[5/189] gen BunProcess.lut.h
Generating /workspace/bun/build/debug/codegen/BunProcess.lut.h from /workspace/bun/src/jsc/bindings/BunProcess.cpp
[6/189] gen JSSink.{cpp,h,lut.h,rs}
generated_jssink.rs: 7 sinks, 84 exported symbols
Generating /workspace/bun/build/debug/codegen/JSSink.lut.h from /workspace/bun/build/debug/codegen/JSSink.lut.txt
[7/189] gen generated_host_exports.rs
generated_host_exports.rs: 94 exports (host=3, lazy=10, generic=81, rust=0); 238 extern-C blocks audited
[8/189] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMe
... (truncated)

release without fix: all passed
bun test v1.4.0-canary.1 (809cf6046)

test/js/bun/transpiler/unstable-parse.test.ts:
(pass) Bun.Transpiler.unstable_parse > exists on prototype [0.04ms]
(pass) Bun.Transpiler.unstable_parse > returns buffer + lazy root + visit [2.01ms]
(pass) Bun.Transpiler.unstable_parse > top-level shape [0.17ms]
(pass) Bun.Transpiler.unstable_parse > empty source [0.08ms]
(pass) Bun.Transpiler.unstable_parse > every object with a kind also has a loc [3.02ms]
(pass) Bun.Transpiler.unstable_parse > s_local / e_binary / b_identifier [0.22ms]
(pass) Bun.Transpiler.unstable_parse > functions and arrows [0.38ms]
(pass) Bun.Transpiler.unstable_parse > classes [0.25ms]
(pass) Bun.Transpiler.unstable_parse > destructuring bindings [0.16ms]
(pass) Bun.Transpiler.unstable_parse > imports, exports, and importRecords [0.31ms]
(pass) Bun.Transpiler.unstable_parse > call, dot, index, optional chain [0.13ms]
(pass) Bun.Transpiler.unstable_parse > template literals [0.14ms]
(pass) Bun.Transpiler.unstable_parse > regex [0.09ms]
(pass) Bun.Transpiler.unstable_parse > try / catch / finally / switch [0.16ms]
(pass) Bun.Transpiler.unstable_parse > jsx is lowered before the AST is exposed [0.22ms]
(pas
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/transpiler/unstable-parse.test.ts
bun test v1.4.0 (221a16240)

test/js/bun/transpiler/unstable-parse.test.ts:
(pass) Bun.Transpiler.unstable_parse > exists on prototype [2.33ms]
(pass) Bun.Transpiler.unstable_parse > returns buffer + lazy root + visit [33.98ms]
(pass) Bun.Transpiler.unstable_parse > top-level shape [10.51ms]
(pass) Bun.Transpiler.unstable_parse > empty source [5.94ms]
(pass) Bun.Transpiler.unstable_parse > every object with a kind also has a loc [118.97ms]
(pass) Bun.Transpiler.unstable_parse > s_local / e_binary / b_identifier [11.75ms]
(pass) Bun.Transpiler.unstable_parse > functions and arrows [21.46ms]
(pass) Bun.Transpiler.unstable_parse > classes [13.67ms]
(pass) Bun.Transpiler.unstable_parse > destructuring bindings [12.78ms]
(pass) Bun.Transpiler.unstable_parse > imports, exports, and importRecords [20.79ms]
(pass) Bun.Transpiler.unstable_parse > call, dot, index, optional chain [9.35ms]
(pass) Bun.Transpiler.unstable_parse > template literals [9.50ms]
(pass) Bun.Transpiler.unstable_parse > regex [6
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 681ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/146] gen ErrorCode+*.h
[2/146] gen JSEvent.lut.h
Generating /workspace/bun/build/release/codegen/JSEvent.lut.h from /workspace/bun/src/jsc/bindings/webcore/JSEvent.cpp
[3/146] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[4/146] gen cpp.rs (cppbind)
[5/146] gen BunProcess.lut.h
Generating /workspace/bun/build/release/codegen/BunProcess.lut.h from /workspace/bun/src/jsc/bindings/BunProcess.cpp
[6/146] gen JSSink.{cpp,h,lut.h,rs}
generated_jssink.rs: 7 sinks, 84 exported symbols
Generating /workspace/bun/build/release/codegen/JSSink.lut.h from /workspace/bun/build/release/codegen/JSSink.lut.txt
[7/146] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (13 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime
... (truncated)
diff hotspot
docs/runtime/transpiler.mdx                        |   61 +
 packages/bun-types/bun.d.ts                        |  288 +++++
 src/js/builtins/BunBuiltinNames.h                  |    1 +
 src/js/builtins/Transpiler.ts                      |  276 +++++
 src/runtime/api.rs                                 |    2 +
 src/runtime/api/JSBundler.classes.ts               |    9 +
 src/runtime/api/JSTranspiler.rs                    |  102 ++
 src/runtime/api/js_transpiler_ast.rs               | 1307 ++++++++++++++++++++
 .../bun-types/fixture/transpiler-unstable-parse.ts |   47 +
 test/js/bun/transpiler/unstable-parse.test.ts      |  477 +++++++
 10 files changed, 2570 insertions(+)

gate history · 15 passed · 1 rejected · iteration 12

evidence per changed file
file                                                      reads  edits  tests
docs/runtime/transpiler.mdx                                   3      5      0
packages/bun-types/bun.d.ts                                   3      4      0
src/js/builtins/BunBuiltinNames.h                             0      0      0
src/js/builtins/Transpiler.ts                                 4     11      0
src/runtime/api.rs                                            1      1      0
src/runtime/api/JSBundler.classes.ts                          1      1      0
src/runtime/api/JSTranspiler.rs                               7     17      0
src/runtime/api/js_transpiler_ast.rs                         21     51      0
…egration/bun-types/fixture/transpiler-unstable-parse.ts      0      2      0
test/js/bun/transpiler/unstable-parse.test.ts                 6     26      0

Adds Bun.Transpiler.prototype.unstable_parse(code, loader?), which parses
JS/TS/JSX and returns Bun's internal AST as a plain JavaScript object tree.

The output is explicitly not ESTree: node 'kind' strings are the snake_case
tags the bun_ast enums already derive via strum, so no mapping table has to
be maintained. The API is documented and typed as unstable; the shape may
change or be removed in any patch release.

Implementation: the AST is serialized to a single ASCII-only JSON buffer in
Rust, then handed to JSC's JSONParse in one call. This avoids one FFI
crossing + Structure transition per property that a direct putDirect-per-
field approach would pay, and keeps the buffer on the zero-copy
StringImpl::createWithoutCopying path.

Closes #2729
Closes #12896
@robobun
robobun requested a review from alii as a code owner August 1, 2026 19:16
@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds the unstable Bun.Transpiler.unstable_parse() API. It parses source into Bun’s internal AST in object or raw tape format, exposes lazy AST proxies and traversal, adds declarations and documentation, and includes comprehensive tests.

Changes

Transpiler AST API

Layer / File(s) Summary
API contract and exposure
packages/bun-types/bun.d.ts, src/runtime/api.rs, src/runtime/api/JSBundler.classes.ts, src/js/builtins/BunBuiltinNames.h, docs/runtime/transpiler.mdx
Adds unstable_parse, raw-format overloads, AST interfaces, runtime bindings, builtin registration, and documentation for the unstable AST.
Parsing and JSON serialization
src/runtime/api/JSTranspiler.rs, src/runtime/api/js_transpiler_ast.rs
Validates source and loader options, parses input, and serializes AST metadata and node types to JSON.
Raw tape serialization and lazy AST runtime
src/runtime/api/js_transpiler_ast.rs, src/js/builtins/Transpiler.ts
Adds the versioned binary tape format, typed decoding, lazy node and array proxies, JSON conversion, and kind-based traversal.
AST behavior validation
test/js/bun/transpiler/unstable-parse.test.ts
Tests AST structure, locations, modules, expressions, JSX, loaders, errors, binary input, symbols, raw output, proxies, and traversal.

Suggested reviewers: alii, jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes expose Bun's internal AST through a JavaScript Transpiler API, supporting AST tooling, analysis, and reduced reliance on third-party parsers [#2729, #12896].
Out of Scope Changes check ✅ Passed The implementation, typings, documentation, builtins, serializers, and tests directly support the linked AST parser and transformation objectives.
Title check ✅ Passed The title clearly identifies the addition of unstable_parse() and its purpose of exposing the internal AST.
Description check ✅ Passed The description includes both required sections and provides detailed implementation, behavior, and verification information.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the claude label Aug 1, 2026
Comment thread src/runtime/api/JSTranspiler.rs Outdated
Comment thread src/runtime/api/JSTranspiler.rs Outdated
Comment thread src/runtime/api/JSTranspiler.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/api/js_transpiler_ast.rs`:
- Around line 1081-1083: Update g_property so its serialized object includes the
property location alongside kind, preserving the documented node contract for
consumers that access loc after identifying nodes by kind. Reuse the existing
location serialization pattern and ensure the output remains valid JSON with
both fields.

In `@test/js/bun/transpiler/unstable-parse.test.ts`:
- Around line 248-251: Extend the “parse errors throw” test in the
unstable_parse suite with a deeply nested expression built using
Buffer.alloc(count, fill).toString(), and assert that parsing it throws the
stack-overflow error produced by the depth-limit path. Ensure the input is deep
enough to exercise the stmt, expr, or binding recursion guards rather than a
normal syntax error.
- Around line 239-251: Replace the bare toThrow() assertions in the “loader
argument overrides constructor default” and “parse errors throw” tests with
assertions matching the exact diagnostic messages emitted by unstable_parse.
Keep the existing valid AST assertion unchanged and ensure each failure case
verifies the intended parser error rather than merely any thrown value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9279e391-d673-4ab4-b886-36727d29b1b4

📥 Commits

Reviewing files that changed from the base of the PR and between f91d5c9 and d75933f.

📒 Files selected for processing (7)
  • docs/runtime/transpiler.mdx
  • packages/bun-types/bun.d.ts
  • src/runtime/api.rs
  • src/runtime/api/JSBundler.classes.ts
  • src/runtime/api/JSTranspiler.rs
  • src/runtime/api/js_transpiler_ast.rs
  • test/js/bun/transpiler/unstable-parse.test.ts

Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread test/js/bun/transpiler/unstable-parse.test.ts
Comment thread test/js/bun/transpiler/unstable-parse.test.ts
…clash; tighten toThrow matchers; add depth-limit test

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/runtime/transpiler.mdx (1)

216-218: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Document the approximateNewlineCount field.

The serializer in js_transpiler_ast.rs emits a top-level approximateNewlineCount field. This sentence lists stmts, symbols, importRecords, hashbang, directive, and exportsKind, but omits approximateNewlineCount. Add it to the list so the documented contract matches the actual output.

📝 Proposed fix
-The top-level result carries `stmts`, `symbols`, `importRecords`, `hashbang`,
-`directive`, and `exportsKind`. Identifier nodes resolve their `Ref` to the
+The top-level result carries `stmts`, `symbols`, `importRecords`, `hashbang`,
+`directive`, `exportsKind`, and `approximateNewlineCount`. Identifier nodes resolve their `Ref` to the
 symbol's original name string.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/runtime/transpiler.mdx` around lines 216 - 218, Update the top-level
result field list in the transpiler documentation to include
approximateNewlineCount alongside stmts, symbols, importRecords, hashbang,
directive, and exportsKind, preserving the existing description of identifier
Ref resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/runtime/transpiler.mdx`:
- Around line 216-218: Update the top-level result field list in the transpiler
documentation to include approximateNewlineCount alongside stmts, symbols,
importRecords, hashbang, directive, and exportsKind, preserving the existing
description of identifier Ref resolution.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7e959b55-7ad9-4590-a65a-a439f794745f

📥 Commits

Reviewing files that changed from the base of the PR and between d75933f and 3b6c16f.

📒 Files selected for processing (3)
  • docs/runtime/transpiler.mdx
  • src/runtime/api/JSTranspiler.rs
  • src/runtime/api/js_transpiler_ast.rs

Comment thread test/js/bun/transpiler/unstable-parse.test.ts Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found, but this adds a new user-facing API (unstable_parse) backed by a ~1,100-line hand-rolled AST→JSON serializer covering all 84 Stmt/Expr/Binding variants — a maintainer should sign off on the API surface and output-shape decisions.

What was reviewed:

  • unstable_parse host fn: arena/log lifetime via TranspilerStateGuard matches scan(); code_holder outlives the JSON buffer.
  • str() WTF-8 decoding now matches decode_wtf8_one (unclamped width, stray continuation → U+FFFD); e_string rope handling checked for the UTF-16-head case (falls through to str16 on the head only — parser doesn't produce mixed-encoding ropes).
  • f64() non-finite → null, -0 → 0 (acceptable for an unstable AST dump); ref_() bounds-checks symbols.
  • Prior review feedback (bare toThrow, kind/loc contract, depth-limit test, comment-cop) all addressed at HEAD.
Extended reasoning...

Overview

Adds Bun.Transpiler.prototype.unstable_parse(code, loader?), which parses JS/TS/JSX and returns Bun's internal AST as a plain JS object tree. The implementation is a new ~1,100-line src/runtime/api/js_transpiler_ast.rs that walks every Stmt (33), Expr (47), Binding (4), and shared G::* variant, emitting a single ASCII-only JSON buffer that is then fed to JSC's JSONParse. The host fn in JSTranspiler.rs (~90 lines) mirrors scan()'s arena/log/guard scaffolding exactly. Also touches: .classes.ts proto entry, bun.d.ts types (UnstableAST/UnstableASTNode), transpiler.mdx docs, api.rs module wiring, and a 288-line test file (22 tests).

Security risks

Low. Input is user-supplied source code (already handled by every other Bun.Transpiler method); the only new attack surface is the JSON serializer, which writes into a growable Vec<u8> with no fixed-size buffers. Recursion is guarded by StackCheck at stmt/expr/binding entry (with a test asserting RangeError on a 200k-deep e_binary chain). Malformed UTF-8 via Uint8Array was the one edge I flagged in a prior run — now fixed in db81afb with a regression test. The output goes through JSC's own JSON.parse, so a malformed emitter would surface as a thrown SyntaxError, not memory corruption.

Level of scrutiny

High — should not be auto-approved. This is new public API surface, and REVIEW.md's API-design section (.claude/docs/landing-prs.md) explicitly calls for maintainer agreement on new user-facing APIs. Even labeled unstable_, this closes two long-standing issues (#2729, #12896) and makes concrete product decisions a human should ratify: the non-ESTree shape, snake_case strum tags as node kinds, Ref→name resolution, flattening parts into one stmts array, the JSON-buffer-then-JSONParse strategy vs. direct putDirect construction, and what "unstable" commits Bun to going forward. The serializer itself is also large enough (every AST variant hand-matched) that a reviewer familiar with bun_ast should skim for missing/mis-mapped fields.

Other factors

  • All prior automated feedback (comment-cop, CodeRabbit, my earlier inline comments on bare toThrow() and the str() WTF-8 clamp bug) is resolved and verified at HEAD.
  • Test coverage is solid: 22 tests spanning locals/functions/classes/destructuring/imports/optional-chain/templates/regex/try-switch/UTF-16 round-trip/non-finite numbers/loader override/parse errors/depth limit/Uint8Array/malformed UTF-8/JSON round-trip, plus a recursive "every kind has a loc" invariant walker.
  • The host-fn lifetime handling (TranspilerStateGuard, detach_lifetime_ref, ASTMemoryAllocator scope) is copied verbatim from the adjacent scan() method, so it inherits that path's review history.
  • The .classes.ts entry maps unstable_parse → unstableParse; the #[bun_jsc::host_fn(method)] attribute on unstable_parse should generate the matching camelCase symbol — worth a quick sanity check by whoever builds it, but it follows the same pattern as transformSync/transform_sync.

Comment thread src/js/builtins/Transpiler.ts
…eError

A single UTF-16 string literal with >~500K code units would throw under
JSC's apply-argument cap while object mode streams per-unit escapes.
Chunk at 0x1000 units like src/node-fallbacks/buffer.js. Covered by the
new 'large UTF-16 string literal' test (20K units across 5 chunks).
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
…ches raw mode

Object mode emitted one U+FFFD per byte on ill-formed UTF-8 (i += 1
after each failure), while raw mode decodes via TextDecoder which
consumes the maximal subpart per FFFD. Align str() with the WHATWG
table so both modes agree. The malformed-UTF-8 test now asserts the
WHATWG count in both modes (one FFFD for a truncated [F0 9F 9F]; three
for [F0 80 80] since F0's second byte must be 90..BF).
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
decode_wtf8_rune_t accepts [ED A0..BF 80..BF] and returns the surrogate
code point, so object mode emitted \uD800 while raw mode (strict UTF-8
via TextDecoder) emits 3x FFFD. Route 0xD800..=0xDFFF through
utf8_maximal_subpart so both modes agree. Covered by a new ED A0 80
case in the malformed-UTF-8 test.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues on 31daac7 — the earlier findings (options-getter UAF, empty-string pool collision, BOM stripping, lone-surrogate divergence, apply arg cap, maximal-subpart FFFD count, WTF-8 surrogate handling) are all addressed and covered by tests. Deferring to a maintainer given this is ~3.5K lines adding a new user-facing API with a novel binary tape format and two parallel AST walkers.

What was reviewed:

  • unstable_parse host fn: options [[Get]]s now precede the buffer borrow; arena/log restore via TranspilerStateGuard matches scan().
  • Tape format: u32 overflow guard is sticky and rechecked at recursive entry; string-pool interning handles the empty-string offset collision on the reader side.
  • Object/raw equivalence: str() now uses WHATWG maximal-subpart and rejects WTF-8 surrogates; readString16 chunks fromCharCode.apply; TextDecoder uses ignoreBOM.
  • Builtin JS: uses $-intrinsics ($Map, $Array, $get/$set, $apply, $call, $isArray) throughout.
Extended reasoning...

Overview

Adds Bun.Transpiler.prototype.unstable_parse() exposing Bun's internal AST in two output modes: an object tree (Rust → ASCII JSON → JSONParse) and a self-describing binary tape wrapped in lazy Proxies with a visit() walker. The bulk is src/runtime/api/js_transpiler_ast.rs (2435 lines: two complete AST walkers over all 33 Stmt / 47 Expr / 4 Binding variants plus shared G::* nodes) and src/js/builtins/Transpiler.ts (276 lines: DataView-backed tape reader). Also touches .classes.ts, BunBuiltinNames.h, bun.d.ts, docs, and adds a 496-line test file.

Security risks

Low. The API is read-only over an in-memory AST the caller already controls. The one memory-safety concern in prior rounds (a Uint8Array backing store detached via an options-object getter between borrow and parse) was fixed by moving all [[Get]]s before code_holder.slice(), with a regression test. The tape reader operates on a Rust-produced buffer, not user bytes; StackCheck guards recursion in both walkers; u32 tape-offset overflow throws RangeError rather than truncating.

Level of scrutiny

This warrants a maintainer look, not auto-approval. It introduces a new user-facing API (even if unstable_-prefixed and @experimental), a novel binary tape encoding, and two large parallel walker implementations that must stay in sync with bun_ast and with each other. Per the repo's review guidance, new cross-cutting API surface and design decisions of this scope should be signed off by a maintainer. The author agent's own summary says "Ready for a maintainer look."

Other factors

Eight review rounds have already tightened the object/raw equivalence invariant substantially, and each fix landed with a targeted test (lone surrogates, BOM, empty-string pool aliasing, malformed UTF-8 maximal-subpart, WTF-8 surrogates, large UTF-16 literals, hostile options getters). Test coverage is broad (39 tests) and CI is green on the touched file. No outstanding unresolved review threads. The remaining questions are design/ownership ones — whether the tape format and visit() API shape are what maintainers want to commit to — not correctness concerns I can point at a line for.

Per maintainer direction: the lazy-proxy tape reader is strictly faster
and exposes the same shape, so remove the JSON serializer path and the
{format:'raw'} option entirely.

- js_transpiler_ast.rs: delete ast_to_json, Writer, utf8_maximal_subpart
  (~1130 lines). Only the tape writer remains.
- JSTranspiler.rs: remove format-option parsing and the JSON.parse path;
  always build the tape and return {buffer}.
- Transpiler.ts: drop the object-mode passthrough.
- bun.d.ts / transpiler.mdx: collapse to a single overload returning
  UnstableParseResult = {buffer, root, visit}.
- tests: merge the two describe blocks; all assertions go through root.
Comment thread src/runtime/api/js_transpiler_ast.rs Outdated
Comment thread src/js/builtins/Transpiler.ts
Comment thread src/runtime/api/js_transpiler_ast.rs
- nodeHandler.get now falls through to Reflect.get(target, prop) for
  non-tape keys (like arrayHandler does), so String(node), template
  interpolation, and .hasOwnProperty() no longer throw 'No default
  value'. Covered by 'node proxy inherits Object.prototype'.
- f_f64's i32 fast path now compares bit patterns so -0.0 keeps the
  TY_F64 slot instead of collapsing to +0. Covered by an Object.is(-0)
  assertion in the non-finite test.
Comment thread src/js/builtins/Transpiler.ts
Comment thread packages/bun-types/bun.d.ts Outdated
Helper nodes (g_decl, g_arg, catch, ...) are now traversed but never
passed to enter/visitors, so enter's UnstableASTNode parameter type
(kind: string, loc: number|null) is honored at runtime. Updated the
expected nodeCount (20 -> 15) and the helper-node test to assert
typeof n.kind is always string inside enter.
Comment thread src/js/builtins/Transpiler.ts Outdated
The native host fn always returns {buffer: ArrayBuffer} or throws; the
guard was a leftover from when object mode could pass through, and it
routed through user-overridable ArrayBuffer[Symbol.hasInstance].
Comment thread src/runtime/api/js_transpiler_ast.rs
approximate_newline_count * 96 has no ceiling, so newline-dense input
could request a multi-GB reserve that aborts on Windows / strict
overcommit. The Vec grows as needed regardless; this is just a hint.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found this run — all prior findings have been addressed. Deferring to a human because this adds a new public API surface (unstable_parse) with a novel binary tape format and ~2,300 lines of new code, which is an API-design decision a maintainer should sign off on.

What was reviewed:

  • Tape writer memory safety: StackCheck guards on every recursive entry, sticky overflow flag rechecked, reserve hint clamped to 16 MiB.
  • Arena/log lifetime in unstable_parse matches the scan() pattern via TranspilerStateGuard; input buffer borrowed only after option getters run.
  • JS builtin proxy traps: get/has fall through to Reflect, readString16 chunked, empty-string cache collision short-circuited, ignoreBOM: true.
  • -0 preserved via bitwise compare; visit() only dispatches enter on kind-bearing nodes.
Extended reasoning...

Overview

This PR adds Bun.Transpiler.prototype.unstable_parse(), exposing Bun's internal AST via a self-describing binary tape (src/runtime/api/js_transpiler_ast.rs, ~1,300 LOC) plus a lazy-Proxy reader in a new JS builtin (src/js/builtins/Transpiler.ts, ~276 LOC). It wires a host fn into JSTranspiler.rs reusing the scan() arena/log/guard pattern, registers the builtin via .classes.ts + BunBuiltinNames.h, and ships docs, .d.ts types, and 34 tests. Net +2,323 lines across 9 files.

Security risks

Low. The API only parses user-supplied source and serializes an already-parsed AST into a fresh Vec<u8> handed back as an ArrayBuffer; no filesystem, network, or privilege boundaries are crossed. The bounds concerns (unbounded reserve from newline count, u32 tape offsets) were flagged in prior rounds and are now clamped/checked. The options-getter-detaches-buffer hazard is handled by borrowing the input after all [[Get]]s.

Level of scrutiny

High — but for design reasons rather than correctness risk. This is new user-facing API surface with a bespoke binary encoding, a lazy-Proxy contract, and a visit() traversal API. Even though it is explicitly unstable_-prefixed and @experimental, the shape (tape header, snake_case kinds, {buffer, root, visit}, which fields each node exposes) is a design commitment that closes two long-standing issues. Per the repo's review guidance, new API surface and cross-cutting abstractions warrant maintainer agreement.

Other factors

The PR has been through 12 review iterations; every inline finding I raised (empty-string pool aliasing, BOM stripping, apply arg cap, WHATWG maximal-subpart divergence, -0 fast-path, nodeHandler prototype fall-through, has/get asymmetry, enter on kind-less helpers, tamperable instanceof ArrayBuffer, unbounded reserve) is now resolved with a corresponding test. The Rust host fn mirrors scan() closely, so the arena/log RAII contract is well-trodden. No human reviewer has weighed in yet, and given the scope this should not merge on bot approval alone.

- Add UnstableStmtKind/UnstableExprKind/UnstableBindingKind string-literal
  unions (81 kinds) derived from bun_ast's Tag enums, and UnstableASTKind
  as their union.
- UnstableParseVisitors is now a mapped type over UnstableASTKind, so
  visitor keys autocomplete, unknown keys are a type error, and each
  handler's node parameter has kind narrowed to its specific literal.
- UnstableASTNode<K> is now generic over the kind.
- Full JSDoc on every UnstableParseResult/UnstableAST field:
  buffer/root/visit semantics, what approximateNewlineCount actually
  counts and why it's a hint not a line count, exportsKind values,
  importRecords/symbols provenance, loc as a byte offset, etc.
- Add a types-integration fixture that asserts narrowing, rejects unknown
  keys via @ts-expect-error, and checks the overload shapes.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No bugs found in this revision, but this is a ~2,300-line new user-facing API (tape format + lazy Proxy reader + 1,300-line AST walker) — worth a maintainer sign-off on the API shape before it lands.

What was reviewed:

  • Tape writer memory safety: StackCheck at every recursive entry, u32 overflow guard, reserve hint clamped to 16 MiB, TranspilerStateGuard restores arena/log on all exits.
  • Options-getter ordering: StringOrBuffer::from_js runs after every options [[Get]], so a hostile getter can't detach the input buffer under a live slice (test pins this).
  • v_e_string UTF-16 rope handling: verifiers confirmed is_utf8() is false when the head is UTF-16, so the rope-flatten branch is not taken and slice16() returns the full string.
  • Proxy reader edge cases from prior rounds (empty-string pool aliasing, BOM stripping, apply-arg cap, -0, lone surrogates, WTF-8 surrogate bytes, Reflect.get/has fallthrough) — all fixed with tests.
Extended reasoning...

Overview

This PR adds Bun.Transpiler.prototype.unstable_parse(), a new public method that parses JS/TS/JSX and returns Bun's internal AST as { buffer, root, visit }. The implementation spans:

  • src/runtime/api/js_transpiler_ast.rs (1,307 lines, new): a TapeWriter that walks the entire bun_ast::Ast (all 33 Stmt, 47 Expr, 4 Binding variants plus G::* shared nodes) into a self-describing binary tape with a header, key-name table, 12-byte field slots, and a deduped string pool.
  • src/js/builtins/Transpiler.ts (276 lines, new): a JS builtin that wraps the tape in DataView-backed lazy Proxy objects with get/has/ownKeys/getOwnPropertyDescriptor/mutation-reject traps, plus a visit() walker and toJSON support.
  • src/runtime/api/JSTranspiler.rs (+102): the unstable_parse host fn, which reuses the existing get_parse_result/TranspilerStateGuard scaffolding from scan().
  • packages/bun-types/bun.d.ts (+288): full type declarations including UnstableStmtKind/UnstableExprKind/UnstableBindingKind unions, UnstableParseResult, UnstableParseVisitors, and thorough JSDoc.
  • test/js/bun/transpiler/unstable-parse.test.ts (477 lines, 34 tests) plus a bun-types fixture and docs.

Security risks

None identified. Input is user-provided source code fed to the same parser scan()/transformSync() already use; the new code only reads the resulting AST. The tape reserve hint is clamped so newline-dense input can't drive an unbounded allocation, deep nesting throws RangeError via StackCheck rather than overflowing, and tape offsets are guarded against u32 overflow. The options-object [[Get]]s run before the input buffer is borrowed, closing the detach-under-slice hazard. The JS builtin uses $-prefixed intrinsics ($Map, $Array, .$get/.$set/.$apply/.$call, $isArray) and the earlier tamper-able instanceof ArrayBuffer guard was removed.

Level of scrutiny

High. This is a brand-new user-facing API surface with a bespoke binary format and Proxy-based lazy reader — even with the unstable_ prefix and @experimental tags, the API shape (method name, {buffer, root, visit} return, kind/loc contract, visitor semantics, whether buffer should be exposed at all) is a design decision a maintainer should sign off on. REVIEW.md's API-design guidance in .claude/docs/landing-prs.md applies. The implementation is also large enough (1,300+ lines of new native code exhaustively matching every AST variant) that a human pass over the field selections is warranted.

Other factors

  • The PR has been through 12 iterations; every prior bot finding (empty-string pool aliasing, BOM stripping, String.fromCharCode.apply arg cap, -0 fast-path, lone surrogates, WTF-8-encoded surrogates, Reflect.get/has fallthrough, kind-less-node dispatch, unclamped reserve hint, instanceof ArrayBuffer tamper) has a corresponding fix + test in the current diff, and all threads are resolved.
  • Test coverage is thorough (34 tests including UTF-16 round-trip, malformed UTF-8, options-getter detach, deep-nesting RangeError, proxy mutation rejection, visit() non-dispatch on helper nodes) and USE_SYSTEM_BUN=1 fail-before is verified.
  • No human reviewer has weighed in yet; only bots have commented.
  • Verifiers examined and refuted a concern about v_e_string dropping rope-tail segments when the head is UTF-16 — is_utf8() returns false in that case, so the rope-flatten branch is skipped and slice16() returns the full flattened string.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support AST transforms Bun AST Parser

2 participants