diff --git a/parser.go b/parser.go index e52e83a..af0ecf1 100644 --- a/parser.go +++ b/parser.go @@ -171,8 +171,13 @@ func (p *Parser) parseNative(constraint string, scheme string) (*Range, error) { } } -// ToVersString converts a Range back to a vers URI string. +// ToVersString converts a Range back to a vers URI string. The scheme is +// canonicalized; when the range already has a scheme it takes precedence. func (p *Parser) ToVersString(r *Range, scheme string) string { + if r.Scheme != "" { + scheme = r.Scheme + } + scheme = canonicalScheme(scheme) if r.IsUnbounded() && len(r.Exclusions) == 0 && len(r.RawConstraints) == 0 { return fmt.Sprintf("vers:%s/*", scheme) } diff --git a/parser_test.go b/parser_test.go index bfdc802..3923688 100644 --- a/parser_test.go +++ b/parser_test.go @@ -548,6 +548,29 @@ func TestToVersString(t *testing.T) { } } +func TestToVersStringCanonicalizesScheme(t *testing.T) { + gem, err := ParseNative("~> 1.2", "rubygems") + if err != nil { + t.Fatal(err) + } + if got := ToVersString(gem, "rubygems"); got != "vers:gem/>=1.2|<2" { + t.Errorf("ToVersString with alias scheme = %q, want vers:gem/>=1.2|<2", got) + } + + untyped := NewRange([]Interval{GreaterThanInterval("v1.2.3", true)}) + if got := ToVersString(untyped, "golang"); got != "vers:go/>=v1.2.3" { + t.Errorf("ToVersString untyped with alias = %q, want vers:go/>=v1.2.3", got) + } + + npm, err := ParseNative("^1.2.3", "npm") + if err != nil { + t.Fatal(err) + } + if got := ToVersString(npm, "pypi"); got != "vers:npm/>=1.2.3|<2.0.0" { + t.Errorf("ToVersString ignored typed range scheme = %q, want vers:npm/>=1.2.3|<2.0.0", got) + } +} + func TestToVersStringEncodesMetacharacters(t *testing.T) { parser := NewParser() diff --git a/range.go b/range.go index 8f19b13..841509b 100644 --- a/range.go +++ b/range.go @@ -269,49 +269,56 @@ func (r *Range) MinimumVersion() (string, bool) { } // Union returns a new Range that is the union of this range and another. +// The operands are assumed to use compatible schemes; use UnionChecked to +// have that verified. func (r *Range) Union(other *Range) *Range { - if r.IsEmpty() { - return other + left, right := rangesWithCommonScheme(r, other) + if left.IsEmpty() { + return right } - if other.IsEmpty() { - return r + if right.IsEmpty() { + return left } - // Combine all intervals - allIntervals := make([]Interval, 0, len(r.Intervals)+len(other.Intervals)) - allIntervals = append(allIntervals, r.Intervals...) - allIntervals = append(allIntervals, other.Intervals...) + cmp := compareFuncFor(left.Scheme) - cmp := compareFuncFor(r.Scheme) + // Combine all intervals + allIntervals := make([]Interval, 0, len(left.Intervals)+len(right.Intervals)) + allIntervals = append(allIntervals, left.Intervals...) + allIntervals = append(allIntervals, right.Intervals...) // Merge overlapping intervals for containment checking merged := mergeIntervals(allIntervals, cmp) - // Combine exclusions (intersection of exclusions for union) - exclusions := make([]string, 0) - for _, e := range r.Exclusions { - for _, oe := range other.Exclusions { - if cmp(e, oe) == 0 { - exclusions = append(exclusions, e) - break - } + // An exclusion survives the union only when the other operand does not + // independently supply the excluded version. + var exclusions []string + for _, e := range left.Exclusions { + if !right.Contains(e) { + exclusions = append(exclusions, e) + } + } + for _, e := range right.Exclusions { + if left.Contains(e) || containsExclusion(exclusions, e, cmp) { + continue } + exclusions = append(exclusions, e) } // Combine raw constraints (unmerged) for VERS output - rawConstraints := make([]Interval, 0, len(r.RawConstraints)+len(other.RawConstraints)) - if len(r.RawConstraints) > 0 { - rawConstraints = append(rawConstraints, r.RawConstraints...) + rawConstraints := make([]Interval, 0, len(left.RawConstraints)+len(right.RawConstraints)) + if len(left.RawConstraints) > 0 { + rawConstraints = append(rawConstraints, left.RawConstraints...) } else { - rawConstraints = append(rawConstraints, r.Intervals...) + rawConstraints = append(rawConstraints, left.Intervals...) } - if len(other.RawConstraints) > 0 { - rawConstraints = append(rawConstraints, other.RawConstraints...) + if len(right.RawConstraints) > 0 { + rawConstraints = append(rawConstraints, right.RawConstraints...) } else { - rawConstraints = append(rawConstraints, other.Intervals...) + rawConstraints = append(rawConstraints, right.Intervals...) } - return &Range{Intervals: merged, Exclusions: exclusions, RawConstraints: rawConstraints, Scheme: r.Scheme} + return &Range{Intervals: merged, Exclusions: exclusions, RawConstraints: rawConstraints, Scheme: left.Scheme} } // UnionChecked returns the union of ranges that use compatible schemes. @@ -319,35 +326,38 @@ func (r *Range) UnionChecked(other *Range) (*Range, error) { if err := checkRangeSchemes(r, other); err != nil { return nil, err } - left, right := rangesWithCommonScheme(r, other) - return left.Union(right), nil + return r.Union(other), nil } -// Intersect returns a new Range that is the intersection of this range and another. +// Intersect returns a new Range that is the intersection of this range and +// another. The operands are assumed to use compatible schemes; use +// IntersectChecked to have that verified. func (r *Range) Intersect(other *Range) *Range { + left, right := rangesWithCommonScheme(r, other) + // Combine raw constraints for VERS output (preserved even if result is empty) - rawConstraints := make([]Interval, 0, len(r.RawConstraints)+len(other.RawConstraints)) - if len(r.RawConstraints) > 0 { - rawConstraints = append(rawConstraints, r.RawConstraints...) + rawConstraints := make([]Interval, 0, len(left.RawConstraints)+len(right.RawConstraints)) + if len(left.RawConstraints) > 0 { + rawConstraints = append(rawConstraints, left.RawConstraints...) } else { - rawConstraints = append(rawConstraints, r.Intervals...) + rawConstraints = append(rawConstraints, left.Intervals...) } - if len(other.RawConstraints) > 0 { - rawConstraints = append(rawConstraints, other.RawConstraints...) + if len(right.RawConstraints) > 0 { + rawConstraints = append(rawConstraints, right.RawConstraints...) } else { - rawConstraints = append(rawConstraints, other.Intervals...) + rawConstraints = append(rawConstraints, right.Intervals...) } - if r.IsEmpty() || other.IsEmpty() { - return &Range{RawConstraints: rawConstraints, Scheme: r.Scheme} + if left.IsEmpty() || right.IsEmpty() { + return &Range{RawConstraints: rawConstraints, Scheme: left.Scheme} } - cmp := compareFuncFor(r.Scheme) + cmp := compareFuncFor(left.Scheme) // Intersect each pair of intervals var result []Interval - for _, i1 := range r.Intervals { - for _, i2 := range other.Intervals { + for _, i1 := range left.Intervals { + for _, i2 := range right.Intervals { intersection := i1.intersectCmp(i2, cmp) if !intersection.isEmptyCmp(cmp) { result = append(result, intersection) @@ -359,22 +369,15 @@ func (r *Range) Intersect(other *Range) *Range { merged := mergeIntervals(result, cmp) // Combine exclusions (union of exclusions for intersection) - exclusions := make([]string, 0, len(r.Exclusions)+len(other.Exclusions)) - exclusions = append(exclusions, r.Exclusions...) - for _, e := range other.Exclusions { - found := false - for _, existing := range exclusions { - if e == existing { - found = true - break - } - } - if !found { + exclusions := make([]string, 0, len(left.Exclusions)+len(right.Exclusions)) + exclusions = append(exclusions, left.Exclusions...) + for _, e := range right.Exclusions { + if !containsExclusion(exclusions, e, cmp) { exclusions = append(exclusions, e) } } - return &Range{Intervals: merged, Exclusions: exclusions, RawConstraints: rawConstraints, Scheme: r.Scheme} + return &Range{Intervals: merged, Exclusions: exclusions, RawConstraints: rawConstraints, Scheme: left.Scheme} } // IntersectChecked returns the intersection of ranges that use compatible schemes. @@ -382,8 +385,7 @@ func (r *Range) IntersectChecked(other *Range) (*Range, error) { if err := checkRangeSchemes(r, other); err != nil { return nil, err } - left, right := rangesWithCommonScheme(r, other) - return left.Intersect(right), nil + return r.Intersect(other), nil } func checkRangeSchemes(a, b *Range) error { @@ -397,6 +399,15 @@ func checkRangeSchemes(a, b *Range) error { return nil } +func containsExclusion(exclusions []string, version string, cmp func(a, b string) int) bool { + for _, existing := range exclusions { + if cmp(existing, version) == 0 { + return true + } + } + return false +} + func rangesWithCommonScheme(a, b *Range) (*Range, *Range) { scheme := a.Scheme if scheme == "" { @@ -407,16 +418,22 @@ func rangesWithCommonScheme(a, b *Range) (*Range, *Range) { return &left, &right } -// Exclude returns a new Range that excludes the given version. +// Exclude returns a Range that excludes the given version. If the range does +// not contain the version, the receiver is returned unchanged. func (r *Range) Exclude(version string) *Range { + if !r.Contains(version) { + return r + } + exclusions := make([]string, len(r.Exclusions), len(r.Exclusions)+1) copy(exclusions, r.Exclusions) exclusions = append(exclusions, version) return &Range{ - Intervals: r.Intervals, - Exclusions: exclusions, - Scheme: r.Scheme, + Intervals: r.Intervals, + Exclusions: exclusions, + RawConstraints: r.RawConstraints, + Scheme: r.Scheme, } } diff --git a/range_test.go b/range_test.go index b549348..e07c908 100644 --- a/range_test.go +++ b/range_test.go @@ -404,6 +404,123 @@ func TestRangeExclude(t *testing.T) { } } +func TestRangeExcludePreservesRawConstraints(t *testing.T) { + original, err := ParseNative(">=1.0, <2.0", "gem") + if err != nil { + t.Fatal(err) + } + if len(original.RawConstraints) == 0 { + t.Fatal("test precondition: native range should carry raw constraints") + } + + excluded := original.Exclude("1.5") + if excluded.Contains("1.5") { + t.Error("excluded version should not be contained") + } + if len(excluded.RawConstraints) != len(original.RawConstraints) { + t.Errorf("Exclude dropped RawConstraints: got %d, want %d", len(excluded.RawConstraints), len(original.RawConstraints)) + } + + serialized := ToVersString(excluded, "gem") + roundTripped, err := Parse(serialized) + if err != nil { + t.Fatalf("Parse(%q): %v", serialized, err) + } + if roundTripped.Contains("1.5") || !roundTripped.Contains("1.4") || roundTripped.Contains("2.0") { + t.Errorf("serialized exclusion did not round-trip: %q", serialized) + } +} + +func TestRangeExcludeIgnoresIrrelevantVersion(t *testing.T) { + original, err := Parse("vers:npm/>=1.0.0|<2.0.0") + if err != nil { + t.Fatal(err) + } + + unchanged := original.Exclude("3.0.0") + if len(unchanged.Exclusions) != 0 { + t.Errorf("Exclude added an irrelevant version: %v", unchanged.Exclusions) + } + if got, want := ToVersString(unchanged, "npm"), ToVersString(original, "npm"); got != want { + t.Errorf("Exclude changed serialization for a version outside the range: %q vs %q", got, want) + } + + stillExcluded := original.Exclude("1.5.0").Exclude("1.5.0") + if len(stillExcluded.Exclusions) != 1 { + t.Errorf("Exclude added a duplicate exclusion: %v", stillExcluded.Exclusions) + } +} + +func TestRangeUnionExclusionRetention(t *testing.T) { + base, _ := Parse("vers:npm/>=1.0.0") + excluded := base.Exclude("1.5.0") + later, _ := Parse("vers:npm/>=2.0.0") + + if excluded.Union(later).Contains("1.5.0") { + t.Error("Union should keep an exclusion the other operand does not contain") + } + if later.Union(excluded).Contains("1.5.0") { + t.Error("Union should keep an exclusion the other operand does not contain (reversed)") + } + + serialized := ToVersString(excluded.Union(later), "npm") + roundTripped, err := Parse(serialized) + if err != nil { + t.Fatalf("Parse(%q): %v", serialized, err) + } + if roundTripped.Contains("1.5.0") { + t.Errorf("serialized union lost the exclusion: %q", serialized) + } + + covering, _ := Parse("vers:npm/>=1.4.0") + if !excluded.Union(covering).Contains("1.5.0") { + t.Error("Union should drop an exclusion the other operand contains") + } + if !covering.Union(excluded).Contains("1.5.0") { + t.Error("Union should drop an exclusion the other operand contains (reversed)") + } +} + +func TestRangeUnionInheritsScheme(t *testing.T) { + generic := NewRange([]Interval{GreaterThanInterval("1.0.dev1", true)}) + typed, _ := Parse("vers:pypi/<2.0") + + if got := generic.Union(typed).Scheme; got != "pypi" { + t.Errorf("Union scheme = %q, want pypi", got) + } + if got := typed.Union(generic).Scheme; got != "pypi" { + t.Errorf("Union scheme (reversed) = %q, want pypi", got) + } + if got := generic.Intersect(typed).Scheme; got != "pypi" { + t.Errorf("Intersect scheme = %q, want pypi", got) + } + if got := typed.Intersect(generic).Scheme; got != "pypi" { + t.Errorf("Intersect scheme (reversed) = %q, want pypi", got) + } + + typedEmpty, _ := Parse("vers:pypi/") + if got := typedEmpty.Union(generic).Scheme; got != "pypi" { + t.Errorf("Union with typed empty operand scheme = %q, want pypi", got) + } + if got := generic.Union(typedEmpty).Scheme; got != "pypi" { + t.Errorf("Union with typed empty operand (reversed) scheme = %q, want pypi", got) + } +} + +func TestRangeAlgebraDedupsEquivalentExclusions(t *testing.T) { + left, _ := Parse("vers:semver/>=1.0.0") + right, _ := Parse("vers:semver/>=1.0.0") + left = left.Exclude("1.5") + right = right.Exclude("1.5.0") + + if got := left.Union(right).Exclusions; len(got) != 1 { + t.Errorf("Union exclusions = %v, want one entry", got) + } + if got := left.Intersect(right).Exclusions; len(got) != 1 { + t.Errorf("Intersect exclusions = %v, want one entry", got) + } +} + func TestRangeString(t *testing.T) { tests := []struct { name string diff --git a/schemes.go b/schemes.go index 05f8ffc..82836eb 100644 --- a/schemes.go +++ b/schemes.go @@ -10,6 +10,7 @@ type semverValue struct { } func compareSemver(a, b string) int { + a, b = strings.TrimSpace(a), strings.TrimSpace(b) va, okA := parseSemverValue(a) vb, okB := parseSemverValue(b) if !okA || !okB { @@ -23,11 +24,8 @@ func compareSemver(a, b string) int { return compareSemverPrereleaseStrings(va.pre, vb.pre) } -func compareNPM(a, b string) int { - return compareSemver(strings.TrimSpace(a), strings.TrimSpace(b)) -} - func compareCargo(a, b string) int { + a, b = strings.TrimSpace(a), strings.TrimSpace(b) comparison := compareSemver(a, b) if comparison != 0 { return comparison diff --git a/schemes_test.go b/schemes_test.go index be31ba9..92938f5 100644 --- a/schemes_test.go +++ b/schemes_test.go @@ -294,6 +294,25 @@ func TestSchemeAwareSharedAPIs(t *testing.T) { } } +func TestSemverComparisonTrimsWhitespace(t *testing.T) { + for _, scheme := range []string{"semver", "hex", "cargo", "npm"} { + if got := CompareWithScheme(" 1.0.0 ", "1.0.0", scheme); got != 0 { + t.Errorf("CompareWithScheme(%q, %q, %q) = %d, want 0", " 1.0.0 ", "1.0.0", scheme, got) + } + if got := CompareWithScheme(" 1.0.0-alpha ", "1.0.0", scheme); got >= 0 { + t.Errorf("CompareWithScheme(%q, %q, %q) = %d, want < 0", " 1.0.0-alpha ", "1.0.0", scheme, got) + } + } + + r, err := Parse("vers:semver/>=1.0.0|<2.0.0") + if err != nil { + t.Fatal(err) + } + if !r.Contains(" 1.5.0 ") { + t.Error("semver range should contain a version with surrounding whitespace") + } +} + func TestCheckedRangeAlgebraRejectsMixedSchemes(t *testing.T) { pypi, _ := Parse("vers:pypi/>=1.0") maven, _ := Parse("vers:maven/<2.0") diff --git a/version.go b/version.go index 080f52c..2f37796 100644 --- a/version.go +++ b/version.go @@ -349,10 +349,8 @@ func compareFuncFor(scheme string) func(a, b string) int { switch scheme { case schemeBazel: return compareBazel - case schemeSemVer, schemeHex, schemeElixir, schemeNginx: + case schemeSemVer, schemeHex, schemeElixir, schemeNginx, schemeNPM: return compareSemver - case schemeNPM: - return compareNPM case schemeCargo: return compareCargo case schemeGo, schemeGolang: