Skip to content

Remove dead code from bun_format, bun_sema, bun_install, and bun_core - #45402

Open
robobun wants to merge 1 commit into
mainfrom
robobun/38f195de/dead-code-lint-fmt-check
Open

robobun wants to merge 1 commit into
mainfrom
robobun/38f195de/dead-code-lint-fmt-check

Conversation

@robobun

@robobun robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Nine trait impls, one function and one generated method have no user. rustc's dead_code lint exempts trait impls and pub items, so the deny lints do not report them.
  • They are: six Spanned impls in bun_format, Default for Session and Files::default_of_module in bun_sema, PartialEq<crate::Error> for ForTarballError and Display for Id in bun_install, and StoredTrace::from in bun_core.
  • bun_lint_react lists bun_glob as a dependency and does not use it.

Fix

  • Delete the items and the dependency line: 12 files, 81 lines.
  • The linker names the candidates: a relink of the debug build with --gc-sections drops every function that nothing reaches. The compiler decides next: a candidate stays deleted only if every configuration compiles without it.
  • Verified: bun bd, bun run rust:check-all (12 targets), cargo check --workspace --all-targets, cargo clippy --workspace --no-deps. Five test files under test/cli pass (see Notes).

Background

  • To rustc a trait impl is always reachable, because generic code could call it. Only a whole-program view shows that nothing does.
  • resolutions! in src/sema/program.rs generates one forwarding method on Files for each signature in its list. The default_of_module line generated a public method that nothing calls. The Resolve method of the same name stays.
  • StoredTrace::from lost its last caller in install: consolidate the open package manager pull requests #44968.

Downsides

  • None found. Checked: every configuration in Notes compiles without the removed items, and no open pull request adds a use of one.

Behaviour change: none

Notes

Removed, by file

  • src/bun_core/Global.rs
    • StoredTrace::from
  • src/format/js/format.rs
    • impl Spanned for FormatIdentifier
    • impl Spanned for FormatDeclaration<'_>
  • src/format/js/print/binary_like_expression.rs
    • impl Spanned for BinaryLikeExpression<'_>
  • src/format/js/print/object_pattern_like.rs
    • impl Spanned for ObjectPatternLike<'_>
  • src/format/js/print/template.rs
    • impl Spanned for TemplateExpression<'_>
  • src/format/js/utils/conditional.rs
    • impl Spanned for Operand<'_>
  • src/install/NetworkTask.rs
    • impl PartialEq<crate::Error> for ForTarballError
  • src/install/PackageManagerTask.rs
    • impl core::fmt::Display for Id
  • src/sema/session.rs
    • impl Default for Session
  • src/sema/program.rs
    • the default_of_module line of resolutions!
  • src/lint/react/Cargo.toml, Cargo.lock
    • the bun_glob dependency of bun_lint_react

Method

  • Link: the debug link command from build.ninja plus -Wl,--gc-sections -Wl,--print-gc-sections. The export list and the version script stay, so the exported NAPI, V8 and uv surface is still a root.
  • Functions: 355 hand-written functions have no kept symbol. In a scratch worktree each free function and inherent method among them (174) got #[deprecated], and cargo check --force-warn deprecated listed every use site for linux, windows and darwin. One function has no use on any target: StoredTrace::from. The others have a caller on another target, in a const context, in a release-only path, or in a test.
  • Trait impls: 129 impl blocks have no kept method. Open dead-code pull requests already delete 58 of them. 34 are hand-written Debug impls, which Remove dead code: 49 trait impls nothing uses, across 12 crates #43664 kept as a debugging aid. One Clone impl and one Drop impl were never candidates. Six stayed because a doc comment gives them a purpose or because they are the only reader of a field.
  • Restore loop for the other 29: delete all, run cargo check --workspace --all-targets --keep-going, put back each impl that an error names, repeat. Seventeen came back. Three more stayed by choice (see the follow-up list). Nine are in this diff.
  • Restore loop configurations, each with --all-targets: linux, x86_64-pc-windows-msvc, aarch64-apple-darwin, --release, linux with --cfg bun_debug --cfg bun_asan --cfg socket_fault_injection, and --release with --cfg bun_codegen_embed.
  • New crates (bun_lint*, bun_format*, bun_sema*): in a scratch worktree every pub fn, const, static and field whose name no other crate mentions became pub(crate), so that dead_code could judge it. That found Files::default_of_module, which mordant's unused_pub skips because a macro generates it.
  • Open pull requests: I fetched the heads of all 5504 open pull requests and searched the Rust lines that each one adds against its merge base for the removed items. No pull request uses one.
  • mordant-baseline.toml is unchanged. The unused_pub count for src/bun_core/Global.rs was already 1 before install: consolidate the open package manager pull requests #44968 removed the caller of StoredTrace::from, so that entry is for another item.

Tests

  • Debug build, --timeout 60000: test/cli/format/format.test.ts (1015 pass, 44 skip), test/cli/format/sort-imports.test.ts (64 pass), test/cli/lint/bun-rules.test.ts (100 pass), test/cli/check/entry-points.test.ts (160 pass), test/cli/install/bun-install-offline.test.ts (11 pass). No failure.
  • With the default 5 s timeout, some tests of these files time out on the debug build in my container. They do so with and without this change.

Self-review

  • A self-review of the diff ran for seven hours and was stopped before it produced a verdict.

Scanned and clean, or already claimed

Follow-up candidates, not in this diff

  • 34 hand-written Debug impls that nothing formats, 24 of them in bun_lint and bun_format.
  • Display for Difference (src/format/verify.rs) has no caller. It is the only reader of the fields of Difference.
  • impl StaticallyNamed for Ident, Pat and Node (src/lint/types/utils/type_or_value_specifier.rs) have no caller. The doc comment of value_matches_specifier names them as accepted inputs.
  • BaseTypeOfClassMember::heritage_token and FinallyCall::on_finally in src/lint/types/utils are written and never read. They mirror the typescript-eslint utilities that the files port.
  • Default for SendWindow and Default for RecvWindow (src/runtime/api/bun/h2/flow_control.rs) have no caller. Remove 7 stale module-level dead_code escapes from the node:http2 engine and react_compiler::hir #42278 changes that file.

Nine trait impls, StoredTrace::from, and the default_of_module line of
resolutions! have no user. The impls are six Spanned impls in
bun_format, Default for Session in bun_sema, and PartialEq<crate::Error>
for ForTarballError and Display for Id in bun_install.

bun_lint_react does not use its bun_glob dependency.
@robobun

robobun commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

Status

  • How the dead code was found: a relink of the debug build with --gc-sections lists every function that nothing reaches. Each removed item then had to compile away in cargo check --workspace --all-targets on linux, windows and darwin, in debug and release.
  • Checks on this branch: bun bd, bun run rust:check-all (12 targets), cargo check --workspace --all-targets, cargo clippy --workspace --no-deps, and five test files under test/cli. All pass.
  • Pull request: Remove dead code from bun_format, bun_sema, bun_install, and bun_core #45402

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 98221468-85e7-4f6a-93f7-bc21c9ed8888
📥 Commits

Reviewing files that changed from the base of the PR and between a407eae and 8efb153.

⛔ Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 11
  • src/bun_core/Global.rs
  • src/format/js/format.rs
  • src/format/js/print/binary_like_expression.rs
  • src/format/js/print/object_pattern_like.rs
  • src/format/js/print/template.rs
  • src/format/js/utils/conditional.rs
  • src/install/NetworkTask.rs
  • src/install/PackageManagerTask.rs
  • src/lint/react/Cargo.toml
  • src/sema/program.rs
  • src/sema/session.rs
💤 Files with no reviewable changes: 11
  • src/lint/react/Cargo.toml
  • src/install/NetworkTask.rs
  • src/format/js/utils/conditional.rs
  • src/install/PackageManagerTask.rs
  • src/sema/session.rs
  • src/format/js/print/binary_like_expression.rs
  • src/format/js/print/object_pattern_like.rs
  • src/format/js/format.rs
  • src/bun_core/Global.rs
  • src/format/js/print/template.rs
  • src/sema/program.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The pull request removes several trait implementations and APIs, plus the bun_glob workspace dependency from bun_lint_react. It adds no replacement implementations or APIs in the summarized changes.

Changes

Formatter span implementations

Layer / File(s) Summary
Remove formatter span implementations
src/format/js/format.rs, src/format/js/print/binary_like_expression.rs, src/format/js/print/object_pattern_like.rs, src/format/js/print/template.rs, src/format/js/utils/conditional.rs
The Spanned implementations for formatter identifiers, declarations, expressions, patterns, templates, and operands were removed.

Installer trait implementations

Layer / File(s) Summary
Remove installer trait implementations
src/install/NetworkTask.rs, src/install/PackageManagerTask.rs
ForTarballError no longer implements PartialEq<crate::Error>. Id no longer implements Display.

Stored trace conversion

Layer / File(s) Summary
Remove stored trace conversion
src/bun_core/Global.rs
StoredTrace::from was removed.

Semantic APIs

Layer / File(s) Summary
Remove semantic APIs
src/sema/program.rs, src/sema/session.rs
Files::default_of_module and Default for Session were removed. Session::new() remains available.

React lint dependency

Layer / File(s) Summary
Remove React lint dependency
src/lint/react/Cargo.toml
The bun_glob workspace dependency was removed from bun_lint_react.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 8efb1

No merge-blocking behavior change is identified; this change is ready for normal merge checks.

Pre-merge checks: Passed 4
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 summarizes the primary change: removing dead code from the affected Bun components.
Description check Passed The description explains the problem, lists the removed items, documents the fix, and provides detailed verification results. It does not use the template headings exactly, but it includes the require…
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@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 — pure deletion of unused impls, one function, one macro-generated wrapper, and one dependency edge; each removed item was checked for remaining users.

What was reviewed:

  • The six removed Spanned impls: every Spanned-bounded consumer in bun_format (JoinNodesBuilder::entries, entries_with_trailing_separator, FormatNodeWithoutTrailingComments, write_array_node, get_cached_element, text_for, speculate_will_break) is only ever handed AST handles or other Format* wrappers; f.join_with(..).entries(..) has no Spanned bound, so identifier(part) at src/format/js/print/ts_types.rs:26 still compiles. All six files import Span/Spanned via crate::prelude::*, so no unused-import warning.
  • Files::default_of_module: the two remaining self.default_of_module calls at src/sema/program.rs:2003 and :2033 are inside trait Resolve's default methods and resolve to Resolve::default_of_module, not the deleted forwarding wrapper; no other crate names it.
  • Display for Id and PartialEq<crate::Error> for ForTarballError: no {}/to_string formatting of a Task::Id and no == against crate::Error anywhere under src/install; Default for Session: every construction site uses Session::new() and new_without_default is allowed at the workspace level; StoredTrace::from has no caller repo-wide and StackTrace is still used elsewhere in Global.rs.
Extended reasoning...

The change is a single 81-line, deletion-only commit across 12 files that removes trait impls, one inherent function, one macro-generated forwarding method, and the unused bun_glob dependency of bun_lint_react (with the matching Cargo.lock line). It touches no security-sensitive surface: no input parsing, auth, crypto, or allocation paths change, and no runtime behavior changes. Approval was decided by confirming via repo-wide search that each deleted item has no remaining caller, generic-bound use, or format-string use, that the files rely on glob prelude imports so no unused-import warning is introduced, and that no CODEOWNERS entry covers the changed paths. The compile could not be run in this session, so the verification is by reading every consumer of the relevant traits and types.

@robobun

robobun commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 3:05 PM PT - Oct 15th, 2026

@robobun, your commit 8efb153 is building: #126163

@robobun

robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 3:05 PM PT - Oct 15th, 2026

❌ @robobun, your commit 8efb153 has some failures in Build #126163 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 45402

That installs a local version of the PR into your bun-45402 executable, so you can run:

bun-45402 --bun

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.

2 participants