semver: collapse a || union containing a match-all branch to * before the prerelease rule - #33739
Open
robobun wants to merge 12 commits into
Open
semver: collapse a || union containing a match-all branch to * before the prerelease rule#33739robobun wants to merge 12 commits into
robobun wants to merge 12 commits into
Claude / Claude Code Review
completed
Jul 8, 2026 in 40m 46s
Code review found 5 potential issues
Found 2 candidates, confirmed 5. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 5 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/semver/SemverQuery.rs:587-591 |
npm.rs find_best_version Eql fast path bypasses MATCH_ALL_BRANCH |
| 🟡 Nit | src/semver/SemverQuery.rs:1127-1130 |
parse_result.len == 0 guard defeated by Version::parse leading v/=/whitespace skip |
Annotations
Check warning on line 591 in src/semver/SemverQuery.rs
claude / Claude Code Review
npm.rs find_best_version Eql fast path bypasses MATCH_ALL_BRANCH
🟡 Same predicate-sync class as the `is_exact()`/`get_exact_version()` guards applied in 0d85b615 and the open `Group::eql()` note, but at a different site: `find_best_version`/`find_best_version_with_filter` in `src/install/npm.rs` (~1823/~1893) open-code `group.head.head.range.left.op == Eql` without checking `MATCH_ALL_BRANCH`. For `"1.2.3-alpha.1 || *"` the fast path returns `1.2.3-alpha.1`, but `Group::satisfies(1.2.3-alpha.1)` now returns `false` via the new early return here — the resolver
Check warning on line 1130 in src/semver/SemverQuery.rs
claude / Claude Code Review
parse_result.len == 0 guard defeated by Version::parse leading v/=/whitespace skip
The `parse_result.len == 0` guard added in 5ae87f1f only covers a bare operator at end-of-input; `Version::parse` also treats input made entirely of `v`/`=`/whitespace as "no operand" but returns `len > 0` (Version.rs:486-511 counts skipped chars into `result.len`). So `Bun.semver.satisfies("1.2.3-alpha.1", "1.2.3-alpha.1 || vv")` (also `"|| =="`, `"|| ^v"`, `"|| ^\t"`) leaves `branch_has_non_any` false, sets `MATCH_ALL_BRANCH`, and returns `false` — a regression from pre-PR Bun and npm-loose (b
Loading