Fix conversion of zero valued scalar pointers to a dynamic value (#1433)

## Changes
This PR also fixes empty values variable overrides using the --var flag.
Now, using `--var="my_variable="` will set the value of `my_variable` to
the empty string instead of ignoring the flag altogether.

## Tests
The change using a unit test. Manually verified the `--var` flag works
now.
This commit is contained in:
shreyas-goenka 2024-05-21 17:23:00 +05:30 committed by GitHub
parent 3f8036f2df
commit c5032644a0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 73 additions and 14 deletions

View File

@ -67,4 +67,49 @@ func TestAdditional(t *testing.T) {
SliceOfPointer: []*string{nil}, SliceOfPointer: []*string{nil},
}) })
}) })
t.Run("pointer to a empty string", func(t *testing.T) {
s := ""
assertFromTypedToTypedEqual(t, &s)
})
t.Run("nil pointer", func(t *testing.T) {
var s *string
assertFromTypedToTypedEqual(t, s)
})
t.Run("pointer to struct with scalar values", func(t *testing.T) {
s := ""
type foo struct {
A string `json:"a"`
B int `json:"b"`
C bool `json:"c"`
D *string `json:"d"`
}
assertFromTypedToTypedEqual(t, &foo{
A: "a",
B: 1,
C: true,
D: &s,
})
assertFromTypedToTypedEqual(t, &foo{
A: "",
B: 0,
C: false,
D: nil,
})
})
t.Run("map with scalar values", func(t *testing.T) {
assertFromTypedToTypedEqual(t, map[string]string{
"a": "a",
"b": "b",
"c": "",
})
assertFromTypedToTypedEqual(t, map[string]int{
"a": 1,
"b": 0,
"c": 2,
})
})
} }

View File

@ -12,16 +12,22 @@ import (
type fromTypedOptions int type fromTypedOptions int
const ( const (
// Use the zero value instead of setting zero values to nil. This is useful // If this flag is set, zero values for scalars (strings, bools, ints, floats)
// for types where the zero values and nil are semantically different. That is // would resolve to corresponding zero values in the dynamic representation.
// strings, bools, ints, floats. // Otherwise, zero values for scalars resolve to dyn.NilValue.
// //
// Note: this is not needed for structs because dyn.NilValue is converted back // This flag exists to reconcile the default values for scalars in a Go struct
// to a zero value when using the convert.ToTyped function. // being zero values with zero values in a dynamic representation. In a Go struct,
// zero values are the same as the values not being set at all. This is not the case
// in the dynamic representation.
// //
// Values in maps and slices should be set to zero values, and not nil in the // If a scalar value in a typed Go struct is zero, in the dynamic representation
// dynamic representation. // we would set it to dyn.NilValue, i.e. equivalent to the value not being set at all.
includeZeroValues fromTypedOptions = 1 << iota //
// If a scalar value in a Go map, slice or pointer is set to zero, we will set it
// to the zero value in the dynamic representation, and not dyn.NilValue. This is
// equivalent to the value being intentionally set to zero.
includeZeroValuedScalars fromTypedOptions = 1 << iota
) )
// FromTyped converts changes made in the typed structure w.r.t. the configuration value // FromTyped converts changes made in the typed structure w.r.t. the configuration value
@ -41,6 +47,14 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
return dyn.NilValue, nil return dyn.NilValue, nil
} }
srcv = srcv.Elem() srcv = srcv.Elem()
// If a pointer to a scalar type points to a zero value, we should include
// that zero value in the dynamic representation.
// This is because by default a pointer is nil in Go, and it not being nil
// indicates its value was intentionally set to zero.
if !slices.Contains(options, includeZeroValuedScalars) {
options = append(options, includeZeroValuedScalars)
}
} }
switch srcv.Kind() { switch srcv.Kind() {
@ -129,7 +143,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
} }
// 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 dyn.NilValue).
nv, err := fromTyped(v.Interface(), refv, includeZeroValues) nv, err := fromTyped(v.Interface(), refv, includeZeroValuedScalars)
if err != nil { if err != nil {
return dyn.InvalidValue, err return dyn.InvalidValue, err
} }
@ -160,7 +174,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
v := src.Index(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 dyn.NilValue).
nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValues) nv, err := fromTyped(v.Interface(), ref.Index(i), includeZeroValuedScalars)
if err != nil { if err != nil {
return dyn.InvalidValue, err return dyn.InvalidValue, err
} }
@ -183,7 +197,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
case dyn.KindNil: 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. We set it to nil if it's zero
// valued in the typed representation and the includeZeroValues option is not set. // valued in the typed representation and the includeZeroValues option is not set.
if src.IsZero() && !slices.Contains(options, includeZeroValues) { if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
return dyn.NilValue, nil return dyn.NilValue, nil
} }
return dyn.V(src.String()), nil return dyn.V(src.String()), nil
@ -203,7 +217,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions
case dyn.KindNil: 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. We set it to nil if it's zero
// valued in the typed representation and the includeZeroValues option is not set. // valued in the typed representation and the includeZeroValues option is not set.
if src.IsZero() && !slices.Contains(options, includeZeroValues) { if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
return dyn.NilValue, nil return dyn.NilValue, nil
} }
return dyn.V(src.Bool()), nil return dyn.V(src.Bool()), nil
@ -228,7 +242,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions)
case dyn.KindNil: 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. We set it to nil if it's zero
// valued in the typed representation and the includeZeroValues option is not set. // valued in the typed representation and the includeZeroValues option is not set.
if src.IsZero() && !slices.Contains(options, includeZeroValues) { if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
return dyn.NilValue, nil return dyn.NilValue, nil
} }
return dyn.V(src.Int()), nil return dyn.V(src.Int()), nil
@ -253,7 +267,7 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOption
case dyn.KindNil: 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. We set it to nil if it's zero
// valued in the typed representation and the includeZeroValues option is not set. // valued in the typed representation and the includeZeroValues option is not set.
if src.IsZero() && !slices.Contains(options, includeZeroValues) { if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
return dyn.NilValue, nil return dyn.NilValue, nil
} }
return dyn.V(src.Float()), nil return dyn.V(src.Float()), nil