From 93aadf62d0b84dee94620da54a0a8dd7ea2c7ebf Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 30 Aug 2020 18:08:42 -0700 Subject: [PATCH 01/17] Prepare for a potential future 1.6.1 release --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c263472..2a29e9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +# 1.6.1 (Unreleased) + # 1.6.0 (Unreleased) * Fixed various defects in the handling of sets containing unknown values. This will cause unknown values to now be returned in more situations, whereas before `cty` would often return incorrect results when working with sets containing unknown values. The list of defects fixed in this release includes: From 51d8b5af4f6e75a70c8306ae5dc91cc8204c1359 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 2 Sep 2020 14:53:21 -0400 Subject: [PATCH 02/17] build: remove go 1.11 and 1.12 from travis config --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4b48858..1a1eb03 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,7 @@ language: go go: - - 1.11.x - - 1.12.x + - 1.13.x - tip before_install: From 26034786fda72e6b71a2eab1147bc71dcab1ae6b Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 2 Sep 2020 18:36:54 -0400 Subject: [PATCH 03/17] cty: RawEquals correct behavior for sets with partially-unknown values Sets with the same type and same unknown values are equal enough for RawEqual's purposes. This commit takes advantage of the setRules's defined ordering to compare each element of the sets as lists. --- cty/value_ops.go | 40 +-- cty/value_ops_test.go | 783 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 799 insertions(+), 24 deletions(-) diff --git a/cty/value_ops.go b/cty/value_ops.go index c491023..297119b 100644 --- a/cty/value_ops.go +++ b/cty/value_ops.go @@ -465,35 +465,27 @@ func (val Value) RawEquals(other Value) bool { return false case ty.IsSetType(): - s1 := val.v.(set.Set) - s2 := other.v.(set.Set) - - // Raw equality for sets is a little tricky because our rules for a - // set of values say that unknown values are never equal. However, - // if both physical sets have the same length and they have all of - // their _known_ values in common, we know that both sets also have - // the same number of unknown values. - if s1.Length() != s2.Length() { + // Convert the set values into a slice so that we can compare each + // value. This is safe because the underlying sets are ordered (see + // setRules in set_internals.go), and so the results are guaranteed to + // be in a consistent order for two equal sets + setList1 := val.AsValueSlice() + setList2 := other.AsValueSlice() + + // If both physical sets have the same length and they have all of their + // _known_ values in common, we know that both sets also have the same + // number of unknown values. + if len(setList1) != len(setList2) { return false } - for it := s1.Iterator(); it.Next(); { - v := it.Value() - if v == unknown { // "unknown" is the internal representation of unknown-ness - continue - } - if !s2.Has(v) { - return false - } - } - for it := s2.Iterator(); it.Next(); { - v := it.Value() - if v == unknown { // "unknown" is the internal representation of unknown-ness - continue - } - if !s1.Has(v) { + + for i := range setList1 { + eq := setList1[i].RawEquals(setList2[i]) + if !eq { return false } } + // If we got here without returning false already then our sets are // equal enough for RawEquals purposes. return true diff --git a/cty/value_ops_test.go b/cty/value_ops_test.go index 3f9a153..c928055 100644 --- a/cty/value_ops_test.go +++ b/cty/value_ops_test.go @@ -765,6 +765,789 @@ func TestValueEquals(t *testing.T) { } } +func TestValueRawEquals(t *testing.T) { + capsuleA := CapsuleVal(capsuleTestType1, &capsuleTestType1Native{"capsuleA"}) + capsuleB := CapsuleVal(capsuleTestType1, &capsuleTestType1Native{"capsuleB"}) + capsuleC := CapsuleVal(capsuleTestType2, &capsuleTestType2Native{"capsuleC"}) + + tests := []struct { + LHS Value + RHS Value + Expected bool + }{ + // Booleans + { + BoolVal(true), + BoolVal(true), + true, + }, + { + BoolVal(false), + BoolVal(false), + true, + }, + { + BoolVal(true), + BoolVal(false), + false, + }, + + // Numbers + { + NumberIntVal(1), + NumberIntVal(2), + false, + }, + { + NumberIntVal(2), + NumberIntVal(2), + true, + }, + + // Strings + { + StringVal(""), + StringVal(""), + true, + }, + { + StringVal("hello"), + StringVal("hello"), + true, + }, + { + StringVal("hello"), + StringVal("world"), + false, + }, + { + StringVal("0"), + StringVal(""), + false, + }, + { + StringVal("años"), + StringVal("años"), + true, + }, + { + // Combining marks are normalized by StringVal + StringVal("años"), // (precomposed tilde-n) + StringVal("años"), // (combining tilde followed by bare n) + true, + }, + { + // tilde-n does not normalize with bare n + StringVal("años"), + StringVal("anos"), + false, + }, + + // Objects + { + ObjectVal(map[string]Value{}), + ObjectVal(map[string]Value{}), + true, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + }), + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + }), + true, + }, + { + ObjectVal(map[string]Value{ + "h\u00e9llo": NumberIntVal(1), // precombined é + }), + ObjectVal(map[string]Value{ + "he\u0301llo": NumberIntVal(1), // e with combining acute accent + }), + true, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + }), + ObjectVal(map[string]Value{}), + false, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + "flag": BoolVal(true), + }), + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + "flag": BoolVal(true), + }), + true, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + }), + ObjectVal(map[string]Value{ + "num": NumberIntVal(2), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + }), + ObjectVal(map[string]Value{ + "othernum": NumberIntVal(1), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + "flag": BoolVal(true), + }), + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + "flag": BoolVal(true), + }), + ObjectVal(map[string]Value{ + "num": NumberIntVal(1), + "flag": BoolVal(false), + }), + false, + }, + + // Tuples + { + EmptyTupleVal, + EmptyTupleVal, + true, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + TupleVal([]Value{NumberIntVal(1)}), + true, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + TupleVal([]Value{NumberIntVal(2)}), + false, + }, + { + TupleVal([]Value{StringVal("hi")}), + TupleVal([]Value{NumberIntVal(1)}), + false, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + TupleVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + false, + }, + { + TupleVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + TupleVal([]Value{NumberIntVal(1)}), + false, + }, + { + TupleVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + TupleVal([]Value{NumberIntVal(1), NumberIntVal(2)}), + true, + }, + { + TupleVal([]Value{UnknownVal(Number)}), + TupleVal([]Value{NumberIntVal(1)}), + false, + }, + { + TupleVal([]Value{UnknownVal(Number)}), + TupleVal([]Value{UnknownVal(Number)}), + true, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + TupleVal([]Value{UnknownVal(Number)}), + false, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + TupleVal([]Value{DynamicVal}), + false, + }, + { + TupleVal([]Value{DynamicVal}), + TupleVal([]Value{NumberIntVal(1)}), + false, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + UnknownVal(Tuple([]Type{Number})), + false, + }, + { + UnknownVal(Tuple([]Type{Number})), + TupleVal([]Value{NumberIntVal(1)}), + false, + }, + { + DynamicVal, + TupleVal([]Value{NumberIntVal(1)}), + false, + }, + { + TupleVal([]Value{NumberIntVal(1)}), + DynamicVal, + false, + }, + + // Lists + { + ListValEmpty(Number), + ListValEmpty(Number), + true, + }, + { + ListValEmpty(Number), + ListValEmpty(Bool), + false, + }, + { + ListVal([]Value{ + NumberIntVal(1), + }), + ListVal([]Value{ + NumberIntVal(1), + }), + true, + }, + { + ListVal([]Value{ + NumberIntVal(1), + }), + ListValEmpty(String), + false, + }, + { + ListVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + ListVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + true, + }, + { + ListVal([]Value{ + NumberIntVal(1), + }), + ListVal([]Value{ + NumberIntVal(2), + }), + false, + }, + { + ListVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + ListVal([]Value{ + NumberIntVal(1), + }), + false, + }, + { + ListVal([]Value{ + NumberIntVal(1), + }), + ListVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + false, + }, + + // Maps + { + MapValEmpty(Number), + MapValEmpty(Number), + true, + }, + { + MapValEmpty(Number), + MapValEmpty(Bool), + false, + }, + { + MapVal(map[string]Value{ + "num": NumberIntVal(1), + }), + MapVal(map[string]Value{ + "num": NumberIntVal(1), + }), + true, + }, + { + MapVal(map[string]Value{ + "h\u00e9llo": NumberIntVal(1), // precombined é + }), + MapVal(map[string]Value{ + "he\u0301llo": NumberIntVal(1), // e with combining acute accent + }), + true, + }, + { + MapVal(map[string]Value{ + "num": NumberIntVal(1), + }), + MapValEmpty(String), + false, + }, + { + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + "num2": NumberIntVal(2), + }), + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + "num2": NumberIntVal(2), + }), + true, + }, + { + MapVal(map[string]Value{ + "num": NumberIntVal(1), + }), + MapVal(map[string]Value{ + "num": NumberIntVal(2), + }), + false, + }, + { + MapVal(map[string]Value{ + "num": NumberIntVal(1), + }), + MapVal(map[string]Value{ + "othernum": NumberIntVal(1), + }), + false, + }, + { + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + "num2": NumberIntVal(2), + }), + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + }), + false, + }, + { + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + }), + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + "num2": NumberIntVal(2), + }), + false, + }, + { + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + "num2": NumberIntVal(2), + }), + MapVal(map[string]Value{ + "num1": NumberIntVal(1), + "num2": NumberIntVal(3), + }), + false, + }, + + // Sets + { + SetValEmpty(Number), + SetValEmpty(Number), + true, + }, + { + SetValEmpty(Number), + SetValEmpty(Bool), + false, + }, + { + SetVal([]Value{ + NumberIntVal(1), + }), + SetVal([]Value{ + NumberIntVal(1), + }), + true, + }, + { + SetVal([]Value{ + NumberIntVal(1), + }), + SetValEmpty(String), + false, + }, + { + SetVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + SetVal([]Value{ + NumberIntVal(2), + NumberIntVal(1), + }), + true, + }, + { + SetVal([]Value{ + NumberIntVal(1), + }), + SetVal([]Value{ + NumberIntVal(2), + }), + false, + }, + { + SetVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + SetVal([]Value{ + NumberIntVal(1), + }), + false, + }, + { + SetVal([]Value{ + NumberIntVal(1), + }), + SetVal([]Value{ + NumberIntVal(1), + NumberIntVal(2), + }), + false, + }, + + // Capsules + { + capsuleA, + capsuleA, + true, + }, + { + capsuleA, + capsuleB, + false, + }, + { + capsuleA, + capsuleC, + false, + }, + { + capsuleA, + UnknownVal(capsuleTestType1), // same type + false, + }, + { + capsuleA, + UnknownVal(capsuleTestType2), // different type + false, + }, + + // Unknowns and Dynamics + { + NumberIntVal(2), + UnknownVal(Number), + false, + }, + { + NumberIntVal(1), + DynamicVal, + false, + }, + { + DynamicVal, + BoolVal(true), + false, + }, + { + DynamicVal, + DynamicVal, + true, //? + }, + { + ListVal([]Value{ + StringVal("hi"), + DynamicVal, + }), + ListVal([]Value{ + StringVal("hi"), + DynamicVal, + }), + true, + }, + { + ListVal([]Value{ + StringVal("hi"), + UnknownVal(String), + }), + ListVal([]Value{ + StringVal("hi"), + UnknownVal(String), + }), + true, + }, + { + MapVal(map[string]Value{ + "static": StringVal("hi"), + "dynamic": DynamicVal, + }), + MapVal(map[string]Value{ + "static": StringVal("hi"), + "dynamic": DynamicVal, + }), + true, + }, + { + MapVal(map[string]Value{ + "static": StringVal("hi"), + "dynamic": UnknownVal(String), + }), + MapVal(map[string]Value{ + "static": StringVal("hi"), + "dynamic": UnknownVal(String), + }), + true, + }, + + { + NullVal(String), + NullVal(DynamicPseudoType), + false, + }, + { + NullVal(String), + NullVal(String), + true, + }, + { + UnknownVal(String), + UnknownVal(Number), + false, + }, + { + StringVal(""), + NullVal(DynamicPseudoType), + false, + }, + { + StringVal(""), + NullVal(String), + false, + }, + { + StringVal(""), + UnknownVal(String), + false, + }, + { + NullVal(DynamicPseudoType), + NullVal(DynamicPseudoType), + true, + }, + { + NullVal(String), + UnknownVal(Number), + false, // because second operand might eventually be null + }, + { + UnknownVal(String), + NullVal(Number), + false, // because first operand might eventually be null + }, + { + UnknownVal(String), + UnknownVal(Number), + false, // because both operands might eventually be null + }, + { + StringVal("hello"), + UnknownVal(Number), + false, // because no number value -- even null -- can be equal to a non-null string + }, + { + UnknownVal(String), + NumberIntVal(1), + false, // because no string value -- even null -- can be equal to a non-null number + }, + { + ObjectVal(map[string]Value{ + "a": StringVal("a"), + }), + // A null value is always known + ObjectVal(map[string]Value{ + "a": NullVal(DynamicPseudoType), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "a": NullVal(DynamicPseudoType), + }), + ObjectVal(map[string]Value{ + "a": NullVal(DynamicPseudoType), + }), + true, + }, + { + ObjectVal(map[string]Value{ + "a": StringVal("a"), + "b": UnknownVal(Number), + }), + // While we have a dynamic type, the different object types should + // still compare false + ObjectVal(map[string]Value{ + "a": NullVal(DynamicPseudoType), + "c": UnknownVal(Number), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "a": StringVal("a"), + "b": UnknownVal(Number), + }), + // While we have a dynamic type, the different object types should + // still compare false + ObjectVal(map[string]Value{ + "a": DynamicVal, + "c": UnknownVal(Number), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "a": NullVal(DynamicPseudoType), + }), + ObjectVal(map[string]Value{ + "a": DynamicVal, + }), + false, + }, + { + ObjectVal(map[string]Value{ + "a": NullVal(List(String)), + }), + // While the unknown val does contain dynamic types, the overall + // container types can't conform. + ObjectVal(map[string]Value{ + "a": UnknownVal(List(List(DynamicPseudoType))), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "a": NullVal(List(List(String))), + }), + ObjectVal(map[string]Value{ + "a": UnknownVal(List(List(DynamicPseudoType))), + }), + false, + }, + { + ObjectVal(map[string]Value{ + "a": SetVal([]Value{ + ObjectVal(map[string]Value{ + "b": UnknownVal(String), + }), + }), + }), + ObjectVal(map[string]Value{ + "a": SetVal([]Value{ + ObjectVal(map[string]Value{ + "b": UnknownVal(String), + }), + }), + }), + true, + }, + { + ObjectVal(map[string]Value{ + "a": SetVal([]Value{ + ObjectVal(map[string]Value{ + "b": UnknownVal(String), + "c": StringVal("cee"), + }), + }), + }), + ObjectVal(map[string]Value{ + "a": SetVal([]Value{ + ObjectVal(map[string]Value{ + "b": UnknownVal(String), + "c": StringVal("cee"), + }), + }), + }), + true, + }, + { + ObjectVal(map[string]Value{ + "a": SetVal([]Value{ + ObjectVal(map[string]Value{ + "b": UnknownVal(String), + }), + }), + }), + ObjectVal(map[string]Value{ + "a": SetVal([]Value{ + ObjectVal(map[string]Value{ + "c": UnknownVal(String), + }), + }), + }), + false, + }, + // Marks + { + StringVal("a").Mark(1), + StringVal("b"), + false, + }, + { + StringVal("a"), + StringVal("b").Mark(2), + false, + }, + { + StringVal("a").Mark(1), + StringVal("b").Mark(2), + false, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%#v.RawEquals(%#v)", test.LHS, test.RHS), func(t *testing.T) { + got := test.LHS.RawEquals(test.RHS) + if !got == test.Expected { + t.Fatalf("wrong Equals result\ngot: %#v\nwant: %#v", got, test.Expected) + } + }) + } +} + func TestValueAdd(t *testing.T) { tests := []struct { LHS Value From c4ddf7db0334e998caf934f222d0de925d28248c Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 2 Sep 2020 15:38:43 -0700 Subject: [PATCH 04/17] v1.6.1 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a29e9d..6c81166 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # 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)) + # 1.6.0 (Unreleased) * Fixed various defects in the handling of sets containing unknown values. This will cause unknown values to now be returned in more situations, whereas before `cty` would often return incorrect results when working with sets containing unknown values. The list of defects fixed in this release includes: From 667b56a633301e43208ac2355e0a4911d820091d Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 2 Sep 2020 15:40:01 -0700 Subject: [PATCH 05/17] Prepare for an assumed future v1.6.2 release --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c81166..56cd760 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ -# 1.6.1 (Unreleased) +# 1.6.2 (Unreleased) + + +# 1.6.1 (September 2, 2020) * 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)) From c7d1c9590e39c15e8c8010bbff87a4b811225b48 Mon Sep 17 00:00:00 2001 From: Pam Selle Date: Tue, 8 Sep 2020 16:33:16 -0400 Subject: [PATCH 06/17] cty: Utilities to unmark with marks at specific paths This allows a caller to call a method like Unmark, but instead of getting a superset of marks, get an array of marks and their associated paths. This allows for a Value to be unmarked and then remarked later without losing so much detail about the marks. --- cty/json/marshal.go | 2 +- cty/marks.go | 48 +++++++++++++++++++++++ cty/marks_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++ cty/msgpack/marshal.go | 2 +- 4 files changed, 138 insertions(+), 2 deletions(-) diff --git a/cty/json/marshal.go b/cty/json/marshal.go index 728ab01..30832db 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 3898e45..e3e5ca7 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,21 @@ func (val Value) Mark(mark interface{}) Value { } } +// 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, _ := Transform(val, func(p Path, v Value) (Value, error) { + for _, path := range pvm { + if p.Equals(path.Path) { + return v.WithMarks(path.Marks), nil + } + } + return v, nil + }) + 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. // @@ -209,6 +241,22 @@ func (val Value) UnmarkDeep() (Value, ValueMarks) { 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) { + var marks []PathValueMarks + ret, _ := Transform(val, func(p Path, v Value) (Value, error) { + unmarkedV, valueMarks := v.Unmark() + if v.IsMarked() { + marks = append(marks, PathValueMarks{p, valueMarks}) + } + return unmarkedV, nil + }) + return ret, marks +} + func (val Value) unmarkForce() Value { unw, _ := val.Unmark() return unw diff --git a/cty/marks_test.go b/cty/marks_test.go index 5699708..661d2b3 100644 --- a/cty/marks_test.go +++ b/cty/marks_test.go @@ -1,6 +1,7 @@ package cty import ( + "fmt" "testing" ) @@ -61,4 +62,91 @@ 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 TestPathValueMarks(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 TestUnmarkDeep(t *testing.T) { + v := NumberIntVal(1).Mark("a") + v1 := NumberIntVal(2) + l := ListVal([]Value{v, v1}) + if l.IsMarked() { + t.Error("Value containing marks should not be marked itself") + } + if !l.ContainsMarked() { + t.Error("Value containing marks should be caught by ContainsMarked") + } + + l1, marks := l.UnmarkDeep() + if got, want := l1, ListVal([]Value{NumberIntVal(1), v1}); !want.RawEquals(got) { + t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + } + if got, want := marks, NewValueMarks("a"); !want.Equal(got) { + t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + } + + l2, paths := l.UnmarkDeepWithPaths() + if got, want := l2, ListVal([]Value{NumberIntVal(1), v1}); !want.RawEquals(got) { + t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + } + expectedPathValueMarks := []PathValueMarks{{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, {}, {}} + for i, p := range paths { + if got, want := p, expectedPathValueMarks[i]; !want.Equal(got) { + t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + } + } } diff --git a/cty/msgpack/marshal.go b/cty/msgpack/marshal.go index 91301c1..419bc0d 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 From 9a0281a59d25dcef5fc2d1fddafb0aa089360559 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 8 Sep 2020 13:35:37 -0700 Subject: [PATCH 07/17] Update CHANGELOG.md --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cd760..787076b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,10 @@ -# 1.6.2 (Unreleased) +# 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. # 1.6.1 (September 2, 2020) -* 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) From 8577dd31e284c671460be4bdb42e6c3dd0eb8444 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 23 Sep 2020 14:00:24 -0400 Subject: [PATCH 08/17] cty: Fix deep marks functions with new transformer The UnmarkDeep, UnmarkDeepWithPaths, and MarkWithPaths functions could previously panic with nested complex values. This was due to the Transform callback function only being called as part of a postorder traversal, when it is not permitted to traverse a marked value. This commit introduces a new TransformWithTransformer function, requiring an implementation of a new Transformer interface. Using this interface, callers can implement either preorder or postorder traversals. In turn, this allows us to write deep transformation for marks which successfully cope with all nested structures. The commit includes significantly more tests for these functions, which should now cover all complex value types. --- cty/marks.go | 66 ++++++--- cty/marks_test.go | 348 +++++++++++++++++++++++++++++++++++++++++++--- cty/walk.go | 56 +++++++- cty/walk_test.go | 96 ++++++++++++- 4 files changed, 508 insertions(+), 58 deletions(-) diff --git a/cty/marks.go b/cty/marks.go index e3e5ca7..e3eaec3 100644 --- a/cty/marks.go +++ b/cty/marks.go @@ -191,18 +191,28 @@ 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, _ := Transform(val, func(p Path, v Value) (Value, error) { - for _, path := range pvm { - if p.Equals(path.Path) { - return v.WithMarks(path.Marks), nil - } - } - return v, nil - }) + ret, _ := TransformWithTransformer(val, &applyPathValueMarksTransformer{pvm}) return ret } @@ -223,6 +233,22 @@ 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 { + t.pvm = append(t.pvm, PathValueMarks{p, 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. // @@ -230,14 +256,16 @@ 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 } @@ -246,15 +274,9 @@ func (val Value) UnmarkDeep() (Value, ValueMarks) { // a caller to know which marks are associated with which paths // in the Value. func (val Value) UnmarkDeepWithPaths() (Value, []PathValueMarks) { - var marks []PathValueMarks - ret, _ := Transform(val, func(p Path, v Value) (Value, error) { - unmarkedV, valueMarks := v.Unmark() - if v.IsMarked() { - marks = append(marks, PathValueMarks{p, valueMarks}) - } - return unmarkedV, nil - }) - return ret, marks + t := unmarkTransformer{} + ret, _ := TransformWithTransformer(val, &t) + return ret, t.pvm } func (val Value) unmarkForce() Value { diff --git a/cty/marks_test.go b/cty/marks_test.go index 661d2b3..d7275e8 100644 --- a/cty/marks_test.go +++ b/cty/marks_test.go @@ -5,6 +5,101 @@ import ( "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) @@ -78,7 +173,7 @@ func TestValueMarks(t *testing.T) { } } -func TestPathValueMarks(t *testing.T) { +func TestPathValueMarksEqual(t *testing.T) { tests := []struct { original PathValueMarks compare PathValueMarks @@ -120,33 +215,242 @@ func TestPathValueMarks(t *testing.T) { } } -func TestUnmarkDeep(t *testing.T) { - v := NumberIntVal(1).Mark("a") - v1 := NumberIntVal(2) - l := ListVal([]Value{v, v1}) - if l.IsMarked() { - t.Error("Value containing marks should not be marked itself") +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) + } + } } - if !l.ContainsMarked() { - t.Error("Value containing marks should be caught by ContainsMarked") + + // 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") - l1, marks := l.UnmarkDeep() - if got, want := l1, ListVal([]Value{NumberIntVal(1), v1}); !want.RawEquals(got) { - t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + // 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) } - if got, want := marks, NewValueMarks("a"); !want.Equal(got) { - t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + 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"), + }, } - l2, paths := l.UnmarkDeepWithPaths() - if got, want := l2, ListVal([]Value{NumberIntVal(1), v1}); !want.RawEquals(got) { - t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, want) + 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) + } + }) } - expectedPathValueMarks := []PathValueMarks{{Path{IndexStep{Key: NumberIntVal(0)}}, NewValueMarks("a")}, {}, {}} - for i, p := range paths { - if got, want := p, expectedPathValueMarks[i]; !want.Equal(got) { - t.Errorf("wrong result\ngot: #%v\nwant: %#v", got, 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")}, + }, + }, + } + + 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/walk.go b/cty/walk.go index a6943ba..921d953 100644 --- a/cty/walk.go +++ b/cty/walk.go @@ -61,6 +61,31 @@ 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). +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 +102,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 +111,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}) } -func transform(path Path, val Value, cb func(Path, Value) (Value, error)) (Value, error) { +// 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, t Transformer) (Value, error) { + val, err := t.Enter(path, val) + if err != nil { + return DynamicVal, err + } + ty := val.Type() var newVal Value @@ -112,7 +150,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 +181,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 +203,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 +216,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 0a6882c..8da00e3 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) + } +} From 603ae02ba440414f6f95dec41d470784cb3a5da1 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 23 Sep 2020 14:06:49 -0400 Subject: [PATCH 09/17] cty: Remove already-fixed FIXME for SetVal The immediately preceding code already unmarks the values and applies the marking to the set, so this comment and panic are out of date. --- cty/value_init.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cty/value_init.go b/cty/value_init.go index 853a5a7..3a8b305 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) { From 604042bc87a51dd90dd214a6bf28030da1e29ba5 Mon Sep 17 00:00:00 2001 From: OwenTuz Date: Mon, 5 Oct 2020 19:42:10 +0100 Subject: [PATCH 10/17] functions/stdlib: formatdate to correctly handle literals at the end of the format string --- cty/function/stdlib/datetime.go | 11 ++++++++--- cty/function/stdlib/datetime_test.go | 5 +++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/cty/function/stdlib/datetime.go b/cty/function/stdlib/datetime.go index 4224814..cae7323 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 3e88cf4..509ce85 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"), From 860524c6c3c929757ae0a195b4a9468b2e47f150 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 5 Oct 2020 11:44:02 -0700 Subject: [PATCH 11/17] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 787076b..0aadc45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # 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/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 (September 2, 2020) From f45ff625753a4d4c5e34b40b511a37e3024a1a96 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 6 Oct 2020 10:53:43 -0400 Subject: [PATCH 12/17] cty: Fix path array reuse with UnmarkDeepWithPaths When unmarking a complex value and retaining the marked paths, a sufficiently deep structure would result in incorrect duplicate path output. For example, this structure (in HCL syntax): { environment = [ { variables = { "x" = 1 "y" = 2 } } ] } If the 1 and 2 values are marked, the resulting path value marks from UnmarkDeepWithPaths would have two entries, as expected. However, both would have the same Path attribute, like so: [ { environment[0].variables["x"], "mark" }, { environment[0].variables["x"], "mark" }, ] This is caused by calling `append` in the walk transform function with the same path object repeatedly, which eventually does not result in array reallocation, and therefore the path object is modified in-place. --- cty/marks.go | 4 +++- cty/marks_test.go | 26 ++++++++++++++++++++++++++ cty/walk.go | 3 +++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/cty/marks.go b/cty/marks.go index e3eaec3..4e1d3b1 100644 --- a/cty/marks.go +++ b/cty/marks.go @@ -240,7 +240,9 @@ type unmarkTransformer struct { func (t *unmarkTransformer) Enter(p Path, v Value) (Value, error) { unmarkedVal, marks := v.Unmark() if len(marks) > 0 { - t.pvm = append(t.pvm, PathValueMarks{p, marks}) + path := make(Path, len(p), len(p)+1) + copy(path, p) + t.pvm = append(t.pvm, PathValueMarks{path, marks}) } return unmarkedVal, nil } diff --git a/cty/marks_test.go b/cty/marks_test.go index d7275e8..7177557 100644 --- a/cty/marks_test.go +++ b/cty/marks_test.go @@ -422,6 +422,32 @@ func TestPathValueMarks(t *testing.T) { {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 { diff --git a/cty/walk.go b/cty/walk.go index 921d953..d17f48c 100644 --- a/cty/walk.go +++ b/cty/walk.go @@ -69,6 +69,9 @@ func walk(path Path, val Value, cb func(Path, Value) (bool, error)) error { // 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) From f4ac520662513761bdf16b3063ded092b9558ae2 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 7 Oct 2020 16:53:40 -0400 Subject: [PATCH 13/17] stdlib: Add tests for join function --- cty/function/stdlib/string_test.go | 66 ++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/cty/function/stdlib/string_test.go b/cty/function/stdlib/string_test.go index 6e78df8..cec0f7e 100644 --- a/cty/function/stdlib/string_test.go +++ b/cty/function/stdlib/string_test.go @@ -393,3 +393,69 @@ 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"), + }, + } + + 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) + } + }) + } +} From 2709c23df71e05debba00b85848c4ea78d446ea5 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Wed, 7 Oct 2020 17:02:53 -0400 Subject: [PATCH 14/17] function: Deeply unmark function arguments For functions which do not explicitly support marks, we now deeply unmark arguments before calling, and reapply those marks to the return value. This ensures that these functions do not panic with arguments which are collection values with marked descendants. This does result in overly conservatively applying marks in some cases, but that can be fixed with later work to add explicit support for marks to those functions. --- cty/function/function.go | 44 ++++++++++++++++-------------- cty/function/stdlib/string_test.go | 14 ++++++++++ 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/cty/function/function.go b/cty/function/function.go index bc5e688..8227a40 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/string_test.go b/cty/function/stdlib/string_test.go index cec0f7e..95351a5 100644 --- a/cty/function/stdlib/string_test.go +++ b/cty/function/stdlib/string_test.go @@ -443,6 +443,20 @@ func TestJoin(t *testing.T) { }, 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 { From ddae4f3f5ca9eaf6d22e9e6a19e02c3997a32298 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 13 Oct 2020 13:06:40 -0700 Subject: [PATCH 15/17] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aadc45..4ec9460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # 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 (September 2, 2020) From d4543790c7579d645cc5a2598b2081568f3a6671 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 20 Oct 2020 17:02:46 -0700 Subject: [PATCH 16/17] v1.7.0 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ec9460..998664e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -# 1.7.0 (Unreleased) +# 1.7.0 (October 20, 2020) * `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)) From 5c915fcf308fc7da8315a91a4d6d775691901003 Mon Sep 17 00:00:00 2001 From: Baraa Basata Date: Tue, 25 Mar 2025 09:07:48 -0400 Subject: [PATCH 17/17] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 998664e..9667554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,10 @@ -# 1.7.0 (October 20, 2020) +# 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 (September 2, 2020) +# 1.6.1 (Unreleased) * `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))