repo: remove empty files, re-export shims, and phase_c_exports.rs - #33925
Merged
Conversation
Delete 50 vestigial .rs files left over from the Zig port and consolidate
their contents into their parent modules:
- 9 literally-empty files (css/properties/{effects,list,svg},
bake/DevServer/{RouteBundle,WatcherAtomics,DirectoryWatchStore,
IncrementalGraph}, http/websocket_http_client, webcore/lib,
io/stub_event_loop)
- 8 single-line re-export shims inlined into parent mod decls
(versioned_url, ExternalSlice, JSInternalPromise, ThreadLock, thread_id,
StaticPipeWriter, multipart_options, NoticeResponse, ValkeyContext)
- 27 one-line expect() matchers folded into expect/simple_matchers.rs
- bake/DevServer/ directory dissolved into dev_server/ (3 files moved,
*_body suffix dropped)
- empty bun_js crate removed from workspace
- runtime/ffi_imports.rs deleted (empty placeholder); jsc/ffi_imports.rs
renamed jsc_abi.rs (it only holds the jsc_abi_extern! macro)
phase_c_exports.rs dissolved into real homes:
- Bun__panic / Bun__outOfMemory -> bun_bin/c_abi_exports.rs
- Bun__VM__scriptExecutionStatus -> virtual_machine_exports.rs with a
REAL body (was a stub that always returned Running, masking
VirtualMachine::script_execution_status)
- Bun__LifecycleAgent{PreventExit,StopPreventingExit}: dead on both
sides; extern decls removed from Debugger.rs and
InspectorLifecycleAgent.cpp
- JSC__JSValue__parseJSON: never defined in C++, never called from Rust;
extern + wrapper + header decl removed
- BunString__toErrorInstance: nothing referenced it; removed
- DNSResolver__getConstructor: fixed at the source. generate-classes.ts
now gates the getConstructor extern on !noConstructor, and dns.rs
Resolver gets #[JsClass(no_constructor)] to match its .classes.ts
Net: 85 files changed, +219 -718.
Collaborator
Author
Contributor
WalkthroughThe PR removes the standalone ChangesABI and crate boundaries
DevServer request and accounting flow
Test runner matcher consolidation
Public module consolidation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
Collaborator
Author
|
This is a maintainer-directed structural cleanup (deleting empty files, inlining re-export shims, consolidating one-line modules). The diff is behaviorally a no-op except for one incidental fix:
Every other change is file moves and re-export inlining; |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/bake/dev_server/error_report_request.rs`:
- Around line 1-17: Update the module-level payload documentation to match the
field order and types read by run_with_body: document length-prefixed name,
message, and browser_url strings, followed by the frame count; describe each
frame as line, column, function name, then file name, with accurate
unavailable-value semantics. Remove the nonexistent raw message ID and correct
the frame field ordering.
- Around line 439-462: Guard the length before subtracting in the line-scanning
logic that computes index_of_first_line: replace the contents.len() - 2 check
with an overflow-safe condition such as contents.len() < 2 or an equivalent
checked comparison. Ensure contents[i + 1] is only accessed after proving both
bytes are in bounds, while preserving the existing early return behavior.
In `@src/runtime/test_runner/expect/simple_matchers.rs`:
- Around line 36-43: The to_be_negative and to_be_positive matchers incorrectly
round numbers before checking their sign. Replace n.round() with direct n < 0.0
and n > 0.0 comparisons, respectively, while preserving the !n.is_infinite() and
!n.is_nan() guards.
🪄 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: e1ac34c2-cb9d-413a-b8c8-4b9e00f4f5a9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
Cargo.tomlsrc/bun_bin/c_abi_exports.rssrc/bun_bin/lib.rssrc/bun_bin/phase_c_exports.rssrc/codegen/generate-classes.tssrc/codegen/generate-host-exports.tssrc/css/properties/effects.rssrc/css/properties/list.rssrc/css/properties/mod.rssrc/css/properties/svg.rssrc/http/lib.rssrc/http/websocket_http_client.rssrc/install/ExternalSlice.rssrc/install/lib.rssrc/install/versioned_url.rssrc/io/lib.rssrc/io/stub_event_loop.rssrc/js/Cargo.tomlsrc/js/lib.rssrc/jsc/Debugger.rssrc/jsc/JSInternalPromise.rssrc/jsc/JSValue.rssrc/jsc/bindings/InspectorLifecycleAgent.cppsrc/jsc/bindings/headers.hsrc/jsc/ffi_imports.rssrc/jsc/jsc_abi.rssrc/jsc/lib.rssrc/jsc/virtual_machine_exports.rssrc/runtime/Cargo.tomlsrc/runtime/api/bun/subprocess.rssrc/runtime/api/bun/subprocess/StaticPipeWriter.rssrc/runtime/bake/DevServer.rssrc/runtime/bake/DevServer/DirectoryWatchStore.rssrc/runtime/bake/DevServer/IncrementalGraph.rssrc/runtime/bake/DevServer/RouteBundle.rssrc/runtime/bake/DevServer/WatcherAtomics.rssrc/runtime/bake/dev_server/assets.rssrc/runtime/bake/dev_server/error_report_request.rssrc/runtime/bake/dev_server/hmr_socket.rssrc/runtime/bake/dev_server/memory_cost.rssrc/runtime/bake/dev_server/mod.rssrc/runtime/dns_jsc/dns.rssrc/runtime/ffi_imports.rssrc/runtime/lib.rssrc/runtime/test_runner/expect.rssrc/runtime/test_runner/expect/simple_matchers.rssrc/runtime/test_runner/expect/toBeArray.rssrc/runtime/test_runner/expect/toBeBoolean.rssrc/runtime/test_runner/expect/toBeDate.rssrc/runtime/test_runner/expect/toBeDefined.rssrc/runtime/test_runner/expect/toBeFalse.rssrc/runtime/test_runner/expect/toBeFalsy.rssrc/runtime/test_runner/expect/toBeFinite.rssrc/runtime/test_runner/expect/toBeFunction.rssrc/runtime/test_runner/expect/toBeGreaterThan.rssrc/runtime/test_runner/expect/toBeGreaterThanOrEqual.rssrc/runtime/test_runner/expect/toBeInteger.rssrc/runtime/test_runner/expect/toBeLessThan.rssrc/runtime/test_runner/expect/toBeLessThanOrEqual.rssrc/runtime/test_runner/expect/toBeNaN.rssrc/runtime/test_runner/expect/toBeNegative.rssrc/runtime/test_runner/expect/toBeNil.rssrc/runtime/test_runner/expect/toBeNull.rssrc/runtime/test_runner/expect/toBeNumber.rssrc/runtime/test_runner/expect/toBePositive.rssrc/runtime/test_runner/expect/toBeString.rssrc/runtime/test_runner/expect/toBeSymbol.rssrc/runtime/test_runner/expect/toBeTrue.rssrc/runtime/test_runner/expect/toBeTruthy.rssrc/runtime/test_runner/expect/toBeUndefined.rssrc/runtime/test_runner/expect/toEndWith.rssrc/runtime/test_runner/expect/toInclude.rssrc/runtime/test_runner/expect/toStartWith.rssrc/runtime/test_runner/mod.rssrc/runtime/valkey_jsc/ValkeyContext.rssrc/runtime/valkey_jsc/mod.rssrc/runtime/webcore.rssrc/runtime/webcore/lib.rssrc/runtime/webcore/s3/multipart_options.rssrc/safety/ThreadLock.rssrc/safety/lib.rssrc/safety/thread_id.rssrc/sql/lib.rssrc/sql/postgres/protocol/NoticeResponse.rs
💤 Files with no reviewable changes (60)
- src/css/properties/svg.rs
- src/runtime/bake/DevServer/WatcherAtomics.rs
- src/css/properties/effects.rs
- src/css/properties/list.rs
- src/http/websocket_http_client.rs
- src/runtime/test_runner/expect/toBeBoolean.rs
- src/runtime/test_runner/expect/toBeUndefined.rs
- src/runtime/test_runner/expect/toBeNil.rs
- src/runtime/bake/DevServer/DirectoryWatchStore.rs
- src/runtime/test_runner/expect/toBeFinite.rs
- src/sql/postgres/protocol/NoticeResponse.rs
- src/runtime/bake/DevServer/RouteBundle.rs
- src/runtime/test_runner/expect/toBeLessThan.rs
- src/runtime/test_runner/expect/toBeGreaterThanOrEqual.rs
- src/runtime/test_runner/expect/toBeLessThanOrEqual.rs
- src/runtime/test_runner/expect/toBeArray.rs
- src/runtime/test_runner/expect/toBeNaN.rs
- src/runtime/bake/DevServer/IncrementalGraph.rs
- src/runtime/test_runner/expect/toBeTruthy.rs
- src/runtime/test_runner/expect/toBeGreaterThan.rs
- src/js/Cargo.toml
- src/runtime/test_runner/expect/toBeTrue.rs
- src/runtime/test_runner/expect/toBeInteger.rs
- src/runtime/test_runner/expect/toBeString.rs
- src/runtime/test_runner/expect/toEndWith.rs
- src/install/ExternalSlice.rs
- src/runtime/test_runner/expect/toBePositive.rs
- src/runtime/test_runner/expect/toBeFalsy.rs
- src/runtime/test_runner/expect/toBeNull.rs
- src/runtime/test_runner/expect/toBeDate.rs
- src/runtime/api/bun/subprocess/StaticPipeWriter.rs
- src/runtime/test_runner/expect/toBeNumber.rs
- src/io/lib.rs
- src/runtime/webcore/s3/multipart_options.rs
- src/jsc/JSInternalPromise.rs
- src/jsc/bindings/headers.h
- src/runtime/test_runner/expect/toBeSymbol.rs
- src/safety/ThreadLock.rs
- src/install/versioned_url.rs
- src/runtime/valkey_jsc/ValkeyContext.rs
- src/io/stub_event_loop.rs
- src/runtime/ffi_imports.rs
- src/js/lib.rs
- src/http/lib.rs
- src/runtime/test_runner/expect/toBeFalse.rs
- src/runtime/test_runner/expect/toBeDefined.rs
- src/runtime/test_runner/expect/toBeNegative.rs
- src/runtime/test_runner/expect/toInclude.rs
- src/runtime/test_runner/expect/toStartWith.rs
- Cargo.toml
- src/safety/thread_id.rs
- src/runtime/test_runner/expect/toBeFunction.rs
- src/jsc/Debugger.rs
- src/runtime/Cargo.toml
- src/jsc/bindings/InspectorLifecycleAgent.cpp
- src/bun_bin/phase_c_exports.rs
- src/jsc/ffi_imports.rs
- src/jsc/JSValue.rs
- src/css/properties/mod.rs
- src/runtime/lib.rs
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/bake/dev_server/error_report_request.rs`:
- Around line 1-17: Update the module-level payload documentation to match the
field order and types read by run_with_body: document length-prefixed name,
message, and browser_url strings, followed by the frame count; describe each
frame as line, column, function name, then file name, with accurate
unavailable-value semantics. Remove the nonexistent raw message ID and correct
the frame field ordering.
- Around line 439-462: Guard the length before subtracting in the line-scanning
logic that computes index_of_first_line: replace the contents.len() - 2 check
with an overflow-safe condition such as contents.len() < 2 or an equivalent
checked comparison. Ensure contents[i + 1] is only accessed after proving both
bytes are in bounds, while preserving the existing early return behavior.
In `@src/runtime/test_runner/expect/simple_matchers.rs`:
- Around line 36-43: The to_be_negative and to_be_positive matchers incorrectly
round numbers before checking their sign. Replace n.round() with direct n < 0.0
and n > 0.0 comparisons, respectively, while preserving the !n.is_infinite() and
!n.is_nan() guards.
🪄 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: e1ac34c2-cb9d-413a-b8c8-4b9e00f4f5a9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
Cargo.tomlsrc/bun_bin/c_abi_exports.rssrc/bun_bin/lib.rssrc/bun_bin/phase_c_exports.rssrc/codegen/generate-classes.tssrc/codegen/generate-host-exports.tssrc/css/properties/effects.rssrc/css/properties/list.rssrc/css/properties/mod.rssrc/css/properties/svg.rssrc/http/lib.rssrc/http/websocket_http_client.rssrc/install/ExternalSlice.rssrc/install/lib.rssrc/install/versioned_url.rssrc/io/lib.rssrc/io/stub_event_loop.rssrc/js/Cargo.tomlsrc/js/lib.rssrc/jsc/Debugger.rssrc/jsc/JSInternalPromise.rssrc/jsc/JSValue.rssrc/jsc/bindings/InspectorLifecycleAgent.cppsrc/jsc/bindings/headers.hsrc/jsc/ffi_imports.rssrc/jsc/jsc_abi.rssrc/jsc/lib.rssrc/jsc/virtual_machine_exports.rssrc/runtime/Cargo.tomlsrc/runtime/api/bun/subprocess.rssrc/runtime/api/bun/subprocess/StaticPipeWriter.rssrc/runtime/bake/DevServer.rssrc/runtime/bake/DevServer/DirectoryWatchStore.rssrc/runtime/bake/DevServer/IncrementalGraph.rssrc/runtime/bake/DevServer/RouteBundle.rssrc/runtime/bake/DevServer/WatcherAtomics.rssrc/runtime/bake/dev_server/assets.rssrc/runtime/bake/dev_server/error_report_request.rssrc/runtime/bake/dev_server/hmr_socket.rssrc/runtime/bake/dev_server/memory_cost.rssrc/runtime/bake/dev_server/mod.rssrc/runtime/dns_jsc/dns.rssrc/runtime/ffi_imports.rssrc/runtime/lib.rssrc/runtime/test_runner/expect.rssrc/runtime/test_runner/expect/simple_matchers.rssrc/runtime/test_runner/expect/toBeArray.rssrc/runtime/test_runner/expect/toBeBoolean.rssrc/runtime/test_runner/expect/toBeDate.rssrc/runtime/test_runner/expect/toBeDefined.rssrc/runtime/test_runner/expect/toBeFalse.rssrc/runtime/test_runner/expect/toBeFalsy.rssrc/runtime/test_runner/expect/toBeFinite.rssrc/runtime/test_runner/expect/toBeFunction.rssrc/runtime/test_runner/expect/toBeGreaterThan.rssrc/runtime/test_runner/expect/toBeGreaterThanOrEqual.rssrc/runtime/test_runner/expect/toBeInteger.rssrc/runtime/test_runner/expect/toBeLessThan.rssrc/runtime/test_runner/expect/toBeLessThanOrEqual.rssrc/runtime/test_runner/expect/toBeNaN.rssrc/runtime/test_runner/expect/toBeNegative.rssrc/runtime/test_runner/expect/toBeNil.rssrc/runtime/test_runner/expect/toBeNull.rssrc/runtime/test_runner/expect/toBeNumber.rssrc/runtime/test_runner/expect/toBePositive.rssrc/runtime/test_runner/expect/toBeString.rssrc/runtime/test_runner/expect/toBeSymbol.rssrc/runtime/test_runner/expect/toBeTrue.rssrc/runtime/test_runner/expect/toBeTruthy.rssrc/runtime/test_runner/expect/toBeUndefined.rssrc/runtime/test_runner/expect/toEndWith.rssrc/runtime/test_runner/expect/toInclude.rssrc/runtime/test_runner/expect/toStartWith.rssrc/runtime/test_runner/mod.rssrc/runtime/valkey_jsc/ValkeyContext.rssrc/runtime/valkey_jsc/mod.rssrc/runtime/webcore.rssrc/runtime/webcore/lib.rssrc/runtime/webcore/s3/multipart_options.rssrc/safety/ThreadLock.rssrc/safety/lib.rssrc/safety/thread_id.rssrc/sql/lib.rssrc/sql/postgres/protocol/NoticeResponse.rs
💤 Files with no reviewable changes (60)
- src/css/properties/svg.rs
- src/runtime/bake/DevServer/WatcherAtomics.rs
- src/css/properties/effects.rs
- src/css/properties/list.rs
- src/http/websocket_http_client.rs
- src/runtime/test_runner/expect/toBeBoolean.rs
- src/runtime/test_runner/expect/toBeUndefined.rs
- src/runtime/test_runner/expect/toBeNil.rs
- src/runtime/bake/DevServer/DirectoryWatchStore.rs
- src/runtime/test_runner/expect/toBeFinite.rs
- src/sql/postgres/protocol/NoticeResponse.rs
- src/runtime/bake/DevServer/RouteBundle.rs
- src/runtime/test_runner/expect/toBeLessThan.rs
- src/runtime/test_runner/expect/toBeGreaterThanOrEqual.rs
- src/runtime/test_runner/expect/toBeLessThanOrEqual.rs
- src/runtime/test_runner/expect/toBeArray.rs
- src/runtime/test_runner/expect/toBeNaN.rs
- src/runtime/bake/DevServer/IncrementalGraph.rs
- src/runtime/test_runner/expect/toBeTruthy.rs
- src/runtime/test_runner/expect/toBeGreaterThan.rs
- src/js/Cargo.toml
- src/runtime/test_runner/expect/toBeTrue.rs
- src/runtime/test_runner/expect/toBeInteger.rs
- src/runtime/test_runner/expect/toBeString.rs
- src/runtime/test_runner/expect/toEndWith.rs
- src/install/ExternalSlice.rs
- src/runtime/test_runner/expect/toBePositive.rs
- src/runtime/test_runner/expect/toBeFalsy.rs
- src/runtime/test_runner/expect/toBeNull.rs
- src/runtime/test_runner/expect/toBeDate.rs
- src/runtime/api/bun/subprocess/StaticPipeWriter.rs
- src/runtime/test_runner/expect/toBeNumber.rs
- src/io/lib.rs
- src/runtime/webcore/s3/multipart_options.rs
- src/jsc/JSInternalPromise.rs
- src/jsc/bindings/headers.h
- src/runtime/test_runner/expect/toBeSymbol.rs
- src/safety/ThreadLock.rs
- src/install/versioned_url.rs
- src/runtime/valkey_jsc/ValkeyContext.rs
- src/io/stub_event_loop.rs
- src/runtime/ffi_imports.rs
- src/js/lib.rs
- src/http/lib.rs
- src/runtime/test_runner/expect/toBeFalse.rs
- src/runtime/test_runner/expect/toBeDefined.rs
- src/runtime/test_runner/expect/toBeNegative.rs
- src/runtime/test_runner/expect/toInclude.rs
- src/runtime/test_runner/expect/toStartWith.rs
- Cargo.toml
- src/safety/thread_id.rs
- src/runtime/test_runner/expect/toBeFunction.rs
- src/jsc/Debugger.rs
- src/runtime/Cargo.toml
- src/jsc/bindings/InspectorLifecycleAgent.cpp
- src/bun_bin/phase_c_exports.rs
- src/jsc/ffi_imports.rs
- src/jsc/JSValue.rs
- src/css/properties/mod.rs
- src/runtime/lib.rs
🛑 Comments failed to post (3)
src/runtime/bake/dev_server/error_report_request.rs (2)
1-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Module doc's payload layout is out of sync with
run_with_body.The doc lists
u32 message id, then length+message, then length+error name, with nobrowser_urlfield, and frame fields orderedfilename, function name. The actual reads inrun_with_bodyarename,message,browser_url(all length-prefixed strings, no raw numeric id), then per-frameline, column, function_name, file_name. This mismatch will mislead anyone extending the wire protocol.🤖 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 `@src/runtime/bake/dev_server/error_report_request.rs` around lines 1 - 17, Update the module-level payload documentation to match the field order and types read by run_with_body: document length-prefixed name, message, and browser_url strings, followed by the frame count; describe each frame as line, column, function name, then file name, with accurate unavailable-value semantics. Remove the nonexistent raw message ID and correct the frame field ordering.
439-462: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Potential arithmetic underflow / OOB panic on short
contents.
contents.len() - 2underflows whencontents.len() < 2(reachable when a bundled file'squoted_contents()is a single byte, e.g. an empty/tiny source file that happens to start with a backslash). This panics on subtraction overflow in debug builds, and in release (with overflow checks off) defeats the bounds guard, leading to an out-of-bounds panic atcontents[i + 1]shortly after.Per repo convention, size/index arithmetic on external content must not rely on debug-only overflow checks.
🛡️ Proposed fix to avoid the underflow
- Some(i) => { - if i >= contents.len() - 2 { - return Ok(None); - } + Some(i) => { + if i + 2 > contents.len() { + return Ok(None); + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.let mut line: usize = 0; let mut prev: usize = 0; let index_of_first_line: usize = if target_line == 0 { 0 // no iteration needed } else { 'find: loop { match strings::index_of_char_pos(contents, b'\\', prev) { Some(i) => { if i + 2 > contents.len() { return Ok(None); } // Bun's JSON printer will not use a sillier encoding for newline. if contents[i + 1] == b'n' { line += 1; if line == target_line as usize { break 'find i + 2; } } prev = i + 2; } None => return Ok(None), } } };🤖 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 `@src/runtime/bake/dev_server/error_report_request.rs` around lines 439 - 462, Guard the length before subtracting in the line-scanning logic that computes index_of_first_line: replace the contents.len() - 2 check with an overflow-safe condition such as contents.len() < 2 or an equivalent checked comparison. Ensure contents[i + 1] is only accessed after proving both bytes are in bounds, while preserving the existing early return behavior.src/runtime/test_runner/expect/simple_matchers.rs (1)
36-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' src/runtime/test_runner/expect/simple_matchers.rsRepository: oven-sh/bun
Length of output: 4260
🏁 Script executed:
rg -n "toBeNegative|toBePositive|to_be_negative|to_be_positive" src testRepository: oven-sh/bun
Length of output: 3856
Use a direct sign check for these matchers in
src/runtime/test_runner/expect/simple_matchers.rs:36-43—round()makes values like-0.4and0.3failtoBeNegative/toBePositive; keep the!is_infinite()guard and drop the rounding.🤖 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 `@src/runtime/test_runner/expect/simple_matchers.rs` around lines 36 - 43, The to_be_negative and to_be_positive matchers incorrectly round numbers before checking their sign. Replace n.round() with direct n < 0.0 and n > 0.0 comparisons, respectively, while preserving the !n.is_infinite() and !n.is_nan() guards.
Collaborator
Author
|
All three CodeRabbit findings are pre-existing behavior in code this PR moves verbatim, not code it introduces:
Keeping these out of scope so the diff stays reviewable as a pure file move. Happy to open follow-ups for the underflow guard and the matcher semantics if wanted. |
…nsolidation 27 one-line matcher files were folded into simple_matchers.rs, dropping the expect/ directory from 75 to 49 .rs files. The >50 sanity floor was just guarding against the glob matching nothing; 40 preserves that intent.
Collaborator
Author
|
CI status: the one remaining non-flaky failure is The other annotations ( This diff touches nothing in fetch/HTTP/redirects. |
robobun
added a commit
that referenced
this pull request
Jul 10, 2026
Conflicts with #33925 (repo-wide snake_case cleanup): - src/runtime/bake/dev_server/mod.rs: adopted main's simplified submodule block (plain `mod` declarations instead of the old `#[path = "../DevServer/..."]` attributes) and added `js_escape`. - src/runtime/bake/DevServer/js_escape.rs (file location): git moved it to src/runtime/bake/dev_server/js_escape.rs following the directory rename; the PR's ErrorReportRequest.rs changes were already merged into the renamed error_report_request.rs and its `super::js_escape::decode_js_escape_sequences` call is intact.
dylan-conway
added a commit
that referenced
this pull request
Jul 16, 2026
## What this adds **A spec-exact TOML v1.1.0 parser** (`src/parsers/toml.rs`), replacing the previous JS-lexer-derived implementation, plus the **official [toml-lang/toml-test](https://github.com/toml-lang/toml-test) conformance suite** as a generated `bun:test` file, following the same pattern as the YAML and JSON5 conformance suites. ### Conformance: 100% of the official suite, before → after Measured against the toml-test v1.1.0 manifest (commit `4d77658d`): 217 valid + 481 invalid + 1 out-of-range-integer + 9 invalid-encoding cases — every case in the manifest, zero exclusions. | | before | after | |---|---|---| | total | 160/699 (23%)¹ | **708/708 (100%)** | | valid documents parsed correctly | 148/217 | 217/217 | | invalid documents rejected with the asserted SyntaxError | 12/481 | 481/481 | | invalid documents wrongly **accepted** | 144 | 0 | | invalid-encoding documents (raw byte input) | untestable¹ | 9/9 | | date/time literals | all rejected as syntax errors | parse as strings of their source text | ¹ The 9 invalid-encoding cases contain ill-formed UTF-8 that a JS string cannot carry; they became testable when `TOML.parse` gained binary input in this PR. ### Parser - All four TOML date/time types (offset/local date-time, local date, local time), returned as strings of the source text - Correct `inf`/`nan`, underscore and leading-zero validation, `0x`/`0o`/`0b` integers - Integers outside `Number.MAX_SAFE_INTEGER` throw instead of silently losing precision (TOML requires lossless handling or an error) - Table / array-of-tables / dotted-key / inline-table definition-state rules enforced; duplicate keys rejected, including non-ASCII keys (the old byte-view comparison of UTF-16 keys missed duplicates and falsely rejected distinct keys) - Control characters, bare carriage returns, and ill-formed UTF-8 rejected (simdutf-validated); leading BOM handled - Multi-line string delimiter/trimming rules, CRLF→LF normalization, exact escape validation including TOML 1.1's `\xHH` and `\e` - TOML 1.1 additions: optional seconds in times, multi-line inline tables with trailing commas - Dotted keys are parsed iteratively (the old parser capped them at 512 segments) ### `Bun.TOML.parse` - Throws `SyntaxError` with precise messages (previously a `BuildMessage`), matching `JSON.parse`, `Bun.YAML.parse`, and `Bun.JSON5.parse` - Converts the AST directly to JS values (previously printed to JSON and re-parsed) - Accepts `Blob`/`TypedArray`/`DataView`/`ArrayBuffer` input like the YAML and JSON5 siblings - Parse errors carry the redaction flag so secrets in malformed config files stay out of logs ### `Bun.TOML.stringify` (new) Serializes a JavaScript object to a TOML document — this API did not exist before. Same surface as the YAML/JSON5 siblings: `(value, replacer, space)`, where `replacer` throws and `space` is accepted but ignored (TOML output is line-oriented). - Idiomatic layout: scalar keyvals first, then `[table]` and `[[array-of-tables]]` sections; mixed arrays use inline tables; keys are bare when possible, quoted otherwise - `Date` values become TOML offset date-times - `null`, `BigInt`, circular structures, and non-object top-level values throw (TOML cannot represent them); `undefined`/function/symbol properties are skipped - Integral doubles beyond ±(2^53 − 1) are emitted as floats so documents round-trip through any TOML reader; unpaired surrogates get the same USVString replacement as `TOML.parse` string input ### Tests - `test/js/bun/toml/toml-test-suite.test.ts` — generated official suite (708 tests); every rejection test asserts the exact full error message - `test/js/bun/toml/generate_toml_test_suite.ts` — pins the upstream commit, decodes the suite's tagged-JSON expectations, inlines every case; `--check` mode fails if the committed suite is stale - `test/js/bun/toml/toml.test.ts` — hand-written coverage beyond the official suite: input types (Buffer/TypedArray subviews/DataView/ArrayBuffer/SharedArrayBuffer/Blob), JS value mapping (`__proto__` safety, key ordering, no Unicode normalization), safe-integer boundaries, source-text date/time preservation, multi-line string edge cases, recursion-limit and GC robustness, the SyntaxError message contract, and `TOML.stringify` (exact layout output, escaping, round-trips, every error contract, and a GC stress test) ### Performance 1.15×–1.49× faster than the previous parser at every document size, growing with size. Release builds from this pipeline (darwin-aarch64), interleaved A/B, best of 3 rounds (each sample = median of 15 timed batches), real-world inputs both parsers accept: | document | size | old | new | change | |---|---|---|---|---| | `uv.lock` (apache/airflow) | 2.9 MB | 246 MB/s (12.0 ms) | **367 MB/s (8.0 ms)** | 1.49× | | `uv.lock` (langgenius/dify) | 662 KB | 237 MB/s | **338 MB/s** | 1.43× | | `poetry.lock` (python-poetry/poetry) | 201 KB | 249 MB/s | **341 MB/s** | 1.37× | | `Cargo.lock` (zed-industries/zed) | 495 KB | 194 MB/s | **262 MB/s** | 1.35× | | `Cargo.lock` (this repo) | 71 KB | 199 MB/s | **259 MB/s** | 1.30× | | `pyproject.toml` (python-poetry/poetry) | 5.7 KB | 168 MB/s | **221 MB/s** | 1.31× | | `poetry.lock` (truncated at a package boundary) | 2.4 KB | 16.2 µs/op | 13.5 µs/op | 1.20× | | `bunfig.toml` | 126 B | 3.29 µs/op | 2.85 µs/op | 1.16× | Basic strings borrow the source bytes when no escape requires decoding (the same approach the YAML parser uses for plain scalars), so string-dominated documents — lockfiles in particular — see the largest gains. ### Behavioral note: stricter parsing of invalid config files The old parser silently accepted some invalid TOML, so a small set of existing `bunfig.toml` files (about 1% in a sample of 99 real-world bunfigs from GitHub) will go from silently tolerated to a startup `SyntaxError` that states the fix. The one pattern observed in the wild is unquoted string values — easy to carry over from `.npmrc` syntax — which now produce a purpose-built message: ``` TOML Parse error: Strings must be quoted: "isolated" ``` The other formerly-tolerated patterns (backslash escapes in basic strings being silently dropped, missing newlines between key/value pairs, bare keys containing `:`) were silent data corruption or non-portable syntax that every other TOML implementation already rejects; none appeared in the sample. ### Deletions The old TOML lexer (`src/parsers/toml/lexer.rs`), and the now-caller-less `E::Object::set_rope`, `get_or_put_array`, and `set`. The bake error reporter's JS-escape decoder (which borrowed the old TOML lexer) is extracted standalone with its exact semantics. Closes #22426 Closes #28680 Closes #28681 Closes #28687 Also addresses the TOML half of #32025 (`\u{…}` is now correctly rejected; the JS-lexer half remains). --- ### Merge with main (38cfba6) One conflict, in `src/ast/e.rs`. Main's hardening round (#33072) added `flags: own_key_property_flags(&key)` to the property-construction sites, which marks a `__proto__` key as computed so it becomes an own property rather than setting the prototype. This branch deletes `set`, `set_rope`, and `get_or_put_array`, replacing `set` with `append_property`. Git merged the new flags into `append_property` on its own; the conflict was only the bodies of the two functions this branch removes. Kept them removed, after checking that neither has a caller anywhere in the tree and that every live construction site (`put`, `get_or_put_object`, `append_property`) carries the new flags, so the hardening is preserved on every path that still exists. Verified on the merge result: the TOML suites pass (810 tests, including the full conformance suite), and the INI and `bun init` tests pass (73 tests) since they are the remaining `get_or_put_object` callers. `test/bake/dev/production.test.ts` also passes again now that #33204 pinned the React build the bake harness installs. ### Merge with main (dbac342) Conflicts with #33722, which fixed `\u{...}` overflow and unterminated-brace handling in the old TOML lexer's copy of the JS escape loop. - `src/parsers/toml/lexer.rs` (modify/delete): kept deleted. The new parser rejects `\u{...}` at the opening brace (`\u{…}` is JavaScript syntax, not TOML), so neither the overflow nor the unterminated-brace path is reachable. - `test/js/bun/resolve/toml/toml-parse.test.ts`: kept this branch's content and folded the #30825 crash-regression inputs into the existing `rejects JS-style \u{XX} escapes` test. Main's `still accepts in-range \u{...}` test asserted `\u{41}` decodes to `"A"`, which contradicts spec-compliant TOML and is already asserted to throw by this file. - `src/runtime/bake/DevServer/js_escape.rs`: ported the one-line unterminated-brace fix, since that decoder was extracted from the code #33722 patched and is documented as preserving its exact semantics. Overflow was already prevented by the existing `0x10FFFF` clamp. ### Merge with main (aba8757) Conflicts with #33925 (repo-wide snake_case directory cleanup), which renamed `src/runtime/bake/DevServer/` to `dev_server/` and replaced the `#[path = "../DevServer/..."]` module-path attributes with plain `mod` declarations. - `src/runtime/bake/dev_server/mod.rs`: adopted main's simplified submodule block and added the `js_escape` declaration. - `src/runtime/bake/DevServer/js_escape.rs`: moved to `dev_server/js_escape.rs` following the rename. Git tracked the rename of this branch's `ErrorReportRequest.rs` changes into `error_report_request.rs`, and its `super::js_escape::decode_js_escape_sequences` call is intact. ### Merge with main (ace42f9) Conflicts with #33909, which replaced `bun_core::Error` with per-crate `thiserror` enums across the codebase. - `src/parsers/toml.rs`: main changed the old file's error handling; this branch replaces the whole file. Kept the new parser and adapted `TOML::parse` to return `crate::Result<Expr>`, mapping the internal `PErr` to `parsers::Error::{SyntaxError, Alloc, StackOverflow}`. - `src/parsers/toml/lexer.rs` (modify/delete): kept deleted. - `src/parsers/error.rs`, `src/runtime/error.rs`: removed the `From<toml::lexer::Error>` impls that referenced the deleted file. - `src/runtime/bake/dev_server/js_escape.rs`: adapted to return `Result<(), crate::Error>` with `crate::Error::SyntaxError`, since `bun_core::err!` is gone. - `src/runtime/api/TOMLObject.rs`: matched on `bun_parsers::Error::Alloc(_)` in place of the removed `bun_core::err!("OutOfMemory")`. - `src/ast/lexer_log.rs`: doc comment combining both sides. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sweep of vestigial Rust files left over from the Zig port: empty modules, one-line re-export shims, the
phase_c_exports.rslink-bridge, and duplicate directory layouts. Net -499 lines across 85 files.Deleted (50 files)
Literally empty (9):
css/properties/{effects,list,svg}.rs,bake/DevServer/{RouteBundle,WatcherAtomics,DirectoryWatchStore,IncrementalGraph}.rs,http/websocket_http_client.rs,runtime/webcore/lib.rs,io/stub_event_loop.rsRe-export-only shims (9), inlined into parent mod:
install/{versioned_url,ExternalSlice}.rs,jsc/JSInternalPromise.rs,safety/{ThreadLock,thread_id}.rs,runtime/api/bun/subprocess/StaticPipeWriter.rs,runtime/webcore/s3/multipart_options.rs,sql/postgres/protocol/NoticeResponse.rs,runtime/valkey_jsc/ValkeyContext.rsOne-line
expect()matchers (27): folded intoexpect/simple_matchers.rs. Each was a singleunary_predicate_matcher!call or a one-line delegate to a shared helper.Empty
bun_jscrate:src/js/{lib.rs,Cargo.toml}removed from workspace.src/js/is the TypeScript builtins dir; the Rust crate there had zero items and zero importers.runtime/ffi_imports.rs: an emptyunsafe extern "C" {}placeholder for a migration that never started. Removed.jsc/ffi_imports.rsrenamed tojsc_abi.rssince thejsc_abi_extern!macro is the only thing it provides.Moved
bake/DevServer/dissolved intobake/dev_server/:ErrorReportRequest.rs,HmrSocket.rs,memory_cost.rsmoved and their module names lose the_bodysuffix. The PascalCaseDevServer/directory (which existed alongside snake-casedev_server/and was mounted via#[path = "../DevServer/..."]) is gone.phase_c_exports.rsdissolvedThe "PHASE-C link bridge" was 250 lines of commented-out migration notes plus 7 real symbols. Each now has a proper home:
Bun__panic,Bun__outOfMemorybun_bin/c_abi_exports.rs(binary-level exports)Bun__VM__scriptExecutionStatusjsc/virtual_machine_exports.rswith a real body that callsVirtualMachine::script_execution_status(). The stub always returnedRunning, so worker-termination status never reachedZigGlobalObject::scriptExecutionStatus().Bun__LifecycleAgent{PreventExit,StopPreventingExit}Debugger.rsandInspectorLifecycleAgent.cpp.JSC__JSValue__parseJSONJSValue::parse_jsonnever called from Rust. Extern + wrapper + header decl removed.BunString__toErrorInstanceDNSResolver__getConstructorgenerate-classes.tsnow gates thegetConstructorextern on!noConstructor(matching the C++ definition it already gated), andResolverindns.rsgets#[JsClass(no_constructor)]to matchdns.classes.ts.Verification
bun bdlinks cleanbun bd test test/js/bun/test/expect.test.js: 406 pass / 0 fail (same as main)[review] gate passed · iteration 3 · 86 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 2 rejected · iteration 3
evidence per changed file