Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Include dyn.Path in normalization warnings and errors #1332

Merged
merged 1 commit into from
Apr 3, 2024
Merged

Include dyn.Path in normalization warnings and errors #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libs/diag/diagnostic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ type Diagnostic struct {
// Location is a source code location associated with the diagnostic message.
// It may be zero if there is no associated location.
Location dyn.Location

// Path is a path to the value in a configuration tree that the diagnostic is associated with.
// It may be nil if there is no associated path.
Path dyn.Path
}

// Errorf creates a new error diagnostic.
Expand Down
83 changes: 44 additions & 39 deletions libs/dyn/convert/normalize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,51 +33,53 @@ func Normalize(dst any, src dyn.Value, opts ...NormalizeOption) (dyn.Value, diag
}
}

return n.normalizeType(reflect.TypeOf(dst), src, []reflect.Type{})
return n.normalizeType(reflect.TypeOf(dst), src, []reflect.Type{}, dyn.EmptyPath)
}

func (n normalizeOptions) normalizeType(typ reflect.Type, src dyn.Value, seen []reflect.Type) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeType(typ reflect.Type, src dyn.Value, seen []reflect.Type, path dyn.Path) (dyn.Value, diag.Diagnostics) {
for typ.Kind() == reflect.Pointer {
typ = typ.Elem()
}

switch typ.Kind() {
case reflect.Struct:
return n.normalizeStruct(typ, src, append(seen, typ))
return n.normalizeStruct(typ, src, append(seen, typ), path)
case reflect.Map:
return n.normalizeMap(typ, src, append(seen, typ))
return n.normalizeMap(typ, src, append(seen, typ), path)
case reflect.Slice:
return n.normalizeSlice(typ, src, append(seen, typ))
return n.normalizeSlice(typ, src, append(seen, typ), path)
case reflect.String:
return n.normalizeString(typ, src)
return n.normalizeString(typ, src, path)
case reflect.Bool:
return n.normalizeBool(typ, src)
return n.normalizeBool(typ, src, path)
case reflect.Int, reflect.Int32, reflect.Int64:
return n.normalizeInt(typ, src)
return n.normalizeInt(typ, src, path)
case reflect.Float32, reflect.Float64:
return n.normalizeFloat(typ, src)
return n.normalizeFloat(typ, src, path)
}

return dyn.InvalidValue, diag.Errorf("unsupported type: %s", typ.Kind())
}

func nullWarning(expected dyn.Kind, src dyn.Value) diag.Diagnostic {
func nullWarning(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnostic {
return diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("expected a %s value, found null", expected),
Location: src.Location(),
Path: path,
}
}

func typeMismatch(expected dyn.Kind, src dyn.Value) diag.Diagnostic {
func typeMismatch(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnostic {
return diag.Diagnostic{
Severity: diag.Error,
Summary: fmt.Sprintf("expected %s, found %s", expected, src.Kind()),
Location: src.Location(),
Path: path,
}
}

func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen []reflect.Type) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen []reflect.Type, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics

switch src.Kind() {
Expand All @@ -93,12 +95,13 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen
Severity: diag.Warning,
Summary: fmt.Sprintf("unknown field: %s", pk.MustString()),
Location: pk.Location(),
Path: path,
})
continue
}

// Normalize the value according to the field type.
nv, err := n.normalizeType(typ.FieldByIndex(index).Type, pv, seen)
nv, err := n.normalizeType(typ.FieldByIndex(index).Type, pv, seen, path.Append(dyn.Key(pk.MustString())))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to double check: Append does not mutate in place and returns a new copy, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, as of #1295:

cli/libs/dyn/path.go

Lines 52 to 59 in 8c144a2

// Append appends the given components to the path.
// Mutations to the returned path do not affect the original path.
func (p Path) Append(cs ...pathComponent) Path {
out := make(Path, len(p)+len(cs))
copy(out, p)
copy(out[len(p):], cs)
return out
}

if err != nil {
diags = diags.Extend(err)
// Skip the element if it cannot be normalized.
Expand Down Expand Up @@ -136,17 +139,17 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen
var v dyn.Value
switch ftyp.Kind() {
case reflect.Struct, reflect.Map:
v, _ = n.normalizeType(ftyp, dyn.V(map[string]dyn.Value{}), seen)
v, _ = n.normalizeType(ftyp, dyn.V(map[string]dyn.Value{}), seen, path.Append(dyn.Key(k)))
case reflect.Slice:
v, _ = n.normalizeType(ftyp, dyn.V([]dyn.Value{}), seen)
v, _ = n.normalizeType(ftyp, dyn.V([]dyn.Value{}), seen, path.Append(dyn.Key(k)))
case reflect.String:
v, _ = n.normalizeType(ftyp, dyn.V(""), seen)
v, _ = n.normalizeType(ftyp, dyn.V(""), seen, path.Append(dyn.Key(k)))
case reflect.Bool:
v, _ = n.normalizeType(ftyp, dyn.V(false), seen)
v, _ = n.normalizeType(ftyp, dyn.V(false), seen, path.Append(dyn.Key(k)))
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
v, _ = n.normalizeType(ftyp, dyn.V(int64(0)), seen)
v, _ = n.normalizeType(ftyp, dyn.V(int64(0)), seen, path.Append(dyn.Key(k)))
case reflect.Float32, reflect.Float64:
v, _ = n.normalizeType(ftyp, dyn.V(float64(0)), seen)
v, _ = n.normalizeType(ftyp, dyn.V(float64(0)), seen, path.Append(dyn.Key(k)))
default:
// Skip fields for which we do not have a natural [dyn.Value] equivalent.
// For example, we don't handle reflect.Complex* and reflect.Uint* types.
Expand All @@ -162,10 +165,10 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen
return src, diags
}

return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src, path))
}

func (n normalizeOptions) normalizeMap(typ reflect.Type, src dyn.Value, seen []reflect.Type) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeMap(typ reflect.Type, src dyn.Value, seen []reflect.Type, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics

switch src.Kind() {
Expand All @@ -176,7 +179,7 @@ func (n normalizeOptions) normalizeMap(typ reflect.Type, src dyn.Value, seen []r
pv := pair.Value

// Normalize the value according to the map element type.
nv, err := n.normalizeType(typ.Elem(), pv, seen)
nv, err := n.normalizeType(typ.Elem(), pv, seen, path.Append(dyn.Key(pk.MustString())))
if err != nil {
diags = diags.Extend(err)
// Skip the element if it cannot be normalized.
Expand All @@ -193,18 +196,18 @@ func (n normalizeOptions) normalizeMap(typ reflect.Type, src dyn.Value, seen []r
return src, diags
}

return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindMap, src, path))
}

func (n normalizeOptions) normalizeSlice(typ reflect.Type, src dyn.Value, seen []reflect.Type) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeSlice(typ reflect.Type, src dyn.Value, seen []reflect.Type, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics

switch src.Kind() {
case dyn.KindSequence:
out := make([]dyn.Value, 0, len(src.MustSequence()))
for _, v := range src.MustSequence() {
// Normalize the value according to the slice element type.
v, err := n.normalizeType(typ.Elem(), v, seen)
v, err := n.normalizeType(typ.Elem(), v, seen, path.Append(dyn.Index(len(out))))
if err != nil {
diags = diags.Extend(err)
// Skip the element if it cannot be normalized.
Expand All @@ -221,10 +224,10 @@ func (n normalizeOptions) normalizeSlice(typ reflect.Type, src dyn.Value, seen [
return src, diags
}

return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindSequence, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindSequence, src, path))
}

func (n normalizeOptions) normalizeString(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeString(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics
var out string

Expand All @@ -239,15 +242,15 @@ func (n normalizeOptions) normalizeString(typ reflect.Type, src dyn.Value) (dyn.
out = strconv.FormatFloat(src.MustFloat(), 'f', -1, 64)
case dyn.KindNil:
// Return a warning if the field is present but has a null value.
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindString, src))
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindString, src, path))
default:
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindString, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindString, src, path))
}

return dyn.NewValue(out, src.Location()), diags
}

func (n normalizeOptions) normalizeBool(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeBool(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics
var out bool

Expand All @@ -268,19 +271,19 @@ func (n normalizeOptions) normalizeBool(typ reflect.Type, src dyn.Value) (dyn.Va
}

// Cannot interpret as a boolean.
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src, path))
}
case dyn.KindNil:
// Return a warning if the field is present but has a null value.
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindBool, src))
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindBool, src, path))
default:
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindBool, src, path))
}

return dyn.NewValue(out, src.Location()), diags
}

func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics
var out int64

Expand All @@ -300,19 +303,20 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value) (dyn.Val
Severity: diag.Error,
Summary: fmt.Sprintf("cannot parse %q as an integer", src.MustString()),
Location: src.Location(),
Path: path,
})
}
case dyn.KindNil:
// Return a warning if the field is present but has a null value.
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindInt, src))
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindInt, src, path))
default:
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindInt, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindInt, src, path))
}

return dyn.NewValue(out, src.Location()), diags
}

func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value) (dyn.Value, diag.Diagnostics) {
func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) {
var diags diag.Diagnostics
var out float64

Expand All @@ -332,13 +336,14 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value) (dyn.V
Severity: diag.Error,
Summary: fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()),
Location: src.Location(),
Path: path,
})
}
case dyn.KindNil:
// Return a warning if the field is present but has a null value.
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindFloat, src))
return dyn.InvalidValue, diags.Append(nullWarning(dyn.KindFloat, src, path))
default:
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindFloat, src))
return dyn.InvalidValue, diags.Append(typeMismatch(dyn.KindFloat, src, path))
}

return dyn.NewValue(out, src.Location()), diags
Expand Down
18 changes: 18 additions & 0 deletions libs/dyn/convert/normalize_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestNormalizeStructElementDiagnostic(t *testing.T) {
Severity: diag.Error,
Summary: `expected string, found map`,
Location: dyn.Location{},
Path: dyn.NewPath(dyn.Key("bar")),
}, err[0])

// Elements that encounter an error during normalization are dropped.
Expand All @@ -68,6 +69,7 @@ func TestNormalizeStructUnknownField(t *testing.T) {
Severity: diag.Warning,
Summary: `unknown field: bar`,
Location: vin.Get("foo").Location(),
Path: dyn.EmptyPath,
}, err[0])

// The field that can be mapped to the struct field is retained.
Expand Down Expand Up @@ -101,6 +103,7 @@ func TestNormalizeStructError(t *testing.T) {
Severity: diag.Error,
Summary: `expected map, found string`,
Location: vin.Get("foo").Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -245,6 +248,7 @@ func TestNormalizeMapElementDiagnostic(t *testing.T) {
Severity: diag.Error,
Summary: `expected string, found map`,
Location: dyn.Location{},
Path: dyn.NewPath(dyn.Key("bar")),
}, err[0])

// Elements that encounter an error during normalization are dropped.
Expand All @@ -270,6 +274,7 @@ func TestNormalizeMapError(t *testing.T) {
Severity: diag.Error,
Summary: `expected map, found string`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -333,6 +338,7 @@ func TestNormalizeSliceElementDiagnostic(t *testing.T) {
Severity: diag.Error,
Summary: `expected string, found map`,
Location: dyn.Location{},
Path: dyn.NewPath(dyn.Index(2)),
}, err[0])

// Elements that encounter an error during normalization are dropped.
Expand All @@ -356,6 +362,7 @@ func TestNormalizeSliceError(t *testing.T) {
Severity: diag.Error,
Summary: `expected sequence, found string`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -410,6 +417,7 @@ func TestNormalizeStringNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a string value, found null`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -446,6 +454,7 @@ func TestNormalizeStringError(t *testing.T) {
Severity: diag.Error,
Summary: `expected string, found map`,
Location: dyn.Location{},
Path: dyn.EmptyPath,
}, err[0])
}

Expand All @@ -466,6 +475,7 @@ func TestNormalizeBoolNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a bool value, found null`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -507,6 +517,7 @@ func TestNormalizeBoolFromStringError(t *testing.T) {
Severity: diag.Error,
Summary: `expected bool, found string`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand All @@ -519,6 +530,7 @@ func TestNormalizeBoolError(t *testing.T) {
Severity: diag.Error,
Summary: `expected bool, found map`,
Location: dyn.Location{},
Path: dyn.EmptyPath,
}, err[0])
}

Expand All @@ -539,6 +551,7 @@ func TestNormalizeIntNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a int value, found null`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -567,6 +580,7 @@ func TestNormalizeIntFromStringError(t *testing.T) {
Severity: diag.Error,
Summary: `cannot parse "abc" as an integer`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand All @@ -579,6 +593,7 @@ func TestNormalizeIntError(t *testing.T) {
Severity: diag.Error,
Summary: `expected int, found map`,
Location: dyn.Location{},
Path: dyn.EmptyPath,
}, err[0])
}

Expand All @@ -599,6 +614,7 @@ func TestNormalizeFloatNil(t *testing.T) {
Severity: diag.Warning,
Summary: `expected a float value, found null`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand Down Expand Up @@ -627,6 +643,7 @@ func TestNormalizeFloatFromStringError(t *testing.T) {
Severity: diag.Error,
Summary: `cannot parse "abc" as a floating point number`,
Location: vin.Location(),
Path: dyn.EmptyPath,
}, err[0])
}

Expand All @@ -639,5 +656,6 @@ func TestNormalizeFloatError(t *testing.T) {
Severity: diag.Error,
Summary: `expected float, found map`,
Location: dyn.Location{},
Path: dyn.EmptyPath,
}, err[0])
}
Loading