Skip to content

Retain location metadata for values in convert.FromTyped #1523

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.

Sign up for GitHub

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

Merged
shreyas-goenka merged 5 commits into from
Jun 25, 2024
Merged

Retain location metadata for values in convert.FromTyped #1523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4436bd
Retain location metadata for values in `dyn.FromTyped`
shreyas-goenka Jun 24, 2024
63717dc
return errors
shreyas-goenka Jun 24, 2024
7880850
fix failing tests
shreyas-goenka Jun 25, 2024
10527a9
add tests that nil values retain location
shreyas-goenka Jun 25, 2024
8ca3353
add tests that locations are retained for changed primitive types
shreyas-goenka Jun 25, 2024
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: 21 additions & 13 deletions libs/dyn/convert/from_typed.go
View file
Open in desktop
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, nil
return dyn.NilValue.WithLocation(ref.Location()), nil
}
srcv = srcv.Elem()

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

var v dyn.Value
var err error
switch srcv.Kind() {
case reflect.Struct:
return fromTypedStruct(srcv, ref, options...)
v, err = fromTypedStruct(srcv, ref, options...)
case reflect.Map:
return fromTypedMap(srcv, ref)
v, err = fromTypedMap(srcv, ref)
case reflect.Slice:
return fromTypedSlice(srcv, ref)
v, err = fromTypedSlice(srcv, ref)
case reflect.String:
return fromTypedString(srcv, ref, options...)
v, err = fromTypedString(srcv, ref, options...)
case reflect.Bool:
return fromTypedBool(srcv, ref, options...)
v, err = fromTypedBool(srcv, ref, options...)
case reflect.Int, reflect.Int32, reflect.Int64:
return fromTypedInt(srcv, ref, options...)
v, err = fromTypedInt(srcv, ref, options...)
case reflect.Float32, reflect.Float64:
return fromTypedFloat(srcv, ref, options...)
v, err = 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.
return dyn.NilValue, nil
v, err = dyn.NilValue, nil
default:
return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
}

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

func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
Expand Down Expand Up @@ -117,7 +125,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.NewValue(out, ref.Location()), nil
return dyn.V(out), nil
}

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

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

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

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

func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
Expand Down
109 changes: 84 additions & 25 deletions libs/dyn/convert/from_typed_test.go
View file
Open in desktop
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 a nil.
// For an initialized pointer with a nil reference we expect an empty map.
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 TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
func TestFromTypedStructSetFieldsRetainLocation(t *testing.T) {
type Tmp struct {
Foo string `json:"foo"`
Bar string `json:"bar"`
Expand All @@ -122,11 +122,9 @@ func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
nv, err := FromTyped(src, ref)
require.NoError(t, err)

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

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

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

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

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

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

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

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

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

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

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

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

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

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

// case: value has not been changed
var src string = "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 @@ -529,12 +531,20 @@ func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(true), nv)
}

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

// case: value has not been changed
var src bool = true
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 @@ -584,12 +594,20 @@ func TestFromTypedIntNonEmptyOverwrite(t *testing.T) {
assert.Equal(t, dyn.V(int64(1234)), nv)
}

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

// case: value has not been changed
var src int = 1234
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)
Copy link
Copy Markdown
Contributor Author

@shreyas-goenka shreyas-goenka Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FromTyped normalizes all integer values to int64

}

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

func TestFromTypedFloatRetainsLocationsIfUnchanged(t *testing.T) {
var src float64 = 1.23
func TestFromTypedFloatRetainsLocations(t *testing.T) {
var src float64
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 @@ -669,3 +696,35 @@ 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)
}