Make `Append` function to `dyn.Path` return independent slice (#1295)

## Changes

While working on #1273, I found that calls to `Append` on a
`dyn.Pattern` were mutating the original slice. This is expected because
appending to a slice will mutate in place if the capacity of the
original slice is large enough. This change updates the `Append` call on
the `dyn.Path` as well to return a newly allocated slice to avoid
inadvertently mutating the originals.

We have existing call sites in the `dyn` package that mutate a
`dyn.Path` (e.g. walk or visit) and these are modified to continue to do
this with a direct call to `append`. Callbacks that use the `dyn.Path`
argument outside of the callback need to make a copy to ensure it isn't
mutated (this is no different from existing semantics).

The `Join` function wasn't used and is removed as part of this change.

## Tests

Unit tests.
This commit is contained in:
Pieter Noordhuis 2024-03-19 10:49:26 +01:00 committed by GitHub
parent de89af6f8c
commit 8255c9d9fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 31 additions and 30 deletions

View File

@ -49,17 +49,13 @@ func NewPath(cs ...pathComponent) Path {
return cs
}
// Join joins the given paths.
func (p Path) Join(qs ...Path) Path {
for _, q := range qs {
p = p.Append(q...)
}
return p
}
// 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 {
return append(p, cs...)
out := make(Path, len(p)+len(cs))
copy(out, p)
copy(out[len(p):], cs)
return out
}
// Equal returns true if the paths are equal.

View File

@ -19,16 +19,14 @@ func TestPathAppend(t *testing.T) {
assert.True(t, p2.Equal(dyn.NewPath(dyn.Key("foo"), dyn.Key("bar"), dyn.Index(1))))
}
func TestPathJoin(t *testing.T) {
p := dyn.NewPath(dyn.Key("foo"))
func TestPathAppendAlwaysNew(t *testing.T) {
p := make(dyn.Path, 0, 2)
p = append(p, dyn.Key("foo"))
// Single arg.
p1 := p.Join(dyn.NewPath(dyn.Key("bar")))
assert.True(t, p1.Equal(dyn.NewPath(dyn.Key("foo"), dyn.Key("bar"))))
// Multiple args.
p2 := p.Join(dyn.NewPath(dyn.Key("bar")), dyn.NewPath(dyn.Index(1)))
assert.True(t, p2.Equal(dyn.NewPath(dyn.Key("foo"), dyn.Key("bar"), dyn.Index(1))))
// There is room for a second element in the slice.
p1 := p.Append(dyn.Index(1))
p2 := p.Append(dyn.Index(2))
assert.NotEqual(t, p1, p2)
}
func TestPathEqualEmpty(t *testing.T) {

View File

@ -58,7 +58,7 @@ func (c anyKeyComponent) visit(v Value, prefix Path, suffix Pattern, opts visitO
m = maps.Clone(m)
for key, value := range m {
var err error
nv, err := visit(value, prefix.Append(Key(key)), suffix, opts)
nv, err := visit(value, append(prefix, Key(key)), suffix, opts)
if err != nil {
// Leave the value intact if the suffix pattern didn't match any value.
if IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) {
@ -89,7 +89,7 @@ func (c anyIndexComponent) visit(v Value, prefix Path, suffix Pattern, opts visi
s = slices.Clone(s)
for i, value := range s {
var err error
nv, err := visit(value, prefix.Append(Index(i)), suffix, opts)
nv, err := visit(value, append(prefix, Index(i)), suffix, opts)
if err != nil {
// Leave the value intact if the suffix pattern didn't match any value.
if IsNoSuchKeyError(err) || IsIndexOutOfBoundsError(err) {

View File

@ -28,13 +28,20 @@ func TestNewPatternFromPath(t *testing.T) {
}
func TestPatternAppend(t *testing.T) {
p1 := dyn.NewPattern(dyn.Key("foo"), dyn.Index(1))
p2 := dyn.NewPattern(dyn.Key("foo")).Append(dyn.Index(1))
assert.Equal(t, p1, p2)
p := dyn.NewPattern(dyn.Key("foo"))
// Single arg.
p1 := p.Append(dyn.Key("bar"))
assert.Equal(t, dyn.NewPattern(dyn.Key("foo"), dyn.Key("bar")), p1)
// Multiple args.
p2 := p.Append(dyn.Key("bar"), dyn.Index(1))
assert.Equal(t, dyn.NewPattern(dyn.Key("foo"), dyn.Key("bar"), dyn.Index(1)), p2)
}
func TestPatternAppendAlwaysNew(t *testing.T) {
p := make(dyn.Pattern, 0, 2).Append(dyn.Key("foo"))
p := make(dyn.Pattern, 0, 2)
p = append(p, dyn.Key("foo"))
// There is room for a second element in the slice.
p1 := p.Append(dyn.Index(1))

View File

@ -66,7 +66,7 @@ func visit(v Value, prefix Path, suffix Pattern, opts visitOptions) (Value, erro
}
func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts visitOptions) (Value, error) {
path := prefix.Append(component)
path := append(prefix, component)
switch {
case component.isKey():

View File

@ -18,7 +18,7 @@ func Foreach(fn MapFunc) MapFunc {
m := maps.Clone(v.MustMap())
for key, value := range m {
var err error
m[key], err = fn(p.Append(Key(key)), value)
m[key], err = fn(append(p, Key(key)), value)
if err != nil {
return InvalidValue, err
}
@ -28,7 +28,7 @@ func Foreach(fn MapFunc) MapFunc {
s := slices.Clone(v.MustSequence())
for i, value := range s {
var err error
s[i], err = fn(p.Append(Index(i)), value)
s[i], err = fn(append(p, Index(i)), value)
if err != nil {
return InvalidValue, err
}

View File

@ -30,7 +30,7 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) {
return visit(v, EmptyPath, NewPatternFromPath(p), visitOptions{
fn: func(prefix Path, v Value) (Value, error) {
path := prefix.Append(component)
path := append(prefix, component)
switch {
case component.isKey():

View File

@ -36,7 +36,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
m := v.MustMap()
out := make(map[string]Value, len(m))
for k := range m {
nv, err := walk(m[k], p.Append(Key(k)), fn)
nv, err := walk(m[k], append(p, Key(k)), fn)
if err == ErrDrop {
continue
}
@ -50,7 +50,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro
s := v.MustSequence()
out := make([]Value, 0, len(s))
for i := range s {
nv, err := walk(s[i], p.Append(Index(i)), fn)
nv, err := walk(s[i], append(p, Index(i)), fn)
if err == ErrDrop {
continue
}