Skip to content

Remove dead code from bun_sema: the unused bun_semver dependency - #44673

Open
robobun wants to merge 1 commit into
mainfrom
robobun/36735356/dead-code-sweep
Open

robobun wants to merge 1 commit into
mainfrom
robobun/36735356/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Behaviour change: none

Problem

  • bun_sema lists bun_semver under [dependencies] (src/sema/Cargo.toml:18). No file of the crate names bun_semver. bun check - a TypeScript type checker builtin to Bun #44361 added the crate with this entry.
  • Cargo reports it on every check: warning: unused dependency `bun_semver` (cargo::unused_dependencies). The warning comes from cargo, not from rustc, so the workspace's deny lints do not stop the build.

Fix

  • Remove the entry from src/sema/Cargo.toml and the matching line from Cargo.lock.
  • Correct because rg bun_semver src/sema finds only the manifest line. The checker compares versions with its own port of semver.Version in src/sema/resolve.rs.
  • Verified: bun bd, bun run rust:check-all (12 targets), cargo check --all-targets for the four sema crates (also --release, and with --cfg bun_debug --cfg bun_asan), and test/cli/check/entry-points.test.ts (158 pass).

Background

  • A [dependencies] entry makes cargo build that crate first and pass it to rustc with --extern. An entry that no code names only adds an edge to the build graph.
  • Eight other crates still depend on bun_semver, so the link takes the same set of rlibs.
  • Considered a deny of unused_dependencies for the whole workspace. Cargo reports nine more entries on linux. Six have a user on another target, under a build cfg, or in unit tests, and Remove dead code from bun-uws, bun-usockets, the require.extensions FFI, and unused Cargo dependencies #40294 removes the other three (Notes). That is a separate change.

Downsides

  • None found. Checked every target and profile above, the baselines feature, and the test targets of the four sema crates. No source file changes.
Notes

Why this pull request is small

This is the result of a full dead-code scan of main at bbdc5a5. 41 dead-code pull requests are open. Together they remove 58,944 lines in 1,017 files. Outside those diffs, the scan found one item that is certain: this dependency entry.

The other nine unused_dependencies warnings on linux

Entry Why it stays here
bun_ini -> bun_js_parser, bun_platform -> bun_core, bun_ptr -> scopeguard #40294 removes them.
bun_resolver -> bun_zstd Used under cfg(bun_codegen_embed) (src/resolver/node_fallbacks.rs:55).
bun_wyhash -> bstr Used by unit tests (src/wyhash/lib.rs:1017).
bun_errno -> bun_windows_sys, bun_libuv_sys -> libc, bun_libuv_sys -> strum Used on Windows.
bun_perf -> bun_sys Used on macOS (src/perf/lib.rs:133).

What the scan covered, and what it found

Each candidate was compared with the lines that the 41 open pull requests remove.

Tools

  • hawk: oven-sh/hawk at c9ca301 does not build with nightly-2026-09-15. Two imports moved: rustc_session::config::CrateType is now rustc_structures::CrateType, and rustc_session::lint::Level is now rustc_lint_defs::Level. With those two changes it builds and the analysis completes.
  • mordant: c5e56d0 with nightly-2026-09-01, MORDANT_TOML without the baseline line, MORDANT_RUSTFLAGS="-A unknown_lints --cap-lints=warn".

Still dead on main, not in this diff (unchanged from the notes in #43664 and #43976)

  • The PerformanceResourceTiming cluster in src/jsc/bindings/webcore (about 2,000 lines). The linker drops every constructor. The two globals are public, so a removal is a rewrite of the two wrappers and needs a decision.
  • Clone for JsPoster and the JsPosterVTable::clone slot. Display for ErrorLocation and the filename field that only it reads. Each is a small refactor.
  • 13 hand-written Debug impls that nothing formats. They are a debugging aid.
  • Bun__napi_get_version in napi.cpp. Remove dead code from the JSC bindings, headers.h, the console builtin, and bun_jsc #40232 has the hunk under the old name of the function.
  • The <Type>Class__call shims that the class generator emits for the seven Expect* matchers.
  • packages/h3blast and the scripts that a person runs by hand.

Tests

test/cli/check/entry-points.test.ts with the debug build: 158 pass with --timeout 120000. With the default 5 s timeout, 4 tests time out in this container under ASAN (the slowest takes 5.1 s). test/internal/source-lints/workspace-lints.test.ts and build-rust.test.ts: 14 pass.


[policy-decision:dep] gate passed · iteration 0 · 2 files touched

passes on PR (with fix)
Dependency/toolchain change; no test proof to run.
diff hotspot
Cargo.lock          | 1 -
 src/sema/Cargo.toml | 1 -
 2 files changed, 2 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                 reads  edits  tests
Cargo.lock               0      0      0
src/sema/Cargo.toml      1      1      0

No file of bun_sema names bun_semver. The checker has its own port of
semver.Version in src/sema/resolve.rs. cargo reports the entry with its
unused_dependencies lint.
@coderabbitai

coderabbitai Bot commented Oct 7, 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: Essentials
  • Run ID: 913100a0-d517-4045-8a9d-494fde9b44da
📥 Commits

Reviewing files that changed from the base of the PR and between bbdc5a5 and 87a827a.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • src/sema/Cargo.toml
💤 Files with no reviewable changes (1)
  • src/sema/Cargo.toml

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


Walkthrough

The bun_semver.workspace dependency was removed from src/sema/Cargo.toml.

Changes

Sema manifest update

Layer / File(s) Summary
Remove semver dependency
src/sema/Cargo.toml
The manifest no longer declares bun_semver as a workspace dependency.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 87a82

No actionable risk from the dependency removal remains; the PR is mergeable after normal checks.

🚥 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 identifies the main change: removing the unused bun_semver dependency from bun_sema.
Description check ✅ Passed The description explains the problem, the fix, and how the author verified it. It does not use the template’s exact section headings, but it provides the required information.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@robobun

robobun commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. buildkite/bun, cargo clippy, cargo miri test, mordant, Format and Lint JavaScript pass.

How to reproduce: on main, run cargo check -p bun_sema. Cargo prints:

warning: unused dependency `bun_semver`
  --> src/sema/Cargo.toml:18:1

On this branch the same command prints no unused dependency warning for bun_sema.

The red check: claude-find-issues does not build or test this change. It looks for related issues. The same job also failed on the last 11 completed runs for other branches.

@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, straightforward removal of an unused manifest entry.

What was reviewed:

  • Searched all of src/sema/ (including the baselines, driver, and standalone sub-crates; no build.rs exists) for bun_semver — no code, cfg-gated, or test references, only the manifest line being removed.
  • Checked Cargo.lock: only the bun_sema dependency-list entry is dropped; the bun_semver package itself stays, since other crates still consume it.
  • Confirmed the lockfile edit matches the manifest edit, so the ninja/cargo build plan stays consistent.
Extended reasoning...

The change deletes one bun_semver.workspace = true line from src/sema/Cargo.toml and the matching entry in the bun_sema dependency list in Cargo.lock; nothing is added and no source files change. It touches no security-sensitive surface. A repo-wide search of src/sema finds no reference to bun_semver in any Rust file, sub-crate manifest, or build script, and bun_semver remains a workspace package with other consumers in Cargo.lock. The changed paths are not covered by .github/CODEOWNERS, and the bug hunt ran dry with no findings, so this is a mechanical two-line manifest cleanup safe to approve.

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