remove go-cmp dependency (#501)

.changelog/5faed85326334a93a9b0a96284389287.json

@@ -0,0 +1,8 @@
+{
+    "id": "5faed853-2633-4a93-a9b0-a96284389287",
+    "type": "bugfix",
+    "description": "Remove runtime dependency on go-cmp.",
+    "modules": [
+        "."
+    ]
+}

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/HttpProtocolUnitTestGenerator.java

@@ -445,29 +445,7 @@ protected void writeAssertComplexEqual(
             GoWriter writer, String expect, String actual, String[] ignoreTypes
     ) {
         writer.addUseImports(SmithyGoDependency.SMITHY_TESTING);
-        writer.addUseImports(SmithyGoDependency.GO_CMP);
-        writer.addUseImports(SmithyGoDependency.GO_CMP_OPTIONS);
-        writer.addUseImports(SmithyGoDependency.SMITHY_DOCUMENT);
-        writer.addUseImports(SmithyGoDependency.MATH);
-
-        writer.openBlock("opts := cmp.Options{", "}", () -> {
-            writer.openBlock("cmpopts.IgnoreUnexported(", "),", () -> {
-                for (String ignoreType : ignoreTypes) {
-                    writer.write("$L,", ignoreType);
-                }
-            });
-            writer.write("cmp.FilterValues(func(x, y float64) bool {\n"
-                    + "\treturn math.IsNaN(x) && math.IsNaN(y)\n"
-                    + "},"
-                    + "cmp.Comparer(func(_, _ interface{}) bool { return true })),");
-            writer.write("cmp.FilterValues(func(x, y float32) bool {\n"
-                    + "\treturn math.IsNaN(float64(x)) && math.IsNaN(float64(y))\n"
-                    + "},"
-                    + "cmp.Comparer(func(_, _ interface{}) bool { return true })),");
-            writer.write("cmpopts.IgnoreTypes(smithydocument.NoSerde{}),");
-        });
-
-        writer.openBlock("if err := smithytesting.CompareValues($L, $L, opts...); err != nil {", "}",
+        writer.openBlock("if err := smithytesting.CompareValues($L, $L); err != nil {", "}",
                 expect, actual, () -> {
                     writer.write("t.Errorf(\"expect $L value match:\\n%v\", err)", expect);
                 });

go.mod

@@ -1,5 +1,3 @@
 module github.com/aws/smithy-go
 
 go 1.20
-
-require github.com/google/go-cmp v0.5.8

go.sum

@@ -1,2 +0,0 @@
-github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
-github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=

modman.toml

@@ -1,5 +1,4 @@
 [dependencies]
-  "github.com/google/go-cmp" = "v0.5.8"
   "github.com/jmespath/go-jmespath" = "v0.4.0"
 
 [modules]

testing/document.go

@@ -4,12 +4,11 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"sort"
 	"strings"
 
 	"github.com/aws/smithy-go/testing/xml"
-
-	"github.com/google/go-cmp/cmp"
 )
 
 // JSONEqual compares two JSON documents and identifies if the documents contain
@@ -25,8 +24,8 @@ func JSONEqual(expectBytes, actualBytes []byte) error {
 		return fmt.Errorf("failed to unmarshal actual bytes, %v", err)
 	}
 
-	if diff := cmp.Diff(expect, actual); len(diff) != 0 {
-		return fmt.Errorf("JSON mismatch (-expect +actual):\n%s", diff)
+	if !reflect.DeepEqual(expect, actual) {
+		return fmt.Errorf("JSON mismatch: %v != %v", expect, actual)
 	}
 
 	return nil
@@ -60,8 +59,8 @@ func XMLEqual(expectBytes, actualBytes []byte) error {
 		return err
 	}
 
-	if diff := cmp.Diff(actualString, expectedString); len(diff) != 0 {
-		return fmt.Errorf("XML mismatch (-expect +actual):\n%s", diff)
+	if expectedString != actualString {
+		return fmt.Errorf("XML mismatch: %v != %v", expectedString, actualString)
 	}
 
 	return nil
@@ -85,8 +84,10 @@ func AssertXMLEqual(t T, expect, actual []byte) bool {
 // contain the same values. Returns an error if the two documents are not
 // equal.
 func URLFormEqual(expectBytes, actualBytes []byte) error {
-	if diff := cmp.Diff(parseFormBody(expectBytes), parseFormBody(actualBytes)); len(diff) != 0 {
-		return fmt.Errorf("Query mismatch (-expect +actual):\n%s", diff)
+	expect := parseFormBody(expectBytes)
+	actual := parseFormBody(actualBytes)
+	if !reflect.DeepEqual(expect, actual) {
+		return fmt.Errorf("Query mismatch: %v != %v", expect, actual)
 	}
 	return nil
 }

testing/struct.go

@@ -4,41 +4,148 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
-	"github.com/aws/smithy-go/document"
 	"io"
-	"io/ioutil"
-	"net/http"
+	"math"
+	"reflect"
 
-	"github.com/google/go-cmp/cmp"
+	"github.com/aws/smithy-go/document"
+	"github.com/aws/smithy-go/middleware"
 )
 
-// CompareValues compares two values to determine if they are equal.
-func CompareValues(expect, actual interface{}, opts ...cmp.Option) error {
-	opts = append(make([]cmp.Option, 0, len(opts)+1), opts...)
-
-	var skippedReaders filterSkipDifferentIoReader
-
-	opts = append(opts,
-		cmp.Transformer("http.NoBody", transformHTTPNoBodyToNil),
-		cmp.FilterValues(skippedReaders.filter, cmp.Ignore()),
-		cmp.Comparer(compareDocumentTypes),
-	)
+// CompareValues compares two values to determine if they are equal,
+// specialized for comparison of SDK operation output types.
+//
+// CompareValues expects the two values to be of the same underlying type.
+// Doing otherwise will result in undefined behavior.
+//
+// The third variadic argument is vestigial from a previous implementation that
+// depended on go-cmp. Values passed therein have no effect.
+func CompareValues(expect, actual interface{}, _ ...interface{}) error {
+	return deepEqual(reflect.ValueOf(expect), reflect.ValueOf(actual), "<root>")
+}
 
-	if diff := cmp.Diff(expect, actual, opts...); len(diff) != 0 {
-		return fmt.Errorf("values do not match\n%s", diff)
+func deepEqual(expect, actual reflect.Value, path string) error {
+	if et, at := expect.Kind(), actual.Kind(); et != at {
+		return fmt.Errorf("%s: kind %s != %s", path, et, at)
 	}
 
-	var errs []error
-	for _, s := range skippedReaders {
-		if err := CompareReaders(s.A, s.B); err != nil {
-			errs = append(errs, err)
+	// there are a handful of short-circuit cases here within the context of
+	// operation responses:
+	//   - ResultMetadata     (we don't care)
+	//   - document.Interface (check for marshaled []byte equality)
+	//   - io.Reader          (check for Read() []byte equality)
+	ei, ai := expect.Interface(), actual.Interface()
+	if _, _, ok := asMetadatas(ei, ai); ok {
+		return nil
+	}
+	if e, a, ok := asDocuments(ei, ai); ok {
+		if !compareDocumentTypes(e, a) {
+			return fmt.Errorf("%s: document values unequal", path)
 		}
+		return nil
 	}
-	if len(errs) != 0 {
-		return fmt.Errorf("io.Readers have different values\n%v", errs)
+	if e, a, ok := asReaders(ei, ai); ok {
+		if err := CompareReaders(e, a); err != nil {
+			return fmt.Errorf("%s: %w", path, err)
+		}
+		return nil
 	}
 
-	return nil
+	switch expect.Kind() {
+	case reflect.Pointer:
+		if expect.Type() != actual.Type() {
+			return fmt.Errorf("%s: type mismatch", path)
+		}
+
+		expect = deref(expect)
+		actual = deref(actual)
+		ek, ak := expect.Kind(), actual.Kind()
+		if ek == reflect.Invalid || ak == reflect.Invalid {
+			// one was a nil pointer, so they both must be nil
+			if ek == ak {
+				return nil
+			}
+			return fmt.Errorf("%s: %s != %s", path, fmtNil(ek), fmtNil(ak))
+		}
+		if err := deepEqual(expect, actual, path); err != nil {
+			return err
+		}
+		return nil
+	case reflect.Slice:
+		if expect.Len() != actual.Len() {
+			return fmt.Errorf("%s: slice length unequal", path)
+		}
+		for i := 0; i < expect.Len(); i++ {
+			ipath := fmt.Sprintf("%s[%d]", path, i)
+			if err := deepEqual(expect.Index(i), actual.Index(i), ipath); err != nil {
+				return err
+			}
+		}
+		return nil
+	case reflect.Map:
+		if expect.Len() != actual.Len() {
+			return fmt.Errorf("%s: map length unequal", path)
+		}
+		for _, k := range expect.MapKeys() {
+			kpath := fmt.Sprintf("%s[%q]", path, k.String())
+			if err := deepEqual(expect.MapIndex(k), actual.MapIndex(k), kpath); err != nil {
+				return err
+			}
+		}
+		return nil
+	case reflect.Struct:
+		for i := 0; i < expect.NumField(); i++ {
+			if !expect.Field(i).CanInterface() {
+				continue // unexported
+			}
+			fpath := fmt.Sprintf("%s.%s", path, expect.Type().Field(i).Name)
+			if err := deepEqual(expect.Field(i), actual.Field(i), fpath); err != nil {
+				return err
+			}
+		}
+		return nil
+	case reflect.Float32, reflect.Float64:
+		// NaN != NaN by definition but we just care about bitwise equality
+		ef, af := math.Float64bits(expect.Float()), math.Float64bits(actual.Float())
+		if ef != af {
+			return fmt.Errorf("%s: float 0x%x != 0x%x", path, ef, af)
+		}
+		return nil
+	default:
+		// everything else is just scalars and can be delegated
+		if !reflect.DeepEqual(ei, ai) {
+			return fmt.Errorf("%s: %v != %v", path, ei, ai)
+		}
+		return nil
+	}
+}
+
+func asMetadatas(i, j interface{}) (ii, jj middleware.Metadata, ok bool) {
+	ii, iok := i.(middleware.Metadata)
+	jj, jok := j.(middleware.Metadata)
+	return ii, jj, iok || jok
+}
+
+func asDocuments(i, j interface{}) (ii, jj documentInterface, ok bool) {
+	ii, iok := i.(documentInterface)
+	jj, jok := j.(documentInterface)
+	return ii, jj, iok || jok
+}
+
+func asReaders(i, j interface{}) (ii, jj io.Reader, ok bool) {
+	ii, iok := i.(io.Reader)
+	jj, jok := j.(io.Reader)
+	return ii, jj, iok || jok
+}
+
+func deref(v reflect.Value) reflect.Value {
+	switch v.Kind() {
+	case reflect.Interface, reflect.Ptr:
+		for v.Kind() == reflect.Interface || v.Kind() == reflect.Ptr {
+			v = v.Elem()
+		}
+	}
+	return v
 }
 
 type documentInterface interface {
@@ -47,6 +154,13 @@ type documentInterface interface {
 }
 
 func compareDocumentTypes(x documentInterface, y documentInterface) bool {
+	if x == nil {
+		x = nopMarshaler{}
+	}
+	if y == nil {
+		y = nopMarshaler{}
+	}
+
 	xBytes, err := x.MarshalSmithyDocument()
 	if err != nil {
 		panic(fmt.Sprintf("MarshalSmithyDocument error: %v", err))
@@ -58,49 +172,22 @@ func compareDocumentTypes(x documentInterface, y documentInterface) bool {
 	return JSONEqual(xBytes, yBytes) == nil
 }
 
-func transformHTTPNoBodyToNil(v io.Reader) io.Reader {
-	if v == http.NoBody {
-		return nil
-	}
-	return v
-}
-
-type filterSkipDifferentIoReader []skippedReaders
-
-func (f *filterSkipDifferentIoReader) filter(a, b io.Reader) bool {
-	if a == nil || b == nil {
-		return false
-	}
-	//at, bt := reflect.TypeOf(a), reflect.TypeOf(b)
-	//for at.Kind() == reflect.Ptr {
-	//	at = at.Elem()
-	//}
-	//for bt.Kind() == reflect.Ptr {
-	//	bt = bt.Elem()
-	//}
-
-	//// The underlying reader types are the same they can be compared directly.
-	//if at == bt {
-	//	return false
-	//}
-
-	*f = append(*f, skippedReaders{A: a, B: b})
-	return true
-}
-
-type skippedReaders struct {
-	A, B io.Reader
-}
-
 // CompareReaders two io.Reader values together to determine if they are equal.
 // Will read the contents of the readers until they are empty.
 func CompareReaders(expect, actual io.Reader) error {
-	e, err := ioutil.ReadAll(expect)
+	if expect == nil {
+		expect = nopReader{}
+	}
+	if actual == nil {
+		actual = nopReader{}
+	}
+
+	e, err := io.ReadAll(expect)
 	if err != nil {
 		return fmt.Errorf("failed to read expect body, %w", err)
 	}
 
-	a, err := ioutil.ReadAll(actual)
+	a, err := io.ReadAll(actual)
 	if err != nil {
 		return fmt.Errorf("failed to read actual body, %w", err)
 	}
@@ -112,3 +199,19 @@ func CompareReaders(expect, actual io.Reader) error {
 
 	return nil
 }
+
+func fmtNil(k reflect.Kind) string {
+	if k == reflect.Invalid {
+		return "nil"
+	}
+	return "non-nil"
+}
+
+type nopReader struct{}
+
+func (nopReader) Read(p []byte) (int, error) { return 0, io.EOF }
+
+type nopMarshaler struct{}
+
+func (nopMarshaler) MarshalSmithyDocument() ([]byte, error)      { return nil, nil }
+func (nopMarshaler) UnmarshalSmithyDocument(v interface{}) error { return nil }