Skip to content

refactor: drop Zig-shadowing underscore affixes from Rust fn names - #36067

Merged
Jarred-Sumner merged 5 commits into
mainfrom
claude/farm/1abae27c/rename-zig-underscore-fns
Jul 27, 2026
Merged

Jarred-Sumner merged 5 commits into
mainfrom
claude/farm/1abae27c/rename-zig-underscore-fns

Conversation

@robobun

@robobun robobun commented Jul 27, 2026 •

Copy link
Copy Markdown
Collaborator

What

Renames ~60 functions and ~12 struct fields across 75 files to drop leading/trailing underscores that were carried over from the Zig codebase as shadowing workarounds and are not needed in Rust.

Example: resume_ is named that way because resume is a Zig keyword. It is not a Rust keyword, so Response::resume() is fine.

Example: Expect::_pass was named that way because other functions in the file had a local pass variable, which Zig refuses to shadow. Rust doesn't care, so it's now Expect::pass (jest.classes.ts updated to match).

Example: AST node fields test_/catch_ exist because test and catch are Zig keywords. Neither is a Rust keyword.

Functions renamed

Trailing underscore dropped (was a Zig keyword or shadowed a local):

  • resume_ (uws Response/h3)
  • or_ (VendorPrefix; or is a Zig keyword, not Rust)
  • jsdom_file_construct_
  • on_handshake_ (valkey, mysql, postgres SocketHandler)
  • server_set_{idle_timeout,on_client_error,on_connection,app_flags,max_http_header_size}_
  • RusageFields::{maxrss,ixrss,nswap,inblock,oublock,msgsnd,msgrcv,nsignals,nvcsw,nivcsw}_
  • SQLDataCell::bool_, Frame::u32_, Reader::u32_

Leading underscore dropped (Zig private-impl convention):

  • Expect::_pass (+ jest.classes.ts)
  • _advance _parse (dotenv) _resolve (jsc_hooks) _boot_and_handle_error _stat _socket _decode _format_t _count_with_hash _buf_append_input _join_abs_string_buf_windows _on_read_chunk _on_structured_clone_deserialize _compare _compare_ipv6 _generic_flush _generic_write _events_cb _schedule _stop
  • node_fs: _cp_async_directory _cp_symlink _cp_open_dest_with_mkdir _copy_single_file_sync
  • ipc: _socket_closed _write _on_write_complete _on_after_ipc_closed _close_socket_task _windows_close _windows_on_closed _windows_on_write_complete

For cp_symlink/cp_open_dest_with_mkdir (only called on some platforms) the dead-code suppression moves from the _ prefix to an explicit #[cfg_attr(..., allow(dead_code))]. The unused #[cfg(not(windows))] _windows_close stub in ipc.rs and the dead NodeFS::_cp_async_directory wrapper are removed.

Struct fields renamed

  • test_ -> test (AST If/For/DoWhile/While/Switch, e::If; ~105 access sites)
  • catch_ -> catch (AST Try)
  • cache_directory_ (PackageManager), error_ (NativeBrotli Context), event_type_ (ChangeEvent), file_polls_ (RareData, MiniEventLoop), process_ (WindowsSpawnResult)

Not renamed

  • Rust keywords/reserved: ref_ loop_ type_ final_ mut_ do_ typeof_ static_ match_ as_ use_ impl_ for_ fn_ in_ self_ move_ macro_. These still need the suffix.
  • Real C symbol names: deflateInit_ inflateInit2_ gzgetc_ _dyld_* _lwp_self _strnicmp _umask etc.
  • repr(C) fields matching the C struct: libuv sys_errno_/unused_, libwebp private_, ExternSocketConfig::unix_ (C macro collision), SQLDataCell::Value::bool_.
  • Same-scope wrapper/impl pairs: html_rewriter on_/get_attribute_/..., CryptoHasher hash_/digest_, FetchHeaders get_/cast_/fast_*_, NthSelectorData::is_function_, start_/next_/ptr_/generate_js_renamer_/_parse (js_parser)/_exec/_resolve (VM)/etc. In each case the clean name already exists on the same type as a field, arg-decode wrapper, or public entrypoint, so the _ variant still needs disambiguation. Happy to follow up with an _impl suffix convention if preferred.
  • Field+accessor pairs (clean name is the getter): Entry::base_, PackedMap::vlq_, RareData::{mysql,valkey,ws_*}_group_, spawn_sync_event_loop_, c-ares AddrInfo::name_.
  • Intentional Rust #[doc(hidden)]/macro helpers: _atomic_* _dispatch_* _ws_minify* _wired _needs_nl _scoped_use_ansi.
  • deref_: kept for symmetry with ref_.

Verification

bun run rust:check-all passes (linux/macos/windows, x64/aarch64). Smoke-tested expect.test.js, html-rewriter.test, fs/cp.test.ts, blocklist-gc.test.ts, transpiler.test, bundler/esbuild/default.test.ts.

In the original Zig codebase many functions were named with a leading or
trailing underscore to work around Zig compiler restrictions on variable
shadowing (e.g. `resume_` because `resume` is a Zig keyword, `_pass`
because another function had a local `pass` variable). Rust has no such
restriction, so these names can be cleaned up.

This renames ~55 functions across 42 files where the clean name does not
collide with anything in the same scope:

Trailing underscore dropped (Zig keyword or shadowing holdover):
  resume_ is_function_ or_ jsdom_file_construct_ on_handshake_ (valkey/mysql)
  server_set_{idle_timeout,on_client_error,on_connection,app_flags,max_http_header_size}_
  RusageFields::{maxrss,ixrss,nswap,inblock,oublock,msgsnd,msgrcv,nsignals,nvcsw,nivcsw}_
  SQLDataCell::bool_ Frame/Reader::u32_

Leading underscore dropped (Zig private-impl convention):
  _pass (expect), _advance _parse (dotenv) _resolve (jsc_hooks)
  _boot_and_handle_error _stat _socket _decode _format_t _count_with_hash
  _buf_append_input _join_abs_string_buf_windows _on_read_chunk
  _on_structured_clone_deserialize _compare _compare_ipv6 _generic_flush
  _generic_write _events_cb _schedule _stop _cp_async_directory _cp_symlink
  _cp_open_dest_with_mkdir _copy_single_file_sync
  ipc: _socket_closed _write _on_write_complete _on_after_ipc_closed
       _close_socket_task _windows_close _windows_on_closed _windows_on_write_complete

Not renamed (need the underscore in Rust too):
  ref_ loop_ final_ mut_ do_ typeof_ static_ match_ as_ use_ impl_ for_
    (Rust keywords/reserved)
  deflateInit_ inflateInit_ gzgetc_ _dyld_* _lwp_self _strnicmp _umask
    (real C symbol names)
  on_/get_attribute_/hash_/digest_/cast_/get_/fast_*_/start_/next_/ptr_/etc.
    (same impl block already has the clean name as a wrapper)
  _atomic_* _dispatch_* _ws_minify* _wired _needs_nl
    (intentional Rust doc-hidden / macro helpers)
@coderabbitai

coderabbitai Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 74f9daec-aa40-4aad-a99c-a1f5499543de

📥 Commits

Reviewing files that changed from the base of the PR and between 45974dd and f226854.

📒 Files selected for processing (37)
  • src/ast/e.rs
  • src/ast/expr.rs
  • src/ast/s.rs
  • src/bunfig/bunfig.rs
  • src/css/selectors/selector.rs
  • src/event_loop/MiniEventLoop.rs
  • src/install/PackageManager.rs
  • src/install/PackageManager/PackageManagerDirectories.rs
  • src/install/PackageManagerTask.rs
  • src/js_parser/fold.rs
  • src/js_parser/lower/lower_decorators.rs
  • src/js_parser/p.rs
  • src/js_parser/parse/parse_stmt.rs
  • src/js_parser/parse/parse_suffix.rs
  • src/js_parser/scan/scan_side_effects.rs
  • src/js_parser/visit/mod.rs
  • src/js_parser/visit/visit_expr.rs
  • src/js_parser/visit/visit_stmt.rs
  • src/js_printer/lib.rs
  • src/jsc/VirtualMachine.rs
  • src/jsc/event_loop.rs
  • src/jsc/rare_data.rs
  • src/react_compiler/codegen.rs
  • src/react_compiler/lowering/build_hir/expr.rs
  • src/react_compiler/lowering/build_hir/mod.rs
  • src/react_compiler/lowering/build_hir/stmt.rs
  • src/react_compiler/lowering/find_context_identifiers.rs
  • src/react_compiler/program.rs
  • src/runtime/cli/run_command.rs
  • src/runtime/jsc_hooks.rs
  • src/runtime/node/node_fs.rs
  • src/runtime/node/path_watcher.rs
  • src/runtime/node/zlib/NativeBrotli.rs
  • src/runtime/valkey_jsc/js_valkey.rs
  • src/spawn/process.rs
  • src/sql_jsc/mysql/JSMySQLConnection.rs
  • src/sql_jsc/postgres/PostgresSQLConnection.rs

Walkthrough

Changes

This PR removes trailing underscores from numerous public and internal Rust helpers, callbacks, accessors, and codec methods. Call sites, FFI shims, comments, and JavaScript host mappings are updated while preserving the described behavior.

Underscore-free API and data contracts

Layer / File(s) Summary
Core APIs and data contracts
src/css/*, src/jsc/NodeModuleModule.rs, src/spawn_sys/*, src/sql_jsc/shared/*, src/uws_sys/*
Renames core public helpers, resource counters, SQL boolean construction, response resumption methods, and the stat export.
Codec and consumer updates
src/runtime/cli/test/parallel/*, src/sql_jsc/*, src/runtime/api/bun/subprocess/*
Updates frame encoding/decoding, SQL boolean decoding, resource getters, and stat dispatch to use the renamed APIs.

Runtime and filesystem integration

Layer / File(s) Summary
Network and server lifecycle
src/jsc/ipc.rs, src/runtime/node/fs_events.rs, src/*Connection.rs, src/runtime/server/*, src/runtime/valkey_jsc/*
Renames socket callbacks, close/write handlers, event scheduling entrypoints, database socket helpers, and server setter implementations.
Filesystem and parsing helpers
src/paths/*, src/runtime/node/node_fs.rs, src/runtime/node/net/*, src/dotenv/*, src/install/*, src/resolver/*, src/io/*
Renames path, copy, comparison, parsing, counting, decoding, and Windows pipe helpers with corresponding call-site updates.
Runtime bindings and test execution
src/runtime/api/bun/h2_frame_parser.rs, src/runtime/cli/run_command.rs, src/runtime/jsc_hooks.rs, src/runtime/test_runner/*, src/runtime/webcore/Blob.rs
Renames runtime writing, boot, resolution, test advancement, expectation, and Blob construction/deserialization helpers and their mappings.

Possibly related PRs

  • oven-sh/bun#36006: Updates callers of the renamed uWS response resumption methods in stream backpressure paths.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely summarizes the main change: removing Zig-style underscore affixes from Rust function names.
Description check ✅ Passed The description covers the required what-and-verification content, though it uses different headings than the template.

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

@robobun

robobun commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

No new tests: this is a mechanical rename refactor with zero behavioral change (every renamed function has identical signature and body; call sites are updated 1:1). There is no test that would fail on main and pass here, since the JS-visible surface is unchanged. Existing coverage (expect.test.js, html-rewriter.test, fs/cp.test.ts, blocklist-gc.test.ts) exercises the renamed paths and continues to pass. bun run rust:check-all is green on all six targets.

Comment thread src/jsc/ipc.rs
Comment thread src/jsc/ipc.rs

@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: 2

🤖 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/cli/run_command.rs`:
- Around line 1744-1746: Update the doc comment above boot_and_handle_error to
reference the current helper name instead of the obsolete _bootAndHandleError
identifier, leaving the described behavior unchanged.

In `@src/runtime/node/fs_events.rs`:
- Line 527: Update the lifecycle documentation associated with events_cb and
schedule in path_watcher.rs, replacing the outdated _events_cb and _schedule
references with the current symbol names. Keep the teardown contract description
accurate and limited to durable, non-obvious information.
🪄 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: 437e2802-46c3-435c-b721-eb278ea4a7f0

📥 Commits

Reviewing files that changed from the base of the PR and between f93a5dc and 45974dd.

📒 Files selected for processing (42)
  • src/css/lib.rs
  • src/css/selectors/parser.rs
  • src/css/selectors/selector.rs
  • src/dotenv/env_loader.rs
  • src/install/lockfile.rs
  • src/io/PipeReader.rs
  • src/jsc/NodeModuleModule.rs
  • src/jsc/ipc.rs
  • src/paths/Path.rs
  • src/paths/resolve_path.rs
  • src/resolver/data_url.rs
  • src/runtime/api/bun/h2_frame_parser.rs
  • src/runtime/api/bun/subprocess/ResourceUsage.rs
  • src/runtime/api/standalone_graph_jsc.rs
  • src/runtime/cli/run_command.rs
  • src/runtime/cli/test/parallel/Coordinator.rs
  • src/runtime/cli/test/parallel/Frame.rs
  • src/runtime/cli/test/parallel/Worker.rs
  • src/runtime/cli/test/parallel/runner.rs
  • src/runtime/hw_exports.rs
  • src/runtime/jsc_hooks.rs
  • src/runtime/node/fs_events.rs
  • src/runtime/node/net/BlockList.rs
  • src/runtime/node/node_fs.rs
  • src/runtime/node/path.rs
  • src/runtime/server/NodeHTTPResponse.rs
  • src/runtime/server/mod.rs
  • src/runtime/server/server_body.rs
  • src/runtime/test_runner/bun_test.rs
  • src/runtime/test_runner/expect.rs
  • src/runtime/test_runner/jest.classes.ts
  • src/runtime/valkey_jsc/js_valkey.rs
  • src/runtime/webcore/Blob.rs
  • src/spawn_sys/spawn_process.rs
  • src/sql_jsc/mysql/JSMySQLConnection.rs
  • src/sql_jsc/mysql/protocol/DecodeBinaryValue.rs
  • src/sql_jsc/mysql/protocol/ResultSet.rs
  • src/sql_jsc/postgres/DataCell.rs
  • src/sql_jsc/postgres/PostgresSQLConnection.rs
  • src/sql_jsc/shared/SQLDataCell.rs
  • src/uws_sys/Response.rs
  • src/uws_sys/h3.rs

Comment thread src/runtime/cli/run_command.rs Outdated
Comment thread src/runtime/node/fs_events.rs
Comment thread src/runtime/node/fs_events.rs
Comment thread src/css/selectors/parser.rs Outdated
Comment thread src/runtime/node/node_fs.rs Outdated
Comment thread src/sql_jsc/postgres/PostgresSQLConnection.rs
Follow-up to the function renames: struct fields named with a trailing
underscore because the clean name is a Zig keyword (or shadowed a local
in Zig). None of these are Rust keywords.

Fields renamed:
  test_  -> test   (AST If/For/DoWhile/While/Switch, e::If; ~105 sites)
  catch_ -> catch  (AST Try; ~11 sites)
  cache_directory_ -> cache_directory  (PackageManager)
  error_           -> error            (NativeBrotli Context)
  event_type_      -> event_type       (ChangeEvent)
  file_polls_      -> file_polls       (RareData, MiniEventLoop)
  process_         -> process          (WindowsSpawnResult)

Not renamed (kept with underscore):
  Rust keywords: type_ loop_ ref_ impl_ macro_ fn_ in_ static_ do_ self_ move_
  Field+accessor-method pairs (clean name is the accessor): Entry::base_,
    PackedMap::vlq_, RareData::{mysql,valkey,ws_*}_group_, spawn_sync_event_loop_
  repr(C) mirrors of C struct fields that carry the underscore in C:
    libuv sys_errno_/unused_, libwebp private_, ExternSocketConfig::unix_
    (C macro), SQLDataCell Value::bool_
  cares AddrInfo::name_/cnames_ (fn name() accessor uses the clean name)

Also from review:
  - revert NthSelectorData::is_function_ (field with same name exists and
    answers a different question)
  - rename postgres SocketHandler::on_handshake_ to match valkey/mysql
    (delegates to a different type, no collision)
  - delete dead NodeFS::_cp_async_directory wrapper
  - fix stale doc-comment references to renamed fns
Comment thread src/runtime/cli/run_command.rs
Comment thread src/runtime/jsc_hooks.rs Outdated
robobun and others added 2 commits July 27, 2026 07:54
The _resolve -> resolve sed in jsc_hooks.rs over-matched two doc comments
that were cross-referencing VirtualMachine::_resolve (which this PR did
not rename, as VirtualMachine already has a separate public resolve()).
Restore the underscore so the comments point at the right method.
@Jarred-Sumner
Jarred-Sumner merged commit bfd1e92 into main Jul 27, 2026
48 of 52 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the claude/farm/1abae27c/rename-zig-underscore-fns branch July 27, 2026 08:20

@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.

LGTM — all prior review findings addressed; mechanical rename with compiler-enforced call-site coverage.

What was reviewed:

  • Verified all five earlier findings are fixed on HEAD: path_watcher.rs _events_cb refs updated, NthSelectorData::is_function_ revert kept, dead NodeFS::_cp_async_directory removed, postgres on_handshake renamed, VirtualMachine._resolve comment refs restored at jsc_hooks.rs:4807/4850.
  • Spot-checked the jest.classes.ts fn: "pass" binding matches the renamed Expect::pass.
  • Grepped for lingering .test_/.catch_ AST field accesses — none remain.
  • Confirmed the deleted #[cfg(not(windows))] _windows_close stub had no callers (Windows path uses the #[cfg(windows)] variant).
Extended reasoning...

Overview

Pure identifier-rename refactor across 71 files: ~60 functions and ~12 struct fields drop leading/trailing underscores that existed only as Zig keyword/shadowing workarounds (test_ → test, catch_ → catch, resume_ → resume, _resolve → resolve, etc.). Bodies and signatures are byte-identical; call sites updated 1:1. One codegen input (jest.classes.ts) updated to match Expect::_pass → pass. Two dead-code items removed (unused #[cfg(not(windows))] _windows_close stub, uncalled NodeFS::_cp_async_directory wrapper). Two #[cfg_attr(..., allow(dead_code))] attributes added where the _ prefix was previously suppressing the lint on platform-conditionally-unused fns.

Security risks

None. No user-facing surface changes, no parsing/validation logic touched, no auth/crypto/permissions paths affected. The renamed identifiers are internal Rust symbols; the one .classes.ts change maps a JS prototype method name to its (renamed) Rust impl — the JS-visible name pass is unchanged.

Level of scrutiny

Low-to-medium. The compiler is the primary reviewer for a rename refactor: bun run rust:check-all passes on all six targets (linux/macos/windows × x64/aarch64), which proves every call site was updated and no #[cfg]-gated path was missed. The residual risk was (a) comment drift, (b) accidental field/method name collisions, and (c) missed sibling sites — all three were caught in earlier review rounds and fixed in c72ccc4 / cd5ed70. This run's bug-hunting pass found nothing new.

Other factors

Two prior review rounds surfaced five nits (stale cross-file comment refs, an is_function field/method collision, a dead wrapper, a missed postgres sibling, and a sed over-match on VirtualMachine._resolve); the author addressed each and I verified the fixes on HEAD via targeted greps. All inline threads are resolved. The PR description's "Not renamed" section documents the deliberate exclusions (Rust keywords, C ABI names, field+accessor pairs), and the one item that violated its own exclusion rule (NthSelectorData::is_function_) was reverted. Existing test coverage (expect.test.js, fs/cp.test.ts, transpiler.test, bundler tests) exercises the renamed paths.

Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
Main fails to build on all lanes since bfd1e92 (Buildkite main
#83211, #83215).

## Cause

#36067 renamed `AnyResponse::resume_` to `AnyResponse::resume` in
`src/uws_sys/Response.rs`. #36006, merged just before it, added six new
callers of the old name. Both passed CI on their own branches; combined
on main:

```
error[E0599]: no method named `resume_` found for enum `AnyResponse` in the current scope
    --> src/runtime/server/RequestContext.rs:2382:22
    --> src/runtime/server/RequestContext.rs:4126:18
    --> src/runtime/server/RequestContext.rs:4182:22
    --> src/runtime/webcore/streams.rs:1408:25
    --> src/runtime/webcore/streams.rs:1785:17
    --> src/runtime/webcore/streams.rs:1873:21
```

## Fix

Rename the six call sites to `resume()`.

## Verification

- `cargo check -p bun_runtime` on main: 6× E0599; with this change:
clean.
- `bun bd` builds.
- `bun bd test test/js/bun/http/serve.test.ts -t 'request body
backpressure'`: 5/5 pass (the tests #36006 added that exercise these
call sites).

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 1 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/http/serve.test.ts

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
## What

Adds `src/runtime/node/node_fs.rs: 2` to the dead-code escape inventory
so the `dead-code-escapes` source-lint passes again on main.

## Why

#36067 (bfd1e92) renamed `_cp_symlink` → `cp_symlink` and
`_cp_open_dest_with_mkdir` → `cp_open_dest_with_mkdir`, moving the
dead-code suppression from the leading underscore (which the inventory
regex does not match) to explicit `#[cfg_attr(..., allow(dead_code))]`
attributes:

```rust
#[cfg_attr(any(windows, target_os = "macos"), allow(dead_code))]
fn cp_symlink(&mut self, ...) { ... }

#[cfg_attr(windows, allow(dead_code))]
fn cp_open_dest_with_mkdir(&mut self, ...) { ... }
```

Both functions are called from the Linux/FreeBSD arms of
`copy_single_file_sync` (lines 8613, 8680, 8706, 8852, 8878) and are
only dead on the platforms named in the predicate, so the escapes are
legitimate per the test's own rules. The inventory file was simply not
regenerated, and the GitHub Actions `source-lints` workflow has been
failing on every main commit since:

```
(fail) #[allow(dead_code)] escapes > src/runtime/node/node_fs.rs (0)
error: src/runtime/node/node_fs.rs has 2 item-level #[allow(dead_code)] escapes, up from 0.
```

Failed runs: 30249533282 (bfd1e92), 30249969141, 30250537591,
30250867335.

## How

Regenerated with `bun
./test/internal/source-lints/dead-code-escapes.test.ts` per the test's
instructions.

## Verification

```
$ bun test test/internal/source-lints/dead-code-escapes.test.ts
...
(pass) #[allow(dead_code)] escapes > src/runtime/node/node_fs.rs (2)
...
 26 pass
 0 fail
```

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · docs-only change; test-proof not
applicable

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
…36067)

## What

Renames ~60 functions and ~12 struct fields across 75 files to drop
leading/trailing underscores that were carried over from the Zig
codebase as shadowing workarounds and are not needed in Rust.

Example: `resume_` is named that way because `resume` is a Zig keyword.
It is not a Rust keyword, so `Response::resume()` is fine.

Example: `Expect::_pass` was named that way because other functions in
the file had a local `pass` variable, which Zig refuses to shadow. Rust
doesn't care, so it's now `Expect::pass` (jest.classes.ts updated to
match).

Example: AST node fields `test_`/`catch_` exist because `test` and
`catch` are Zig keywords. Neither is a Rust keyword.

## Functions renamed

**Trailing underscore dropped** (was a Zig keyword or shadowed a local):
- `resume_` (uws Response/h3)
- `or_` (VendorPrefix; `or` is a Zig keyword, not Rust)
- `jsdom_file_construct_`
- `on_handshake_` (valkey, mysql, postgres SocketHandler)
-
`server_set_{idle_timeout,on_client_error,on_connection,app_flags,max_http_header_size}_`
-
`RusageFields::{maxrss,ixrss,nswap,inblock,oublock,msgsnd,msgrcv,nsignals,nvcsw,nivcsw}_`
- `SQLDataCell::bool_`, `Frame::u32_`, `Reader::u32_`

**Leading underscore dropped** (Zig private-impl convention):
- `Expect::_pass` (+ jest.classes.ts)
- `_advance` `_parse` (dotenv) `_resolve` (jsc_hooks)
`_boot_and_handle_error` `_stat` `_socket` `_decode` `_format_t`
`_count_with_hash` `_buf_append_input` `_join_abs_string_buf_windows`
`_on_read_chunk` `_on_structured_clone_deserialize` `_compare`
`_compare_ipv6` `_generic_flush` `_generic_write` `_events_cb`
`_schedule` `_stop`
- node_fs: `_cp_async_directory` `_cp_symlink`
`_cp_open_dest_with_mkdir` `_copy_single_file_sync`
- ipc: `_socket_closed` `_write` `_on_write_complete`
`_on_after_ipc_closed` `_close_socket_task` `_windows_close`
`_windows_on_closed` `_windows_on_write_complete`

For `cp_symlink`/`cp_open_dest_with_mkdir` (only called on some
platforms) the dead-code suppression moves from the `_` prefix to an
explicit `#[cfg_attr(..., allow(dead_code))]`. The unused
`#[cfg(not(windows))]` `_windows_close` stub in ipc.rs and the dead
`NodeFS::_cp_async_directory` wrapper are removed.

## Struct fields renamed

- `test_` -> `test` (AST `If`/`For`/`DoWhile`/`While`/`Switch`, `e::If`;
~105 access sites)
- `catch_` -> `catch` (AST `Try`)
- `cache_directory_` (PackageManager), `error_` (NativeBrotli Context),
`event_type_` (ChangeEvent), `file_polls_` (RareData, MiniEventLoop),
`process_` (WindowsSpawnResult)

## Not renamed

- **Rust keywords/reserved**: `ref_` `loop_` `type_` `final_` `mut_`
`do_` `typeof_` `static_` `match_` `as_` `use_` `impl_` `for_` `fn_`
`in_` `self_` `move_` `macro_`. These still need the suffix.
- **Real C symbol names**: `deflateInit_` `inflateInit2_` `gzgetc_`
`_dyld_*` `_lwp_self` `_strnicmp` `_umask` etc.
- **repr(C) fields matching the C struct**: libuv
`sys_errno_`/`unused_`, libwebp `private_`, `ExternSocketConfig::unix_`
(C macro collision), `SQLDataCell::Value::bool_`.
- **Same-scope wrapper/impl pairs**: html_rewriter
`on_`/`get_attribute_`/..., CryptoHasher `hash_`/`digest_`, FetchHeaders
`get_`/`cast_`/`fast_*_`, `NthSelectorData::is_function_`,
`start_`/`next_`/`ptr_`/`generate_js_renamer_`/`_parse`
(js_parser)/`_exec`/`_resolve` (VM)/etc. In each case the clean name
already exists on the same type as a field, arg-decode wrapper, or
public entrypoint, so the `_` variant still needs disambiguation. Happy
to follow up with an `_impl` suffix convention if preferred.
- **Field+accessor pairs** (clean name is the getter): `Entry::base_`,
`PackedMap::vlq_`, `RareData::{mysql,valkey,ws_*}_group_`,
`spawn_sync_event_loop_`, c-ares `AddrInfo::name_`.
- **Intentional Rust `#[doc(hidden)]`/macro helpers**: `_atomic_*`
`_dispatch_*` `_ws_minify*` `_wired` `_needs_nl` `_scoped_use_ansi`.
- `deref_`: kept for symmetry with `ref_`.

## Verification

`bun run rust:check-all` passes (linux/macos/windows, x64/aarch64).
Smoke-tested `expect.test.js`, `html-rewriter.test`, `fs/cp.test.ts`,
`blocklist-gc.test.ts`, `transpiler.test`,
`bundler/esbuild/default.test.ts`.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
Main fails to build on all lanes since bfd1e92 (Buildkite main
#83211, #83215).

## Cause

#36067 renamed `AnyResponse::resume_` to `AnyResponse::resume` in
`src/uws_sys/Response.rs`. #36006, merged just before it, added six new
callers of the old name. Both passed CI on their own branches; combined
on main:

```
error[E0599]: no method named `resume_` found for enum `AnyResponse` in the current scope
    --> src/runtime/server/RequestContext.rs:2382:22
    --> src/runtime/server/RequestContext.rs:4126:18
    --> src/runtime/server/RequestContext.rs:4182:22
    --> src/runtime/webcore/streams.rs:1408:25
    --> src/runtime/webcore/streams.rs:1785:17
    --> src/runtime/webcore/streams.rs:1873:21
```

## Fix

Rename the six call sites to `resume()`.

## Verification

- `cargo check -p bun_runtime` on main: 6× E0599; with this change:
clean.
- `bun bd` builds.
- `bun bd test test/js/bun/http/serve.test.ts -t 'request body
backpressure'`: 5/5 pass (the tests #36006 added that exercise these
call sites).

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 1 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/http/serve.test.ts

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
## What

Adds `src/runtime/node/node_fs.rs: 2` to the dead-code escape inventory
so the `dead-code-escapes` source-lint passes again on main.

## Why

#36067 (bfd1e92) renamed `_cp_symlink` → `cp_symlink` and
`_cp_open_dest_with_mkdir` → `cp_open_dest_with_mkdir`, moving the
dead-code suppression from the leading underscore (which the inventory
regex does not match) to explicit `#[cfg_attr(..., allow(dead_code))]`
attributes:

```rust
#[cfg_attr(any(windows, target_os = "macos"), allow(dead_code))]
fn cp_symlink(&mut self, ...) { ... }

#[cfg_attr(windows, allow(dead_code))]
fn cp_open_dest_with_mkdir(&mut self, ...) { ... }
```

Both functions are called from the Linux/FreeBSD arms of
`copy_single_file_sync` (lines 8613, 8680, 8706, 8852, 8878) and are
only dead on the platforms named in the predicate, so the escapes are
legitimate per the test's own rules. The inventory file was simply not
regenerated, and the GitHub Actions `source-lints` workflow has been
failing on every main commit since:

```
(fail) #[allow(dead_code)] escapes > src/runtime/node/node_fs.rs (0)
error: src/runtime/node/node_fs.rs has 2 item-level #[allow(dead_code)] escapes, up from 0.
```

Failed runs: 30249533282 (bfd1e92), 30249969141, 30250537591,
30250867335.

## How

Regenerated with `bun
./test/internal/source-lints/dead-code-escapes.test.ts` per the test's
instructions.

## Verification

```
$ bun test test/internal/source-lints/dead-code-escapes.test.ts
...
(pass) #[allow(dead_code)] escapes > src/runtime/node/node_fs.rs (2)
...
 26 pass
 0 fail
```

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 0 · docs-only change; test-proof not
applicable

<!-- robobun:evidence:end -->
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.

2 participants