Revert "Fix dynamic representation of zero values in maps and slices (#1154)"

This reverts commit 6beda4405e.

libs/dyn/convert/end_to_end_test.go

@@ -39,15 +39,6 @@ func TestAdditional(t *testing.T) {
 		})
 	})
 
-	t.Run("map with empty string value", func(t *testing.T) {
-		s := ""
-		assertFromTypedToTypedEqual(t, Tmp{
-			MapToPointer: map[string]*string{
-				"key": &s,
-			},
-		})
-	})
-
 	t.Run("map with nil value", func(t *testing.T) {
 		assertFromTypedToTypedEqual(t, Tmp{
 			MapToPointer: map[string]*string{

libs/dyn/convert/from_typed.go

@@ -3,35 +3,13 @@ package convert
 import (
 	"fmt"
 	"reflect"
-	"slices"
 
 	"github.com/databricks/cli/libs/dyn"
 )
 
-type fromTypedOptions int
-
-const (
-	// Use the zero value instead of setting zero values to nil. This is useful
-	// for types where the zero values and nil are semantically different. That is
-	// strings, bools, ints, floats.
-	//
-	// Note: this is not needed for structs because dyn.NilValue is converted back
-	// to a zero value when using the convert.ToTyped function.
-	//
-	// Values in maps and slices should be set to zero values, and not nil in the
-	// dynamic representation.
-	includeZeroValues fromTypedOptions = 1 << iota
-)
-
 // FromTyped converts changes made in the typed structure w.r.t. the configuration value
 // back to the configuration value, retaining existing location information where possible.
 func FromTyped(src any, ref dyn.Value) (dyn.Value, error) {
-	return fromTyped(src, ref)
-}
-
-// Private implementation of FromTyped that allows for additional options not exposed
-// in the public API.
-func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
 	srcv := reflect.ValueOf(src)
 
 	// Dereference pointer if necessary
@@ -50,13 +28,13 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
 	case reflect.Slice:
 		return fromTypedSlice(srcv, ref)
 	case reflect.String:
-		return fromTypedString(srcv, ref, options...)
+		return fromTypedString(srcv, ref)
 	case reflect.Bool:
-		return fromTypedBool(srcv, ref, options...)
+		return fromTypedBool(srcv, ref)
 	case reflect.Int, reflect.Int32, reflect.Int64:
-		return fromTypedInt(srcv, ref, options...)
+		return fromTypedInt(srcv, ref)
 	case reflect.Float32, reflect.Float64:
-		return fromTypedFloat(srcv, ref, options...)
+		return fromTypedFloat(srcv, ref)
 	}
 
 	return dyn.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
@@ -74,7 +52,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	info := getStructInfo(src.Type())
 	for k, v := range info.FieldValues(src) {
 		// Convert the field taking into account the reference value (may be equal to config.NilValue).
-		nv, err := fromTyped(v.Interface(), ref.Get(k))
+		nv, err := FromTyped(v.Interface(), ref.Get(k))
 		if err != nil {
 			return dyn.Value{}, err
 		}
@@ -111,8 +89,8 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 		k := iter.Key().String()
 		v := iter.Value()
 
-		// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
+		// Convert entry taking into account the reference value (may be equal to config.NilValue).
-		nv, err := fromTyped(v.Interface(), ref.Get(k), includeZeroValues)
+		nv, err := FromTyped(v.Interface(), ref.Get(k))
 		if err != nil {
 			return dyn.Value{}, err
 		}
@@ -142,8 +120,8 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	for i := 0; i < src.Len(); i++ {
 		v := src.Index(i)
 
-		// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
+		// Convert entry taking into account the reference value (may be equal to config.NilValue).
-		nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValues)
+		nv, err := FromTyped(v.Interface(), ref.Index(i))
 		if err != nil {
 			return dyn.Value{}, err
 		}
@@ -154,7 +132,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	return dyn.NewValue(out, ref.Location()), nil
 }
 
-func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
+func fromTypedString(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindString:
 		value := src.String()
@@ -164,9 +142,9 @@ func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
 
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference. We set it to nil if it's zero
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// valued in the typed representation and the includeZeroValues option is not set.
+		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
+		if src.IsZero() {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.String()), nil
@@ -175,7 +153,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
 	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
-func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
+func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindBool:
 		value := src.Bool()
@@ -184,9 +162,9 @@ func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions
 		}
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference. We set it to nil if it's zero
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// valued in the typed representation and the includeZeroValues option is not set.
+		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
+		if src.IsZero() {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Bool()), nil
@@ -195,7 +173,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions
 	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
-func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
+func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindInt:
 		value := src.Int()
@@ -204,9 +182,9 @@ func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions)
 		}
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference. We set it to nil if it's zero
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// valued in the typed representation and the includeZeroValues option is not set.
+		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
+		if src.IsZero() {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Int()), nil
@@ -215,7 +193,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions)
 	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
-func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
+func fromTypedFloat(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindFloat:
 		value := src.Float()
@@ -224,9 +202,9 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOption
 		}
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference. We set it to nil if it's zero
+		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// valued in the typed representation and the includeZeroValues option is not set.
+		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
+		if src.IsZero() {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Float()), nil

libs/dyn/convert/from_typed_test.go

@@ -68,190 +68,6 @@ func TestFromTypedStructSetFieldsRetainLocationIfUnchanged(t *testing.T) {
 	assert.Equal(t, dyn.NewValue("qux", dyn.Location{}), nv.Get("bar"))
 }
 
-func TestFromTypedStringMapWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := map[string]string{
-		"foo": "",
-		"bar": "fuzz",
-	}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(""),
-		"bar": dyn.V("fuzz"),
-	}), nv)
-}
-
-func TestFromTypedStringSliceWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := []string{"a", "", "c"}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V([]dyn.Value{
-		dyn.V("a"), dyn.V(""), dyn.V("c"),
-	}), nv)
-}
-
-func TestFromTypedStringStructWithZeroValue(t *testing.T) {
-	type Tmp struct {
-		Foo string `json:"foo"`
-		Bar string `json:"bar"`
-	}
-
-	ref := dyn.NilValue
-	src := Tmp{
-		Foo: "foo",
-		Bar: "",
-	}
-
-	// Note, the zero value is not included in the output.
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V("foo"),
-	}), nv)
-}
-
-func TestFromTypedBoolMapWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := map[string]bool{
-		"foo": false,
-		"bar": true,
-	}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(false),
-		"bar": dyn.V(true),
-	}), nv)
-}
-
-func TestFromTypedBoolSliceWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := []bool{true, false, true}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V([]dyn.Value{
-		dyn.V(true), dyn.V(false), dyn.V(true),
-	}), nv)
-}
-
-func TestFromTypedBoolStructWithZeroValue(t *testing.T) {
-	type Tmp struct {
-		Foo bool `json:"foo"`
-		Bar bool `json:"bar"`
-	}
-
-	ref := dyn.NilValue
-	src := Tmp{
-		Foo: true,
-		Bar: false,
-	}
-
-	// Note, the zero value is not included in the output.
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(true),
-	}), nv)
-}
-
-func TestFromTypedIntMapWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := map[string]int{
-		"foo": 0,
-		"bar": 1,
-	}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(int64(0)),
-		"bar": dyn.V(int64(1)),
-	}), nv)
-}
-
-func TestFromTypedIntSliceWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := []int{1, 0, 2}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V([]dyn.Value{
-		dyn.V(int64(1)), dyn.V(int64(0)), dyn.V(int64(2)),
-	}), nv)
-}
-
-func TestFromTypedIntStructWithZeroValue(t *testing.T) {
-	type Tmp struct {
-		Foo int `json:"foo"`
-		Bar int `json:"bar"`
-	}
-
-	ref := dyn.NilValue
-	src := Tmp{
-		Foo: 1,
-		Bar: 0,
-	}
-
-	// Note, the zero value is not included in the output.
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(int64(1)),
-	}), nv)
-}
-
-func TestFromTypedFloatMapWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := map[string]float64{
-		"foo": 0.0,
-		"bar": 1.0,
-	}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(0.0),
-		"bar": dyn.V(1.0),
-	}), nv)
-}
-
-func TestFromTypedFloatSliceWithZeroValue(t *testing.T) {
-	ref := dyn.NilValue
-	src := []float64{1.0, 0.0, 2.0}
-
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V([]dyn.Value{
-		dyn.V(1.0), dyn.V(0.0), dyn.V(2.0),
-	}), nv)
-}
-
-func TestFromTypedFloatStructWithZeroValue(t *testing.T) {
-	type Tmp struct {
-		Foo float64 `json:"foo"`
-		Bar float64 `json:"bar"`
-	}
-
-	ref := dyn.NilValue
-	src := Tmp{
-		Foo: 1.0,
-		Bar: 0.0,
-	}
-
-	// Note, the zero value is not included in the output.
-	nv, err := FromTyped(src, ref)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(1.0),
-	}), nv)
-}
-
 func TestFromTypedMapNil(t *testing.T) {
 	var src map[string]string = nil
 
@@ -323,7 +139,7 @@ func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
 	nv, err := FromTyped(src, ref)
 	require.NoError(t, err)
 	assert.Equal(t, dyn.V(map[string]dyn.Value{
-		"foo": dyn.V(""),
+		"foo": dyn.NilValue,
 	}), nv)
 }