From 01ca7ed97b836958d6f39484fd8cf23168fa4ee2 Mon Sep 17 00:00:00 2001
From: Charles Lowell <10964656+chlowell@users.noreply.github.com>
Date: Mon, 16 Sep 2024 16:50:58 -0700
Subject: [PATCH 1/2] Replace MethodByName with type assertions

---
 apps/internal/json/json.go | 54 +++++++++++++-------------------------
 1 file changed, 18 insertions(+), 36 deletions(-)

diff --git a/apps/internal/json/json.go b/apps/internal/json/json.go
index 2238521f..2134e57c 100644
--- a/apps/internal/json/json.go
+++ b/apps/internal/json/json.go
@@ -18,10 +18,6 @@ import (
 )
 
 const addField = "AdditionalFields"
-const (
-	marshalJSON   = "MarshalJSON"
-	unmarshalJSON = "UnmarshalJSON"
-)
 
 var (
 	leftBrace  = []byte("{")[0]
@@ -106,48 +102,38 @@ func delimIs(got json.Token, want rune) bool {
 // hasMarshalJSON will determine if the value or a pointer to this value has
 // the MarshalJSON method.
 func hasMarshalJSON(v reflect.Value) bool {
-	if method := v.MethodByName(marshalJSON); method.Kind() != reflect.Invalid {
-		_, ok := v.Interface().(json.Marshaler)
-		return ok
-	}
-
-	if v.Kind() == reflect.Ptr {
-		v = v.Elem()
-	} else {
-		if !v.CanAddr() {
-			return false
+	ok := false
+	if _, ok = v.Interface().(json.Marshaler); !ok {
+		var i any
+		if v.Kind() == reflect.Ptr {
+			i = v.Elem().Interface()
+		} else if v.CanAddr() {
+			i = v.Addr().Interface()
 		}
-		v = v.Addr()
-	}
-
-	if method := v.MethodByName(marshalJSON); method.Kind() != reflect.Invalid {
-		_, ok := v.Interface().(json.Marshaler)
-		return ok
+		_, ok = i.(json.Marshaler)
 	}
-	return false
+	return ok
 }
 
 // callMarshalJSON will call MarshalJSON() method on the value or a pointer to this value.
 // This will panic if the method is not defined.
 func callMarshalJSON(v reflect.Value) ([]byte, error) {
-	if method := v.MethodByName(marshalJSON); method.Kind() != reflect.Invalid {
-		marsh := v.Interface().(json.Marshaler)
+	if marsh, ok := v.Interface().(json.Marshaler); ok {
 		return marsh.MarshalJSON()
 	}
 
 	if v.Kind() == reflect.Ptr {
-		v = v.Elem()
+		if marsh, ok := v.Elem().Interface().(json.Marshaler); ok {
+			return marsh.MarshalJSON()
+		}
 	} else {
 		if v.CanAddr() {
-			v = v.Addr()
+			if marsh, ok := v.Addr().Interface().(json.Marshaler); ok {
+				return marsh.MarshalJSON()
+			}
 		}
 	}
 
-	if method := v.MethodByName(unmarshalJSON); method.Kind() != reflect.Invalid {
-		marsh := v.Interface().(json.Marshaler)
-		return marsh.MarshalJSON()
-	}
-
 	panic(fmt.Sprintf("callMarshalJSON called on type %T that does not have MarshalJSON defined", v.Interface()))
 }
 
@@ -162,12 +148,8 @@ func hasUnmarshalJSON(v reflect.Value) bool {
 		v = v.Addr()
 	}
 
-	if method := v.MethodByName(unmarshalJSON); method.Kind() != reflect.Invalid {
-		_, ok := v.Interface().(json.Unmarshaler)
-		return ok
-	}
-
-	return false
+	_, ok := v.Interface().(json.Unmarshaler)
+	return ok
 }
 
 // hasOmitEmpty indicates if the field has instructed us to not output

From bddb53b2b3f62f4c74fdb472ef7b3811e7ac5366 Mon Sep 17 00:00:00 2001
From: Charles Lowell <10964656+chlowell@users.noreply.github.com>
Date: Tue, 24 Sep 2024 15:21:01 -0700
Subject: [PATCH 2/2] add test coverage of Marshal

---
 apps/internal/json/json_test.go | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/apps/internal/json/json_test.go b/apps/internal/json/json_test.go
index 266be059..be7f8222 100644
--- a/apps/internal/json/json_test.go
+++ b/apps/internal/json/json_test.go
@@ -41,7 +41,7 @@ type StructE struct {
 	AdditionalFields map[string]interface{}
 }
 
-func TestUnmarshal(t *testing.T) {
+func TestUnmarshalRoundTrip(t *testing.T) {
 	now := time.Now()
 	nowJSON, err := now.MarshalJSON()
 	if err != nil {
@@ -147,6 +147,21 @@ func TestUnmarshal(t *testing.T) {
 		}
 		if diff := (&pretty.Config{IncludeUnexported: false}).Compare(test.want, test.got); diff != "" {
 			t.Errorf("TestUnmarshal(%s): -want/+got:\n%s", test.desc, diff)
+			continue
+		}
+		b, err := Marshal(test.got)
+		if err != nil {
+			t.Errorf("TestUnmarshal(%s): Marshal failed: %s", test.desc, err)
+			continue
+		}
+		err = Unmarshal(b, test.got)
+		if err != nil {
+			t.Errorf("TestUnmarshal(%s): Unmarshal round trip failed: %s", test.desc, err)
+			continue
+		}
+		if diff := (&pretty.Config{IncludeUnexported: false}).Compare(test.want, test.got); diff != "" {
+			t.Errorf("TestUnmarshal(%s): Round trip failed. -want/+got:\n%s", test.desc, diff)
+			continue
 		}
 	}
 }