semver: drop redundant store and two dead guards in the parser

[robobun]

Three no-op statements left over from the Zig port, all provably unreachable or redundant. No behavior change.

List::and_range (SemverQuery.rs)

let mut tail = Box::new(Query { range: *range, next: None });
tail.range = *range;

range: &Range is an immutable reference and Range is Copy, so the second store writes the same value it already holds.

query::parse hyphen lookahead (SemverQuery.rs)

i += strings::length_of_leading_whitespace_ascii(&input[i..]);
if i == input.len() { break 'possibly_hyphenate false; }
if !(i < input.len() && matches!(input[i], b'0'..=b'9' | b'X' | b'x' | b'*')) { ... }

length_of_leading_whitespace_ascii returns at most its slice's length, so after the addition i <= input.len(). The preceding i == input.len() check then leaves i < input.len() as the only remaining case, so the guard in the next condition is always true.

Version::parse tag arm (Version.rs)

b'-' | b'+' => {
    ...
    part_start_i = i;
    while i < input.len() && matches!(input[i], b' ') { i += 1; }

This arm is entered via match input[i] with i unchanged between dispatch and the loop, so input[i] is - or + and the loop condition is false on the first iteration. Tag::parse reads from part_start_i (set before the loop) regardless.

Why no new test

These are identity transformations on the compiled function: the dropped store writes the value already present, and the dropped guards evaluate to the branch already taken on every input. There is no input that can distinguish before from after. Regression coverage is the existing test/cli/install/semver.test.ts, which exercises List::and_range (multi-comparator ranges), the hyphen-range lookahead, and version tag parsing.

Verification

$ cargo clippy -p bun_semver
    Finished `dev` profile
$ bun bd test test/cli/install/semver.test.ts
 28 pass
 0 fail
 1946 expect() calls

[robobun]

Status: Ready for a maintainer.

• cargo clippy -p bun_semver clean, all 28 tests in test/cli/install/semver.test.ts pass (1946 assertions).
• Self-review raised 27 concerns and refuted all of them; claude[bot] independently verified each deletion is an identity transformation.
• No new test: each dropped statement is provably a no-op on every input (the store writes the value already present; the two guards evaluate to the branch already taken), so there is no distinguishing input. Regression coverage is the existing semver suite, which exercises List::and_range, the hyphen lookahead, and tag parsing.

[robobun]

^{Updated 1:43 AM PT - Jul 20th, 2026}

@robobun, your commit 05c8831 is building: #76177

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a299e471-ce98-47b2-914f-3be3f38be042

📥 Commits

Reviewing files that changed from the base of the PR and between 01e913e and 05c8831.

📒 Files selected for processing (2)

• src/semver/SemverQuery.rs
• src/semver/Version.rs

---

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

[claude]

LGTM — three provably dead statements removed, no behavior change.

What was reviewed:

• List::and_range: the dropped tail.range = *range re-stores the same Copy value just written in the struct literal.
• Hyphen lookahead: verified length_of_leading_whitespace_ascii returns ≤ slice length, so the preceding i == input.len() break makes the i < input.len() guard tautological — input[i] cannot panic.
• Version::parse tag arm: i is unchanged between match input[i] and the loop, so input[i] is -/+, never ' '; the loop body is unreachable.

Extended reasoning...

Overview

Three deletions in the semver parser (src/semver/SemverQuery.rs, src/semver/Version.rs): a redundant field re-assignment in List::and_range, an always-true i < input.len() conjunct in the hyphen-range lookahead, and a while-loop whose condition is false on the first iteration in the b'-' | b'+' match arm of Version::parse. Each is a Zig-port leftover.

Verification of each claim

• Redundant store: range: &Range is an immutable borrow and Range is Copy; the Box::new(Query { range: *range, .. }) already wrote the same bits. No interior mutability is involved.
• Hyphen guard: read length_of_leading_whitespace_ascii at src/bun_core/string/immutable.rs:2134 — it returns an index into its slice or the slice length, never more. After i += ...(&input[i..]) we have i <= input.len(); the immediately preceding if i == input.len() { break } leaves i < input.len() as the only remaining case, so the removed conjunct is always true and input[i] is in bounds.
• Dead space-skip loop: control reaches the loop only when match input[i] matched b'-' | b'+' and i has not been mutated since (the early-break arm above exits the outer loop). matches!(input[i], b' ') is therefore false on entry, so the body never runs. part_start_i was set before the loop and is what Tag::parse reads, so even the intent was preserved.

Security risks

None. Pure Rust, no unsafe touched, no new indexing sites — the one indexing site simplified is proven in-bounds by the line above it.

Level of scrutiny

Low. Each deletion is locally provable as identity from the surrounding lines; there is no input that can distinguish before from after. Existing coverage (test/cli/install/semver.test.ts, 28 tests / 1946 asserts) passes with the change and cargo clippy -p bun_semver is clean.

Other factors

The PR body includes a per-site proof matching what I verified. The "no new test" justification is sound under the repo rule: identity transformations have no distinguishing input, and the existing suite already exercises all three code paths.

[robobun]

Closing: this is a no-behavior-change cleanup (three statements that are provably no-ops on every input; see the per-site proof in the PR body and the bot review above), so there is no input that can produce a test which fails without the diff and passes with it. Under the repo's test requirement for src/** changes that leaves nothing to gate against.

The deletions are correct; a maintainer who wants them can reopen and merge directly. Otherwise the larger dead-code sweep in #32000 is the better vehicle for this class of change.

[claude]

LGTM — three provably-dead statements removed with no behavior change.

What was reviewed:

• List::and_range: the dropped tail.range = *range re-stores the same Copy value just written by the struct literal; range: &Range is immutable between the two.
• Hyphen lookahead: verified length_of_leading_whitespace_ascii returns at most slice.len() (src/bun_core/string/immutable.rs:2134), so after the preceding i == input.len() break, i < input.len() is guaranteed and the dropped guard is always true.
• Version::parse tag arm: i is unchanged between the match input[i] dispatch and the loop, so input[i] is -/+ and the space-skip loop body never executes; part_start_i was set before it and Tag::parse reads from there.

Extended reasoning...

Overview

Three deletions in the semver parser (src/semver/SemverQuery.rs, src/semver/Version.rs), each a leftover from the Zig port: a redundant field store immediately after construction, an always-true i < input.len() conjunct guarded by a preceding equality check, and a whitespace-skip loop whose condition is false on the first iteration because the match arm was entered on a non-space byte at the same index.

Security risks

None. Pure dead-code removal in a byte parser; no new indexing, no widened input acceptance, no allocation or FFI changes. The one indexing site touched (input[i] in the hyphen lookahead) remains guarded by the immediately preceding if i == input.len() { break }, and I confirmed length_of_leading_whitespace_ascii cannot return more than its slice length, so i cannot exceed input.len() at that point.

Level of scrutiny

Low. Each deletion is an identity transformation provable from local control flow — no cross-function invariants are relied on beyond the bounded return of length_of_leading_whitespace_ascii, which I read directly. There is no input that can distinguish before from after, so the absence of a new test is justified per the repo's own rule (a test that passes both with and without the change is worse than no test). The existing test/cli/install/semver.test.ts (28 tests, 1946 assertions) exercises all three sites and passes.

Other factors

cargo clippy -p bun_semver is clean. The bug-hunting system found nothing. The PR body's per-site proofs are correct on independent verification.