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.

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

Track multiple locations associated with a dyn.Value #1510

Merged
merged 24 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion bundle/config/generate/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func ConvertJobToValue(job *jobs.Job) (dyn.Value, error) {
tasks = append(tasks, v)
}
// We're using location lines to define the order of keys in exported YAML.
value["tasks"] = dyn.NewValue(tasks, dyn.Location{Line: jobOrder.Get("tasks")})
value["tasks"] = dyn.NewValue(tasks, []dyn.Location{{Line: jobOrder.Get("tasks")}})
}

return yamlsaver.ConvertToMapValue(job.Settings, jobOrder, []string{"format", "new_cluster", "existing_cluster_id"}, value)
Expand Down
4 changes: 2 additions & 2 deletions bundle/config/mutator/expand_pipeline_glob_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (m *expandPipelineGlobPaths) expandLibrary(v dyn.Value) ([]dyn.Value, error
if err != nil {
return nil, err
}
nv, err := dyn.SetByPath(v, p, dyn.NewValue(m, pv.Location()))
nv, err := dyn.SetByPath(v, p, dyn.NewValue(m, pv.Locations()))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -90,7 +90,7 @@ func (m *expandPipelineGlobPaths) expandSequence(p dyn.Path, v dyn.Value) (dyn.V
vs = append(vs, v...)
}

return dyn.NewValue(vs, v.Location()), nil
return dyn.NewValue(vs, v.Locations()), nil
}

func (m *expandPipelineGlobPaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
Expand Down
20 changes: 9 additions & 11 deletions bundle/config/mutator/python/python_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ type createOverrideVisitorTestCase struct {
}

func TestCreateOverrideVisitor(t *testing.T) {
left := dyn.NewValue(42, dyn.Location{})
right := dyn.NewValue(1337, dyn.Location{})
left := dyn.V(42)
right := dyn.V(1337)

testCases := []createOverrideVisitorTestCase{
{
Expand Down Expand Up @@ -470,47 +470,45 @@ func TestCreateOverrideVisitor_omitempty(t *testing.T) {
// this is not happening, but adding for completeness
name: "undo delete of empty variables",
path: dyn.MustPathFromString("variables"),
left: dyn.NewValue([]dyn.Value{}, location),
left: dyn.NewValue([]dyn.Value{}, []dyn.Location{location}),
expectedErr: merge.ErrOverrideUndoDelete,
phases: allPhases,
},
{
name: "undo delete of empty job clusters",
path: dyn.MustPathFromString("resources.jobs.job0.job_clusters"),
left: dyn.NewValue([]dyn.Value{}, location),
left: dyn.NewValue([]dyn.Value{}, []dyn.Location{location}),
expectedErr: merge.ErrOverrideUndoDelete,
phases: allPhases,
},
{
name: "allow delete of non-empty job clusters",
path: dyn.MustPathFromString("resources.jobs.job0.job_clusters"),
left: dyn.NewValue([]dyn.Value{dyn.NewValue("abc", location)}, location),
left: dyn.NewValue([]dyn.Value{dyn.NewValue("abc", []dyn.Location{location})}, []dyn.Location{location}),
expectedErr: nil,
// deletions aren't allowed in 'load' phase
phases: []phase{PythonMutatorPhaseInit},
},
{
name: "undo delete of empty tags",
path: dyn.MustPathFromString("resources.jobs.job0.tags"),
left: dyn.NewValue(map[string]dyn.Value{}, location),
left: dyn.NewValue(map[string]dyn.Value{}, []dyn.Location{location}),
expectedErr: merge.ErrOverrideUndoDelete,
phases: allPhases,
},
{
name: "allow delete of non-empty tags",
path: dyn.MustPathFromString("resources.jobs.job0.tags"),
left: dyn.NewValue(
map[string]dyn.Value{"dev": dyn.NewValue("true", location)},
location,
),
left: dyn.NewValue(map[string]dyn.Value{"dev": dyn.NewValue("true", []dyn.Location{location})}, []dyn.Location{location}),

expectedErr: nil,
// deletions aren't allowed in 'load' phase
phases: []phase{PythonMutatorPhaseInit},
},
{
name: "undo delete of nil",
path: dyn.MustPathFromString("resources.jobs.job0.tags"),
left: dyn.NilValue.WithLocation(location),
left: dyn.NilValue.WithLocations([]dyn.Location{location}),
expectedErr: merge.ErrOverrideUndoDelete,
phases: allPhases,
},
Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/rewrite_sync_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (m *rewriteSyncPaths) makeRelativeTo(root string) dyn.MapFunc {
return dyn.InvalidValue, err
}

return dyn.NewValue(filepath.Join(rel, v.MustString()), v.Location()), nil
return dyn.NewValue(filepath.Join(rel, v.MustString()), v.Locations()), nil
}
}

Expand Down
2 changes: 1 addition & 1 deletion bundle/config/mutator/translate_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func (t *translateContext) rewriteValue(p dyn.Path, v dyn.Value, fn rewriteFunc,
return dyn.InvalidValue, err
}

return dyn.NewValue(out, v.Location()), nil
return dyn.NewValue(out, v.Locations()), nil
}

func (t *translateContext) rewriteRelativeTo(p dyn.Path, v dyn.Value, fn rewriteFunc, dir, fallback string) (dyn.Value, error) {
Expand Down
10 changes: 5 additions & 5 deletions bundle/config/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (r *Root) MergeTargetOverrides(name string) error {

// Below, we're setting fields on the bundle key, so make sure it exists.
if root.Get("bundle").Kind() == dyn.KindInvalid {
root, err = dyn.Set(root, "bundle", dyn.NewValue(map[string]dyn.Value{}, dyn.Location{}))
root, err = dyn.Set(root, "bundle", dyn.V(map[string]dyn.Value{}))
if err != nil {
return err
}
Expand All @@ -404,7 +404,7 @@ func (r *Root) MergeTargetOverrides(name string) error {
if v := target.Get("git"); v.Kind() != dyn.KindInvalid {
ref, err := dyn.GetByPath(root, dyn.NewPath(dyn.Key("bundle"), dyn.Key("git")))
if err != nil {
ref = dyn.NewValue(map[string]dyn.Value{}, dyn.Location{})
ref = dyn.V(map[string]dyn.Value{})
}

// Merge the override into the reference.
Expand All @@ -415,7 +415,7 @@ func (r *Root) MergeTargetOverrides(name string) error {

// If the branch was overridden, we need to clear the inferred flag.
if branch := v.Get("branch"); branch.Kind() != dyn.KindInvalid {
out, err = dyn.SetByPath(out, dyn.NewPath(dyn.Key("inferred")), dyn.NewValue(false, dyn.Location{}))
out, err = dyn.SetByPath(out, dyn.NewPath(dyn.Key("inferred")), dyn.V(false))
if err != nil {
return err
}
Expand Down Expand Up @@ -456,7 +456,7 @@ func rewriteShorthands(v dyn.Value) (dyn.Value, error) {
// configuration will convert this to a string if necessary.
return dyn.NewValue(map[string]dyn.Value{
"default": variable,
}, variable.Location()), nil
}, variable.Locations()), nil

case dyn.KindMap, dyn.KindSequence:
// Check if the original definition of variable has a type field.
Expand All @@ -469,7 +469,7 @@ func rewriteShorthands(v dyn.Value) (dyn.Value, error) {
return dyn.NewValue(map[string]dyn.Value{
"type": typeV,
"default": variable,
}, variable.Location()), nil
}, variable.Locations()), nil
}

return variable, nil
Expand Down
4 changes: 2 additions & 2 deletions bundle/internal/bundletest/location.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ func SetLocation(b *bundle.Bundle, prefix string, filePath string) {
return dyn.Walk(root, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
// If the path has the given prefix, set the location.
if p.HasPrefix(start) {
return v.WithLocation(dyn.Location{
return v.WithLocations([]dyn.Location{{
File: filePath,
}), nil
}}), nil
}

// The path is not nested under the given prefix.
Expand Down
4 changes: 2 additions & 2 deletions libs/dyn/convert/from_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
// Dereference pointer if necessary
for srcv.Kind() == reflect.Pointer {
if srcv.IsNil() {
return dyn.NilValue.WithLocation(ref.Location()), nil
return dyn.NilValue.WithLocations(ref.Locations()), nil
}
srcv = srcv.Elem()

Expand Down Expand Up @@ -83,7 +83,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
if err != nil {
return dyn.InvalidValue, err
}
return v.WithLocation(ref.Location()), err
return v.WithLocations(ref.Locations()), err
}

func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
Expand Down
60 changes: 30 additions & 30 deletions libs/dyn/convert/from_typed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,16 @@ func TestFromTypedStructSetFieldsRetainLocation(t *testing.T) {
}

ref := dyn.V(map[string]dyn.Value{
"foo": dyn.NewValue("bar", dyn.Location{File: "foo"}),
"bar": dyn.NewValue("baz", dyn.Location{File: "bar"}),
"foo": dyn.NewValue("bar", []dyn.Location{{File: "foo"}}),
"bar": dyn.NewValue("baz", []dyn.Location{{File: "bar"}}),
})

nv, err := FromTyped(src, ref)
require.NoError(t, err)

// Assert foo and bar have retained their location.
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))
assert.Equal(t, dyn.NewValue("bar", []dyn.Location{{File: "foo"}}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", []dyn.Location{{File: "bar"}}), nv.Get("bar"))
}

func TestFromTypedStringMapWithZeroValue(t *testing.T) {
Expand Down Expand Up @@ -359,16 +359,16 @@ func TestFromTypedMapNonEmptyRetainLocation(t *testing.T) {
}

ref := dyn.V(map[string]dyn.Value{
"foo": dyn.NewValue("bar", dyn.Location{File: "foo"}),
"bar": dyn.NewValue("baz", dyn.Location{File: "bar"}),
"foo": dyn.NewValue("bar", []dyn.Location{{File: "foo"}}),
"bar": dyn.NewValue("baz", []dyn.Location{{File: "bar"}}),
})

nv, err := FromTyped(src, ref)
require.NoError(t, err)

// Assert foo and bar have retained their locations.
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", dyn.Location{File: "bar"}), nv.Get("bar"))
assert.Equal(t, dyn.NewValue("bar", []dyn.Location{{File: "foo"}}), nv.Get("foo"))
assert.Equal(t, dyn.NewValue("qux", []dyn.Location{{File: "bar"}}), nv.Get("bar"))
}

func TestFromTypedMapFieldWithZeroValue(t *testing.T) {
Expand Down Expand Up @@ -432,16 +432,16 @@ func TestFromTypedSliceNonEmptyRetainLocation(t *testing.T) {
}

ref := dyn.V([]dyn.Value{
dyn.NewValue("foo", dyn.Location{File: "foo"}),
dyn.NewValue("bar", dyn.Location{File: "bar"}),
dyn.NewValue("foo", []dyn.Location{{File: "foo"}}),
dyn.NewValue("bar", []dyn.Location{{File: "bar"}}),
})

nv, err := FromTyped(src, ref)
require.NoError(t, err)

// Assert foo and bar have retained their locations.
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv.Index(0))
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "bar"}), nv.Index(1))
assert.Equal(t, dyn.NewValue("foo", []dyn.Location{{File: "foo"}}), nv.Index(0))
assert.Equal(t, dyn.NewValue("bar", []dyn.Location{{File: "bar"}}), nv.Index(1))
}

func TestFromTypedStringEmpty(t *testing.T) {
Expand Down Expand Up @@ -477,19 +477,19 @@ func TestFromTypedStringNonEmptyOverwrite(t *testing.T) {
}

func TestFromTypedStringRetainsLocations(t *testing.T) {
var ref = dyn.NewValue("foo", dyn.Location{File: "foo"})
var ref = dyn.NewValue("foo", []dyn.Location{{File: "foo"}})

// case: value has not been changed
var src string = "foo"
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue("foo", dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue("foo", []dyn.Location{{File: "foo"}}), nv)

// case: value has been changed
src = "bar"
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue("bar", dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue("bar", []dyn.Location{{File: "foo"}}), nv)
}

func TestFromTypedStringTypeError(t *testing.T) {
Expand Down Expand Up @@ -532,19 +532,19 @@ func TestFromTypedBoolNonEmptyOverwrite(t *testing.T) {
}

func TestFromTypedBoolRetainsLocations(t *testing.T) {
var ref = dyn.NewValue(true, dyn.Location{File: "foo"})
var ref = dyn.NewValue(true, []dyn.Location{{File: "foo"}})

// case: value has not been changed
var src bool = true
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(true, dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue(true, []dyn.Location{{File: "foo"}}), nv)

// case: value has been changed
src = false
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(false, dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue(false, []dyn.Location{{File: "foo"}}), nv)
}

func TestFromTypedBoolVariableReference(t *testing.T) {
Expand Down Expand Up @@ -595,19 +595,19 @@ func TestFromTypedIntNonEmptyOverwrite(t *testing.T) {
}

func TestFromTypedIntRetainsLocations(t *testing.T) {
var ref = dyn.NewValue(1234, dyn.Location{File: "foo"})
var ref = dyn.NewValue(1234, []dyn.Location{{File: "foo"}})

// case: value has not been changed
var src int = 1234
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1234, dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue(1234, []dyn.Location{{File: "foo"}}), nv)

// case: value has been changed
src = 1235
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(int64(1235), dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue(int64(1235), []dyn.Location{{File: "foo"}}), nv)
}

func TestFromTypedIntVariableReference(t *testing.T) {
Expand Down Expand Up @@ -659,19 +659,19 @@ func TestFromTypedFloatNonEmptyOverwrite(t *testing.T) {

func TestFromTypedFloatRetainsLocations(t *testing.T) {
var src float64
var ref = dyn.NewValue(1.23, dyn.Location{File: "foo"})
var ref = dyn.NewValue(1.23, []dyn.Location{{File: "foo"}})

// case: value has not been changed
src = 1.23
nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1.23, dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue(1.23, []dyn.Location{{File: "foo"}}), nv)

// case: value has been changed
src = 1.24
nv, err = FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(1.24, dyn.Location{File: "foo"}), nv)
assert.Equal(t, dyn.NewValue(1.24, []dyn.Location{{File: "foo"}}), nv)
}

func TestFromTypedFloatVariableReference(t *testing.T) {
Expand Down Expand Up @@ -740,27 +740,27 @@ func TestFromTypedNilPointerRetainsLocations(t *testing.T) {
}

var src *Tmp
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
ref := dyn.NewValue(nil, []dyn.Location{{File: "foobar"}})

nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
assert.Equal(t, dyn.NewValue(nil, []dyn.Location{{File: "foobar"}}), nv)
}

func TestFromTypedNilMapRetainsLocation(t *testing.T) {
var src map[string]string
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
ref := dyn.NewValue(nil, []dyn.Location{{File: "foobar"}})

nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
assert.Equal(t, dyn.NewValue(nil, []dyn.Location{{File: "foobar"}}), nv)
}

func TestFromTypedNilSliceRetainsLocation(t *testing.T) {
var src []string
ref := dyn.NewValue(nil, dyn.Location{File: "foobar"})
ref := dyn.NewValue(nil, []dyn.Location{{File: "foobar"}})

nv, err := FromTyped(src, ref)
require.NoError(t, err)
assert.Equal(t, dyn.NewValue(nil, dyn.Location{File: "foobar"}), nv)
assert.Equal(t, dyn.NewValue(nil, []dyn.Location{{File: "foobar"}}), nv)
}
Loading
Loading