From 6aa02775615f84b9cd6a6cb9af05b7eca969f599 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Mon, 16 Dec 2024 15:33:35 +0100 Subject: [PATCH] internal/core/export: sort conjuncts in binary expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to help reduce diffs in Unity. It sorts both the values in adt.Conjunction as well as adt.BinaryExpr. In the latter case, it first flattens the three, then sorts, then rebuilds the binary tree. Signed-off-by: Marcel van Lohuizen Change-Id: I9fa6b91816a15c3a6cfa78c25b34851bd791011e Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1205862 Reviewed-by: Daniel Martí TryBot-Result: CUEcueckoo --- .../cmd/testdata/script/def_jsonschema.txtar | 2 +- internal/core/export/adt.go | 99 +++++++ .../export/testdata/main/conjunctsort.txtar | 258 ++++++++++++++++++ .../core/export/testdata/main/simplify.txtar | 8 +- internal/core/export/value.go | 23 +- pkg/list/testdata/list.txtar | 2 +- pkg/list/testdata/unique.txtar | 51 ++-- pkg/struct/testdata/struct.txtar | 62 ++--- tools/flow/testdata/template.txtar | 4 +- 9 files changed, 435 insertions(+), 74 deletions(-) create mode 100644 internal/core/export/testdata/main/conjunctsort.txtar diff --git a/cmd/cue/cmd/testdata/script/def_jsonschema.txtar b/cmd/cue/cmd/testdata/script/def_jsonschema.txtar index f53f26a5b..d3cc92a19 100644 --- a/cmd/cue/cmd/testdata/script/def_jsonschema.txtar +++ b/cmd/cue/cmd/testdata/script/def_jsonschema.txtar @@ -49,7 +49,7 @@ import "strings" lastName?: strings.MinRunes(1) // Age in years which must be equal to or greater than zero. - age?: >=0 & int + age?: int & >=0 ... } -- schema.json -- diff --git a/internal/core/export/adt.go b/internal/core/export/adt.go index 2d2a14c49..f623cccf1 100644 --- a/internal/core/export/adt.go +++ b/internal/core/export/adt.go @@ -16,7 +16,9 @@ package export import ( "bytes" + "cmp" "fmt" + "slices" "strings" "cuelang.org/go/cue/ast" @@ -230,6 +232,9 @@ func (e *exporter) adt(env *adt.Environment, expr adt.Elem) ast.Expr { } case *adt.BinaryExpr: + if x.Op == adt.AndOp || x.Op == adt.OrOp { + return e.sortBinaryTree(env, x) + } return &ast.BinaryExpr{ Op: x.Op.Token(), X: e.innerExpr(env, x.X), @@ -301,6 +306,100 @@ func (e *exporter) adt(env *adt.Environment, expr adt.Elem) ast.Expr { } } +// sortBinaryTree converte x to a binary tree and sorts it's elements +// using sortLeafAdt. +func (e *exporter) sortBinaryTree(env *adt.Environment, x *adt.BinaryExpr) (b ast.Expr) { + var exprs []adt.Node + + var flatten func(expr adt.Expr) + flatten = func(expr adt.Expr) { + if y, ok := expr.(*adt.BinaryExpr); ok && x.Op == y.Op { + flatten(y.X) + flatten(y.Y) + } else { + exprs = append(exprs, expr) + } + } + flatten(x) + + // Sort the expressions + slices.SortStableFunc(exprs, cmpLeafNodes) + + nodes := make([]ast.Expr, 0, len(exprs)) + for _, x := range exprs { + switch y := x.(type) { + case *adt.Top: + case *adt.BasicType: + if y.K != adt.TopKind { + nodes = append(nodes, e.expr(env, y)) + } + default: + nodes = append(nodes, e.innerExpr(env, y.(adt.Expr))) + } + } + + if len(nodes) == 0 { + return e.adt(env, &adt.Top{}) + } + + return ast.NewBinExpr(x.Op.Token(), nodes...) +} + +// cmpConjuncts compares two Conjunct based on their element using cmpLeafNodes. +func cmpConjuncts(a, b adt.Conjunct) int { + return cmpLeafNodes(a.Expr(), b.Expr()) +} + +// cmpLeafNodes compares two adt.Expr values. The values may not be a binary +// expressions. It returns true if a is less than b. +func cmpLeafNodes[T adt.Node](a, b T) int { + if c := cmp.Compare(typeOrder(a), typeOrder(b)); c != 0 { + return c + } + + srcA := a.Source() + srcB := b.Source() + + if srcA == nil || srcB == nil { + // TODO: some tie breaker + return 0 + } + + posA := srcA.Pos() + posB := srcB.Pos() + + if c := cmp.Compare(posA.Filename(), posB.Filename()); c != 0 { + return c + } + + if c := cmp.Compare(posA.Offset(), posB.Offset()); c != 0 { + return c + } + + return 0 +} + +func typeOrder(x adt.Node) int { + switch x.(type) { + case *adt.Top: + return 0 + case *adt.BasicType: + return 1 + case *adt.FieldReference: + return 2 // sometimes basic types are represented as field references. + case *adt.Bool, *adt.Null, *adt.Num, *adt.String, *adt.Bytes: + return 10 + case *adt.BoundValue: + return 20 + case *adt.StructLit, *adt.ListLit: + return 500 + case adt.Expr: + return 25 + default: + return 100 + } +} + var dummyTop = &ast.Ident{Name: "_"} func (e *exporter) resolve(env *adt.Environment, r adt.Resolver) ast.Expr { diff --git a/internal/core/export/testdata/main/conjunctsort.txtar b/internal/core/export/testdata/main/conjunctsort.txtar new file mode 100644 index 000000000..eb4d2c5db --- /dev/null +++ b/internal/core/export/testdata/main/conjunctsort.txtar @@ -0,0 +1,258 @@ +-- in.cue -- +import "list" + +basicFirst: { + sort: Z & {a: _x} // string first + _x: y + "" + y: string + Z: a: string +} + +literalLast: { + p1: { + [string]: list.UniqueItems() + a: [_, _] + } + p2: { + a: [_, _] + [string]: list.UniqueItems() + } +} + +posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: a & b + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: a & b + } +} +-- out/definition -- +import "list" + +basicFirst: { + sort: Z & { + a: _x + } + _x: y + "" + y: string + Z: { + a: string + } +} +literalLast: { + p1: { + [string]: list.UniqueItems() + a: [_, _] + } + p2: { + a: [_, _] + [string]: list.UniqueItems() + } +} +posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: a & b + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: a & b + } +} +-- out/doc -- +[] +[basicFirst] +[basicFirst sort] +[basicFirst sort a] +[basicFirst _x] +[basicFirst y] +[basicFirst Z] +[basicFirst Z a] +[literalLast] +[literalLast p1] +[literalLast p1 a] +[literalLast p1 a 0] +[literalLast p1 a 1] +[literalLast p2] +[literalLast p2 a] +[literalLast p2 a 0] +[literalLast p2 a 1] +[posTieBreaker] +[posTieBreaker p1] +[posTieBreaker p1 a] +[posTieBreaker p1 b] +[posTieBreaker p1 c] +[posTieBreaker p2] +[posTieBreaker p2 b] +[posTieBreaker p2 a] +[posTieBreaker p2 c] +-- out/value -- +== Simplified +{ + basicFirst: { + sort: { + a: string & _x + } + y: string + Z: { + a: string + } + } + literalLast: { + p1: { + a: list.UniqueItems() & [_, _] + } + p2: { + a: list.UniqueItems() & [_, _] + } + } + posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: list.UniqueItems() & list.MinItems(3) + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: list.MinItems(3) & list.UniqueItems() + } + } +} +== Raw +{ + basicFirst: { + sort: { + a: string & _x + } + _x: y + "" + y: string + Z: { + a: string + } + } + literalLast: { + p1: { + a: list.UniqueItems() & [_, _] + } + p2: { + a: list.UniqueItems() & [_, _] + } + } + posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: list.UniqueItems() & list.MinItems(3) + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: list.MinItems(3) & list.UniqueItems() + } + } +} +== Final +{ + basicFirst: { + sort: { + a: _|_ // basicFirst.sort.a: non-concrete value string in operand to + + } + y: string + Z: { + a: string + } + } + literalLast: { + p1: { + a: _|_ // literalLast.p1.a: invalid value [_,_] (does not satisfy list.UniqueItems): equal values at position 0 and 1 + } + p2: { + a: _|_ // literalLast.p2.a: invalid value [_,_] (does not satisfy list.UniqueItems): equal values at position 0 and 1 + } + } + posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: list.UniqueItems() & list.MinItems(3) + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: list.MinItems(3) & list.UniqueItems() + } + } +} +== All +{ + basicFirst: { + sort: { + a: string & _x + } + _x: y + "" + y: string + Z: { + a: string + } + } + literalLast: { + p1: { + a: list.UniqueItems() & [_, _] + } + p2: { + a: list.UniqueItems() & [_, _] + } + } + posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: list.UniqueItems() & list.MinItems(3) + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: list.MinItems(3) & list.UniqueItems() + } + } +} +== Eval +{ + basicFirst: { + sort: { + a: string & _x + } + y: string + Z: { + a: string + } + } + literalLast: { + p1: { + a: list.UniqueItems() & [_, _] + } + p2: { + a: list.UniqueItems() & [_, _] + } + } + posTieBreaker: { + p1: { + a: list.UniqueItems() + b: list.MinItems(3) + c: list.UniqueItems() & list.MinItems(3) + } + p2: { + b: list.MinItems(3) + a: list.UniqueItems() + c: list.MinItems(3) & list.UniqueItems() + } + } +} diff --git a/internal/core/export/testdata/main/simplify.txtar b/internal/core/export/testdata/main/simplify.txtar index 29952ab73..27153e72c 100644 --- a/internal/core/export/testdata/main/simplify.txtar +++ b/internal/core/export/testdata/main/simplify.txtar @@ -58,7 +58,7 @@ Benign change in error message. == Raw { x: { - y: >=-9223372036854775808 & <=9223372036854775807 & int + y: int & >=-9223372036854775808 & <=9223372036854775807 } s: strings.MinRunes(4) & strings.MaxRunes(7) additional: { @@ -95,7 +95,7 @@ Benign change in error message. == Eval { x: { - y: >=-9223372036854775808 & <=9223372036854775807 & int + y: int & >=-9223372036854775808 & <=9223372036854775807 } s: strings.MinRunes(4) & strings.MaxRunes(7) additional: { @@ -135,7 +135,7 @@ diff old new == Raw { x: { - y: >=-9223372036854775808 & <=9223372036854775807 & int + y: int & >=-9223372036854775808 & <=9223372036854775807 } s: strings.MinRunes(4) & strings.MaxRunes(7) additional: { @@ -172,7 +172,7 @@ diff old new == Eval { x: { - y: >=-9223372036854775808 & <=9223372036854775807 & int + y: int & >=-9223372036854775808 & <=9223372036854775807 } s: strings.MinRunes(4) & strings.MaxRunes(7) additional: { diff --git a/internal/core/export/value.go b/internal/core/export/value.go index 55b66db13..f476a51bc 100644 --- a/internal/core/export/value.go +++ b/internal/core/export/value.go @@ -16,6 +16,7 @@ package export import ( "fmt" + "slices" "strings" "cuelang.org/go/cue/ast" @@ -102,14 +103,23 @@ func (e *exporter) vertex(n *adt.Vertex) (result ast.Expr) { } if result == nil { // fall back to expression mode - a := []ast.Expr{} + a := []adt.Conjunct{} n.VisitLeafConjuncts(func(c adt.Conjunct) bool { - if x := e.expr(c.Env, c.Elem()); x != dummyTop { - a = append(a, x) - } + a = append(a, c) return true }) - result = ast.NewBinExpr(token.AND, a...) + // Use stable sort to ensure that tie breaks (for instance if elements + // are not associated with a position) are deterministic. + slices.SortStableFunc(a, cmpConjuncts) + + exprs := make([]ast.Expr, 0, len(a)) + for _, c := range a { + if x := e.expr(c.Env, c.Elem()); x != dummyTop { + exprs = append(exprs, x) + } + } + + result = ast.NewBinExpr(token.AND, exprs...) } if len(s.Elts) > 0 { @@ -195,12 +205,15 @@ func (e *exporter) value(n adt.Value, a ...adt.Conjunct) (result ast.Expr) { a = x.Values } + slices.SortStableFunc(a, cmpLeafNodes) + for _, x := range a { result = wrapBin(result, e.bareValue(x), adt.AndOp) } case *adt.Disjunction: a := []ast.Expr{} + for i, v := range x.Values { var expr ast.Expr if e.cfg.Simplify { diff --git a/pkg/list/testdata/list.txtar b/pkg/list/testdata/list.txtar index e9eec3e16..05d4b18e8 100644 --- a/pkg/list/testdata/list.txtar +++ b/pkg/list/testdata/list.txtar @@ -182,7 +182,7 @@ unique: { } // Issue #2099 minItems: { - incomplete1: [...] & list.MinItems(1) + incomplete1: list.MinItems(1) & [...] fail1: _|_ // minItems.fail1: invalid value [] (does not satisfy list.MinItems(1)): len(list) < MinItems(1) (0 < 1) ok1: [0, ...] ok2: [0] diff --git a/pkg/list/testdata/unique.txtar b/pkg/list/testdata/unique.txtar index bd2306bce..b7fb5565c 100644 --- a/pkg/list/testdata/unique.txtar +++ b/pkg/list/testdata/unique.txtar @@ -79,7 +79,7 @@ import "list" } #abErr: { a: 1 - b?: string & int + b?: int & string } #b: { b: 1 @@ -101,38 +101,38 @@ ok: { // These have all equal elements, but incomplete: { - top: [_, _] & list.UniqueItems() + top: list.UniqueItems() & [_, _] // These two elements are considered equal, but the error is an "incomplete" // errors, as the items may still differ once they become more specific. - withOptional1: [{ + withOptional1: list.UniqueItems() & [{ a: int }, { a: int b?: string - }] & list.UniqueItems() + }] // Ditto. This is an incomplete error, even though the matching elements // are "final": there is still an optional field. - withOptional2: [{ + withOptional2: list.UniqueItems() & [{ a: 1 }, { a: 1 b?: string - }] & list.UniqueItems() + }] // Ditto. This time with actually closed fields. - withOptional3: [#a, #ab] & list.UniqueItems() + withOptional3: list.UniqueItems() & [#a, #ab] // Ditto. There are not optional fields, but the structs are open. - openSpecific: [{ + openSpecific: list.UniqueItems() & [{ a: 1 }, { a: 1 - }] & list.UniqueItems() + }] // Fully identical closed structs, but with non-concrete values. - structs: [#c, #c] & list.UniqueItems() + structs: list.UniqueItems() & [#c, #c] } fail: { ints: _|_ // fail.ints: invalid value [1,2,1] (does not satisfy list.UniqueItems): equal value (1) at position 0 and 2 @@ -164,17 +164,8 @@ diff old new ./in.cue:3:21 ./in.cue:45:11 -@@ -21,7 +24,7 @@ - } - #abErr: { - a: 1 -- b?: int & string -+ b?: string & int - } - #b: { - b: 1 @@ -64,12 +67,7 @@ - }] & list.UniqueItems() + }] // Ditto. This time with actually closed fields. - withOptional3: [{ @@ -183,10 +174,10 @@ diff old new - a: 1 - b?: int - }] -+ withOptional3: [#a, #ab] & list.UniqueItems() ++ withOptional3: list.UniqueItems() & [#a, #ab] // Ditto. There are not optional fields, but the structs are open. - openSpecific: [{ + openSpecific: list.UniqueItems() & [{ @@ -83,7 +81,7 @@ } fail: { @@ -254,25 +245,25 @@ ok: { // These have all equal elements, but incomplete: { - top: [_, _] & list.UniqueItems() + top: list.UniqueItems() & [_, _] // These two elements are considered equal, but the error is an "incomplete" // errors, as the items may still differ once they become more specific. - withOptional1: [{ + withOptional1: list.UniqueItems() & [{ a: int }, { a: int b?: string - }] & list.UniqueItems() + }] // Ditto. This is an incomplete error, even though the matching elements // are "final": there is still an optional field. - withOptional2: [{ + withOptional2: list.UniqueItems() & [{ a: 1 }, { a: 1 b?: string - }] & list.UniqueItems() + }] // Ditto. This time with actually closed fields. withOptional3: [{ @@ -283,14 +274,14 @@ incomplete: { }] // Ditto. There are not optional fields, but the structs are open. - openSpecific: [{ + openSpecific: list.UniqueItems() & [{ a: 1 }, { a: 1 - }] & list.UniqueItems() + }] // Fully identical closed structs, but with non-concrete values. - structs: [#c, #c] & list.UniqueItems() + structs: list.UniqueItems() & [#c, #c] } fail: { ints: _|_ // fail.ints: invalid value [1,2,1] (does not satisfy list.UniqueItems): equal value (1) at position 0 and 2 diff --git a/pkg/struct/testdata/struct.txtar b/pkg/struct/testdata/struct.txtar index 92f0c2e4d..30e5f34e8 100644 --- a/pkg/struct/testdata/struct.txtar +++ b/pkg/struct/testdata/struct.txtar @@ -65,14 +65,14 @@ Result: import "struct" minFields1: { - incomplete1: {} & struct.MinFields(1) - optIncomplete: { + incomplete1: struct.MinFields(1) & {} + optIncomplete: struct.MinFields(1) & { a?: string - } & struct.MinFields(1) + } fail1: _|_ // minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1) - optCloseIncomplete: close({ + optCloseIncomplete: struct.MinFields(1) & close({ a?: 1 - }) & struct.MinFields(1) + }) failHidden1: _|_ // minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1) ok4: { a: 1 @@ -92,24 +92,24 @@ minFields1: { } } minFields2: { - incomplete1: close({ + incomplete1: struct.MinFields(2) & close({ a?: string b: 1 - }) & struct.MinFields(2) - incomplete2: close({ + }) + incomplete2: struct.MinFields(2) & close({ a?: string b?: int - }) & struct.MinFields(2) - incomplete3: close({ + }) + incomplete3: struct.MinFields(2) & close({ a?: string b?: int c: 1 - }) & struct.MinFields(2) - incomplete4: close({ + }) + incomplete4: struct.MinFields(2) & close({ a?: string b?: int c?: int - }) & struct.MinFields(2) + }) fail: _|_ // minFields2.fail: invalid value {a?:string} (does not satisfy struct.MinFields(2)): len(fields) < MinFields(2) (0 < 2) } maxFields: { @@ -151,10 +151,10 @@ diff old new @@ -61,9 +65,7 @@ b?: int c?: int - }) & struct.MinFields(2) -- fail: close({ + }) +- fail: struct.MinFields(2) & close({ - a?: string -- }) & struct.MinFields(2) +- }) + fail: _|_ // minFields2.fail: invalid value {a?:string} (does not satisfy struct.MinFields(2)): len(fields) < MinFields(2) (0 < 2) } maxFields: { @@ -183,14 +183,14 @@ Result: import "struct" minFields1: { - incomplete1: {} & struct.MinFields(1) - optIncomplete: { + incomplete1: struct.MinFields(1) & {} + optIncomplete: struct.MinFields(1) & { a?: string - } & struct.MinFields(1) + } fail1: _|_ // minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1) - optCloseIncomplete: close({ + optCloseIncomplete: struct.MinFields(1) & close({ a?: 1 - }) & struct.MinFields(1) + }) failHidden1: _|_ // minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1) ok4: { a: 1 @@ -210,27 +210,27 @@ minFields1: { } } minFields2: { - incomplete1: close({ + incomplete1: struct.MinFields(2) & close({ a?: string b: 1 - }) & struct.MinFields(2) - incomplete2: close({ + }) + incomplete2: struct.MinFields(2) & close({ a?: string b?: int - }) & struct.MinFields(2) - incomplete3: close({ + }) + incomplete3: struct.MinFields(2) & close({ a?: string b?: int c: 1 - }) & struct.MinFields(2) - incomplete4: close({ + }) + incomplete4: struct.MinFields(2) & close({ a?: string b?: int c?: int - }) & struct.MinFields(2) - fail: close({ + }) + fail: struct.MinFields(2) & close({ a?: string - }) & struct.MinFields(2) + }) } maxFields: { ok1: {} diff --git a/tools/flow/testdata/template.txtar b/tools/flow/testdata/template.txtar index 6deb66cee..6794a403f 100644 --- a/tools/flow/testdata/template.txtar +++ b/tools/flow/testdata/template.txtar @@ -101,7 +101,7 @@ diff old new env: {} | [] stdout: "foo" stderr: null -- stdin: (*null | string | bytes) & GET.response.body +- stdin: GET.response.body & (*null | string | bytes) success: bool mustSucceed: true @@ -140,7 +140,7 @@ graph TD env: {} | [] stdout: "foo" stderr: null - stdin: (*null | string | bytes) & GET.response.body + stdin: GET.response.body & (*null | string | bytes) success: bool mustSucceed: true -- 2.51.2