Range and serialization parity fixes

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)
 	}

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()
 

range.go

@@ -269,85 +269,95 @@ 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.
 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,31 +369,23 @@ 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.
 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,
 	}
 }
 

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