Ignore `dyn.NilValue` when traversing value from `dyn.Map` (#1547)

## Changes

The map function ignores cases where either a key in a map is not
present or an index in a sequence is out of bounds. As of recently, we
retain nil values as valid values in a configuration tree. As such, it
makes sense to also ignore cases where a map or sequence is expected but
nil is found. This is semantically no different from an empty map where
a key is not found.

Without this fix, all calls to `dyn.Map` would need to be updated with
nil-checks at every path component.

Related PRs:
* #1507
* #1511

## Tests

Unit tests pass.
This commit is contained in:
Pieter Noordhuis 2024-07-01 15:00:31 +02:00 committed by GitHub
parent e8b76a7f13
commit da603c6ead
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 63 additions and 10 deletions

View File

@ -6,6 +6,28 @@ import (
"slices" "slices"
) )
// This error is returned if the path indicates that a map or sequence is expected, but the value is nil.
type cannotTraverseNilError struct {
p Path
}
func (e cannotTraverseNilError) Error() string {
component := e.p[len(e.p)-1]
switch {
case component.isKey():
return fmt.Sprintf("expected a map to index %q, found nil", e.p)
case component.isIndex():
return fmt.Sprintf("expected a sequence to index %q, found nil", e.p)
default:
panic("invalid component")
}
}
func IsCannotTraverseNilError(err error) bool {
var target cannotTraverseNilError
return errors.As(err, &target)
}
type noSuchKeyError struct { type noSuchKeyError struct {
p Path p Path
} }
@ -70,11 +92,17 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts
switch { switch {
case component.isKey(): case component.isKey():
// Expect a map to be set if this is a key. // Expect a map to be set if this is a key.
m, ok := v.AsMap() switch v.Kind() {
if !ok { case KindMap:
// OK
case KindNil:
return InvalidValue, cannotTraverseNilError{path}
default:
return InvalidValue, fmt.Errorf("expected a map to index %q, found %s", path, v.Kind()) return InvalidValue, fmt.Errorf("expected a map to index %q, found %s", path, v.Kind())
} }
m := v.MustMap()
// Lookup current value in the map. // Lookup current value in the map.
ev, ok := m.GetByString(component.key) ev, ok := m.GetByString(component.key)
if !ok { if !ok {
@ -103,11 +131,17 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts
case component.isIndex(): case component.isIndex():
// Expect a sequence to be set if this is an index. // Expect a sequence to be set if this is an index.
s, ok := v.AsSequence() switch v.Kind() {
if !ok { case KindSequence:
// OK
case KindNil:
return InvalidValue, cannotTraverseNilError{path}
default:
return InvalidValue, fmt.Errorf("expected a sequence to index %q, found %s", path, v.Kind()) return InvalidValue, fmt.Errorf("expected a sequence to index %q, found %s", path, v.Kind())
} }
s := v.MustSequence()
// Lookup current value in the sequence. // Lookup current value in the sequence.
if component.index < 0 || component.index >= len(s) { if component.index < 0 || component.index >= len(s) {
return InvalidValue, indexOutOfBoundsError{path} return InvalidValue, indexOutOfBoundsError{path}

View File

@ -10,9 +10,12 @@ type MapFunc func(Path, Value) (Value, error)
// Foreach returns a [MapFunc] that applies the specified [MapFunc] to each // Foreach returns a [MapFunc] that applies the specified [MapFunc] to each
// value in a map or sequence and returns the new map or sequence. // value in a map or sequence and returns the new map or sequence.
// If the input is nil, it returns nil.
func Foreach(fn MapFunc) MapFunc { func Foreach(fn MapFunc) MapFunc {
return func(p Path, v Value) (Value, error) { return func(p Path, v Value) (Value, error) {
switch v.Kind() { switch v.Kind() {
case KindNil:
return v, nil
case KindMap: case KindMap:
m := v.MustMap().Clone() m := v.MustMap().Clone()
for _, pair := range m.Pairs() { for _, pair := range m.Pairs() {
@ -75,8 +78,10 @@ func MapByPattern(v Value, p Pattern, fn MapFunc) (Value, error) {
return nv, nil return nv, nil
} }
// Return original value if a key or index is missing. // Return original value if:
if IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) { // - any map or sequence is a nil, or
// - a key or index is missing
if IsCannotTraverseNilError(err) || IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) {
return v, nil return v, nil
} }

View File

@ -20,11 +20,14 @@ func TestMapWithEmptyPath(t *testing.T) {
} }
func TestMapOnNilValue(t *testing.T) { func TestMapOnNilValue(t *testing.T) {
var nv dyn.Value
var err error var err error
_, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Key("foo")), nil) nv, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Key("foo")), nil)
assert.ErrorContains(t, err, `expected a map to index "foo", found nil`) assert.NoError(t, err)
_, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Index(42)), nil) assert.Equal(t, dyn.NilValue, nv)
assert.ErrorContains(t, err, `expected a sequence to index "[42]", found nil`) nv, err = dyn.MapByPath(dyn.NilValue, dyn.NewPath(dyn.Index(42)), nil)
assert.NoError(t, err)
assert.Equal(t, dyn.NilValue, nv)
} }
func TestMapFuncOnMap(t *testing.T) { func TestMapFuncOnMap(t *testing.T) {
@ -269,6 +272,17 @@ func TestMapForeachOnOtherError(t *testing.T) {
assert.ErrorContains(t, err, "expected a map or sequence, found int") assert.ErrorContains(t, err, "expected a map or sequence, found int")
} }
func TestMapForeachOnNil(t *testing.T) {
vin := dyn.NilValue
// Check that if foreach is applied to nil, it returns nil.
vout, err := dyn.Map(vin, ".", dyn.Foreach(func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
return dyn.InvalidValue, nil
}))
assert.NoError(t, err)
assert.Equal(t, dyn.NilValue, vout)
}
func TestMapByPatternOnNilValue(t *testing.T) { func TestMapByPatternOnNilValue(t *testing.T) {
var err error var err error
_, err = dyn.MapByPattern(dyn.NilValue, dyn.NewPattern(dyn.AnyKey()), nil) _, err = dyn.MapByPattern(dyn.NilValue, dyn.NewPattern(dyn.AnyKey()), nil)