From 6adcc7ab73a4e14e35caee46d8d785814068b425 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 30 Apr 2019 15:29:47 -0700 Subject: [PATCH] vendor: go get github.com/zclconf/go-cty@master cty now guarantees that sets of primitive values will iterate in a reasonable order. Previously it was the caller's responsibility to deal with that, but we invariably neglected to do so, causing inconsistent ordering. Since cty prioritizes consistent behavior over performance, it now imposes its own sort on set elements as part of iterating over them so that calling applications don't have to worry so much about it. This change also causes cty to consistently push unknown and null values in sets to the end of iteration, where before that was undefined. This means that our diff output will now consistently list additions before removals when showing sets, rather than the ordering being undefined as before. The ordering of known, non-null, non-primitive values is still not contractually fixed but remains consistent for a particular version of cty. --- command/format/diff_test.go | 12 ++-- go.mod | 2 +- go.sum | 4 +- lang/funcs/collection_test.go | 6 +- .../zclconf/go-cty/cty/set/iterator.go | 31 ++-------- .../github.com/zclconf/go-cty/cty/set/ops.go | 51 +++++++++------- .../zclconf/go-cty/cty/set/rules.go | 18 ++++++ .../zclconf/go-cty/cty/set_internals.go | 58 +++++++++++++++++++ vendor/modules.txt | 2 +- 9 files changed, 125 insertions(+), 59 deletions(-) diff --git a/command/format/diff_test.go b/command/format/diff_test.go index d93e26c2c..afbed3c8d 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -2564,13 +2564,13 @@ func TestResourceChange_nestedSet(t *testing.T) { ~ ami = "ami-BEFORE" -> "ami-AFTER" id = "i-02ae66f368e8518a9" - - root_block_device { - - volume_type = "gp2" -> null - } + root_block_device { + new_field = "new_value" + volume_type = "gp2" } + - root_block_device { + - volume_type = "gp2" -> null + } } `, }, @@ -2624,12 +2624,12 @@ func TestResourceChange_nestedSet(t *testing.T) { ~ ami = "ami-BEFORE" -> "ami-AFTER" id = "i-02ae66f368e8518a9" - - root_block_device { # forces replacement - - volume_type = "gp2" -> null - } + root_block_device { # forces replacement + volume_type = "different" } + - root_block_device { # forces replacement + - volume_type = "gp2" -> null + } } `, }, diff --git a/go.mod b/go.mod index fbb759943..04f45c119 100644 --- a/go.mod +++ b/go.mod @@ -107,7 +107,7 @@ require ( github.com/xanzy/ssh-agent v0.2.1 github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18 // indirect github.com/xlab/treeprint v0.0.0-20161029104018-1d6e34225557 - github.com/zclconf/go-cty v0.0.0-20190426224007-b18a157db9e2 + github.com/zclconf/go-cty v0.0.0-20190430221426-d36a6f0dbffd go.uber.org/atomic v1.3.2 // indirect go.uber.org/multierr v1.1.0 // indirect go.uber.org/zap v1.9.1 // indirect diff --git a/go.sum b/go.sum index 8c8c07596..bc2d0b857 100644 --- a/go.sum +++ b/go.sum @@ -403,8 +403,8 @@ github.com/xlab/treeprint v0.0.0-20161029104018-1d6e34225557/go.mod h1:ce1O1j6Ut github.com/zclconf/go-cty v0.0.0-20181129180422-88fbe721e0f8/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v0.0.0-20190124225737-a385d646c1e9 h1:hHCAGde+QfwbqXSAqOmBd4NlOrJ6nmjWp+Nu408ezD4= github.com/zclconf/go-cty v0.0.0-20190124225737-a385d646c1e9/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= -github.com/zclconf/go-cty v0.0.0-20190426224007-b18a157db9e2 h1:Ai1LhlYNEqE39zGU07qHDNJ41iZVPZfZr1dSCoXrp1w= -github.com/zclconf/go-cty v0.0.0-20190426224007-b18a157db9e2/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= +github.com/zclconf/go-cty v0.0.0-20190430221426-d36a6f0dbffd h1:NZOOU7h+pDtcKo6xlqm8PwnarS8nJ+6+I83jT8ZfLPI= +github.com/zclconf/go-cty v0.0.0-20190430221426-d36a6f0dbffd/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= go.opencensus.io v0.18.0 h1:Mk5rgZcggtbvtAun5aJzAtjKKN/t0R3jJPlWILlv938= go.opencensus.io v0.18.0/go.mod h1:vKdFvxhtzZ9onBp9VKHK8z/sRpBMnKAsufL7wlDrCOA= go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4= diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 19574730e..ac686fa72 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -2005,12 +2005,12 @@ func TestReverse(t *testing.T) { }, { cty.SetVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")}), - cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")}), + cty.ListVal([]cty.Value{cty.StringVal("b"), cty.StringVal("a")}), // set-of-string iterates in lexicographical order "", }, { - cty.SetVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b"), cty.StringVal("c")}), - cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("c"), cty.StringVal("b")}), + cty.SetVal([]cty.Value{cty.StringVal("b"), cty.StringVal("a"), cty.StringVal("c")}), + cty.ListVal([]cty.Value{cty.StringVal("c"), cty.StringVal("b"), cty.StringVal("a")}), // set-of-string iterates in lexicographical order "", }, { diff --git a/vendor/github.com/zclconf/go-cty/cty/set/iterator.go b/vendor/github.com/zclconf/go-cty/cty/set/iterator.go index f15498e2f..4a60494f9 100644 --- a/vendor/github.com/zclconf/go-cty/cty/set/iterator.go +++ b/vendor/github.com/zclconf/go-cty/cty/set/iterator.go @@ -1,36 +1,15 @@ package set type Iterator struct { - bucketIds []int - vals map[int][]interface{} - bucketIdx int - valIdx int + vals []interface{} + idx int } func (it *Iterator) Value() interface{} { - return it.currentBucket()[it.valIdx] + return it.vals[it.idx] } func (it *Iterator) Next() bool { - if it.bucketIdx == -1 { - // init - if len(it.bucketIds) == 0 { - return false - } - - it.valIdx = 0 - it.bucketIdx = 0 - return true - } - - it.valIdx++ - if it.valIdx >= len(it.currentBucket()) { - it.valIdx = 0 - it.bucketIdx++ - } - return it.bucketIdx < len(it.bucketIds) -} - -func (it *Iterator) currentBucket() []interface{} { - return it.vals[it.bucketIds[it.bucketIdx]] + it.idx++ + return it.idx < len(it.vals) } diff --git a/vendor/github.com/zclconf/go-cty/cty/set/ops.go b/vendor/github.com/zclconf/go-cty/cty/set/ops.go index 726e7077a..fd1555f21 100644 --- a/vendor/github.com/zclconf/go-cty/cty/set/ops.go +++ b/vendor/github.com/zclconf/go-cty/cty/set/ops.go @@ -75,8 +75,12 @@ func (s Set) Copy() Set { return ret } -// Iterator returns an iterator over values in the set, in an undefined order -// that callers should not depend on. +// Iterator returns an iterator over values in the set. If the set's rules +// implement OrderedRules then the result is ordered per those rules. If +// no order is provided, or if it is not a total order, then the iteration +// order is undefined but consistent for a particular version of cty. Do not +// rely on specific ordering between cty releases unless the rules order is a +// total order. // // The pattern for using the returned iterator is: // @@ -89,18 +93,11 @@ func (s Set) Copy() Set { // Once an iterator has been created for a set, the set *must not* be mutated // until the iterator is no longer in use. func (s Set) Iterator() *Iterator { - // Sort the bucketIds to ensure that we always traverse in a - // consistent order. - bucketIds := make([]int, 0, len(s.vals)) - for id := range s.vals { - bucketIds = append(bucketIds, id) - } - sort.Ints(bucketIds) + vals := s.Values() return &Iterator{ - bucketIds: bucketIds, - vals: s.vals, - bucketIdx: -1, + vals: vals, + idx: -1, } } @@ -113,16 +110,30 @@ func (s Set) EachValue(cb func(interface{})) { } } -// Values returns a slice of all of the values in the set in no particular -// order. This is just a wrapper around EachValue that accumulates the results -// in a slice for caller convenience. -// -// The returned slice will be nil if there are no values in the set. +// Values returns a slice of all the values in the set. If the set rules have +// an order then the result is in that order. If no order is provided or if +// it is not a total order then the result order is undefined, but consistent +// for a particular set value within a specific release of cty. func (s Set) Values() []interface{} { var ret []interface{} - s.EachValue(func(v interface{}) { - ret = append(ret, v) - }) + // Sort the bucketIds to ensure that we always traverse in a + // consistent order. + bucketIDs := make([]int, 0, len(s.vals)) + for id := range s.vals { + bucketIDs = append(bucketIDs, id) + } + sort.Ints(bucketIDs) + + for _, bucketID := range bucketIDs { + ret = append(ret, s.vals[bucketID]...) + } + + if orderRules, ok := s.rules.(OrderedRules); ok { + sort.SliceStable(ret, func(i, j int) bool { + return orderRules.Less(ret[i], ret[j]) + }) + } + return ret } diff --git a/vendor/github.com/zclconf/go-cty/cty/set/rules.go b/vendor/github.com/zclconf/go-cty/cty/set/rules.go index 7200184b0..51f744b5e 100644 --- a/vendor/github.com/zclconf/go-cty/cty/set/rules.go +++ b/vendor/github.com/zclconf/go-cty/cty/set/rules.go @@ -23,3 +23,21 @@ type Rules interface { // be equivalent. Equivalent(interface{}, interface{}) bool } + +// OrderedRules is an extension of Rules that can apply a partial order to +// element values. When a set's Rules implements OrderedRules an iterator +// over the set will return items in the order described by the rules. +// +// If the given order is not a total order (that is, some pairs of non-equivalent +// elements do not have a defined order) then the resulting iteration order +// is undefined but consistent for a particular version of cty. The exact +// order in that case is not part of the contract and is subject to change +// between versions. +type OrderedRules interface { + Rules + + // Less returns true if and only if the first argument should sort before + // the second argument. If the second argument should sort before the first + // or if there is no defined order for the values, return false. + Less(interface{}, interface{}) bool +} diff --git a/vendor/github.com/zclconf/go-cty/cty/set_internals.go b/vendor/github.com/zclconf/go-cty/cty/set_internals.go index 1d7a731aa..3fd4fb2df 100644 --- a/vendor/github.com/zclconf/go-cty/cty/set_internals.go +++ b/vendor/github.com/zclconf/go-cty/cty/set_internals.go @@ -6,6 +6,8 @@ import ( "hash/crc32" "math/big" "sort" + + "github.com/zclconf/go-cty/cty/set" ) // setRules provides a Rules implementation for the ./set package that @@ -19,6 +21,8 @@ type setRules struct { Type Type } +var _ set.OrderedRules = setRules{} + // Hash returns a hash value for the receiver that can be used for equality // checks where some inaccuracy is tolerable. // @@ -58,6 +62,60 @@ func (r setRules) Equivalent(v1 interface{}, v2 interface{}) bool { return eqv.v == true } +// Less is an implementation of set.OrderedRules so that we can iterate over +// set elements in a consistent order, where such an order is possible. +func (r setRules) Less(v1, v2 interface{}) bool { + v1v := Value{ + ty: r.Type, + v: v1, + } + v2v := Value{ + ty: r.Type, + v: v2, + } + + if v1v.RawEquals(v2v) { // Easy case: if they are equal then v1 can't be less + return false + } + + // Null values always sort after non-null values + if v2v.IsNull() && !v1v.IsNull() { + return true + } else if v1v.IsNull() { + return false + } + // Unknown values always sort after known values + if v1v.IsKnown() && !v2v.IsKnown() { + return true + } else if !v1v.IsKnown() { + return false + } + + switch r.Type { + case String: + // String values sort lexicographically + return v1v.AsString() < v2v.AsString() + case Bool: + // Weird to have a set of bools, but if we do then false sorts before true. + if v2v.True() || !v1v.True() { + return true + } + return false + case Number: + v1f := v1v.AsBigFloat() + v2f := v2v.AsBigFloat() + return v1f.Cmp(v2f) < 0 + default: + // No other types have a well-defined ordering, so we just produce a + // default consistent-but-undefined ordering then. This situation is + // not considered a compatibility constraint; callers should rely only + // on the ordering rules for primitive values. + v1h := makeSetHashBytes(v1v) + v2h := makeSetHashBytes(v2v) + return bytes.Compare(v1h, v2h) < 0 + } +} + func makeSetHashBytes(val Value) []byte { var buf bytes.Buffer appendSetHashBytes(val, &buf) diff --git a/vendor/modules.txt b/vendor/modules.txt index bb5d86866..ef888b8b3 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -434,7 +434,7 @@ github.com/vmihailenco/msgpack/codes github.com/xanzy/ssh-agent # github.com/xlab/treeprint v0.0.0-20161029104018-1d6e34225557 github.com/xlab/treeprint -# github.com/zclconf/go-cty v0.0.0-20190426224007-b18a157db9e2 +# github.com/zclconf/go-cty v0.0.0-20190430221426-d36a6f0dbffd github.com/zclconf/go-cty/cty github.com/zclconf/go-cty/cty/gocty github.com/zclconf/go-cty/cty/convert