Repository: Masterminds/semver
Pull request #167 (2021-12-21 17:04:17)

add marshalling for constraints
This PR adds marshal and unmarshal for constraints (see #130). I tried to built it similar to the already existing implementations for the version type, but there were some challenges

### Challenges

**json.Marshal escapes certain characters**
Unlike version, constraints can also contain '<' and '>' characters. Unfortunately these get escaped when using `json.Marshal` directly (note the `escapeHTML` option in [source](https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/encoding/json/encode.go;l=161)). As a result we cannot simply call `json.Marshal` inside our custom MarshalJSON. Therefore I have decided to use a custom encoder that disables HTML escaping.

So far so good. Unfortunately our troubles do not end there. As a matter of fact `encoding.json` calls [additional logic](https://cs.opensource.google/go/go/+/master:src/encoding/json/encode.go;l=479;drc=master) after it has called our custom Marshaler. In our case, the troublesome part is the `compact` func. The main issue here is that it runs its [own escaper](https://cs.opensource.google/go/go/+/master:src/encoding/json/indent.go;l=23;drc=master), which we cannot overwrite. As a result if you are marshalling using `json.Marshal`, it will always contain the unicode runes for them (e.g. "\u003c" for "<").

### Implications for end-users

I think for most users this should be fine, as you can use the escaped characters directly within semVer. For example you can use them in the `NewConstraint` func without any issues:

```go
s := "\u003c1.1.1"
cs, err := NewConstraint(s)
if err != nil {
t.Error(err)
}
fmt.Println(cs) // will print <1.1.1
```

And if that is not enough, users can always use a custom Encoder with `SetEscapeHTML` disabled instead of `json.Marshal`. This is similar to how we do it in the unit test:

```go
buf := new(bytes.Buffer)
enc := json.NewEncoder(buf)
enc.SetEscapeHTML(false)
err = enc.Encode(cs)
...
// buf.String() will have nothing escaped
```

### Open Questions

* Should we add some more test-cases to `TestNewConstraint` which contain escaped characters?
* I noticed in the Unmarshal part of Version, that each field is [copied separately](https://github.com/Masterminds/semver/blob/master/version.go#L416). Is there an advantage of this versus directly deepcopying?


Patch:
diff --git a/constraints.go b/constraints.go
index 547613f..7bb62ac 100644
--- a/constraints.go
+++ b/constraints.go
@@ -2,6 +2,7 @@ package semver
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"regexp"
@@ -134,6 +135,33 @@ func (cs Constraints) String() string {
 	return strings.Join(buf, " || ")
 }
 
+// UnmarshalJSON implements JSON.Unmarshaler interface.
+func (cs *Constraints) UnmarshalJSON(b []byte) error {
+	var s string
+	if err := json.Unmarshal(b, &s); err != nil {
+		return err
+	}
+	temp, err := NewConstraint(s)
+	if err != nil {
+		return err
+	}
+	*cs = *temp
+	return nil
+}
+
+// MarshalJSON implements JSON.Marshaler interface.
+func (cs Constraints) MarshalJSON() ([]byte, error) {
+	// we need our own encoder so we don't escape '<' and '>' which json.Marshal does
+	buf := new(bytes.Buffer)
+	enc := json.NewEncoder(buf)
+	enc.SetEscapeHTML(false)
+
+	if err := enc.Encode(cs.String()); err != nil {
+		return nil, err
+	}
+	return bytes.TrimRight(buf.Bytes(), "\n"), nil
+}
+
 var constraintOps map[string]cfunc
 var constraintRegex *regexp.Regexp
 var constraintRangeRegex *regexp.Regexp


Test patch:
diff --git a/constraints_test.go b/constraints_test.go
index 0504399..0820c09 100644
--- a/constraints_test.go
+++ b/constraints_test.go
@@ -1,6 +1,9 @@
 package semver
 
 import (
+	"bytes"
+	"encoding/json"
+	"fmt"
 	"reflect"
 	"testing"
 )
@@ -664,3 +667,56 @@ func TestConstraintString(t *testing.T) {
 		}
 	}
 }
+
+func TestJsonMarshalConstraints(t *testing.T) {
+	tests := []struct {
+		sCs  string
+		want string
+	}{
+		{"1.1.1", "1.1.1"},
+		{">=1.1.1", ">=1.1.1"},
+		{"<=1.1.1", "<=1.1.1"},
+	}
+
+	for _, tc := range tests {
+		cs, err := NewConstraint(tc.sCs)
+		if err != nil {
+			t.Errorf("Error creating constraints: %s", err)
+		}
+		buf := new(bytes.Buffer)
+		enc := json.NewEncoder(buf)
+		enc.SetEscapeHTML(false)
+		err = enc.Encode(cs)
+		if err != nil {
+			t.Errorf("Error unmarshaling version: %s", err)
+		}
+		got := buf.String()
+		want := fmt.Sprintf("%q\n", tc.want)
+		if got != want {
+			t.Errorf("Error marshaling unexpected marshaled content: got=%q want=%q", got, want)
+		}
+	}
+}
+
+func TestJsonUnmarshalConstraints(t *testing.T) {
+	tests := []struct {
+		sCs  string
+		want string
+	}{
+		{"1.1.1", "1.1.1"},
+		{">=1.2.3", ">=1.2.3"},
+		{"<=1.2.3", "<=1.2.3"},
+	}
+
+	for _, tc := range tests {
+		cs := Constraints{}
+		err := json.Unmarshal([]byte(fmt.Sprintf("%q", tc.sCs)), &cs)
+		if err != nil {
+			t.Errorf("Error unmarshaling constraints: %s", err)
+		}
+		got := cs.String()
+		if got != tc.want {
+			t.Errorf("Error unmarshaling unexpected object content: got=%q want=%q", got, tc.want)
+		}
+	}
+}