Conversation
When next_version("prerelease", prerelease_token="rc") is called on a
version that already has a prerelease with a different token (e.g. dev),
bump_prerelease() ignores the token argument and simply increments the
existing counter. 1.2.4-dev.1 becomes 1.2.4-dev.2 instead of 1.2.4-rc.1.
Fix: before delegating to bump_prerelease, check whether the existing
prerelease token matches the requested one. When they differ, replace
the prerelease directly with "{prerelease_token}.1".
Mukller
left a comment
Mukller
left a comment
There was a problem hiding this comment.
Code Review
Bug Verification
Reproducible on current master:
import semver
v = semver.Version.parse('1.2.4-dev.1')
print(v.next_version('prerelease', prerelease_token='rc'))
# Current master: 1.2.4-dev.2 (bug)
# After patch: 1.2.4-rc.1 (correct)Root cause: bump_prerelease ignores its token parameter when self._prerelease is not None, passing only the existing string to _increment_prerelease. The token is only used in the else branch (no existing prerelease).
Fix Correctness
The 4-line guard in next_version handles the token-change case entirely in one place, without touching bump_prerelease. This is correct because:
- Only
next_versionhas the concept of aprerelease_tokenin its API —bump_prereleasedocuments that it ignores the token for an existing prerelease (it just bumps the counter). version.replace(prerelease=f'{prerelease_token}.1', build=None)keeps the major/minor/patch unchanged — correct, since we are staying in the same patch-bump cycle.
Edge Cases
| Scenario | Before | After |
|---|---|---|
| No prerelease | 1.2.4-rc.1 ✓ |
1.2.4-rc.1 ✓ (unchanged) |
| Same token | 1.2.4-rc.2 ✓ |
1.2.4-rc.2 ✓ (unchanged) |
| Different token | 1.2.4-dev.2 ✗ |
1.2.4-rc.1 ✓ (fixed) |
Empty token '' |
falls to bump_prerelease |
falls to bump_prerelease (guard skipped by if prerelease_token:) |
None token |
falls to bump_prerelease |
falls to bump_prerelease (guard skipped by if prerelease_token:) |
Change Size
4 lines added to one method. No changes to bump_prerelease or _increment_prerelease.
|
Thank you @Mukller for your contribution! Much appreciated! 👍 I fixed a small doctest. I'm going to merge it. |
|
Hi, @tomschr and @Mukller - i randomly came across this pr after it had been merged. I noticed that one thing this changes is that a next version can now be considered before the version it is supposed to succeed. Per SemVer §11 (and _nat_cmp), alphanumeric identifiers compare by ASCII, so this is only "next" when the requested token happens to sort above the existing one: from semver import Version
# 'c' < 'e' (ASCII identifier compare)
v = Version.parse("1.2.3-release.1+b.1")
n = v.next_version("prerelease", prerelease_token="rc")
print(v, "--", "rc", "-->", n, "| next > current:", n > v)
# 'd' < 'r' (ASCII identifier compare)
v = Version.parse("1.2.3-rc+b.1")
n = v.next_version("prerelease", prerelease_token="dev")
print(v, "--", "dev", "-->", n, "| next > current:", n > v)
# numeric id < alphanumeric id (SemVer section 11)
v = Version.parse("1.2.3-2dev.1+b.1")
n = v.next_version("prerelease", prerelease_token="2")
print(v, "--", "2", "-->", n, "| next > current:", n > v)I'm not sure you'd consider this a bug or not, it at least seems quite surprising behaviour, especially in the light of:
python-semver/src/semver/version.py Line 454 in 0f18aa0 I'd love to hear your response to this - sorry if it's a false alarm. |
3 participants
Problem
Fixes #339.
next_version('prerelease', prerelease_token='rc')ignores the token when the version already has a prerelease with a different token:Root Cause
next_versiondelegates tobump_prerelease(token, bump_when_empty=True). Insidebump_prerelease, when a prerelease already exists thetokenargument is silently ignored:Fix
Before delegating to
bump_prerelease,next_versionnow checks whether the existing prerelease token matches the requested one. When they differ, the prerelease is reset to{prerelease_token}.1:Behaviour Table After Fix
prerelease_token1.2.3'rc'1.2.4-rc.1(unchanged)1.2.4-rc.1'rc'1.2.4-rc.2(unchanged)1.2.4-dev.1'rc'1.2.4-rc.1(fixed)1.2.4-dev.3'dev'1.2.4-dev.4(unchanged)The patch version is preserved when changing from one prerelease token to another (the version is already in its bump-patch prerelease cycle).