Remove dead code from the bun_runtime re-export hubs, bun_core, bun_css, bun_install, bun_bundler, the FFI crates, and the error-code table - #39582
Conversation
…ss, bun_install, bun_bundler, the FFI crates, and the error-code table
Deletes src/jsc/generated_classes_list.rs (an alias namespace with no
reader), the unused aliases in bun_runtime's api.rs / webcore.rs / bake /
server / cli re-export hubs, a handful of never-constructed enum variants
(streams::Writable::{TemporaryAndDone, IntoArray, IntoArrayAndDone},
WritableFuture::Handler, DrainResult::Empty, Source::Direct,
pretty_format Tag::ArrayBuffer, shell IoKind::Stdin, fs watcher
Event::Close, CalendarError::OutOfMemory, thumbhash
DecodeError::OutOfMemory), the never-called ServerLike consts / vm_mut and
ResponseLike::upgrade, leftover re-exports and helpers in bun_core,
bun_css, bun_install, bun_bundler, bun_collections, bun_sql_jsc and
bun_jsc, two FFI declarations with no caller (SetFileTime,
lsquic_conn_n_pending_streams), and 17 error codes nothing produces.
Each Rust item was reported unused by rustc with the crate's pub items
demoted to pub(crate), on the linux, windows and darwin targets; verified
with bun bd, rust:check-all and the affected test files.
Rebase of #39319 onto current main.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (58)
💤 Files with no reviewable changes (28)
Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review. WalkthroughThe pull request narrows public Rust and runtime exports, removes obsolete APIs and enum variants, restricts internal visibility, updates layering documentation, adjusts WebCore stream handling, and adds ChangesAPI and runtime cleanup
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 5:23 PM PT - Aug 18th, 2026
✅ @robobun, your commit fa018227fcff08605248d9142da0972e8f939ef6 passed in 🧪 To try this PR locally: bunx bun-pr 39582That installs a local version of the PR into your bun-39582 --bun |
|
Status: ready for review. Rebase of #39319 onto current main, now at fa01822, waiting on CI (build 100885; the previous build on 5490234 had 172 of 179 jobs green with no failures when the comment-only push superseded it). Verified on this tree:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…lacement) and pin how ArrayBuffers print in snapshots
|
e5b6757 does two things: restores |
…pshots.test.ts and update the comments that named the removed api aliases
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
d591490 and bd3b0bc: the |
…he GenBlob doc at its actual consumer
|
e7a87c3 through 5490234 are the remaining comment follow-ups from the review: the four comments that still named removed |
|
Two phrases in the generated walkthrough above overstate things, for anyone skimming it: "WebSocket upgrade support is removed" refers only to the |
…e::FrameworkRouter, webcore::jsc::codegen)
|
fa01822 updates the last three comments that named removed paths ( |
There was a problem hiding this comment.
Automated review is now clean — the four earlier rounds (rsplit_once/clippy.toml, and three batches of stale comment references to removed api::*/server::*/bake::*/webcore::jsc::codegen paths) are all addressed as of fa01822, and this pass found nothing new. Given the breadth (60 files across the error-code table, stream/shell/server/fs-watcher enum variants, and ServerLike/ResponseLike trait surface), a maintainer sign-off is still worth having.
What was reviewed:
- Spot-checked deleted error codes (
ERR_REQUIRE_ASYNC_MODULE,ERR_INVALID_PACKAGE_CONFIG,ERR_CRYPTO_INVALID_TAG_LENGTH,ERR_WORKER_INIT_FAILED) — no producers insrc/. - Confirmed
Source::DirectandIoKind::Stdinhave no remaining constructors or match sites. - Re-swept for comments naming removed paths after fa01822 — none found beyond the historical
src/dns/lib.rsnote the author already called out.
Extended reasoning...
Overview
Dead-code sweep across ~60 files in bun_runtime, bun_core, bun_css, bun_install, bun_bundler, bun_jsc, bun_collections, bun_sql_jsc, and the two FFI *_sys crates, plus 17 unused entries removed from ErrorCode.ts. Net −400 lines. The only non-deletion edits are forced by the deletions: match-arm removals for never-constructed variants, seven pub → pub(crate) demotions, reflowed pub use groups, and comment updates. One inline-snapshot test added to pin ArrayBuffer pretty-format output (the one pretty_format match this PR touches).
Security risks
None identified. No auth, crypto, permission, or input-validation logic changes; the crypto-related deletions are unused error-code table rows only. The SetFileTime and lsquic_conn_n_pending_streams FFI extern removals have no callers on any target.
Level of scrutiny
Higher than a typical mechanical cleanup because of breadth: it removes enum variants (DrainResult::Empty, Source::Direct, Writable::{TemporaryAndDone,IntoArray,IntoArrayAndDone}, IoKind::Stdin, Event::Close, Tag::ArrayBuffer) whose match arms are deleted alongside them, drops trait methods from ServerLike/ResponseLike, and prunes the error-code table (which renumbers the generated enum). The methodology is sound — demote pub → pub(crate) and let rustc report unused across linux/windows/macos, with codegen output diffed byte-identical — and bun run rust:check-all passing on all 12 targets is the strongest available signal that nothing platform-gated was missed. Still, the combination of scope and the number of subsystems touched puts this past what I'd approve without a human look.
Other factors
I reviewed this PR four times previously; each round surfaced only stale comment references to removed paths (no behavioral issues), and each was fixed promptly. After fa01822 the author ran a tree-wide sweep for every removed path in ::-qualified form and reported it clean; I re-checked a sample and agree. CI build 100885 is running on the head commit (the previous build on 5490234 was 172/179 green with no failures when superseded). The rsplit_once deletion was correctly reverted after review since clippy.toml names it as the sanctioned replacement.
2 participants
Rebase of #39319 (closed as stale) onto current main. Same content; the review on that PR was addressed there (the
ServerInitContextreport was a false positive, the comment nits are folded in), and pertest/internal/source-lints/CLAUDE.mdthe dead-symbol lint test is not carried over. Net -400 lines insrc/(452 deleted, 52 added as reflowed import groups, visibility adjustments and comment updates), plus one inline-snapshot test intest/js/bun/test/snapshot-tests/bun-snapshots.test.tsthat pins howArrayBuffervalues print (the onepretty_formatmatch this PR edits). No behavior change.Problem
bun_runtimere-export hubs (about 200 lines)src/jsc/generated_classes_list.rs(116 lines), mounted asbun_runtime::GeneratedClassesList, was a flat alias namespace with no reader:generate-classes.tsresolves each.classes.tsclass to its Rust type by walking the crate's ownpub mod/pub usetree (rustModuleResolver, which only registers items at module depth 0, and everything in this file sat insidepub mod Classes { .. }), and the regeneratedgenerated_classes.rsis byte-identical without the file.bun_runtimeis also the only crate that can see its own re-export hubs (nothing butbun_bindepends on it, andbun_binonly callsCli::start), so a hub entry rustc reports unused is unused everywhere.api.rsstill mirrored the oldbun.api.*Zig namespace: 36 aliases (Image,Shell,Timer,Archive,Glob,HTMLRewriter,NativeZlib,Valkey,MySQL,Postgres,FFIObject,SocketHandlers,x509, the 18process::*names other thanRusage, the five PTY types underbun::terminal,bun::H2FrameParser,posix_spawn,bun_spawn as spawn, ...) that no code and no generated code names.TCPSocket/TLSSocketstay: rustc reported them unused on linux, butsocket/WindowsNamedPipeContext.rsimports them throughapi.bun.js.rs(pub use crate::{api, webcore}),webcore.rs(the five sink re-exports,form_data::{AsyncFormData, FormData}, and thewebcore::jsc::codegenforwarding module whose own doc comment says it exists for call sites that "still spell" a path nothing spells),webcore/streams.rs(result::StreamResult),webcore/Request.rs(js_gen::from_js*),webcore/FormData.rs(get_boundary),s3/client.rs(multipart::{self}),bake/mod.rs(FrameworkRouteralias,DynamicRouteMap/EncodedPattern/Route/StaticRouteMap,production::{EntryPointMap, TypeAndFlags}),bake/dev_server/mod.rs(Assets,IncrementalGraph,RouteBundle,SourceMapStore),server/mod.rs(BunInfo,PreparedRequestFor,ServerInitContext,ServePluginsState),lib.rs(fivecli::*_commandcrate-root aliases),cli/mod.rs(command::Commandself-alias),cli/Arguments.rs(load_config),allocators/mod.rs,shell/interpreter.rs(Builtin),node/zlib/NativeZstd.rs(Context),api/JSBundler.rs(Config,MiniImportRecord),api/BunObject.rs(JSZstd::deallocator, whose own comment says "0 C++ refs, 0 Rust refs"),api/bun/Terminal.rs(from_js,from_js_direct) andapi/bun/h2_frame_parser.rs(the CamelCaseH2FrameParserConstructortwin of the snake_case alias the js2native thunk actually calls).server_body.rs,FrameworkRouter.rsandproduction.rswere reachable only through the deleted re-exports; they becomepub(crate)(rustc'sunreachable_pubdemands it).ServerInitContextitself stayspub: it is still reachable throughAnyRoute::from_js, and demoting it tripsprivate_interfaces.bun_runtimeitems (about 110 lines)streams::Writable::{TemporaryAndDone, IntoArray, IntoArrayAndDone}are never produced (theStreamResultvariants of the same names are; theWritableones only hadto_jsarms).WritableFuture::Handleris never armed: the only construction is inside therun()arm that matches an existingHandler, soWritableHandler,WritableHandlerFnand that arm go with it.DrainResult::Empty(webcore.rs, twomatches!inBody.rs, one arm inByteStream.rs),ReadableStream::Source::Direct(the C++Tag::Directmaps toNoneinfrom_js; the FFITagenum itself is untouched),pretty_format::Tag::ArrayBuffer(JSType::ArrayBuffermaps toTag::TypedArray; the now-unreachable_ => {}arm ofprint_asgoes too),shell::IoKind::Stdin,node_fs_watcher::Event::Close(EventType::Close, which is used, stays),cron::CalendarError::OutOfMemory,thumbhash::DecodeError::OutOfMemory.ServerLike::{SSL_ENABLED, DEBUG_MODE, vm_mut}(RequestContextuses its own const generics; the inherentNewServer::vm_mutis still used and stays, and the two SAFETY comments that cited the trait item now cite it) andResponseLike::upgradewith both impls (the HMR upgrade goes through the inherentuwsmethods).Other crates (about 60 lines)
bun_core: theCodePointZerore-export inimmutable.rs, and theZERO_VALUE/MAXassociated consts ofCodePointZero(both impls;from_u32is what the decoders use).bun_css:css_parser::CustomMediaandIdentFns, the crate-rootCustomIdentFns/DashedIdentFns,container::ContainerNameFns,selector::{_PrintErr, _Printer}("re-export alias parity"), and the path re-export of thecss_eql_partialeq!macro (every use is textual).bun_install:PackageManager::{do_patch_commit, prepare_patch, GitResolver}, crate-rootFolderResolutionandPostinstallOptimizer,patch_install::PatchedDep.bun_bundler:LinkerContext::{OutputFileListBuilder, StaticRouteVisitor, do_step5}module aliases,bundle_v2::{AdditionalFile, IndexStringMap},options::BakeExtra.bun_collections::ArrayListAlignedInandbun_sql_jsc::mysql::MySQLRequestQueuecrate-level re-exports,bun_jsc::ZigErrorTypecrate-root re-export (the type is used by path).windows_sys::kernel32::SetFileTime(itsfutimenscaller was rewritten) andlsquic_sys::lsquic_conn_n_pending_streams.ErrorCode.ts: 17 codes with no producer ($ERR_X(in src/js,ErrorCode::ERR_Xin C++,ErrorCode::Xin Rust, none in tests or docs; re-checked against today's main):ERR_BUFFER_CONTEXT_NOT_AVAILABLE,ERR_CRYPTO_INITIALIZATION_FAILED,ERR_CRYPTO_INVALID_COUNTER,ERR_CRYPTO_INVALID_TAG_LENGTH,ERR_CRYPTO_JOB_INIT_FAILED,ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE,ERR_INVALID_ADDRESS,ERR_INVALID_PACKAGE_CONFIG,ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE,ERR_MISSING_PLATFORM_FOR_WORKER,ERR_NON_CONTEXT_AWARE_DISABLED,ERR_REQUIRE_ASYNC_MODULE,ERR_TLS_PSK_SET_IDENTITY_HINT_FAILED,ERR_WASI_NOT_STARTED,ERR_WORKER_INIT_FAILED,ERR_SECRETS_NOT_AVAILABLE. Everything that consumes the table is generated from it, so the renumbering is invisible.Fix
pub usegroups, the sevenpub->pub(crate)changes, and the comments that named the removed items, including theVirtualMachine.rsLAYERING comments and thegenerate-classes.tsresolver comment that used removedapialiases as examples).dead_code = deny, so the only dead Rust left ispubitems, which rustc exempts. For every crate,pubwas temporarily demoted topub(crate)on the items no file outside the crate mentions (and, forbun_runtime, on everything the codegen output andbun_bindo not mention), andcargo check -p <crate>was run forx86_64-unknown-linux-gnu,x86_64-pc-windows-msvcandaarch64-apple-darwin. An item was only deleted when rustc reported it unused on all three; items it reported on one target only (ipc.rs,node_fs.rs,cron.rs's Windows and macOS error variants, theapi.rssocket aliases) and items only reached through macros expanded in other crates (bun_alloc's zone methods,bun_clap's const-eval helpers,js_class_module!'sIntoRawMut) were kept.bun bd(regenerates the class, js2native, host-export and error-code codegen and links;generated_classes.rsandgenerated_js2native.rsare byte-identical to main's,generated_host_exports.rsdiffers only in line-number comments),bun run rust:check-all, andbun bd teston the stream, body, fs.watch, shell and error-code test files listed in the first comment. Checked line by line against the two dead-code PRs still open (Remove dead code from uws_sys, webcore bindings, crash_handler, and scripts #37181, Remove dead code from bun-uws, the native BufferList class, and built-in JS #37659): no shared deletions.Left alone
strings::rsplit_oncehas no caller, butclippy.tomlnames it as the replacement for the disallowedstr::rsplit_once/str::rsplitn/ByteSlice::rsplit_once_str, so it is kept (it was deleted in the first push of this PR and restored after review).webcore.rs'sDOMExceptionCodere-export,install/lib.rs'sTextLockfile,ffi/mod.rs'sffi_object_draft,streams.rs's never-producedStreamResultvariants, theimage/mod.rs/dns_jsc/mod.rs/node_fs.rsre-export groups,Blob.rs's structured-clone helpers,server/mod.rs'sref_,shell_body.rs'sInvalidArguments/Todo,bindgen.rs's three marker structs, and theERR_REDIS_*/ERR_KEY_GENERATION_JOB_FAILEDcodes.napi_statusvariants,SSRKind::Regular,serialized_failure::ErrorKind's JS variants andMessageId::Visualizer(external code tables),bake::Mode::ProductionDynamic(a documented mode placeholder),ERR_POSTGRES_INVALID_BYTE_SEQUENCE_FOR_ENCODING(documented in docs/runtime/sql.mdx), and the unused entries of theO::,EPOLL::,IN::andPOLL_*constant tables inbun_sys.[review] gate passed · iteration 2 · 60 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 2 rejected · iteration 2
evidence per changed file