Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Retain location metadata for values in convert.FromTyped" #1528

Merged
merged 1 commit into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 13 additions & 21 deletions libs/dyn/convert/from_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
// Dereference pointer if necessary
for srcv.Kind() == reflect.Pointer {
if srcv.IsNil() {
return dyn.NilValue.WithLocation(ref.Location()), nil
return dyn.NilValue, nil
}
srcv = srcv.Elem()

Expand All @@ -55,35 +55,27 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
}
}

var v dyn.Value
var err error
switch srcv.Kind() {
case reflect.Struct:
v, err = fromTypedStruct(srcv, ref, options...)
return fromTypedStruct(srcv, ref, options...)
case reflect.Map:
v, err = fromTypedMap(srcv, ref)
return fromTypedMap(srcv, ref)
case reflect.Slice:
v, err = fromTypedSlice(srcv, ref)
return fromTypedSlice(srcv, ref)
case reflect.String:
v, err = fromTypedString(srcv, ref, options...)
return fromTypedString(srcv, ref, options...)
case reflect.Bool:
v, err = fromTypedBool(srcv, ref, options...)
return fromTypedBool(srcv, ref, options...)
case reflect.Int, reflect.Int32, reflect.Int64:
v, err = fromTypedInt(srcv, ref, options...)
return fromTypedInt(srcv, ref, options...)
case reflect.Float32, reflect.Float64:
v, err = fromTypedFloat(srcv, ref, options...)
return fromTypedFloat(srcv, ref, options...)
case reflect.Invalid:
// If the value is untyped and not set (e.g. any type with nil value), we return nil.
v, err = dyn.NilValue, nil
default:
return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
return dyn.NilValue, nil
}

// Ensure the location metadata is retained.
if err != nil {
return dyn.InvalidValue, err
}
return v.WithLocation(ref.Location()), err
return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
}

func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
Expand Down Expand Up @@ -125,7 +117,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
// 2. The reference is a map (i.e. the struct was and still is empty).
// 3. The "includeZeroValues" option is set (i.e. the struct is a non-nil pointer).
if out.Len() > 0 || ref.Kind() == dyn.KindMap || slices.Contains(options, includeZeroValues) {
return dyn.V(out), nil
return dyn.NewValue(out, ref.Location()), nil
}

// Otherwise, return nil.
Expand Down Expand Up @@ -172,7 +164,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
out.Set(refk, nv)
}

return dyn.V(out), nil
return dyn.NewValue(out, ref.Location()), nil
}

func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
Expand Down Expand Up @@ -207,7 +199,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
out[i] = nv
}

return dyn.V(out), nil
return dyn.NewValue(out, ref.Location()), nil
}

func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
Expand Down
109 changes: 25 additions & 84 deletions libs/dyn/convert/from_typed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestFromTypedStructPointerZeroFields(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv)

// For an initialized pointer with a nil reference we expect an empty map.
// For an initialized pointer with a nil reference we expect a nil.
src = &Tmp{}
nv, err = FromTyped(src, dyn.NilValue)
require.NoError(t, err)
Expand Down Expand Up @@ -103,7 +103,7 @@ func TestFromTypedStructSetFields(t *testing.T) {
}), nv)
}

func TestFromTypedStructSetFieldsRetainLocation(t *testing.T) {
func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
type Tmp struct {
Foo string `json:"foo"`
Bar string `json:"bar"`
Expand All @@ -122,9 +122,11 @@ func TestFromTypedStructSetFieldsRetainLocation(t *testing.T) {
nv, err := FromTyped(src, ref)
require.NoError(t, err)

// Assert foo and bar have retained their location.
// Assert foo has retained its location.
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))

// Assert bar lost its location (because it was overwritten).
assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
}

func TestFromTypedStringMapWithZeroValue(t *testing.T) {
Expand Down Expand Up @@ -352,7 +354,7 @@ func TestFromTypedMapNonEmpty(t *testing.T) {
}), nv)
}

func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) {
func TestFromTypedMapNonEmptyRetainLocationIfUnchanged(t *testing.T) {
var src = map[string]string{
"foo": "bar",
"bar": "qux",
Expand All @@ -366,9 +368,11 @@ func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) {
nv, err := FromTyped(src, ref)
require.NoError(t, err)

// Assert foo and bar have retained their locations.
// Assert foo has retained its location.
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))

// Assert bar lost its location (because it was overwritten).
assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
}

func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
Expand Down Expand Up @@ -425,23 +429,25 @@ func TestFromTypedSliceNonEmpty(t *testing.T) {
}), nv)
}

func TestFromTypedSliceNonEmptyRetainLocation(t *testing.T) {
func TestFromTypedSliceNonEmptyRetainLocationIfUnchanged(t *testing.T) {
var src = []string{
"foo",
"bar",
}

ref := dyn.V([]dyn.Value{
dyn.NewValue("foo", dyn.Location{File: "foo"}),
dyn.NewValue("bar", dyn.Location{File: "bar"}),
dyn.NewValue("baz", dyn.Location{File: "baz"}),
})

nv, err := FromTyped(src, ref)
require.NoError(t, err)

// Assert foo and bar have retained their locations.
// Assert foo has retained its location.
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv.Index(0))
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "bar"}), nv.Index(1))

// Assert bar lost its location (because it was overwritten).
assert.Equal(t, dyn.NewValue("bar", dyn.Location{}), nv.Index(1))
}

func TestFromTypedStringEmpty(t *testing.T) {
Expand Down Expand Up @@ -476,20 +482,12 @@ func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V("new"), nv)
}

func TestFromTypedStringRetainsLocations(t *testing.T) {
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})

// case: value has not been changed
func TestFromTypedStringRetainsLocationsIfUnchanged(t *testing.T) {
var src string = "foo"
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv)

// case: value has been changed
src = "bar"
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv)
}

func TestFromTypedStringTypeError(t *testing.T) {
Expand Down Expand Up @@ -531,20 +529,12 @@ func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(true), nv)
}

func TestFromTypedBoolRetainsLocations(t *testing.T) {
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})

// case: value has not been changed
func TestFromTypedBoolRetainsLocationsIfUnchanged(t *testing.T) {
var src bool = true
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(true, dyn.Location{File: "foo"}), nv)

// case: value has been changed
src = false
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(false, dyn.Location{File: "foo"}), nv)
}

func TestFromTypedBoolVariableReference(t *testing.T) {
Expand Down Expand Up @@ -594,20 +584,12 @@ func TestFromTypedIntNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(int64(1234)), nv)
}

func TestFromTypedIntRetainsLocations(t *testing.T) {
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})

// case: value has not been changed
func TestFromTypedIntRetainsLocationsIfUnchanged(t *testing.T) {
var src int = 1234
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1234, dyn.Location{File: "foo"}), nv)

// case: value has been changed
src = 1235
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(int64(1235), dyn.Location{File: "foo"}), nv)
}

func TestFromTypedIntVariableReference(t *testing.T) {
Expand Down Expand Up @@ -657,21 +639,12 @@ func TestFromTypedFloatNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(1.23), nv)
}

func TestFromTypedFloatRetainsLocations(t *testing.T) {
var src float64
func TestFromTypedFloatRetainsLocationsIfUnchanged(t *testing.T) {
var src float64 = 1.23
var ref = dyn.NewValue(1.23, dyn.Location{File: "foo"})

// case: value has not been changed
src = 1.23
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1.23, dyn.Location{File: "foo"}), nv)

// case: value has been changed
src = 1.24
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1.24, dyn.Location{File: "foo"}), nv)
}

func TestFromTypedFloatVariableReference(t *testing.T) {
Expand All @@ -696,35 +669,3 @@ func TestFromTypedAnyNil(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv)
}

func TestFromTypedNilPointerRetainsLocations(t *testing.T) {
type Tmp struct {
Foo string `json:"foo"`
Bar string `json:"bar"`
}

var src *Tmp
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})

nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
}

func TestFromTypedNilMapRetainsLocation(t *testing.T) {
var src map[string]string
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})

nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
}

func TestFromTypedNilSliceRetainsLocation(t *testing.T) {
var src []string
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})

nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
}
Loading