Skip to content

Commit cdf588b

Browse files
authored
fix(internal/semver): change PrereleaseNumber to int (#3114)
1 parent cd2b7a3 commit cdf588b

5 files changed

Lines changed: 117 additions & 80 deletions

File tree

internal/legacylibrarian/legacylibrarian/commit_version_analyzer.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
"github.com/googleapis/librarian/internal/legacylibrarian/legacyconfig"
2424
"github.com/googleapis/librarian/internal/legacylibrarian/legacygitrepo"
25-
legacysemver "github.com/googleapis/librarian/internal/semver"
25+
"github.com/googleapis/librarian/internal/semver"
2626
)
2727

2828
// getConventionalCommitsSinceLastRelease returns all conventional commits for the given library since the
@@ -171,25 +171,25 @@ func isUnderAnyPath(file string, paths []string) bool {
171171
// NextVersion calculates the next semantic version based on a slice of conventional commits.
172172
func NextVersion(commits []*legacygitrepo.ConventionalCommit, currentVersion string) (string, error) {
173173
highestChange := getHighestChange(commits)
174-
return legacysemver.DeriveNext(highestChange, currentVersion)
174+
return semver.DeriveNext(highestChange, currentVersion)
175175
}
176176

177177
// getHighestChange determines the highest-ranking change type from a slice of commits.
178-
func getHighestChange(commits []*legacygitrepo.ConventionalCommit) legacysemver.ChangeLevel {
179-
highestChange := legacysemver.None
178+
func getHighestChange(commits []*legacygitrepo.ConventionalCommit) semver.ChangeLevel {
179+
highestChange := semver.None
180180
for _, commit := range commits {
181-
var currentChange legacysemver.ChangeLevel
181+
var currentChange semver.ChangeLevel
182182
switch {
183183
case commit.IsNested:
184184
// ignore nested commit type for version bump
185185
// this allows for always increase minor version for generation PR
186-
currentChange = legacysemver.Minor
186+
currentChange = semver.Minor
187187
case commit.IsBreaking:
188-
currentChange = legacysemver.Major
188+
currentChange = semver.Major
189189
case commit.Type == "feat":
190-
currentChange = legacysemver.Minor
190+
currentChange = semver.Minor
191191
case commit.Type == "fix":
192-
currentChange = legacysemver.Patch
192+
currentChange = semver.Patch
193193
}
194194
if currentChange > highestChange {
195195
highestChange = currentChange

internal/legacylibrarian/legacylibrarian/commit_version_analyzer_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"github.com/google/go-cmp/cmp/cmpopts"
2424
"github.com/googleapis/librarian/internal/legacylibrarian/legacyconfig"
2525
"github.com/googleapis/librarian/internal/legacylibrarian/legacygitrepo"
26-
legacysemver "github.com/googleapis/librarian/internal/semver"
26+
"github.com/googleapis/librarian/internal/semver"
2727
)
2828

2929
func TestShouldIncludeForRelease(t *testing.T) {
@@ -559,7 +559,7 @@ func TestGetHighestChange(t *testing.T) {
559559
for _, test := range []struct {
560560
name string
561561
commits []*legacygitrepo.ConventionalCommit
562-
expectedChange legacysemver.ChangeLevel
562+
expectedChange semver.ChangeLevel
563563
}{
564564
{
565565
name: "major change",
@@ -568,75 +568,75 @@ func TestGetHighestChange(t *testing.T) {
568568
{Type: "feat"},
569569
{Type: "fix"},
570570
},
571-
expectedChange: legacysemver.Major,
571+
expectedChange: semver.Major,
572572
},
573573
{
574574
name: "minor change",
575575
commits: []*legacygitrepo.ConventionalCommit{
576576
{Type: "feat"},
577577
{Type: "fix"},
578578
},
579-
expectedChange: legacysemver.Minor,
579+
expectedChange: semver.Minor,
580580
},
581581
{
582582
name: "patch change",
583583
commits: []*legacygitrepo.ConventionalCommit{
584584
{Type: "fix"},
585585
},
586-
expectedChange: legacysemver.Patch,
586+
expectedChange: semver.Patch,
587587
},
588588
{
589589
name: "no change",
590590
commits: []*legacygitrepo.ConventionalCommit{
591591
{Type: "docs"},
592592
{Type: "chore"},
593593
},
594-
expectedChange: legacysemver.None,
594+
expectedChange: semver.None,
595595
},
596596
{
597597
name: "no commits",
598598
commits: []*legacygitrepo.ConventionalCommit{},
599-
expectedChange: legacysemver.None,
599+
expectedChange: semver.None,
600600
},
601601
{
602602
name: "nested commit forces minor bump",
603603
commits: []*legacygitrepo.ConventionalCommit{
604604
{Type: "fix"},
605605
{Type: "feat", IsNested: true},
606606
},
607-
expectedChange: legacysemver.Minor,
607+
expectedChange: semver.Minor,
608608
},
609609
{
610610
name: "nested commit with breaking change forces minor bump",
611611
commits: []*legacygitrepo.ConventionalCommit{
612612
{Type: "feat", IsBreaking: true, IsNested: true},
613613
{Type: "feat"},
614614
},
615-
expectedChange: legacysemver.Minor,
615+
expectedChange: semver.Minor,
616616
},
617617
{
618618
name: "major change and nested commit",
619619
commits: []*legacygitrepo.ConventionalCommit{
620620
{Type: "feat", IsBreaking: true},
621621
{Type: "fix", IsNested: true},
622622
},
623-
expectedChange: legacysemver.Major,
623+
expectedChange: semver.Major,
624624
},
625625
{
626626
name: "nested commit before major change",
627627
commits: []*legacygitrepo.ConventionalCommit{
628628
{Type: "fix", IsNested: true},
629629
{Type: "feat", IsBreaking: true},
630630
},
631-
expectedChange: legacysemver.Major,
631+
expectedChange: semver.Major,
632632
},
633633
{
634634
name: "nested commit with only fixes forces minor bump",
635635
commits: []*legacygitrepo.ConventionalCommit{
636636
{Type: "fix"},
637637
{Type: "fix", IsNested: true},
638638
},
639-
expectedChange: legacysemver.Minor,
639+
expectedChange: semver.Minor,
640640
},
641641
} {
642642
t.Run(test.name, func(t *testing.T) {

internal/legacylibrarian/legacylibrarian/release_stage.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"github.com/googleapis/librarian/internal/legacylibrarian/legacyconfig"
2727
"github.com/googleapis/librarian/internal/legacylibrarian/legacydocker"
2828
"github.com/googleapis/librarian/internal/legacylibrarian/legacygitrepo"
29-
legacysemver "github.com/googleapis/librarian/internal/semver"
29+
"github.com/googleapis/librarian/internal/semver"
3030
)
3131

3232
type stageRunner struct {
@@ -248,7 +248,7 @@ func (r *stageRunner) updateLibrary(library *legacyconfig.LibraryState, commits
248248
// If library version was explicitly set, attempt to use it. Otherwise, try to determine the version from the commits.
249249
if r.libraryVersion != "" {
250250
slog.Info("library version override inputted", "currentVersion", library.Version, "inputVersion", r.libraryVersion)
251-
nextVersion = legacysemver.MaxVersion(library.Version, r.libraryVersion)
251+
nextVersion = semver.MaxVersion(library.Version, r.libraryVersion)
252252
slog.Debug("determined the library's next version from version input", "library", library.ID, "nextVersion", nextVersion)
253253
// Currently, nextVersion is the max of current version or input version. If nextVersion is equal to the current version,
254254
// then the input version is either equal or less than current version and cannot be used for release
@@ -304,7 +304,7 @@ func (r *stageRunner) determineNextVersion(commits []*legacygitrepo.Conventional
304304
}
305305

306306
// Compare versions and pick latest
307-
return legacysemver.MaxVersion(nextVersionFromCommits, libraryConfig.NextVersion), nil
307+
return semver.MaxVersion(nextVersionFromCommits, libraryConfig.NextVersion), nil
308308
}
309309

310310
// toCommit converts a slice of legacygitrepo.ConventionalCommit to a slice of legacyconfig.Commit.

internal/semver/semver.go

Lines changed: 71 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
// Package legacysemver provides functionality for parsing, comparing, and manipulating
15+
// Package semver provides functionality for parsing, comparing, and manipulating
1616
// semantic version strings according to the SemVer 2.0.0 spec.
17-
package legacysemver
17+
package semver
1818

1919
import (
2020
"fmt"
@@ -32,15 +32,22 @@ type Version struct {
3232
// PrereleaseSeparator is the separator between the pre-release string and
3333
// its version (e.g., ".").
3434
PrereleaseSeparator string
35-
// PrereleaseNumber is the numeric part of the pre-release string (e.g., "1", "21").
36-
PrereleaseNumber string
35+
// PrereleaseNumber is the numeric part of the pre-release segment of the
36+
// version string (e.g., the 1 in "alpha.1"). Zero is a valid pre-release
37+
// number. If there is no numeric part in the pre-release segment, this
38+
// field is nil.
39+
PrereleaseNumber *int
3740
}
3841

3942
// semverRegex defines format for semantic version.
4043
// Regex from https://semver.org/, with buildmetadata part removed.
4144
// It uses named capture groups for major, minor, patch, and prerelease.
4245
var semverRegex = regexp.MustCompile(`^(?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)\.(?P<patch>0|[1-9]\d*)(?:-(?P<prerelease>(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?$`)
4346

47+
// unsegmentedPrereleaseRegexp extracts the prerelease number, if present, in the
48+
// prerelease portion of the SemVer version parsed by [semverRegex].
49+
var unsegmentedPrereleaseRegexp = regexp.MustCompile(`^(.*?)(\d+)$`)
50+
4451
// Parse parses a version string into a Version struct.
4552
func Parse(versionString string) (*Version, error) {
4653
matches := semverRegex.FindStringSubmatch(versionString)
@@ -78,13 +85,21 @@ func Parse(versionString string) (*Version, error) {
7885
if i := strings.LastIndex(prerelease, "."); i != -1 {
7986
v.Prerelease = prerelease[:i]
8087
v.PrereleaseSeparator = "."
81-
v.PrereleaseNumber = prerelease[i+1:]
88+
num, err := strconv.Atoi(prerelease[i+1:])
89+
if err != nil {
90+
return nil, fmt.Errorf("invalid prerelease number: %w", err)
91+
}
92+
v.PrereleaseNumber = &num
8293
} else {
83-
re := regexp.MustCompile(`^(.*?)(\d+)$`)
84-
matches := re.FindStringSubmatch(prerelease)
94+
matches := unsegmentedPrereleaseRegexp.FindStringSubmatch(prerelease)
8595
if len(matches) == 3 {
8696
v.Prerelease = matches[1]
87-
v.PrereleaseNumber = matches[2]
97+
num, err := strconv.Atoi(matches[2])
98+
if err != nil {
99+
// This should not happen if the regex is correct.
100+
return nil, fmt.Errorf("invalid prerelease number: %w", err)
101+
}
102+
v.PrereleaseNumber = &num
88103
} else {
89104
v.Prerelease = prerelease
90105
}
@@ -130,39 +145,73 @@ func (v *Version) Compare(other *Version) int {
130145
return 1
131146
}
132147
// prerelease number (e.g. "alpha1" vs "alpha2")
133-
if v.PrereleaseNumber < other.PrereleaseNumber {
148+
// Note: Lack of prerelease number is considered lower precedence than
149+
// the same prerelease when it has a number - https://semver.org/#spec-item-11.
150+
if v.PrereleaseNumber == nil && other.PrereleaseNumber != nil {
134151
return -1
135152
}
136-
if v.PrereleaseNumber > other.PrereleaseNumber {
153+
if v.PrereleaseNumber != nil && other.PrereleaseNumber == nil {
137154
return 1
138155
}
156+
if v.PrereleaseNumber != nil && other.PrereleaseNumber != nil {
157+
if *v.PrereleaseNumber < *other.PrereleaseNumber {
158+
return -1
159+
}
160+
if *v.PrereleaseNumber > *other.PrereleaseNumber {
161+
return 1
162+
}
163+
}
139164
return 0
140165
}
141166

142167
// String formats a Version struct into a string.
143168
func (v *Version) String() string {
144169
version := fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch)
145170
if v.Prerelease != "" {
146-
version += "-" + v.Prerelease + v.PrereleaseSeparator + v.PrereleaseNumber
171+
version += "-" + v.Prerelease
172+
if v.PrereleaseNumber != nil {
173+
version += v.PrereleaseSeparator + strconv.Itoa(*v.PrereleaseNumber)
174+
}
147175
}
148176
return version
149177
}
150178

151179
// incrementPrerelease increments the pre-release version number, or appends
152180
// one if it doesn't exist.
153-
func (v *Version) incrementPrerelease() error {
154-
if v.PrereleaseNumber == "" {
181+
func (v *Version) incrementPrerelease() {
182+
if v.PrereleaseNumber == nil {
155183
v.PrereleaseSeparator = "."
156-
v.PrereleaseNumber = "1"
157-
return nil
184+
// Initialize a new int pointer set to 0. Fallthrough to increment to 1.
185+
// We prefer the first prerelease to use 1 instead of 0.
186+
v.PrereleaseNumber = new(int)
158187
}
159-
num, err := strconv.Atoi(v.PrereleaseNumber)
160-
if err != nil {
161-
// This should not happen if Parse is correct.
162-
return fmt.Errorf("invalid prerelease version: %w", err)
188+
*v.PrereleaseNumber++
189+
}
190+
191+
func (v *Version) bump(highestChange ChangeLevel) {
192+
if v.Prerelease != "" {
193+
// Only bump the prerelease version number.
194+
v.incrementPrerelease()
195+
return
196+
}
197+
198+
// Bump the version core.
199+
// Breaking changes and feat result in minor bump for pre-1.0.0 versions.
200+
if (v.Major == 0 && highestChange == Major) || highestChange == Minor {
201+
v.Minor++
202+
v.Patch = 0
203+
return
204+
}
205+
if highestChange == Patch {
206+
v.Patch++
207+
return
208+
}
209+
if highestChange == Major {
210+
v.Major++
211+
v.Minor = 0
212+
v.Patch = 0
213+
return
163214
}
164-
v.PrereleaseNumber = strconv.Itoa(num + 1)
165-
return nil
166215
}
167216

168217
// MaxVersion returns the largest semantic version string among the provided version strings.
@@ -218,36 +267,7 @@ func DeriveNext(highestChange ChangeLevel, currentVersion string) (string, error
218267
return "", fmt.Errorf("failed to parse current version: %w", err)
219268
}
220269

221-
// Handle prerelease versions
222-
if currentSemVer.Prerelease != "" {
223-
if err := currentSemVer.incrementPrerelease(); err != nil {
224-
return "", err
225-
}
226-
return currentSemVer.String(), nil
227-
}
228-
229-
// Handle standard versions
230-
if currentSemVer.Major == 0 {
231-
// breaking change and feat result in minor bump for pre-1.0.0
232-
if highestChange == Major || highestChange == Minor {
233-
currentSemVer.Minor++
234-
currentSemVer.Patch = 0
235-
} else {
236-
currentSemVer.Patch++
237-
}
238-
} else {
239-
switch highestChange {
240-
case Major:
241-
currentSemVer.Major++
242-
currentSemVer.Minor = 0
243-
currentSemVer.Patch = 0
244-
case Minor:
245-
currentSemVer.Minor++
246-
currentSemVer.Patch = 0
247-
case Patch:
248-
currentSemVer.Patch++
249-
}
250-
}
270+
currentSemVer.bump(highestChange)
251271

252272
return currentSemVer.String(), nil
253273
}

0 commit comments

Comments
 (0)