diff --git a/constraints.go b/constraints.go index e8353bc..3ce4f03 100644 --- a/constraints.go +++ b/constraints.go @@ -276,6 +276,39 @@ func (c *constraint) string() string { type cfunc func(v *Version, c *constraint, includePre bool) (bool, error) +// constraintErr is a lazily-formatted error returned when a constraint check +// fails. The message (and the Version.String() conversion it requires) is only +// computed when Error() is called. A Check that discards the error therefore +// performs no message formatting and only allocates this small struct. +type constraintErr struct { + format string + v *Version + orig string + hasOrig bool +} + +func (e *constraintErr) Error() string { + if e.hasOrig { + return fmt.Sprintf(e.format, e.v, e.orig) + } + return fmt.Sprintf(e.format, e.v) +} + +const cerrOneFormat = "%q is a prerelease version and the constraint is only looking for release versions" + +// cerrOne builds a constraintErr referencing only the version, deferring all +// message formatting (including the version string conversion) until Error() +// is called. +func cerrOne(v *Version) *constraintErr { + return &constraintErr{format: cerrOneFormat, v: v} +} + +// cerrTwo builds a constraintErr referencing the version and the constraint's +// original string, deferring all message formatting until Error() is called. +func cerrTwo(v *Version, c *constraint, format string) *constraintErr { + return &constraintErr{format: format, v: v, orig: c.orig, hasOrig: true} +} + func parseConstraint(c string) (*constraint, error) { if len(c) > 0 { m := constraintRegex.FindStringSubmatch(c) @@ -347,7 +380,7 @@ func constraintNotEqual(v *Version, c *constraint, includePre bool) (bool, error // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } if c.dirty { @@ -357,7 +390,7 @@ func constraintNotEqual(v *Version, c *constraint, includePre bool) (bool, error if c.con.Minor() != v.Minor() && !c.minorDirty { return true, nil } else if c.minorDirty { - return false, fmt.Errorf("%q is equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is equal to %q") } else if c.con.Patch() != v.Patch() && !c.patchDirty { return true, nil } else if c.patchDirty { @@ -367,15 +400,15 @@ func constraintNotEqual(v *Version, c *constraint, includePre bool) (bool, error if eq { return true, nil } - return false, fmt.Errorf("%q is equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is equal to %q") } - return false, fmt.Errorf("%q is equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is equal to %q") } } eq := v.Equal(c.con) if eq { - return false, fmt.Errorf("%q is equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is equal to %q") } return true, nil @@ -386,7 +419,7 @@ func constraintGreaterThan(v *Version, c *constraint, includePre bool) (bool, er // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } var eq bool @@ -396,17 +429,17 @@ func constraintGreaterThan(v *Version, c *constraint, includePre bool) (bool, er if eq { return true, nil } - return false, fmt.Errorf("%q is less than or equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than or equal to %q") } if v.Major() > c.con.Major() { return true, nil } else if v.Major() < c.con.Major() { - return false, fmt.Errorf("%q is less than or equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than or equal to %q") } else if c.minorDirty { // This is a range case such as >11. When the version is something like // 11.1.0 is it not > 11. For that we would need 12 or higher - return false, fmt.Errorf("%q is less than or equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than or equal to %q") } else if c.patchDirty { // This is for ranges such as >11.1. A version of 11.1.1 is not greater // which one of 11.2.1 is greater @@ -414,7 +447,7 @@ func constraintGreaterThan(v *Version, c *constraint, includePre bool) (bool, er if eq { return true, nil } - return false, fmt.Errorf("%q is less than or equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than or equal to %q") } // If we have gotten here we are not comparing pre-preleases and can use the @@ -423,21 +456,21 @@ func constraintGreaterThan(v *Version, c *constraint, includePre bool) (bool, er if eq { return true, nil } - return false, fmt.Errorf("%q is less than or equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than or equal to %q") } func constraintLessThan(v *Version, c *constraint, includePre bool) (bool, error) { // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } eq := v.Compare(c.con) < 0 if eq { return true, nil } - return false, fmt.Errorf("%q is greater than or equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is greater than or equal to %q") } func constraintGreaterThanEqual(v *Version, c *constraint, includePre bool) (bool, error) { @@ -445,21 +478,21 @@ func constraintGreaterThanEqual(v *Version, c *constraint, includePre bool) (boo // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } eq := v.Compare(c.con) >= 0 if eq { return true, nil } - return false, fmt.Errorf("%q is less than %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than %q") } func constraintLessThanEqual(v *Version, c *constraint, includePre bool) (bool, error) { // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } var eq bool @@ -469,13 +502,13 @@ func constraintLessThanEqual(v *Version, c *constraint, includePre bool) (bool, if eq { return true, nil } - return false, fmt.Errorf("%q is greater than %q", v, c.orig) + return false, cerrTwo(v, c, "%q is greater than %q") } if v.Major() > c.con.Major() { - return false, fmt.Errorf("%q is greater than %q", v, c.orig) + return false, cerrTwo(v, c, "%q is greater than %q") } else if v.Major() == c.con.Major() && v.Minor() > c.con.Minor() && !c.minorDirty { - return false, fmt.Errorf("%q is greater than %q", v, c.orig) + return false, cerrTwo(v, c, "%q is greater than %q") } return true, nil @@ -491,11 +524,11 @@ func constraintTilde(v *Version, c *constraint, includePre bool) (bool, error) { // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } if v.LessThan(c.con) { - return false, fmt.Errorf("%q is less than %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than %q") } // ~0.0.0 is a special case where all constraints are accepted. It's @@ -506,11 +539,11 @@ func constraintTilde(v *Version, c *constraint, includePre bool) (bool, error) { } if v.Major() != c.con.Major() { - return false, fmt.Errorf("%q does not have same major version as %q", v, c.orig) + return false, cerrTwo(v, c, "%q does not have same major version as %q") } if v.Minor() != c.con.Minor() && !c.minorDirty { - return false, fmt.Errorf("%q does not have same major and minor version as %q", v, c.orig) + return false, cerrTwo(v, c, "%q does not have same major and minor version as %q") } return true, nil @@ -522,7 +555,7 @@ func constraintTildeOrEqual(v *Version, c *constraint, includePre bool) (bool, e // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } if c.dirty { @@ -534,7 +567,7 @@ func constraintTildeOrEqual(v *Version, c *constraint, includePre bool) (bool, e return true, nil } - return false, fmt.Errorf("%q is not equal to %q", v, c.orig) + return false, cerrTwo(v, c, "%q is not equal to %q") } // ^* --> (any) @@ -550,12 +583,12 @@ func constraintCaret(v *Version, c *constraint, includePre bool) (bool, error) { // The existence of prereleases is checked at the group level and passed in. // Exit early if the version has a prerelease but those are to be ignored. if v.Prerelease() != "" && !includePre { - return false, fmt.Errorf("%q is a prerelease version and the constraint is only looking for release versions", v) + return false, cerrOne(v) } // This less than handles prereleases if v.LessThan(c.con) { - return false, fmt.Errorf("%q is less than %q", v, c.orig) + return false, cerrTwo(v, c, "%q is less than %q") } var eq bool @@ -570,12 +603,12 @@ func constraintCaret(v *Version, c *constraint, includePre bool) (bool, error) { if eq { return true, nil } - return false, fmt.Errorf("%q does not have same major version as %q", v, c.orig) + return false, cerrTwo(v, c, "%q does not have same major version as %q") } // ^ when the major is 0 and minor > 0 is >=0.y.z < 0.y+1 if c.con.Major() == 0 && v.Major() > 0 { - return false, fmt.Errorf("%q does not have same major version as %q", v, c.orig) + return false, cerrTwo(v, c, "%q does not have same major version as %q") } // If the con Minor is > 0 it is not dirty if c.con.Minor() > 0 || c.patchDirty { @@ -583,11 +616,11 @@ func constraintCaret(v *Version, c *constraint, includePre bool) (bool, error) { if eq { return true, nil } - return false, fmt.Errorf("%q does not have same minor version as %q. Expected minor versions to match when constraint major version is 0", v, c.orig) + return false, cerrTwo(v, c, "%q does not have same minor version as %q. Expected minor versions to match when constraint major version is 0") } // ^ when the minor is 0 and minor > 0 is =0.0.z if c.con.Minor() == 0 && v.Minor() > 0 { - return false, fmt.Errorf("%q does not have same minor version as %q", v, c.orig) + return false, cerrTwo(v, c, "%q does not have same minor version as %q") } // At this point the major is 0 and the minor is 0 and not dirty. The patch @@ -596,7 +629,7 @@ func constraintCaret(v *Version, c *constraint, includePre bool) (bool, error) { if eq { return true, nil } - return false, fmt.Errorf("%q does not equal %q. Expect version and constraint to equal when major and minor versions are 0", v, c.orig) + return false, cerrTwo(v, c, "%q does not equal %q. Expect version and constraint to equal when major and minor versions are 0") } func isX(x string) bool { diff --git a/constraints_test.go b/constraints_test.go index fe2c14b..c75ff33 100644 --- a/constraints_test.go +++ b/constraints_test.go @@ -854,6 +854,42 @@ func TestConstraintsValidate(t *testing.T) { } } +// TestConstraintErrMessageOne pins the lazy one-argument branch of +// constraintErr.Error() (built by cerrOne); the prerelease guard inside each +// constraint func is the only caller of cerrOne and is otherwise only reached +// when the error is discarded (Check) or re-wrapped by Validate, so it needs +// the message read directly here. +func TestConstraintErrMessageOne(t *testing.T) { + c := &constraint{orig: "2.0.0", con: MustParse("2.0.0")} + v := MustParse("3.1.4-beta") + + ok, err := constraintGreaterThan(v, c, false) + if ok { + t.Fatal("expected constraint check to fail for a prerelease when prereleases are excluded") + } + got, want := err.Error(), `"3.1.4-beta" is a prerelease version and the constraint is only looking for release versions` + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +// TestConstraintErrMessageTwo pins the lazy two-argument branch of +// constraintErr.Error() (built by cerrTwo) end-to-end, reading the message +// straight from the returned error rather than through Validate's own copy. +func TestConstraintErrMessageTwo(t *testing.T) { + c := &constraint{orig: "2.0.0", con: MustParse("2.0.0")} + v := MustParse("1.0.0") + + ok, err := constraintGreaterThan(v, c, false) + if ok { + t.Fatal("expected constraint check to fail for a lower version") + } + got, want := err.Error(), `"1.0.0" is less than or equal to "2.0.0"` + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + func TestConstraintsValidateIncludePrerelease(t *testing.T) { tests := []struct { constraint string diff --git a/version.go b/version.go index 84544f4..02a1832 100644 --- a/version.go +++ b/version.go @@ -1,7 +1,6 @@ package semver import ( - "bytes" "database/sql/driver" "encoding/json" "errors" @@ -344,17 +343,23 @@ func MustParse(v string) *Version { // don't contain a leading v per the spec. Instead it's optional on // implementation. func (v Version) String() string { - var buf bytes.Buffer + buf := make([]byte, 0, 12+len(v.pre)+len(v.metadata)) - fmt.Fprintf(&buf, "%d.%d.%d", v.major, v.minor, v.patch) + buf = strconv.AppendUint(buf, v.major, 10) + buf = append(buf, '.') + buf = strconv.AppendUint(buf, v.minor, 10) + buf = append(buf, '.') + buf = strconv.AppendUint(buf, v.patch, 10) if v.pre != "" { - fmt.Fprintf(&buf, "-%s", v.pre) + buf = append(buf, '-') + buf = append(buf, v.pre...) } if v.metadata != "" { - fmt.Fprintf(&buf, "+%s", v.metadata) + buf = append(buf, '+') + buf = append(buf, v.metadata...) } - return buf.String() + return string(buf) } // Original returns the original value passed in to be parsed. @@ -690,44 +695,41 @@ func compareSegment(v, o uint64) int { } func comparePrerelease(v, o string) int { - // split the prelease versions by their part. The separator, per the spec, - // is a . - sparts := strings.Split(v, ".") - oparts := strings.Split(o, ".") - - // Find the longer length of the parts to know how many loop iterations to - // go through. - slen := len(sparts) - olen := len(oparts) - - l := slen - if olen > slen { - l = olen - } - - // Iterate over each part of the prereleases to compare the differences. - for i := 0; i < l; i++ { - // Since the lentgh of the parts can be different we need to create - // a placeholder. This is to avoid out of bounds issues. - stemp := "" - if i < slen { - stemp = sparts[i] + // Compare the prerelease identifiers part by part (the separator, per the + // spec, is a .) without allocating a slice of parts. Parts that exist in + // only one identifier are compared against the empty string, matching the + // original semantics. + vi, oi := 0, 0 + exv, exo := false, false + for !exv || !exo { + vpart := "" + if !exv { + if j := strings.IndexByte(v[vi:], '.'); j < 0 { + vpart = v[vi:] + exv = true + } else { + vpart = v[vi : vi+j] + vi += j + 1 + } } - otemp := "" - if i < olen { - otemp = oparts[i] + opart := "" + if !exo { + if j := strings.IndexByte(o[oi:], '.'); j < 0 { + opart = o[oi:] + exo = true + } else { + opart = o[oi : oi+j] + oi += j + 1 + } } - d := comparePrePart(stemp, otemp) - if d != 0 { + if d := comparePrePart(vpart, opart); d != 0 { return d } } - // Reaching here means two versions are of equal value but have different - // metadata (the part following a +). They are not identical in string form - // but the version comparison finds them to be equal. + // Reaching here means the compared parts were all equal. return 0 } @@ -785,9 +787,21 @@ func comparePrePart(s, o string) int { // Like strings.ContainsAny but does an only instead of any. func containsOnly(s string, comp string) bool { - return strings.IndexFunc(s, func(r rune) bool { - return !strings.ContainsRune(comp, r) - }) == -1 + if s == "" { + return true + } + // Build a per-byte lookup table once so the per-character check below is + // O(1) instead of scanning comp for every rune. + var ok [256]bool + for i := 0; i < len(comp); i++ { + ok[comp[i]] = true + } + for i := 0; i < len(s); i++ { + if !ok[s[i]] { + return false + } + } + return true } // From the spec, "Identifiers MUST comprise only diff --git a/version_test.go b/version_test.go index 5ebe9e0..3ccaf13 100644 --- a/version_test.go +++ b/version_test.go @@ -70,6 +70,25 @@ func TestStrictNewVersion(t *testing.T) { {"alpha+beta", true}, {"1.2.3-alpha_beta+foo", true}, {"1.0.0-alpha..1", true}, + + // Whitespace anywhere in the version is invalid per the spec. + {" 1.2.3", true}, + {"1.2.3 ", true}, + {"1.2 .3", true}, + {"1.2.3-alpha 1", true}, + + // Dangling separators and empty identifiers are invalid. + {"1.2.3+", true}, // empty build metadata + {"1.2.3-", true}, // empty pre-release + {"1.2.3-alpha.", true}, + {"1.2.3+meta.", true}, + {"1.2.3+meta..meta", true}, + + // Numeric segments and pre-release identifier parts must not be + // negative or lead with zero. + {"-1.2.3", true}, // negative major + {"1.2.3-rc.01", true}, + {"1.2.3-01.2", true}, } for _, tc := range tests { @@ -138,6 +157,24 @@ func TestNewVersion(t *testing.T) { {"9.8.7+meta+meta", true}, // Multiple metadata parts {"1.2.31----RC-SNAPSHOT.12.09.1--.12+788", true}, // Leading 0 in a number part of a pre-release segment + // Whitespace anywhere in the version is invalid, loose or not. + {" 1.2.3", true}, + {"1.2.3 ", true}, + {"1.2 .3", true}, + {"1.2.3-alpha 1", true}, + + // Dangling separators and empty identifiers are invalid. + {"1.2.3+", true}, + {"1.2.3-", true}, + {"1.2.3-alpha.", true}, + {"1.2.3+meta.", true}, + {"1.2.3+meta..meta", true}, + + // Negative and leading-zero numeric parts are invalid even loosely. + {"-1.2.3", true}, + {"1.2.3-rc.01", true}, + {"1.2.3-01.2", true}, + // Versions that are invalid but in loose mode are handled. // This enables a calver-ish style. This pattern has long // been supported by this package even though it technically @@ -218,6 +255,24 @@ func TestNewVersion(t *testing.T) { {"1.1.01", true}, // A leading 0 on a number segment {"9.8.7+meta+meta", true}, // Multiple metadata parts {"1.2.31----RC-SNAPSHOT.12.09.1--.12+788", true}, // Leading 0 in a number part of a pre-release segment + + // Whitespace anywhere in the version is invalid per the spec. + {" 1.2.3", true}, + {"1.2.3 ", true}, + {"1.2 .3", true}, + {"1.2.3-alpha 1", true}, + + // Dangling separators and empty identifiers are invalid. + {"1.2.3+", true}, + {"1.2.3-", true}, + {"1.2.3-alpha.", true}, + {"1.2.3+meta.", true}, + {"1.2.3+meta..meta", true}, + + // Negative and leading-zero numeric parts are invalid. + {"-1.2.3", true}, + {"1.2.3-rc.01", true}, + {"1.2.3-01.2", true}, } for _, tc := range tests {