Skip to content

Commit 482d83c

Browse files
pieternpietern
authored
Revert "Retain location metadata for values in convert.FromTyped" (#1528)
## Changes This reverts commit dac5f09 (#1523). Retaining the location for nil values means equality checks no longer pass. We need #1520 to be merged first. ## Tests Integration test `TestAccPythonWheelTaskDeployAndRunWithWrapper`.
1 parent dac5f09 commit 482d83c

File tree

2 files changed

+38
-105
lines changed

2 files changed

+38
-105
lines changed

‎libs/dyn/convert/from_typed.go‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
4242
// Dereference pointer if necessary
4343
for srcv.Kind() == reflect.Pointer {
4444
if srcv.IsNil() {
45-
return dyn.NilValue.WithLocation(ref.Location()), nil
45+
return dyn.NilValue, nil
4646
}
4747
srcv = srcv.Elem()
4848

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

58-
var v dyn.Value
59-
var err error
6058
switch srcv.Kind() {
6159
case reflect.Struct:
62-
v, err = fromTypedStruct(srcv, ref, options...)
60+
return fromTypedStruct(srcv, ref, options...)
6361
case reflect.Map:
64-
v, err = fromTypedMap(srcv, ref)
62+
return fromTypedMap(srcv, ref)
6563
case reflect.Slice:
66-
v, err = fromTypedSlice(srcv, ref)
64+
return fromTypedSlice(srcv, ref)
6765
case reflect.String:
68-
v, err = fromTypedString(srcv, ref, options...)
66+
return fromTypedString(srcv, ref, options...)
6967
case reflect.Bool:
70-
v, err = fromTypedBool(srcv, ref, options...)
68+
return fromTypedBool(srcv, ref, options...)
7169
case reflect.Int, reflect.Int32, reflect.Int64:
72-
v, err = fromTypedInt(srcv, ref, options...)
70+
return fromTypedInt(srcv, ref, options...)
7371
case reflect.Float32, reflect.Float64:
74-
v, err = fromTypedFloat(srcv, ref, options...)
72+
return fromTypedFloat(srcv, ref, options...)
7573
case reflect.Invalid:
7674
// If the value is untyped and not set (e.g. any type with nil value), we return nil.
77-
v, err = dyn.NilValue, nil
78-
default:
79-
return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
75+
return dyn.NilValue, nil
8076
}
8177

82-
// Ensure the location metadata is retained.
83-
if err != nil {
84-
return dyn.InvalidValue, err
85-
}
86-
return v.WithLocation(ref.Location()), err
78+
return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
8779
}
8880

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

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

175-
return dyn.V(out), nil
167+
return dyn.NewValue(out, ref.Location()), nil
176168
}
177169

178170
func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
@@ -207,7 +199,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
207199
out[i] = nv
208200
}
209201

210-
return dyn.V(out), nil
202+
return dyn.NewValue(out, ref.Location()), nil
211203
}
212204

213205
func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {

‎libs/dyn/convert/from_typed_test.go‎

Lines changed: 25 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func TestFromTypedStructPointerZeroFields(t *testing.T) {
4949
require.NoError(t, err)
5050
assert.Equal(t, dyn.NilValue, nv)
5151

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

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

125-
// Assert foo and bar have retained their location.
125+
// Assert foo has retained its location.
126126
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
127-
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))
127+
128+
// Assert bar lost its location (because it was overwritten).
129+
assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
128130
}
129131

130132
func TestFromTypedStringMapWithZeroValue(t *testing.T) {
@@ -352,7 +354,7 @@ func TestFromTypedMapNonEmpty(t *testing.T) {
352354
}), nv)
353355
}
354356

355-
func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) {
357+
func TestFromTypedMapNonEmptyRetainLocationIfUnchanged(t *testing.T) {
356358
var src = map[string]string{
357359
"foo": "bar",
358360
"bar": "qux",
@@ -366,9 +368,11 @@ func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) {
366368
nv, err := FromTyped(src, ref)
367369
require.NoError(t, err)
368370

369-
// Assert foo and bar have retained their locations.
371+
// Assert foo has retained its location.
370372
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
371-
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))
373+
374+
// Assert bar lost its location (because it was overwritten).
375+
assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
372376
}
373377

374378
func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
@@ -425,23 +429,25 @@ func TestFromTypedSliceNonEmpty(t *testing.T) {
425429
}), nv)
426430
}
427431

428-
func TestFromTypedSliceNonEmptyRetainLocation(t *testing.T) {
432+
func TestFromTypedSliceNonEmptyRetainLocationIfUnchanged(t *testing.T) {
429433
var src = []string{
430434
"foo",
431435
"bar",
432436
}
433437

434438
ref := dyn.V([]dyn.Value{
435439
dyn.NewValue("foo", dyn.Location{File: "foo"}),
436-
dyn.NewValue("bar", dyn.Location{File: "bar"}),
440+
dyn.NewValue("baz", dyn.Location{File: "baz"}),
437441
})
438442

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

442-
// Assert foo and bar have retained their locations.
446+
// Assert foo has retained its location.
443447
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv.Index(0))
444-
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "bar"}), nv.Index(1))
448+
449+
// Assert bar lost its location (because it was overwritten).
450+
assert.Equal(t, dyn.NewValue("bar", dyn.Location{}), nv.Index(1))
445451
}
446452

447453
func TestFromTypedStringEmpty(t *testing.T) {
@@ -476,20 +482,12 @@ func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
476482
assert.Equal(t, dyn.V("new"), nv)
477483
}
478484

479-
func TestFromTypedStringRetainsLocations(t *testing.T) {
480-
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})
481-
482-
// case: value has not been changed
485+
func TestFromTypedStringRetainsLocationsIfUnchanged(t *testing.T) {
483486
var src string = "foo"
487+
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})
484488
nv, err := FromTyped(src, ref)
485489
require.NoError(t, err)
486490
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv)
487-
488-
// case: value has been changed
489-
src = "bar"
490-
nv, err = FromTyped(src, ref)
491-
require.NoError(t, err)
492-
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv)
493491
}
494492

495493
func TestFromTypedStringTypeError(t *testing.T) {
@@ -531,20 +529,12 @@ func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
531529
assert.Equal(t, dyn.V(true), nv)
532530
}
533531

534-
func TestFromTypedBoolRetainsLocations(t *testing.T) {
535-
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})
536-
537-
// case: value has not been changed
532+
func TestFromTypedBoolRetainsLocationsIfUnchanged(t *testing.T) {
538533
var src bool = true
534+
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})
539535
nv, err := FromTyped(src, ref)
540536
require.NoError(t, err)
541537
assert.Equal(t, dyn.NewValue(true, dyn.Location{File: "foo"}), nv)
542-
543-
// case: value has been changed
544-
src = false
545-
nv, err = FromTyped(src, ref)
546-
require.NoError(t, err)
547-
assert.Equal(t, dyn.NewValue(false, dyn.Location{File: "foo"}), nv)
548538
}
549539

550540
func TestFromTypedBoolVariableReference(t *testing.T) {
@@ -594,20 +584,12 @@ func TestFromTypedIntNonEmptyOverwrite(t *testing.T) {
594584
assert.Equal(t, dyn.V(int64(1234)), nv)
595585
}
596586

597-
func TestFromTypedIntRetainsLocations(t *testing.T) {
598-
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})
599-
600-
// case: value has not been changed
587+
func TestFromTypedIntRetainsLocationsIfUnchanged(t *testing.T) {
601588
var src int = 1234
589+
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})
602590
nv, err := FromTyped(src, ref)
603591
require.NoError(t, err)
604592
assert.Equal(t, dyn.NewValue(1234, dyn.Location{File: "foo"}), nv)
605-
606-
// case: value has been changed
607-
src = 1235
608-
nv, err = FromTyped(src, ref)
609-
require.NoError(t, err)
610-
assert.Equal(t, dyn.NewValue(int64(1235), dyn.Location{File: "foo"}), nv)
611593
}
612594

613595
func TestFromTypedIntVariableReference(t *testing.T) {
@@ -657,21 +639,12 @@ func TestFromTypedFloatNonEmptyOverwrite(t *testing.T) {
657639
assert.Equal(t, dyn.V(1.23), nv)
658640
}
659641

660-
func TestFromTypedFloatRetainsLocations(t *testing.T) {
661-
var src float64
642+
func TestFromTypedFloatRetainsLocationsIfUnchanged(t *testing.T) {
643+
var src float64 = 1.23
662644
var ref = dyn.NewValue(1.23, dyn.Location{File: "foo"})
663-
664-
// case: value has not been changed
665-
src = 1.23
666645
nv, err := FromTyped(src, ref)
667646
require.NoError(t, err)
668647
assert.Equal(t, dyn.NewValue(1.23, dyn.Location{File: "foo"}), nv)
669-
670-
// case: value has been changed
671-
src = 1.24
672-
nv, err = FromTyped(src, ref)
673-
require.NoError(t, err)
674-
assert.Equal(t, dyn.NewValue(1.24, dyn.Location{File: "foo"}), nv)
675648
}
676649

677650
func TestFromTypedFloatVariableReference(t *testing.T) {
@@ -696,35 +669,3 @@ func TestFromTypedAnyNil(t *testing.T) {
696669
require.NoError(t, err)
697670
assert.Equal(t, dyn.NilValue, nv)
698671
}
699-
700-
func TestFromTypedNilPointerRetainsLocations(t *testing.T) {
701-
type Tmp struct {
702-
Foo string `json:"foo"`
703-
Bar string `json:"bar"`
704-
}
705-
706-
var src *Tmp
707-
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
708-
709-
nv, err := FromTyped(src, ref)
710-
require.NoError(t, err)
711-
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
712-
}
713-
714-
func TestFromTypedNilMapRetainsLocation(t *testing.T) {
715-
var src map[string]string
716-
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
717-
718-
nv, err := FromTyped(src, ref)
719-
require.NoError(t, err)
720-
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
721-
}
722-
723-
func TestFromTypedNilSliceRetainsLocation(t *testing.T) {
724-
var src []string
725-
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
726-
727-
nv, err := FromTyped(src, ref)
728-
require.NoError(t, err)
729-
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
730-
}

0 commit comments

Comments
 (0)