Retain partially valid structs in `convert.Normalize` (#1203)

## Changes

Before this change, any error in a subtree would cause the entire
subtree to be dropped from the output.

This is not ideal when debugging, so instead we drop only the values
that cannot be normalized. Note that this doesn't change behavior if the
caller is properly checking the returned diagnostics for errors.

Note: this includes a change to use `dyn.InvalidValue` as opposed to
`dyn.NilValue` when returning errors.

## Tests

Added unit tests for the case where nested struct, map, or slice
elements contain an error.

libs/dyn/convert/from_typed.go

@@ -59,7 +59,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
 		return fromTypedFloat(srcv, ref, options...)
 	}
 
-	return dyn.NilValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
+	return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
 }
 
 func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
@@ -67,7 +67,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindMap, dyn.KindNil:
 	default:
-		return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+		return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
 	out := make(map[string]dyn.Value)
@@ -76,7 +76,7 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 		// Convert the field taking into account the reference value (may be equal to config.NilValue).
 		nv, err := fromTyped(v.Interface(), ref.Get(k))
 		if err != nil {
-			return dyn.Value{}, err
+			return dyn.InvalidValue, err
 		}
 
 		if nv != dyn.NilValue {
@@ -92,7 +92,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindMap, dyn.KindNil:
 	default:
-		return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+		return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
 	// Return nil if the map is nil.
@@ -109,7 +109,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).
 		nv, err := fromTyped(v.Interface(), ref.Get(k), includeZeroValues)
 		if err != nil {
-			return dyn.Value{}, err
+			return dyn.InvalidValue, err
 		}
 
 		// Every entry is represented, even if it is a nil.
@@ -125,7 +125,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 	switch ref.Kind() {
 	case dyn.KindSequence, dyn.KindNil:
 	default:
-		return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+		return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 	}
 
 	// Return nil if the slice is nil.
@@ -140,7 +140,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 		// 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
+			return dyn.InvalidValue, err
 		}
 
 		out[i] = nv
@@ -167,7 +167,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
 		return dyn.V(src.String()), nil
 	}
 
-	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+	return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
 func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
@@ -187,7 +187,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions
 		return dyn.V(src.Bool()), nil
 	}
 
-	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+	return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
 func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
@@ -207,7 +207,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions)
 		return dyn.V(src.Int()), nil
 	}
 
-	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+	return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 }
 
 func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
@@ -227,5 +227,5 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOption
 		return dyn.V(src.Float()), nil
 	}
 
-	return dyn.Value{}, fmt.Errorf("unhandled type: %s", ref.Kind())
+	return dyn.InvalidValue, fmt.Errorf("unhandled type: %s", ref.Kind())
 }

libs/dyn/convert/normalize.go

@@ -35,7 +35,7 @@ func normalizeType(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics
 		return normalizeFloat(typ, src)
 	}
 
-	return dyn.NilValue, diag.Errorf("unsupported type: %s", typ.Kind())
+	return dyn.InvalidValue, diag.Errorf("unsupported type: %s", typ.Kind())
 }
 
 func typeMismatch(expected dyn.Kind, src dyn.Value) diag.Diagnostic {
@@ -69,7 +69,7 @@ func normalizeStruct(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnosti
 			if err != nil {
 				diags = diags.Extend(err)
 				// Skip the element if it cannot be normalized.
-				if err.HasError() {
+				if !v.IsValid() {
 					continue
 				}
 			}
@@ -82,7 +82,7 @@ func normalizeStruct(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnosti
 		return src, diags
 	}
 
-	return dyn.NilValue, diags.Append(typeMismatch(dyn.KindMap, src))
+	return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src))
 }
 
 func normalizeMap(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
@@ -97,7 +97,7 @@ func normalizeMap(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics)
 			if err != nil {
 				diags = diags.Extend(err)
 				// Skip the element if it cannot be normalized.
-				if err.HasError() {
+				if !v.IsValid() {
 					continue
 				}
 			}
@@ -110,7 +110,7 @@ func normalizeMap(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics)
 		return src, diags
 	}
 
-	return dyn.NilValue, diags.Append(typeMismatch(dyn.KindMap, src))
+	return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src))
 }
 
 func normalizeSlice(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
@@ -125,7 +125,7 @@ func normalizeSlice(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostic
 			if err != nil {
 				diags = diags.Extend(err)
 				// Skip the element if it cannot be normalized.
-				if err.HasError() {
+				if !v.IsValid() {
 					continue
 				}
 			}
@@ -138,7 +138,7 @@ func normalizeSlice(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostic
 		return src, diags
 	}
 
-	return dyn.NilValue, diags.Append(typeMismatch(dyn.KindSequence, src))
+	return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindSequence, src))
 }
 
 func normalizeString(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
@@ -155,7 +155,7 @@ func normalizeString(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnosti
 	case dyn.KindFloat:
 		out = strconv.FormatFloat(src.MustFloat(), 'f', -1, 64)
 	default:
-		return dyn.NilValue, diags.Append(typeMismatch(dyn.KindString, src))
+		return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindString, src))
 	}
 
 	return dyn.NewValue(out, src.Location()), diags
@@ -177,10 +177,10 @@ func normalizeBool(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics
 			out = false
 		default:
 			// Cannot interpret as a boolean.
-			return dyn.NilValue, diags.Append(typeMismatch(dyn.KindBool, src))
+			return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src))
 		}
 	default:
-		return dyn.NilValue, diags.Append(typeMismatch(dyn.KindBool, src))
+		return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src))
 	}
 
 	return dyn.NewValue(out, src.Location()), diags
@@ -197,14 +197,14 @@ func normalizeInt(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics)
 		var err error
 		out, err = strconv.ParseInt(src.MustString(), 10, 64)
 		if err != nil {
-			return dyn.NilValue, diags.Append(diag.Diagnostic{
+			return dyn.InvalidValue, diags.Append(diag.Diagnostic{
 				Severity: diag.Error,
 				Summary:  fmt.Sprintf("cannot parse %q as an integer", src.MustString()),
 				Location: src.Location(),
 			})
 		}
 	default:
-		return dyn.NilValue, diags.Append(typeMismatch(dyn.KindInt, src))
+		return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindInt, src))
 	}
 
 	return dyn.NewValue(out, src.Location()), diags
@@ -221,14 +221,14 @@ func normalizeFloat(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostic
 		var err error
 		out, err = strconv.ParseFloat(src.MustString(), 64)
 		if err != nil {
-			return dyn.NilValue, diags.Append(diag.Diagnostic{
+			return dyn.InvalidValue, diags.Append(diag.Diagnostic{
 				Severity: diag.Error,
 				Summary:  fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()),
 				Location: src.Location(),
 			})
 		}
 	default:
-		return dyn.NilValue, diags.Append(typeMismatch(dyn.KindFloat, src))
+		return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindFloat, src))
 	}
 
 	return dyn.NewValue(out, src.Location()), diags

libs/dyn/convert/normalize_test.go

@@ -104,6 +104,44 @@ func TestNormalizeStructError(t *testing.T) {
 	}, err[0])
 }
 
+func TestNormalizeStructNestedError(t *testing.T) {
+	type Nested struct {
+		F1 int `json:"f1"`
+		F2 int `json:"f2"`
+	}
+	type Tmp struct {
+		Foo Nested `json:"foo"`
+		Bar Nested `json:"bar"`
+	}
+
+	var typ Tmp
+	vin := dyn.V(map[string]dyn.Value{
+		"foo": dyn.V(map[string]dyn.Value{
+			"f1": dyn.V("error"),
+			"f2": dyn.V(1),
+		}),
+		"bar": dyn.V(map[string]dyn.Value{
+			"f1": dyn.V(1),
+			"f2": dyn.V("error"),
+		}),
+	})
+	vout, err := Normalize(typ, vin)
+	assert.Len(t, err, 2)
+
+	// Verify that valid fields are retained.
+	assert.Equal(t,
+		dyn.V(map[string]dyn.Value{
+			"foo": dyn.V(map[string]dyn.Value{
+				"f2": dyn.V(int64(1)),
+			}),
+			"bar": dyn.V(map[string]dyn.Value{
+				"f1": dyn.V(int64(1)),
+			}),
+		}),
+		vout,
+	)
+}
+
 func TestNormalizeMap(t *testing.T) {
 	var typ map[string]string
 	vin := dyn.V(map[string]dyn.Value{
@@ -157,6 +195,40 @@ func TestNormalizeMapError(t *testing.T) {
 	}, err[0])
 }
 
+func TestNormalizeMapNestedError(t *testing.T) {
+	type Nested struct {
+		F1 int `json:"f1"`
+		F2 int `json:"f2"`
+	}
+
+	var typ map[string]Nested
+	vin := dyn.V(map[string]dyn.Value{
+		"foo": dyn.V(map[string]dyn.Value{
+			"f1": dyn.V("error"),
+			"f2": dyn.V(1),
+		}),
+		"bar": dyn.V(map[string]dyn.Value{
+			"f1": dyn.V(1),
+			"f2": dyn.V("error"),
+		}),
+	})
+	vout, err := Normalize(typ, vin)
+	assert.Len(t, err, 2)
+
+	// Verify that valid fields are retained.
+	assert.Equal(t,
+		dyn.V(map[string]dyn.Value{
+			"foo": dyn.V(map[string]dyn.Value{
+				"f2": dyn.V(int64(1)),
+			}),
+			"bar": dyn.V(map[string]dyn.Value{
+				"f1": dyn.V(int64(1)),
+			}),
+		}),
+		vout,
+	)
+}
+
 func TestNormalizeSlice(t *testing.T) {
 	var typ []string
 	vin := dyn.V([]dyn.Value{
@@ -209,6 +281,40 @@ func TestNormalizeSliceError(t *testing.T) {
 	}, err[0])
 }
 
+func TestNormalizeSliceNestedError(t *testing.T) {
+	type Nested struct {
+		F1 int `json:"f1"`
+		F2 int `json:"f2"`
+	}
+
+	var typ []Nested
+	vin := dyn.V([]dyn.Value{
+		dyn.V(map[string]dyn.Value{
+			"f1": dyn.V("error"),
+			"f2": dyn.V(1),
+		}),
+		dyn.V(map[string]dyn.Value{
+			"f1": dyn.V(1),
+			"f2": dyn.V("error"),
+		}),
+	})
+	vout, err := Normalize(typ, vin)
+	assert.Len(t, err, 2)
+
+	// Verify that valid fields are retained.
+	assert.Equal(t,
+		dyn.V([]dyn.Value{
+			dyn.V(map[string]dyn.Value{
+				"f2": dyn.V(int64(1)),
+			}),
+			dyn.V(map[string]dyn.Value{
+				"f1": dyn.V(int64(1)),
+			}),
+		}),
+		vout,
+	)
+}
+
 func TestNormalizeString(t *testing.T) {
 	var typ string
 	vin := dyn.V("string")