From f4d9c2225da1c5723b3db8e47e8542a71962a7d0 Mon Sep 17 00:00:00 2001 From: Torin Sandall Date: Mon, 20 Sep 2021 02:46:02 -0700 Subject: [PATCH] types: Sort any elements during construction (#3805) * types: Sort any elements during construction This changes updates the implementation of the any type to sort elements during construction. This way the Compare() function does not have to sort elements before recursing (which can result in data races if global type instances from the built-in function declarations or elsewhere are compared.) Fixes #3793 * capabilities.json: changed ordering * internal/presentation: fix json error output Co-authored-by: Torin Sandall Co-authored-by: Stephan Renatus Signed-off-by: Torin Sandall --- capabilities.json | 172 ++++++++++----------- internal/presentation/presentation_test.go | 10 +- types/types.go | 13 +- types/types_test.go | 38 +++++ 4 files changed, 139 insertions(+), 94 deletions(-) diff --git a/capabilities.json b/capabilities.json index 76f8c1c97a..cc391d8ae1 100644 --- a/capabilities.json +++ b/capabilities.json @@ -21,16 +21,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "any" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "any" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -76,16 +76,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "any" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "any" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -472,16 +472,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "string" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "string" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -517,10 +517,7 @@ { "of": [ { - "of": { - "type": "any" - }, - "type": "set" + "type": "string" }, { "dynamic": { @@ -540,7 +537,10 @@ "type": "object" }, { - "type": "string" + "of": { + "type": "any" + }, + "type": "set" } ], "type": "any" @@ -847,16 +847,16 @@ "value": { "of": [ { - "of": { + "dynamic": { "type": "any" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "any" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -867,16 +867,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "any" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "any" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -1747,16 +1747,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "any" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "any" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -1775,16 +1775,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "any" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "any" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -1926,23 +1926,6 @@ }, "type": "array" }, - { - "of": { - "of": [ - { - "type": "string" - }, - { - "dynamic": { - "type": "any" - }, - "type": "array" - } - ], - "type": "any" - }, - "type": "set" - }, { "dynamic": { "key": { @@ -1964,17 +1947,9 @@ } }, "type": "object" - } - ], - "type": "any" - }, - { - "of": [ - { - "type": "string" }, { - "dynamic": { + "of": { "of": [ { "type": "string" @@ -1988,10 +1963,18 @@ ], "type": "any" }, - "type": "array" + "type": "set" + } + ], + "type": "any" + }, + { + "of": [ + { + "type": "string" }, { - "of": { + "dynamic": { "of": [ { "type": "string" @@ -2005,7 +1988,7 @@ ], "type": "any" }, - "type": "set" + "type": "array" }, { "dynamic": { @@ -2028,6 +2011,23 @@ } }, "type": "object" + }, + { + "of": { + "of": [ + { + "type": "string" + }, + { + "dynamic": { + "type": "any" + }, + "type": "array" + } + ], + "type": "any" + }, + "type": "set" } ], "type": "any" @@ -2180,12 +2180,6 @@ }, "type": "array" }, - { - "of": { - "type": "any" - }, - "type": "set" - }, { "dynamic": { "key": { @@ -2196,6 +2190,12 @@ } }, "type": "object" + }, + { + "of": { + "type": "any" + }, + "type": "set" } ], "type": "any" @@ -2258,12 +2258,6 @@ }, "type": "array" }, - { - "of": { - "type": "any" - }, - "type": "set" - }, { "dynamic": { "key": { @@ -2274,6 +2268,12 @@ } }, "type": "object" + }, + { + "of": { + "type": "any" + }, + "type": "set" } ], "type": "any" @@ -2387,16 +2387,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "number" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "number" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -2856,16 +2856,16 @@ { "of": [ { - "of": { + "dynamic": { "type": "number" }, - "type": "set" + "type": "array" }, { - "dynamic": { + "of": { "type": "number" }, - "type": "array" + "type": "set" } ], "type": "any" @@ -3142,16 +3142,16 @@ { "of": [ { - "type": "number" + "type": "null" }, { - "type": "string" + "type": "boolean" }, { - "type": "boolean" + "type": "number" }, { - "type": "null" + "type": "string" } ], "type": "any" diff --git a/internal/presentation/presentation_test.go b/internal/presentation/presentation_test.go index 614376d31b..35ae899f1e 100644 --- a/internal/presentation/presentation_test.go +++ b/internal/presentation/presentation_test.go @@ -193,10 +193,7 @@ func TestOutputJSONErrorStructuredAstErr(t *testing.T) { { "of": [ { - "of": { - "type": "any" - }, - "type": "set" + "type": "string" }, { "dynamic": { @@ -216,7 +213,10 @@ func TestOutputJSONErrorStructuredAstErr(t *testing.T) { "type": "object" }, { - "type": "string" + "of": { + "type": "any" + }, + "type": "set" } ], "type": "any" diff --git a/types/types.go b/types/types.go index 05f13c5e5c..cf102040d1 100644 --- a/types/types.go +++ b/types/types.go @@ -376,6 +376,7 @@ func NewAny(of ...Type) Any { for i := range sl { sl[i] = of[i] } + sort.Sort(typeSlice(sl)) return sl } @@ -411,7 +412,14 @@ func (t Any) Merge(other Type) Any { if t.Contains(other) { return t } - return append(t, other) + cpy := make(Any, len(t)+1) + idx := sort.Search(len(t), func(i int) bool { + return Compare(t[i], other) >= 0 + }) + copy(cpy, t[:idx]) + cpy[idx] = other + copy(cpy[idx+1:], t[idx:]) + return cpy } // Union returns a new Any type that is the union of the two Any types. @@ -431,6 +439,7 @@ func (t Any) Union(other Any) Any { cpy = append(cpy, other[i]) } } + sort.Sort(typeSlice(cpy)) return cpy } @@ -625,8 +634,6 @@ func Compare(a, b Type) int { case Any: sl1 := typeSlice(a.(Any)) sl2 := typeSlice(b.(Any)) - sort.Sort(sl1) - sort.Sort(sl2) return typeSliceCompare(sl1, sl2) case *Function: fA := a.(*Function) diff --git a/types/types_test.go b/types/types_test.go index 2f38e728ea..e2201a34fe 100644 --- a/types/types_test.go +++ b/types/types_test.go @@ -14,6 +14,44 @@ import ( var dynamicPropertyAnyAny = NewDynamicProperty(A, A) +func TestAnySorted(t *testing.T) { + a := NewAny(S, N) + if Compare(a[0], N) != 0 { + t.Fatal("expected any type to be sorted") + } +} + +func TestAnyMerge(t *testing.T) { + x := NewAny(S, B) + + if Compare(x.Merge(N)[1], N) != 0 { + t.Fatal("expected number to be inserted into middle") + } + + if Compare(x.Merge(NewNull())[0], NewNull()) != 0 { + t.Fatal("expected null to be inserted at front") + } + + if Compare(x.Merge(NewArray(nil, A))[2], NewArray(nil, A)) != 0 { + t.Fatal("expected array to be inserted at back") + } +} + +func TestAnyUnion(t *testing.T) { + x := NewAny(NewNull(), N) + y := NewAny(S, B) + z := x.Union(y) + exp := []Type{NewNull(), B, N, S} + if len(z) != len(exp) { + t.Fatalf("expected %v elements in result of union", len(exp)) + } + for i := range z { + if Compare(z[i], exp[i]) != 0 { + t.Fatal("expected", exp[i], "but got", z[i]) + } + } +} + func TestStrings(t *testing.T) { tpe := NewObject([]*StaticProperty{