test: added tests for edge cases

* fixed panic case when attempting to set a non-assignable value
* clarified example and limitation regarding embedded structs

Signed-off-by: Frederic BIDON <[email protected]>

Author: fredbi

pointer.go

@@ -45,6 +45,7 @@ type JSONSetable interface {
 //   - a go map[K]V is interpreted as an object, with type K assignable to a string
 //   - a go slice []T is interpreted as an array
 //   - a go struct is interpreted as an object, with exported fields interpreted as keys
+//   - promoted fields from an embedded struct are traversed
 //   - scalars (e.g. int, float64 ...), channels, functions and go arrays cannot be traversed
 //
 // For struct s resolved by reflection, key mappings honor the conventional struct tag `json`.
@@ -54,7 +55,7 @@ type JSONSetable interface {
 // # Limitations
 //
 //   - Unlike go standard marshaling, untagged fields do not default to the go field name and are ignored.
-//   - anonymous embedded fields are not traversed
+//   - anonymous fields are not traversed if untagged
 type Pointer struct {
 	referenceTokens []string
 }
@@ -362,7 +363,7 @@ func getSingleImpl(node any, decodedToken string, nameProvider *jsonname.NamePro
 	case reflect.Slice:
 		tokenIndex, err := strconv.Atoi(decodedToken)
 		if err != nil {
-			return nil, kind, err
+			return nil, kind, errors.Join(err, ErrPointer)
 		}
 		sLength := rValue.Len()
 		if tokenIndex < 0 || tokenIndex >= sLength {
@@ -396,21 +397,34 @@ func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.N
 			return fmt.Errorf("object has no field %q: %w", decodedToken, ErrPointer)
 		}
 		fld := rValue.FieldByName(nm)
-		if fld.IsValid() {
-			fld.Set(reflect.ValueOf(data))
+		if !fld.CanSet() {
+			return fmt.Errorf("can't set struct field %s to %v: %w", nm, data, ErrPointer)
+		}
+
+		value := reflect.ValueOf(data)
+		valueType := value.Type()
+		assignedType := fld.Type()
+
+		if !valueType.AssignableTo(assignedType) {
+			return fmt.Errorf("can't set value with type %T to field %s with type %v: %w", data, nm, assignedType, ErrPointer)
 		}
+
+		fld.Set(value)
+
 		return nil
 
 	case reflect.Map:
 		kv := reflect.ValueOf(decodedToken)
 		rValue.SetMapIndex(kv, reflect.ValueOf(data))
+
 		return nil
 
 	case reflect.Slice:
 		tokenIndex, err := strconv.Atoi(decodedToken)
 		if err != nil {
-			return err
+			return errors.Join(err, ErrPointer)
 		}
+
 		sLength := rValue.Len()
 		if tokenIndex < 0 || tokenIndex >= sLength {
 			return errOutOfBounds(sLength, tokenIndex)
@@ -420,7 +434,17 @@ func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.N
 		if !elem.CanSet() {
 			return fmt.Errorf("can't set slice index %s to %v: %w", decodedToken, data, ErrPointer)
 		}
-		elem.Set(reflect.ValueOf(data))
+
+		value := reflect.ValueOf(data)
+		valueType := value.Type()
+		assignedType := elem.Type()
+
+		if !valueType.AssignableTo(assignedType) {
+			return fmt.Errorf("can't set value with type %T to slice element %d with type %v: %w", data, tokenIndex, assignedType, ErrPointer)
+		}
+
+		elem.Set(value)
+
 		return nil
 
 	default:

pointer_test.go

@@ -6,6 +6,7 @@ package jsonpointer
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"strconv"
 	"testing"
 
@@ -246,6 +247,7 @@ func TestPointableInterface(t *testing.T) {
 
 	t.Run("with pointable type", func(t *testing.T) {
 		p := &pointableImpl{"hello"}
+
 		result, _, err := GetForToken(p, "some")
 		require.NoError(t, err)
 		assert.Equal(t, p.a, result)
@@ -346,6 +348,95 @@ func TestArray(t *testing.T) {
 	}
 }
 
+func TestStruct(t *testing.T) {
+	t.Parallel()
+
+	t.Run("with untagged struct field", func(t *testing.T) {
+		type Embedded struct {
+			D int `json:"d"`
+		}
+
+		s := struct {
+			Embedded
+
+			A         int `json:"a"`
+			B         int
+			Anonymous struct {
+				C int `json:"c"`
+			}
+		}{}
+
+		{
+			s.A = 1
+			s.B = 2
+			s.Anonymous.C = 3
+			s.D = 4
+		}
+
+		t.Run(`should resolve field A tagged "a"`, func(t *testing.T) {
+			pointerA, err := New("/a")
+			require.NoError(t, err)
+
+			value, kind, err := pointerA.Get(s)
+			require.NoError(t, err)
+			require.Equal(t, reflect.Int, kind)
+			require.Equal(t, 1, value)
+
+			_, err = pointerA.Set(&s, 9)
+			require.NoError(t, err)
+
+			value, _, err = pointerA.Get(s)
+			require.NoError(t, err)
+			require.Equal(t, 9, value)
+		})
+
+		t.Run(`should resolve embedded field D with tag`, func(t *testing.T) {
+			pointerD, err := New("/d")
+			require.NoError(t, err)
+
+			value, kind, err := pointerD.Get(s)
+			require.NoError(t, err)
+			require.Equal(t, reflect.Int, kind)
+			require.Equal(t, 4, value)
+
+			_, err = pointerD.Set(&s, 6)
+			require.NoError(t, err)
+
+			value, _, err = pointerD.Get(s)
+			require.NoError(t, err)
+			require.Equal(t, 6, value)
+		})
+
+		t.Run("with known limitations", func(t *testing.T) {
+			t.Run(`should not resolve field B without tag`, func(t *testing.T) {
+				pointerB, err := New("/B")
+				require.NoError(t, err)
+
+				_, _, err = pointerB.Get(s)
+				require.Error(t, err)
+				require.ErrorContains(t, err, `has no field "B"`)
+
+				_, err = pointerB.Set(&s, 8)
+				require.Error(t, err)
+				require.ErrorContains(t, err, `has no field "B"`)
+			})
+
+			t.Run(`should not resolve field C with tag, but anonymous`, func(t *testing.T) {
+				pointerC, err := New("/c")
+				require.NoError(t, err)
+
+				_, _, err = pointerC.Get(s)
+				require.Error(t, err)
+				require.ErrorContains(t, err, `has no field "c"`)
+
+				_, err = pointerC.Set(&s, 7)
+				require.Error(t, err)
+				require.ErrorContains(t, err, `has no field "c"`)
+			})
+		})
+	})
+}
+
 func TestOtherThings(t *testing.T) {
 	t.Parallel()
 
@@ -367,11 +458,21 @@ func TestOtherThings(t *testing.T) {
 	})
 
 	t.Run("out of bound array index should error", func(t *testing.T) {
-		p, err := New("/foo/3")
-		require.NoError(t, err)
+		t.Run("with index overflow", func(t *testing.T) {
+			p, err := New("/foo/3")
+			require.NoError(t, err)
 
-		_, _, err = p.Get(testDocumentJSON(t))
-		require.Error(t, err)
+			_, _, err = p.Get(testDocumentJSON(t))
+			require.Error(t, err)
+		})
+
+		t.Run("with index unerflow", func(t *testing.T) {
+			p, err := New("/foo/-3")
+			require.NoError(t, err)
+
+			_, _, err = p.Get(testDocumentJSON(t))
+			require.Error(t, err)
+		})
 	})
 
 	t.Run("referring to a key in an array should error", func(t *testing.T) {
@@ -907,4 +1008,72 @@ func TestEdgeCases(t *testing.T) {
 
 		require.Equal(t, doc, newDoc)
 	})
+
+	t.Run("with out of bounds index", func(t *testing.T) {
+		p, err := New("/foo/10")
+		require.NoError(t, err)
+
+		t.Run("should error on Get", func(t *testing.T) {
+			_, _, err := p.Get(testStructJSONDoc(t))
+			require.Error(t, err)
+			require.ErrorContains(t, err, "index out of bounds")
+		})
+
+		t.Run("should error on Set", func(t *testing.T) {
+			_, err := p.Set(testStructJSONPtr(t), "peek-a-boo")
+			require.Error(t, err)
+			require.ErrorContains(t, err, "index out of bounds")
+		})
+	})
+
+	t.Run("Set with invalid pointer token", func(t *testing.T) {
+		doc := testStructJSONDoc(t)
+		pointer, err := New("/foo/x")
+		require.NoError(t, err)
+
+		_, err = pointer.Set(&doc, "yay")
+		require.Error(t, err)
+		require.ErrorContains(t, err, `Atoi: parsing "x"`)
+	})
+
+	t.Run("Set with invalid reference in struct", func(t *testing.T) {
+		doc := struct {
+			A func() `json:"a"`
+			B []int  `json:"b"`
+		}{
+			A: func() {},
+			B: []int{0, 1},
+		}
+
+		t.Run("should error when attempting to set a struct field value that is not assignable", func(t *testing.T) {
+			pointerA, err := New("/a")
+			require.NoError(t, err)
+
+			_, err = pointerA.Set(&doc, "waou")
+			require.Error(t, err)
+			require.ErrorContains(t, err, `can't set value with type string to field A`)
+		})
+
+		t.Run("should error when attempting to set a slice element value that is not assignable", func(t *testing.T) {
+			pointerB, err := New("/b/0")
+			require.NoError(t, err)
+
+			_, err = pointerB.Set(&doc, "waou")
+			require.Error(t, err)
+			require.ErrorContains(t, err, `can't set value with type string to slice element 0 with type int`)
+		})
+
+		t.Run("should error when attempting to set a value that does not exist", func(t *testing.T) {
+			pointerB, err := New("/x")
+			require.NoError(t, err)
+
+			_, _, err = pointerB.Get(&doc)
+			require.Error(t, err)
+			require.ErrorContains(t, err, `no field`)
+
+			_, err = pointerB.Set(&doc, "oops")
+			require.Error(t, err)
+			require.ErrorContains(t, err, `no field`)
+		})
+	})
 }

struct_example_test.go

@@ -10,6 +10,8 @@ import (
 var ErrExampleIface = errors.New("example error")
 
 type ExampleDoc struct {
+	PromotedDoc
+
 	Promoted     EmbeddedDoc `json:"promoted"`
 	AnonPromoted EmbeddedDoc `json:"-"`
 	A            string      `json:"propA"`
@@ -23,8 +25,15 @@ type EmbeddedDoc struct {
 	B string `json:"propB"`
 }
 
+type PromotedDoc struct {
+	C string `json:"propC"`
+}
+
 func Example_struct() {
 	doc := ExampleDoc{
+		PromotedDoc: PromotedDoc{
+			C: "c",
+		},
 		Promoted: EmbeddedDoc{
 			B: "promoted",
 		},
@@ -43,15 +52,27 @@ func Example_struct() {
 		}
 		fmt.Printf("a: %v\n", a)
 	}
+
 	{
-		// tagged embedded field is resolved
+		// tagged struct field is resolved
 		pointerB, _ := jsonpointer.New("/promoted/propB")
-		a, _, err := pointerB.Get(doc)
+		b, _, err := pointerB.Get(doc)
+		if err != nil {
+			fmt.Println(err)
+			return
+		}
+		fmt.Printf("b: %v\n", b)
+	}
+
+	{
+		// tagged embedded field is resolved
+		pointerC, _ := jsonpointer.New("/propC")
+		c, _, err := pointerC.Get(doc)
 		if err != nil {
 			fmt.Println(err)
 			return
 		}
-		fmt.Printf("b: %v\n", a)
+		fmt.Printf("c: %v\n", c)
 	}
 
 	{
@@ -69,7 +90,7 @@ func Example_struct() {
 	}
 
 	{
-		// Limitation: anonymous embedded field is not resolved.
+		// Limitation: anonymous field is not resolved.
 		pointerC, _ := jsonpointer.New("/propB")
 		_, _, err := pointerC.Get(doc)
 		fmt.Printf("anonymous: %v\n", err)
@@ -85,6 +106,7 @@ func Example_struct() {
 	// output:
 	// a: a
 	// b: promoted
+	// c: c
 	// ignored: object has no field "ignored": JSON pointer error
 	// unexported: object has no field "unexported": JSON pointer error
 	// anonymous: object has no field "propB": JSON pointer error