fix(version): apply the 0.x caret rule in satisfiesRange - #1183
Conversation
|
@MRX-72 its updated now |
|
@sonukapoor could. you take a look here |
sonukapoor
left a comment
There was a problem hiding this comment.
Checked this against real node-semver rather than by reading, because "which versions does a caret admit" is exactly the kind of claim that deserves it. Ran 864 operator/version pairs through both implementations and compared.
Caret on released versions: 62 disagreements with npm on main, zero after this PR. Every case behaves as npm does, including the partial forms ^0, ^0.0, ^1 and ^1.2. No new disagreement anywhere, so nothing regressed.
The tests are discriminating. Reverting version.ts kills all three new blocks, and reverting only the first commit still kills one, so the parts refinement in the second commit is independently covered too. 145 suites, 1936 tests, merges cleanly.
The part worth saying out loud: this is not a no-op for users. Tightening the caret rule changes what the override detectors report. With a ^0.25.0 override and copies at 0.24.0, 0.25.12 and 0.26.1:
- OA010 goes from admitting 6 of 8 to 2 of 8. It had been reporting floors as admitting versions npm can never resolve, so that is a false-positive reduction.
- OA005.d, OA006 and OA008 each now fire on a case they were wrongly suppressing. OA006 in particular now matches its own code comment about a parent forcing a different version.
All four are correct, but anyone with a ^0.x override will see new findings after upgrading, so this needs a CHANGELOG line under Fixed. I will add that when it merges.
Two things I found while looking, both pre-existing and neither yours to fix here.
OA008's message is wrong for the copies this newly flags. It prints "installed copies (0.24.0, 0.26.1) are below the floor", but 0.26.1 is above the ceiling, not below the floor. main mislabels it the same way for ^1.2.0 with a 2.0.0 copy, so this PR only makes it more visible. Filing separately.
Tilde has the identical bug you just fixed for caret. ~1 admits only 1.0.x where npm means >=1.0.0 <2.0.0. Same shape, same file, untouched here, and reasonable to leave for a follow-up.
Approving. Holding the merge until next week rather than stacking it on today's, since I would rather each of these landed on its own day.
sonukapoor
left a comment
There was a problem hiding this comment.
Good catch, and the fix is right. I reproduced the bug on current main before reading your diff:
^0.25.0 admits 0.26.0 -> true
^0.0.3 admits 0.0.4 -> true
Both should be false, and your version follows npm's leftmost-non-zero rule properly, including the partial cases ^0 and ^0.0 that are easy to forget.
One thing I checked and want to record, since it looks wrong and is not: v.split(".").map(Number) seemed unsafe for a prerelease like 0.0.3-beta.1, but coerceVersion always returns a clean X.Y.Z, so the parse holds.
This is worth more than its size suggests. satisfiesRange is what enumerates the versions an override admits, so a wrong caret ceiling means OA010 evaluates the wrong version set entirely. This fixes correctness in a rule, not just a helper.
I merge these one at a time rather than in a batch, so it will land shortly.
|
Merged, thank you @prx-my! This one quietly mattered more than its size: |
Closes #1177
Summary
satisfiesRange()treated^as "same major,>=floor", which is correctfor
^1.2.0but wrong for0.x: npm resolves^0.25.0to>=0.25.0 <0.26.0because with a zero major the minor is the breakingposition. The caret branch only compared majors, so
0 === 0admitted every0.y.zabove the floor.Changes
src/utils/version.tsmajor > 0-> same major (unchanged)0.y.z-> same major and minor (breaking position)0.0.z-> exact match~branch style andisBreakingUpgrade()'s 0.xrule (from [Bug] Breaking? not flagged for 0.x cross-minor upgrades (isMajorVersionBump ignores 0.x) #990), so both semver oracles now agree
tests/utils/version-extensions.test.tsWhy it matters
satisfiesRangeis the semver oracle for OA005/OA006/OA008 (false-negativedirection) and OA010's admitted-version scan (false-positive direction): for
"esbuild": "^0.25.0"it admitted every published0.26.x,0.27.x, ...and reported floors as admitting vulnerable versions npm can never resolve.
Verification
Per CONTRIBUTING.md:
npm run lint:tests✓npm run build✓node dist/index.js advisories sync✓ (228435 records)npm test✓ (145 suites, 1935 tests)tests fail before the fix and pass after, and confirmed existing
^1.2.0/
~behavior is unchanged.