diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c811667..9667554f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ +# 1.7.0 (Unreleased) + +* `cty`: `Value.UnmarkDeepWithPaths` and `Value.MarkWithPaths` are like `Value.UnmarkDeep` and `Value.Mark` but they retain path information for each marked value, so that marks can be re-applied later without all the loss of detail that results from `Value.UnmarkDeep` aggregating together all of the nested marks. +* `function`: Unless a parameter has `AllowMarks: true` explicitly set, the functions infrastructure will now guarantee that it never sees a marked value even if the mark is deep inside a data structure. Previously that guarantee was only shallow for the top-level value, similar to `AllowUnknown`, but because marks are a relatively new addition to `cty` and numerous existing functions are not written to deal with them this is the more conservative and robust default. ([#72](https://github.com/zclconf/go-cty/pull/72)) +* `function/stdlib`: The `formatdate` function was not correctly handling literal sequences at the end of the format string. It will now handle those as intended. ([#69](https://github.com/zclconf/go-cty/pull/69)) + # 1.6.1 (Unreleased) -* Fix a regression from 1.6.0 where `Value.RawEqual` no longer returned the correct result given a pair of sets containing partially-unknown values. ([#64](https://github.com/zclconf/go-cty/pull/64)) +* `cty`:: Fix a regression from 1.6.0 where `Value.RawEqual` no longer returned the correct result given a pair of sets containing partially-unknown values. ([#64](https://github.com/zclconf/go-cty/pull/64)) # 1.6.0 (Unreleased) diff --git a/cty/function/function.go b/cty/function/function.go index bc5e688a..8227a405 100644 --- a/cty/function/function.go +++ b/cty/function/function.go @@ -244,19 +244,21 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) { return cty.UnknownVal(expectedType), nil } - if val.IsMarked() && !spec.AllowMarked { - unwrappedVal, marks := val.Unmark() - // In order to avoid additional overhead on applications that - // are not using marked values, we copy the given args only - // if we encounter a marked value we need to unmark. However, - // as a consequence we end up doing redundant copying if multiple - // marked values need to be unwrapped. That seems okay because - // argument lists are generally small. - newArgs := make([]cty.Value, len(args)) - copy(newArgs, args) - newArgs[i] = unwrappedVal - resultMarks = append(resultMarks, marks) - args = newArgs + if !spec.AllowMarked { + unwrappedVal, marks := val.UnmarkDeep() + if len(marks) > 0 { + // In order to avoid additional overhead on applications that + // are not using marked values, we copy the given args only + // if we encounter a marked value we need to unmark. However, + // as a consequence we end up doing redundant copying if multiple + // marked values need to be unwrapped. That seems okay because + // argument lists are generally small. + newArgs := make([]cty.Value, len(args)) + copy(newArgs, args) + newArgs[i] = unwrappedVal + resultMarks = append(resultMarks, marks) + args = newArgs + } } } @@ -266,13 +268,15 @@ func (f Function) Call(args []cty.Value) (val cty.Value, err error) { if !val.IsKnown() && !spec.AllowUnknown { return cty.UnknownVal(expectedType), nil } - if val.IsMarked() && !spec.AllowMarked { - unwrappedVal, marks := val.Unmark() - newArgs := make([]cty.Value, len(args)) - copy(newArgs, args) - newArgs[len(posArgs)+i] = unwrappedVal - resultMarks = append(resultMarks, marks) - args = newArgs + if !spec.AllowMarked { + unwrappedVal, marks := val.UnmarkDeep() + if len(marks) > 0 { + newArgs := make([]cty.Value, len(args)) + copy(newArgs, args) + newArgs[len(posArgs)+i] = unwrappedVal + resultMarks = append(resultMarks, marks) + args = newArgs + } } } } diff --git a/cty/function/stdlib/datetime.go b/cty/function/stdlib/datetime.go index 42248143..cae7323b 100644 --- a/cty/function/stdlib/datetime.go +++ b/cty/function/stdlib/datetime.go @@ -362,9 +362,14 @@ func splitDateFormat(data []byte, atEOF bool) (advance int, token []byte, err er for i := 1; i < len(data); i++ { if data[i] == esc { if (i + 1) == len(data) { - // We need at least one more byte to decide if this is an - // escape or a terminator. - return 0, nil, nil + if atEOF { + // We have a closing quote and are at the end of our input + return len(data), data, nil + } else { + // We need at least one more byte to decide if this is an + // escape or a terminator. + return 0, nil, nil + } } if data[i+1] == esc { i++ // doubled-up quotes are an escape sequence diff --git a/cty/function/stdlib/datetime_test.go b/cty/function/stdlib/datetime_test.go index 3e88cf40..509ce85b 100644 --- a/cty/function/stdlib/datetime_test.go +++ b/cty/function/stdlib/datetime_test.go @@ -39,6 +39,11 @@ func TestFormatDate(t *testing.T) { cty.StringVal("3 o'clock PM"), ``, }, + { + cty.StringVal("H 'o''clock'"), + cty.StringVal("3 o'clock"), + ``, + }, { cty.StringVal("hh:mm:ssZZZZ"), cty.StringVal("15:04:05+0000"), diff --git a/cty/function/stdlib/string_test.go b/cty/function/stdlib/string_test.go index 6e78df80..95351a59 100644 --- a/cty/function/stdlib/string_test.go +++ b/cty/function/stdlib/string_test.go @@ -393,3 +393,83 @@ func TestSubstr(t *testing.T) { }) } } + +func TestJoin(t *testing.T) { + tests := map[string]struct { + Separator cty.Value + Lists []cty.Value + Want cty.Value + }{ + "single two-element list": { + cty.StringVal("-"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}), + }, + cty.StringVal("hello-world"), + }, + "multiple single-element lists": { + cty.StringVal("-"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("chicken")}), + cty.ListVal([]cty.Value{cty.StringVal("egg")}), + }, + cty.StringVal("chicken-egg"), + }, + "single single-element list": { + cty.StringVal("-"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("chicken")}), + }, + cty.StringVal("chicken"), + }, + "blank separator": { + cty.StringVal(""), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("horse"), cty.StringVal("face")}), + }, + cty.StringVal("horseface"), + }, + "marked list": { + cty.StringVal("-"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}).Mark("sensitive"), + }, + cty.StringVal("hello-world").Mark("sensitive"), + }, + "marked separator": { + cty.StringVal("-").Mark("sensitive"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("hello"), cty.StringVal("world")}), + }, + cty.StringVal("hello-world").Mark("sensitive"), + }, + "list with some marked elements": { + cty.StringVal("-"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("hello").Mark("sensitive"), cty.StringVal("world")}), + }, + cty.StringVal("hello-world").Mark("sensitive"), + }, + "multiple marks": { + cty.StringVal("-").Mark("a"), + []cty.Value{ + cty.ListVal([]cty.Value{cty.StringVal("hello").Mark("b"), cty.StringVal("world").Mark("c")}), + }, + cty.StringVal("hello-world").WithMarks(cty.NewValueMarks("a", "b", "c")), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got, err := Join(test.Separator, test.Lists...) + + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if !got.RawEquals(test.Want) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want) + } + }) + } +} diff --git a/cty/json/marshal.go b/cty/json/marshal.go index 728ab010..30832db4 100644 --- a/cty/json/marshal.go +++ b/cty/json/marshal.go @@ -10,7 +10,7 @@ import ( func marshal(val cty.Value, t cty.Type, path cty.Path, b *bytes.Buffer) error { if val.IsMarked() { - return path.NewErrorf("value has marks, so it cannot be seralized") + return path.NewErrorf("value has marks, so it cannot be serialized as JSON") } // If we're going to decode as DynamicPseudoType then we need to save diff --git a/cty/marks.go b/cty/marks.go index 3898e455..4e1d3b12 100644 --- a/cty/marks.go +++ b/cty/marks.go @@ -67,6 +67,23 @@ func (m ValueMarks) GoString() string { return s.String() } +// PathValueMarks is a structure that enables tracking marks +// and the paths where they are located in one type +type PathValueMarks struct { + Path Path + Marks ValueMarks +} + +func (p PathValueMarks) Equal(o PathValueMarks) bool { + if !p.Path.Equals(o.Path) { + return false + } + if !p.Marks.Equal(o.Marks) { + return false + } + return true +} + // IsMarked returns true if and only if the receiving value carries at least // one mark. A marked value cannot be used directly with integration methods // without explicitly unmarking it (and retrieving the markings) first. @@ -174,6 +191,31 @@ func (val Value) Mark(mark interface{}) Value { } } +type applyPathValueMarksTransformer struct { + pvm []PathValueMarks +} + +func (t *applyPathValueMarksTransformer) Enter(p Path, v Value) (Value, error) { + return v, nil +} + +func (t *applyPathValueMarksTransformer) Exit(p Path, v Value) (Value, error) { + for _, path := range t.pvm { + if p.Equals(path.Path) { + return v.WithMarks(path.Marks), nil + } + } + return v, nil +} + +// MarkWithPaths accepts a slice of PathValueMarks to apply +// markers to particular paths and returns the marked +// Value. +func (val Value) MarkWithPaths(pvm []PathValueMarks) Value { + ret, _ := TransformWithTransformer(val, &applyPathValueMarksTransformer{pvm}) + return ret +} + // Unmark separates the marks of the receiving value from the value itself, // removing a new unmarked value and a map (representing a set) of the marks. // @@ -191,6 +233,24 @@ func (val Value) Unmark() (Value, ValueMarks) { }, marks } +type unmarkTransformer struct { + pvm []PathValueMarks +} + +func (t *unmarkTransformer) Enter(p Path, v Value) (Value, error) { + unmarkedVal, marks := v.Unmark() + if len(marks) > 0 { + path := make(Path, len(p), len(p)+1) + copy(path, p) + t.pvm = append(t.pvm, PathValueMarks{path, marks}) + } + return unmarkedVal, nil +} + +func (t *unmarkTransformer) Exit(p Path, v Value) (Value, error) { + return v, nil +} + // UnmarkDeep is similar to Unmark, but it works with an entire nested structure // rather than just the given value directly. // @@ -198,17 +258,29 @@ func (val Value) Unmark() (Value, ValueMarks) { // the returned marks set includes the superset of all of the marks encountered // during the operation. func (val Value) UnmarkDeep() (Value, ValueMarks) { + t := unmarkTransformer{} + ret, _ := TransformWithTransformer(val, &t) + marks := make(ValueMarks) - ret, _ := Transform(val, func(_ Path, v Value) (Value, error) { - unmarkedV, valueMarks := v.Unmark() - for m, s := range valueMarks { + for _, pvm := range t.pvm { + for m, s := range pvm.Marks { marks[m] = s } - return unmarkedV, nil - }) + } + return ret, marks } +// UnmarkDeepWithPaths is like UnmarkDeep, except it returns a slice +// of PathValueMarks rather than a superset of all marks. This allows +// a caller to know which marks are associated with which paths +// in the Value. +func (val Value) UnmarkDeepWithPaths() (Value, []PathValueMarks) { + t := unmarkTransformer{} + ret, _ := TransformWithTransformer(val, &t) + return ret, t.pvm +} + func (val Value) unmarkForce() Value { unw, _ := val.Unmark() return unw diff --git a/cty/marks_test.go b/cty/marks_test.go index 56997088..71775573 100644 --- a/cty/marks_test.go +++ b/cty/marks_test.go @@ -1,9 +1,105 @@ package cty import ( + "fmt" "testing" ) +func TestContainsMarked(t *testing.T) { + testCases := []struct { + val Value + want bool + }{ + { + StringVal("a"), + false, + }, + { + NumberIntVal(1).Mark("a"), + true, + }, + { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + false, + }, + { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2).Mark("a")}), + true, + }, + { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}).Mark("a"), + true, + }, + { + ListValEmpty(String).Mark("c"), + true, + }, + { + MapVal(map[string]Value{"a": StringVal("b").Mark("c"), "x": StringVal("y").Mark("z")}), + true, + }, + { + TupleVal([]Value{NumberIntVal(1).Mark("a"), StringVal("y").Mark("z")}), + true, + }, + { + SetVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2).Mark("z")}), + true, + }, + { + ObjectVal(map[string]Value{ + "x": ListVal([]Value{ + NumberIntVal(1).Mark("a"), + NumberIntVal(2), + }), + "y": StringVal("y"), + "z": BoolVal(true), + }), + true, + }, + } + + for _, tc := range testCases { + if got, want := tc.val.ContainsMarked(), tc.want; got != want { + t.Errorf("wrong result (got %v, want %v) for %#v", got, want, tc.val) + } + } +} + +func TestIsMarked(t *testing.T) { + testCases := []struct { + val Value + want bool + }{ + { + StringVal("a"), + false, + }, + { + NumberIntVal(1).Mark("a"), + true, + }, + { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + false, + }, + { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2).Mark("a")}), + false, + }, + { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}).Mark("a"), + true, + }, + } + + for _, tc := range testCases { + if got, want := tc.val.IsMarked(), tc.want; got != want { + t.Errorf("wrong result (got %v, want %v) for %#v", got, want, tc.val) + } + } +} + func TestValueMarks(t *testing.T) { v := True v1 := v.Mark(1) @@ -61,4 +157,326 @@ func TestValueMarks(t *testing.T) { if got, want := result, False.WithMarks(NewValueMarks("a", "b", "c", "d")); !want.RawEquals(got) { t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, want) } + + // Unmark the result and capture the paths + unmarkedResult, pvm := result.UnmarkDeepWithPaths() + // Remark the result with those paths + remarked := unmarkedResult.MarkWithPaths(pvm) + if got, want := remarked, False.WithMarks(NewValueMarks("a", "b", "c", "d")); !want.RawEquals(got) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, want) + } + + // If we call MarkWithPaths without any matching paths, we should get the unmarked result + markedWithNoPaths := unmarkedResult.MarkWithPaths([]PathValueMarks{{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("z")}}) + if got, want := markedWithNoPaths, False; !want.RawEquals(got) { + t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, want) + } +} + +func TestPathValueMarksEqual(t *testing.T) { + tests := []struct { + original PathValueMarks + compare PathValueMarks + want bool + }{ + { + PathValueMarks{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, + PathValueMarks{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, + true, + }, + { + PathValueMarks{Path{IndexStep{Key: StringVal("p")}}, NewValueMarks(123)}, + PathValueMarks{Path{IndexStep{Key: StringVal("p")}}, NewValueMarks(123)}, + true, + }, + { + PathValueMarks{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, + PathValueMarks{Path{IndexStep{Key: NumberIntVal(1)}}, NewValueMarks("a")}, + false, + }, + { + PathValueMarks{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, + PathValueMarks{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("b")}, + false, + }, + { + PathValueMarks{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, + PathValueMarks{Path{IndexStep{Key: NumberIntVal(1)}}, NewValueMarks("b")}, + false, + }, + } + for _, test := range tests { + t.Run(fmt.Sprintf("Comparing %#v to %#v", test.original, test.compare), func(t *testing.T) { + got := test.original.Equal(test.compare) + if got != test.want { + t.Errorf("wrong result\ngot: %v\nwant: %v", got, test.want) + } + }) + } +} + +func TestMarks(t *testing.T) { + wantMarks := func(marks ValueMarks, expected ...string) { + if len(marks) != len(expected) { + t.Fatalf("wrong marks: %#v", marks) + } + for _, mark := range expected { + if _, ok := marks[mark]; !ok { + t.Fatalf("missing mark %q: %#v", mark, marks) + } + } + } + + // Single mark + val := StringVal("foo").Mark("a") + wantMarks(val.Marks(), "a") + val, marks := val.Unmark() + if val.IsMarked() { + t.Fatalf("still marked after unmark: %#v", marks) + } + wantMarks(marks, "a") + + // Multiple marks + val = val.WithMarks(NewValueMarks("a", "b", "c")) + wantMarks(val.Marks(), "a", "b", "c") + val, marks = val.Unmark() + if val.IsMarked() { + t.Fatalf("still marked after unmark: %#v", marks) + } + wantMarks(marks, "a", "b", "c") +} + +func TestUnmarkDeep(t *testing.T) { + testCases := map[string]struct { + val Value + want Value + marks ValueMarks + }{ + "unmarked string": { + StringVal("a"), + StringVal("a"), + NewValueMarks(), + }, + "marked number": { + NumberIntVal(1).Mark("a"), + NumberIntVal(1), + NewValueMarks("a"), + }, + "unmarked list": { + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + NewValueMarks(), + }, + "list with some elements marked": { + ListVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2)}), + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + NewValueMarks("a"), + }, + "marked list with all elements marked": { + ListVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2).Mark("b")}).Mark("c"), + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + NewValueMarks("a", "b", "c"), + }, + "marked empty list": { + ListValEmpty(String).Mark("c"), + ListValEmpty(String), + NewValueMarks("c"), + }, + "map with elements marked": { + MapVal(map[string]Value{"a": StringVal("b").Mark("c"), "x": StringVal("y").Mark("z")}), + MapVal(map[string]Value{"a": StringVal("b"), "x": StringVal("y")}), + NewValueMarks("c", "z"), + }, + "tuple with elements marked": { + TupleVal([]Value{NumberIntVal(1).Mark("a"), StringVal("y").Mark("z")}), + TupleVal([]Value{NumberIntVal(1), StringVal("y")}), + NewValueMarks("a", "z"), + }, + "set with elements marked": { + SetVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2).Mark("z")}), + SetVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + NewValueMarks("a", "z"), + }, + "complex marked object with lots of marks": { + ObjectVal(map[string]Value{ + "x": ListVal([]Value{ + NumberIntVal(3).Mark("a"), + NumberIntVal(5).Mark("b"), + }).WithMarks(NewValueMarks("c", "d")), + "y": StringVal("y").Mark("e"), + "z": BoolVal(true).Mark("f"), + }).Mark("g"), + ObjectVal(map[string]Value{ + "x": ListVal([]Value{ + NumberIntVal(3), + NumberIntVal(5), + }), + "y": StringVal("y"), + "z": BoolVal(true), + }), + NewValueMarks("a", "b", "c", "d", "e", "f", "g"), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, marks := tc.val.UnmarkDeep() + if !got.RawEquals(tc.want) { + t.Errorf("wrong value\n got: %#v\nwant: %#v", got, tc.want) + } + if !marks.Equal(tc.marks) { + t.Errorf("wrong marks\n got: %#v\nwant: %#v", got, tc.want) + } + }) + } +} + +func TestPathValueMarks(t *testing.T) { + testCases := map[string]struct { + marked Value + unmarked Value + pvms []PathValueMarks + }{ + "unmarked string": { + StringVal("a"), + StringVal("a"), + nil, + }, + "marked number": { + NumberIntVal(1).Mark("a"), + NumberIntVal(1), + []PathValueMarks{ + {Path{}, NewValueMarks("a")}, + }, + }, + "list with some elements marked": { + ListVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2)}), + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + []PathValueMarks{ + {IndexIntPath(0), NewValueMarks("a")}, + }, + }, + "marked list with all elements marked": { + ListVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2).Mark("b")}).Mark("c"), + ListVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + []PathValueMarks{ + {Path{}, NewValueMarks("c")}, + {IndexIntPath(0), NewValueMarks("a")}, + {IndexIntPath(1), NewValueMarks("b")}, + }, + }, + "marked empty list": { + ListValEmpty(String).Mark("c"), + ListValEmpty(String), + []PathValueMarks{ + {Path{}, NewValueMarks("c")}, + }, + }, + "map with elements marked": { + MapVal(map[string]Value{"a": StringVal("b").Mark("c"), "x": StringVal("y").Mark("z")}), + MapVal(map[string]Value{"a": StringVal("b"), "x": StringVal("y")}), + []PathValueMarks{ + {IndexStringPath("a"), NewValueMarks("c")}, + {IndexStringPath("x"), NewValueMarks("z")}, + }, + }, + "tuple with elements marked": { + TupleVal([]Value{NumberIntVal(1).Mark("a"), StringVal("y").Mark("z"), ObjectVal(map[string]Value{"x": True}).Mark("o")}), + TupleVal([]Value{NumberIntVal(1), StringVal("y"), ObjectVal(map[string]Value{"x": True})}), + []PathValueMarks{ + {IndexIntPath(0), NewValueMarks("a")}, + {IndexIntPath(1), NewValueMarks("z")}, + {IndexIntPath(2), NewValueMarks("o")}, + }, + }, + "set with elements marked": { + SetVal([]Value{NumberIntVal(1).Mark("a"), NumberIntVal(2).Mark("z")}), + SetVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + []PathValueMarks{ + {Path{}, NewValueMarks("a", "z")}, + }, + }, + "complex marked object with lots of marks": { + ObjectVal(map[string]Value{ + "x": ListVal([]Value{ + NumberIntVal(3).Mark("a"), + NumberIntVal(5).Mark("b"), + }).WithMarks(NewValueMarks("c", "d")), + "y": StringVal("y").Mark("e"), + "z": BoolVal(true).Mark("f"), + }).Mark("g"), + ObjectVal(map[string]Value{ + "x": ListVal([]Value{ + NumberIntVal(3), + NumberIntVal(5), + }), + "y": StringVal("y"), + "z": BoolVal(true), + }), + []PathValueMarks{ + {Path{}, NewValueMarks("g")}, + {GetAttrPath("x"), NewValueMarks("c", "d")}, + {GetAttrPath("x").IndexInt(0), NewValueMarks("a")}, + {GetAttrPath("x").IndexInt(1), NewValueMarks("b")}, + {GetAttrPath("y"), NewValueMarks("e")}, + {GetAttrPath("z"), NewValueMarks("f")}, + }, + }, + "path array reuse regression test": { + ObjectVal(map[string]Value{ + "environment": ListVal([]Value{ + ObjectVal(map[string]Value{ + "variables": MapVal(map[string]Value{ + "bar": StringVal("secret").Mark("sensitive"), + "foo": StringVal("secret").Mark("sensitive"), + }), + }), + }), + }), + ObjectVal(map[string]Value{ + "environment": ListVal([]Value{ + ObjectVal(map[string]Value{ + "variables": MapVal(map[string]Value{ + "bar": StringVal("secret"), + "foo": StringVal("secret"), + }), + }), + }), + }), + []PathValueMarks{ + {GetAttrPath("environment").IndexInt(0).GetAttr("variables").IndexString("bar"), NewValueMarks("sensitive")}, + {GetAttrPath("environment").IndexInt(0).GetAttr("variables").IndexString("foo"), NewValueMarks("sensitive")}, + }, + }, + } + + for name, tc := range testCases { + t.Run(fmt.Sprintf("unmark: %s", name), func(t *testing.T) { + got, pvms := tc.marked.UnmarkDeepWithPaths() + if !got.RawEquals(tc.unmarked) { + t.Errorf("wrong value\n got: %#v\nwant: %#v", got, tc.unmarked) + } + + if len(pvms) != len(tc.pvms) { + t.Errorf("wrong length\n got: %d\nwant: %d", len(pvms), len(tc.pvms)) + } + + findPvm: + for _, wantPvm := range tc.pvms { + for _, gotPvm := range pvms { + if gotPvm.Path.Equals(wantPvm.Path) && gotPvm.Marks.Equal(wantPvm.Marks) { + continue findPvm + } + } + t.Errorf("missing %#v\nnot found in: %#v", wantPvm, pvms) + } + }) + + t.Run(fmt.Sprintf("mark: %s", name), func(t *testing.T) { + got := tc.unmarked.MarkWithPaths(tc.pvms) + if !got.RawEquals(tc.marked) { + t.Errorf("wrong value\n got: %#v\nwant: %#v", got, tc.marked) + } + }) + } } diff --git a/cty/msgpack/marshal.go b/cty/msgpack/marshal.go index 91301c16..419bc0db 100644 --- a/cty/msgpack/marshal.go +++ b/cty/msgpack/marshal.go @@ -43,7 +43,7 @@ func Marshal(val cty.Value, ty cty.Type) ([]byte, error) { func marshal(val cty.Value, ty cty.Type, path cty.Path, enc *msgpack.Encoder) error { if val.IsMarked() { - return path.NewErrorf("value has marks, so it cannot be seralized") + return path.NewErrorf("value has marks, so it cannot be serialized") } // If we're going to decode as DynamicPseudoType then we need to save diff --git a/cty/value_init.go b/cty/value_init.go index 853a5a7d..3a8b3056 100644 --- a/cty/value_init.go +++ b/cty/value_init.go @@ -247,11 +247,6 @@ func SetVal(vals []Value) Value { val = unmarkedVal markSets = append(markSets, marks) } - if val.ContainsMarked() { - // FIXME: Allow this, but unmark the values and apply the - // marking to the set itself instead. - panic("set cannot contain marked values") - } if elementType == DynamicPseudoType { elementType = val.ty } else if val.ty != DynamicPseudoType && !elementType.Equals(val.ty) { diff --git a/cty/walk.go b/cty/walk.go index a6943bab..d17f48cc 100644 --- a/cty/walk.go +++ b/cty/walk.go @@ -61,6 +61,34 @@ func walk(path Path, val Value, cb func(Path, Value) (bool, error)) error { return nil } +// Transformer is the interface used to optionally transform values in a +// possibly-complex structure. The Enter method is called before traversing +// through a given path, and the Exit method is called when traversal of a +// path is complete. +// +// Use Enter when you want to transform a complex value before traversal +// (preorder), and Exit when you want to transform a value after traversal +// (postorder). +// +// The path passed to the given function may not be used after that function +// returns, since its backing array is re-used for other calls. +type Transformer interface { + Enter(Path, Value) (Value, error) + Exit(Path, Value) (Value, error) +} + +type postorderTransformer struct { + callback func(Path, Value) (Value, error) +} + +func (t *postorderTransformer) Enter(p Path, v Value) (Value, error) { + return v, nil +} + +func (t *postorderTransformer) Exit(p Path, v Value) (Value, error) { + return t.callback(p, v) +} + // Transform visits all of the values in a possibly-complex structure, // calling a given function for each value which has an opportunity to // replace that value. @@ -77,7 +105,7 @@ func walk(path Path, val Value, cb func(Path, Value) (bool, error)) error { // value constructor functions. An easy way to preserve invariants is to // ensure that the transform function never changes the value type. // -// The callback function my halt the walk altogether by +// The callback function may halt the walk altogether by // returning a non-nil error. If the returned error is about the element // currently being visited, it is recommended to use the provided path // value to produce a PathError describing that context. @@ -86,10 +114,23 @@ func walk(path Path, val Value, cb func(Path, Value) (bool, error)) error { // returns, since its backing array is re-used for other calls. func Transform(val Value, cb func(Path, Value) (Value, error)) (Value, error) { var path Path - return transform(path, val, cb) + return transform(path, val, &postorderTransformer{cb}) +} + +// TransformWithTransformer allows the caller to more closely control the +// traversal used for transformation. See the documentation for Transformer for +// more details. +func TransformWithTransformer(val Value, t Transformer) (Value, error) { + var path Path + return transform(path, val, t) } -func transform(path Path, val Value, cb func(Path, Value) (Value, error)) (Value, error) { +func transform(path Path, val Value, t Transformer) (Value, error) { + val, err := t.Enter(path, val) + if err != nil { + return DynamicVal, err + } + ty := val.Type() var newVal Value @@ -112,7 +153,7 @@ func transform(path Path, val Value, cb func(Path, Value) (Value, error)) (Value path := append(path, IndexStep{ Key: kv, }) - newEv, err := transform(path, ev, cb) + newEv, err := transform(path, ev, t) if err != nil { return DynamicVal, err } @@ -143,7 +184,7 @@ func transform(path Path, val Value, cb func(Path, Value) (Value, error)) (Value path := append(path, IndexStep{ Key: kv, }) - newEv, err := transform(path, ev, cb) + newEv, err := transform(path, ev, t) if err != nil { return DynamicVal, err } @@ -165,7 +206,7 @@ func transform(path Path, val Value, cb func(Path, Value) (Value, error)) (Value path := append(path, GetAttrStep{ Name: name, }) - newAV, err := transform(path, av, cb) + newAV, err := transform(path, av, t) if err != nil { return DynamicVal, err } @@ -178,5 +219,9 @@ func transform(path Path, val Value, cb func(Path, Value) (Value, error)) (Value newVal = val } - return cb(path, newVal) + newVal, err = t.Exit(path, newVal) + if err != nil { + return DynamicVal, err + } + return newVal, err } diff --git a/cty/walk_test.go b/cty/walk_test.go index 0a6882c8..8da00e3e 100644 --- a/cty/walk_test.go +++ b/cty/walk_test.go @@ -80,7 +80,20 @@ func TestWalk(t *testing.T) { } } -func TestTransform(t *testing.T) { +type pathTransformer struct{} + +func (pathTransformer) Enter(p Path, v Value) (Value, error) { + return v, nil +} + +func (pathTransformer) Exit(p Path, v Value) (Value, error) { + if v.Type().IsPrimitiveType() { + return StringVal(fmt.Sprintf("%#v", p)), nil + } + return v, nil +} + +func TestTransformWithTransformer(t *testing.T) { val := ObjectVal(map[string]Value{ "string": StringVal("hello"), "number": NumberIntVal(10), @@ -101,12 +114,7 @@ func TestTransform(t *testing.T) { "unknown_map": UnknownVal(Map(Bool)), }) - gotVal, err := Transform(val, func(path Path, v Value) (Value, error) { - if v.Type().IsPrimitiveType() { - return StringVal(fmt.Sprintf("%#v", path)), nil - } - return v, nil - }) + gotVal, err := TransformWithTransformer(val, pathTransformer{}) if err != nil { t.Fatalf("unexpected error: %s", err) } @@ -149,3 +157,77 @@ func TestTransform(t *testing.T) { } } } + +type errorTransformer struct{} + +func (errorTransformer) Enter(p Path, v Value) (Value, error) { + return v, nil +} + +func (errorTransformer) Exit(p Path, v Value) (Value, error) { + ty := v.Type() + if ty.IsPrimitiveType() { + return v, nil + } + return v, p.NewError(fmt.Errorf("expected primitive type, was %#v", ty)) +} + +func TestTransformWithTransformer_error(t *testing.T) { + val := ObjectVal(map[string]Value{ + "string": StringVal("hello"), + "number": NumberIntVal(10), + "bool": True, + "list": ListVal([]Value{True}), + }) + + gotVal, err := TransformWithTransformer(val, errorTransformer{}) + if gotVal != DynamicVal { + t.Fatalf("expected DynamicVal, got %#v", gotVal) + } + if err == nil { + t.Fatal("expected error, got nil") + } + + pathError, ok := err.(PathError) + if !ok { + t.Fatalf("expected PathError, got %#v", err) + } + + if got, want := pathError.Path, GetAttrPath("list"); !got.Equals(want) { + t.Errorf("wrong path\n got: %#v\nwant: %#v", got, want) + } +} + +func TestTransform(t *testing.T) { + val := ObjectVal(map[string]Value{ + "list": ListVal([]Value{True, True, False}), + "set": SetVal([]Value{True, False}), + "map": MapVal(map[string]Value{"a": True, "b": False}), + "object": ObjectVal(map[string]Value{ + "a": True, + "b": ListVal([]Value{False, False, False}), + }), + }) + wantVal := ObjectVal(map[string]Value{ + "list": ListVal([]Value{False, False, True}), + "set": SetVal([]Value{True, False}), + "map": MapVal(map[string]Value{"a": False, "b": True}), + "object": ObjectVal(map[string]Value{ + "a": False, + "b": ListVal([]Value{True, True, True}), + }), + }) + + gotVal, err := Transform(val, func(p Path, v Value) (Value, error) { + if v.Type().Equals(Bool) { + return v.Not(), nil + } + return v, nil + }) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if !gotVal.RawEquals(wantVal) { + t.Fatalf("wrong value\n got: %#v\nwant: %#v", gotVal, wantVal) + } +}