Skip to content

Remove dead code from bun_lint - #47028

Open
robobun wants to merge 2 commits into
mainfrom
robobun/cdef29cd/dead-code-sweep
Open

robobun wants to merge 2 commits into
mainfrom
robobun/cdef29cd/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Two token predicates in bun_lint have no caller: is_not_semicolon_token and is_colon_token (src/lint/utils/eslint_utils/token_predicate.rs).
  • Both came with the port of eslint-utils in Add bun lint and bun format #44805. The file holds 11 predicates. The other 9 have callers.

Fix

  • Delete the function, the punctuators! line, and both names in the pub use list of utils/ast_utils.rs: 10 lines removed, 3 re-wrapped.
  • Correct because a word search over the sources, the tests and the generated code finds no other use, and no open pull request adds one.
  • Verified: bun bd, bun run rust:check-all (12 targets), test/cli/lint/defaults.test.ts (Notes).
  • Self-reviewed: 38 concerns raised, 36 addressed. Rejected: a move of the pair into Remove dead code from bun_lint, bun_react_compiler, and the node:fs Stats bindings #46832 (its open question is about trait impls), and a change to the unused_pub lint (outside this PR).

Background

  • token_predicate.rs mirrors token-predicate.mjs of eslint-utils: one small function for each punctuator, for lint rules that ask if a token is ; or (.
  • punctuators! expands each line to one pub fn that calls token.is_punctuator(text). src/lint/tokens/mod.rs:53 documents that call as the equivalent.
  • A deletion of unreferenced code has one possible place, so no other design was weighed.

Downsides

  • A later port of a rule that calls isColonToken or isNotSemicolonToken must write token.is_punctuator(":") or restore the function.
  • None found for users. Checked: the heads of all 5,860 open pull requests, 12 targets.

Behaviour change: none

Notes

Removed

  • is_not_semicolon_token: the function with its doc comment and #[inline].
  • is_colon_token: its line in the punctuators! list.
  • Both names in pub use super::eslint_utils::{...} of src/lint/utils/ast_utils.rs.
  • git grep -w for the two names and for isColonToken and isNotSemicolonToken finds only deleted lines. grep -rw over build/debug/codegen finds nothing.

What earlier sweeps said about these items

Open pull requests

Found dead in this run, and left alone

How the candidates were found

  • hawk (tools/hawk) for linux-x64 and darwin-arm64, dev profile. Its 44 function, method and type-alias findings outside bun_windows_sys were deleted in a scratch tree, and cargo check --target x86_64-pc-windows-msvc put 39 back. hawk reports nothing inside a macro expansion, and its Windows run failed.
  • rustc dead_code on bun_sema and bun_lint in a scratch tree, with pub narrowed on each item that no dependent crate names. Functions that a macro emits were not judged.
  • A relink of the debug build with -Wl,--gc-sections,--print-gc-sections.
  • tsc --noUnusedLocals over src/js, src, src/runtime/bake and scripts/build: 19 unused locals. The multi-line ones are removed by Remove dead code from the JSC bindings, the codegen scripts, the CI scripts, bun_core, and bun_jsc #40492 and Remove the dead code from the bake error overlay syntax highlighter #44514. #onConnect in http2.ts is a false positive.
  • Dep-info of the linux build: each of the 3250 tracked .rs files is compiled or is behind a cfg. Each of the 766 headers is included or named.
  • Five read-only searches: the code generators and build scripts, the exported macros, the C++ bindings, the built-in JS, and files that nothing references.

Tests, debug build of this branch

  • test/cli/lint/defaults.test.ts --timeout 90000: 261 pass. With the default 5 s timeout, 46 tests time out in the debug build.

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

`is_not_semicolon_token` and `is_colon_token` in `utils/eslint_utils/token_predicate.rs` have no caller on main and none in an open pull request. Delete both, and their names in the `pub use` list of `utils/ast_utils.rs`.
@robobun

robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review

This pull request deletes two bun_lint token predicates that have no caller: is_not_semicolon_token and is_colon_token.

How to check it:

  • git grep -w -e is_not_semicolon_token -e is_colon_token -e isColonToken -e isNotSemicolonToken on main finds 4 lines. This diff deletes all 4.
  • bun bd builds.
  • bun run rust:check-all passes on 12 targets.
  • bun bd test test/cli/lint/defaults.test.ts --timeout 90000 gives 261 pass.
  • No open pull request adds a use of either name (5,860 heads compared, details in the Notes of the description).

CI on 62a1952e (build 133365):

  • 179 of 181 jobs passed. Two darwin x64 - test-bun jobs still wait for an agent.
  • No test failed on every attempt. Five tests failed one time and passed on the retry: test-zlib-invalid-input-memory.js, test/cli/lint/lint.test.ts (the exit code after a reader closes the pipe early, macOS arm64), nth-anplusb-ident.test.ts, webview-chrome.test.ts and test/cli/bun.test.ts. None of them can reach the removed functions, because nothing calls them.
  • The run before it (build 133354) had one red lane, windows x64 - verify-baseline: a JPEG test timed out under emulation. The same lane fails on main (build 133333).

Pull request: #47028

@robobun

robobun commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:33 PM PT - Oct 26th, 2026

✅ @robobun, your commit 62a1952e174d700bb4b353ba2e8cccd1f241cd67 passed in Build #133365! 🎉


🧪   To try this PR locally:

bunx bun-pr 47028

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

bun-47028 --bun

@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 two unreferenced token predicates.

What was reviewed:

  • Repo-wide search (all file types, not just src/) for is_colon_token, is_not_semicolon_token, isColonToken, isNotSemicolonToken — no remaining references after the deletion.
  • The nine names left in the pub use super::eslint_utils::{..} list in src/lint/utils/ast_utils.rs all still resolve: eight come from the punctuators! macro and is_not_closing_paren_token remains defined, so the re-export and the pub use token_predicate::* glob in eslint_utils/mod.rs are unaffected.
  • is_not_closing_paren_token (kept) still has callers in no_extra_bind.rs and prefer_arrow_callback.rs, so the deletion stops at the right boundary.
Extended reasoning...

The change is a single commit deleting two unused functions from the bun_lint crate: one line removed from the punctuators! macro invocation and one six-line standalone function in src/lint/utils/eslint_utils/token_predicate.rs, plus the matching names dropped from the explicit re-export list in src/lint/utils/ast_utils.rs. It touches no security-sensitive surface and has no runtime behavior change since neither symbol had a caller. A repo-wide grep confirms no reference to either name survives, every remaining re-exported name still exists, and no CODEOWNERS entry covers src/lint. The bug hunt ran dry and no third-party objections are on the timeline, so a human look is not needed.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 8a0e49e and c037dac.

Files selected for processing: 2
  • src/lint/utils/ast_utils.rs
  • src/lint/utils/eslint_utils/token_predicate.rs
Files with no reviewable changes: 1
  • src/lint/utils/eslint_utils/token_predicate.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 change removes is_colon_token and is_not_semicolon_token from the token-predicate module and from the eslint_utils re-exports. Other token predicates remain.

Changes

Token predicate cleanup

Layer / File(s) Summary
Remove predicates and re-exports
src/lint/utils/eslint_utils/token_predicate.rs, src/lint/utils/ast_utils.rs
Removes is_colon_token and is_not_semicolon_token and removes their re-exports. Other token predicates remain.

Possibly related PRs

  • oven-sh/bun#46832: Removes other unused bun_lint implementations, including a conversion in ast_utils.rs.

Suggested reviewers: jarred-sumner

Priority: Low

Merge Risk: Minimal · up to c037d

No actionable current-head risk is established; the cleanup is ready to merge.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title "Remove dead code from bun_lint" matches the main change. The diff deletes the unused is_colon_token and is_not_semicolon_token predicates and their re-exports in `src/lint/utils/ast_uti…
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.
Description check Passed The description explains what the PR changes and how the code was verified. It includes the rationale, scope, behavior impact, validation commands, and test results. Although it does not use the exact…
Full details: Title check

Explanation

The title "Remove dead code from bun_lint" matches the main change. The diff deletes the unused is_colon_token and is_not_semicolon_token predicates and their re-exports in src/lint/utils/ast_utils.rs. The title is somewhat broad, since it does not name the two predicates, but it refers to the real change and is not vague or misleading.

Full details: Description check

Explanation

The description explains what the PR changes and how the code was verified. It includes the rationale, scope, behavior impact, validation commands, and test results. Although it does not use the exact template headings, it provides the required information in equivalent sections.

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

@robobun

robobun commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

No change follows from this review. It confirms the two points that the description states: no reference to either name remains, and the nine names left in the pub use list still resolve. The Buildkite run for c037dac is in progress.

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