Fix dynamic representation of zero values in maps and slices

libs/dyn/convert/end_to_end_test.go

@@ -39,6 +39,15 @@ 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,13 +3,35 @@ 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
@@ -28,13 +50,13 @@ func FromTyped(src any, ref dyn.Value) (dyn.Value, error) {
 	case reflect.Slice:
 		return fromTypedSlice(srcv, ref)
 	case reflect.String:
-		return fromTypedString(srcv, ref)
+		return fromTypedString(srcv, ref, options...)
 	case reflect.Bool:
-		return fromTypedBool(srcv, ref)
+		return fromTypedBool(srcv, ref, options...)
 	case reflect.Int, reflect.Int32, reflect.Int64:
-		return fromTypedInt(srcv, ref)
+		return fromTypedInt(srcv, ref, options...)
 	case reflect.Float32, reflect.Float64:
-		return fromTypedFloat(srcv, ref)
+		return fromTypedFloat(srcv, ref, options...)
 	}
 
 	return dyn.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
@@ -52,7 +74,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
 		}
@@ -89,8 +111,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 config.NilValue).
-		nv, err := FromTyped(v.Interface(), ref.Get(k))
+		// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
+		nv, err := fromTyped(v.Interface(), ref.Get(k), includeZeroValues)
 		if err != nil {
 			return dyn.Value{}, err
 		}
@@ -120,8 +142,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 config.NilValue).
-		nv, err := FromTyped(v.Interface(), ref.Index(i))
+		// Convert entry taking into account the reference value (may be equal to dyn.NilValue).
+		nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValues)
 		if err != nil {
 			return dyn.Value{}, err
 		}
@@ -132,7 +154,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) (dyn.Value, error) {
+func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindString:
 		value := src.String()
@@ -142,9 +164,9 @@ func fromTypedString(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() {
+		// This field is not set in the reference. We set it to nil if it's zero
+		// valued in the typed representation and the includeZeroValues option is not set.
+		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.String()), nil
@@ -153,7 +175,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
-func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
+func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindBool:
 		value := src.Bool()
@@ -162,9 +184,9 @@ func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 		}
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() {
+		// This field is not set in the reference. We set it to nil if it's zero
+		// valued in the typed representation and the includeZeroValues option is not set.
+		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Bool()), nil
@@ -173,7 +195,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
-func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
+func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindInt:
 		value := src.Int()
@@ -182,9 +204,9 @@ func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 		}
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() {
+		// This field is not set in the reference. We set it to nil if it's zero
+		// valued in the typed representation and the includeZeroValues option is not set.
+		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Int()), nil
@@ -193,7 +215,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
-func fromTypedFloat(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
+func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindFloat:
 		value := src.Float()
@@ -202,9 +224,9 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 		}
 		return dyn.V(value), nil
 	case dyn.KindNil:
-		// This field is not set in the reference, so we only include it if it has a non-zero value.
-		// Otherwise, we would always include all zero valued fields.
-		if src.IsZero() {
+		// This field is not set in the reference. We set it to nil if it's zero
+		// valued in the typed representation and the includeZeroValues option is not set.
+		if src.IsZero() && !slices.Contains(options, includeZeroValues) {
 			return dyn.NilValue, nil
 		}
 		return dyn.V(src.Float()), nil

libs/dyn/convert/from_typed_test.go

@@ -68,6 +68,190 @@ 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
 
@@ -139,7 +323,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.NilValue,
+		"foo": dyn.V(""),
 	}), nv)
 }