Skip to content

Remove dead code from bun_lint, bun_sema, BunProcess.cpp, and NodeDirent.cpp - #45764

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

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

Conversation

@robobun

@robobun robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • 29 lines in 6 files have no user.
  • source_file, compare_types and symbol_to_string in the queries! list of src/lint/types/mod.rs have no caller. The list generates the only call of the Services functions of the same names in bun_sema. The trait is pub, so rustc does not check it.
  • BunProcess.cpp declares five using X = JSC::X; aliases that it never names. DirEntType in NodeDirent.cpp has three enumerators that nothing names.

Fix

  • Delete the three list lines with their Services functions (21 lines), the five aliases and the three enumerators.
  • Correct because a word search over the sources, the tests and the generated code finds no user, and the workspace compiles for 12 targets.
  • Verified: bun bd, bun run rust:check-all, cargo check --workspace --all-targets, 8 test files (Notes).
  • Self-reviewed: 41 concerns raised, 33 addressed. Rejected: a pub(crate) narrowing of Queries (resolve_name needs a caller first), and a hold until earlier sweeps merge.

Background

  • queries! lists each question that a lint handle (Type, TsSymbol) can ask the type checker. One line generates a Queries method and a forwarding call to Services in bun_sema.
  • BunProcess.cpp compiles as its own unit with using namespace JSC;, so the aliases change no name lookup.
  • A deletion of unreferenced code has one possible place, so no other design was weighed.

Downsides

  • A later lint handle that needs one of the three queries must restore it.
  • None found for users. Checked: the heads of all 5,559 open pull requests, 12 targets, the tests in Notes.

Behaviour change: none

Notes

Removed

  • src/lint/types/mod.rs: the queries! lines source_file, compare_types and symbol_to_string. No call of these names has a Queries receiver. The calls that exist go to other functions: Checker::compare_types and Checker::symbol_to_string in bun_sema, a private compare_types in prefer_optional_chain.rs, and the parser's source_file. With Queries made crate-private in a scratch edit, rustc reports resolve_name as the only query left that is never used.
  • src/sema/check/services.rs, services/types.rs, services/symbols.rs: Services::source_file, Services::compare_types, Services::symbol_to_string. The generated forwarding call was the only user. What they call (Files::by_path, Checker::compare_types, Checker::symbol_to_string, Checker::prop_to_string) keeps other callers.
  • src/jsc/bindings/BunProcess.cpp: the aliases Exception, JSModuleLoader, JSModuleRecord, SourceOrigin, JSNonFinalObject. None of the names appears in the rest of the file. The same block in Path.cpp and ZigGlobalObject.cpp is left to Remove dead code from the class and sink generators, JSBuffer, the bake client, bun-error, and the build scripts #43378 and Remove dead code from the WebCore, node, and sqlite bindings, uWS/uSockets, SecretsLinux, and five Rust crates #42431.
  • src/jsc/bindings/NodeDirent.cpp: the enumerators Whiteout, Door and EventPort of DirEntType, all equal to Unknown. No C or C++ file names them.

Open pull requests

  • Compared at 2026-10-17 16:40 UTC on main 017b243 with the heads of all 5,559 open pull requests, drafts and every author included.
  • 156 of them touch a file of this diff (151 touch BunProcess.cpp). A trial merge of each with main and with this branch (git merge-tree) shows that the branch adds a conflict to none.
  • None deletes one of these lines. None adds a use of a removed name: the added lines were searched for the three query names, the five alias names in BunProcess.cpp, and the three enumerators.

Found dead in this run, and left alone

How the candidates were found

  • Ten text searches, one for each area: src/js, the C++ bindings (three parts), uSockets and uWS, the build scripts, the side packages, the three new crates, and commented-out code.
  • A relink of the debug build with -Wl,--gc-sections, with each dropped function mapped to its source line by llvm-nm -l: 517 C++ functions and 607 Rust function lines. Almost all are Windows or macOS code, release-only code, or are deleted by an open pull request.
  • For bun_sema: pub narrowed to pub(crate) on each item that no other crate names, in a scratch tree. rustc reported nothing new.
  • For 39 trait impls that the linker drops: delete all, compile for linux, windows and darwin and with --all-targets, put back each impl that an error names. What stayed deleted is a debugging aid.

Tests, debug build of this branch (with --timeout 600000, because the lint and check tests need more than the default with a debug build)

  • test/cli/lint/conformance.test.ts: 3 pass.
  • test/cli/lint/rules.test.ts: 16 pass.
  • test/cli/lint/native-rules.test.ts: 166 pass.
  • test/cli/lint/bun-rules.test.ts: 100 pass.
  • test/cli/check/check.test.ts: 599 pass, 42 skip.
  • test/js/node/process/process.test.js: 188 pass, 5 skip.
  • test/js/node/fs/dir.test.ts: 23 pass.
  • test/regression/issue/24129.test.ts: 2 pass.

no test proof · iteration 0 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check

…es in BunProcess.cpp, three DirEntType enumerators

The queries source_file, compare_types and symbol_to_string of the
queries! list in src/lint/types/mod.rs have no caller. The list
generated the only call of the Services functions of the same names
in bun_sema, so those go too.

BunProcess.cpp does not name Exception, JSModuleLoader, JSModuleRecord,
SourceOrigin or JSNonFinalObject after the alias lines. NodeDirent.cpp
does not name Whiteout, Door or EventPort.
@robobun

robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:53 PM PT - Oct 17th, 2026

✅ @robobun, your commit 61ca6f133e48f3a4ace622fab42d411adc7c939a passed in Build #127635! 🎉


🧪   To try this PR locally:

bunx bun-pr 45764

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

bun-45764 --bun

@robobun

robobun commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • The change is a deletion of 29 lines that no code names. It has no test of its own, and the body carries Behaviour change: none.
  • How it was checked: bun bd, bun run rust:check-all (12 targets), cargo check --workspace --all-targets, and the 8 test files in the Notes, all on main 017b243.
  • Open pull requests: the branch was compared with the heads of all 5,559 open pull requests at 2026-10-17 16:40 UTC. It repeats no deletion, and it adds a merge conflict to none.

@coderabbitai

coderabbitai Bot commented Oct 17, 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: 73a73e51-d19e-4e5b-afe8-e3c3eec76b66
📥 Commits

Reviewing files that changed from the base of the PR and between 017b243 and 61ca6f1.

📒 Files selected for processing: 6
  • src/jsc/bindings/BunProcess.cpp
  • src/jsc/bindings/NodeDirent.cpp
  • src/lint/types/mod.rs
  • src/sema/check/services.rs
  • src/sema/check/services/symbols.rs
  • src/sema/check/services/types.rs
💤 Files with no reviewable changes: 6
  • src/sema/check/services.rs
  • src/sema/check/services/symbols.rs
  • src/sema/check/services/types.rs
  • src/jsc/bindings/NodeDirent.cpp
  • src/jsc/bindings/BunProcess.cpp
  • src/lint/types/mod.rs

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

Walkthrough

The changes remove unused C++ aliases and enum members, along with three query methods from the Rust Queries trait and their corresponding Services methods.

Changes

C++ declaration cleanup

Layer / File(s) Summary
Remove unused C++ declarations
src/jsc/bindings/BunProcess.cpp, src/jsc/bindings/NodeDirent.cpp
The Bun namespace no longer defines five aliases. DirEntType no longer defines Whiteout, Door, or EventPort.

Rust query API cleanup

Layer / File(s) Summary
Remove Queries methods
src/lint/types/mod.rs
The Queries trait no longer declares source_file, compare_types, or symbol_to_string.
Remove Services methods
src/sema/check/services.rs, src/sema/check/services/types.rs, src/sema/check/services/symbols.rs
Services no longer provides source_file, compare_types, or symbol_to_string.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 61ca6

No actionable merge-blocking risk is identified from these declaration removals; the change is ready for normal 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 and concisely describes the primary change: removing dead code from the listed components.
Description check Passed The description explains the problem, fix, scope, verification steps, behavior impact, and known downsides. It uses different headings from the template but provides the required information.
✨ 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, a pure deletion of unreferenced items with no behavior change.

What was reviewed:

  • Expanded queries! in src/lint/types/mod.rs: each entry generates only a defaulted Queries trait method plus a forwarding impl to Services; the sole other implementor (NoTypes) overrides nothing, and no Queries receiver calls source_file, compare_types or symbol_to_string anywhere in src/lint (the remaining hits are Checker methods in bun_sema, a private TypeNode comparator in prefer_optional_chain.rs, and the parser's source_file).
  • Confirmed what the deleted Services bodies called (Files::by_path, Checker::compare_types, Checker::symbol_to_string, prop_to_string) still has other callers, so nothing else becomes dead.
  • NodeDirent.cpp: DirEntType is file-local, no switch over it, and Whiteout/Door/EventPort are named nowhere in C++ or in tests beyond a comment in test/regression/issue/24129.test.ts describing the old bug; the remaining enumerators and the uv_dirent_type_t mapping are unchanged.
  • BunProcess.cpp: the five removed aliases are not named in the file, and using namespace JSC; on line 166 would resolve them anyway, including under a unity build.
Extended reasoning...

The change is a single commit of 29 deleted lines across six files: three entries dropped from the lint-side queries! macro list and their matching Services forwarding targets in bun_sema, five unused using aliases in BunProcess.cpp, and three zero-valued enumerators in a file-local C++ enum in NodeDirent.cpp. It touches no security-sensitive surface (no auth, crypto, parsing of untrusted input, or FFI signatures). Approval rests on direct verification that each removed item has no remaining consumer, including macro-expanded code and the second Queries implementor, and that nothing downstream becomes newly dead; the changed paths have no CODEOWNERS entry and there are no outstanding reviewer objections.

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