this repo has no description
encoding/jsonschema: improve mergeAllOf behavior
The `mergeAllOf` function wasn't working quite as expected.
This change applies a few fixes:
- Flattening is now more efficient and hopefully obvious,
using the conjuncts iterator
- A single-element allOf is flattened into its single element
- `item.apply` methods are expected to call the argument function
rather than just invoke `apply` recursively.
We add a unit test to make it easier to test the behavior
directly.
Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I542b7d9874203843b758c9ccc5f45d8bae47d174
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1224260
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
+314
-63
+38
-36
encoding/jsonschema/generate.go
+38
-36
encoding/jsonschema/generate.go
···
17
17
import (
18
18
"cmp"
19
19
"fmt"
20
+
"iter"
20
21
"maps"
21
22
"reflect"
22
23
"slices"
···
120
121
it1 := &itemAllOf{
121
122
elems: make([]item, 0, len(it.elems)),
122
123
}
123
-
for _, e := range it.elems {
124
-
e := mergeAllOf(e)
125
-
if e1, ok := e.(*itemAllOf); ok {
126
-
it1.elems = append(it1.elems, e1.elems...)
127
-
} else {
128
-
it1.elems = append(it1.elems, e)
124
+
loop:
125
+
for e := range conjuncts(it) {
126
+
// Remove elements that are entirely redundant.
127
+
// Note: DeepEqual seems reasonable here because values are generally
128
+
// small and the data structures are well-defined. We could
129
+
// reconsider if these assumptions change.
130
+
// TODO we could unify itemType elements here, for example:
131
+
// allOf(itemType(number), itemType(integer)) -> itemType(integer)
132
+
for _, e1 := range it1.elems {
133
+
if reflect.DeepEqual(e1, e) {
134
+
continue loop
135
+
}
129
136
}
137
+
it1.elems = append(it1.elems, e.apply(mergeAllOf))
130
138
}
131
-
// Remove elements that are entirely redundant.
132
-
// Note: DeepEqual seems reasonable here because values are generally
133
-
// small and the data structures are well-defined. We could
134
-
// reconsider if these assumptions change.
135
-
it1.elems = dedupe(it1.elems, func(x, y item) bool { return reflect.DeepEqual(x, y) })
139
+
if len(it1.elems) == 1 {
140
+
return it1.elems[0]
141
+
}
136
142
return it1
137
143
default:
138
144
return it.apply(mergeAllOf)
139
145
}
146
+
}
147
+
148
+
func conjuncts(it *itemAllOf) iter.Seq[item] {
149
+
return func(yield func(item) bool) {
150
+
yieldConjuncts(it, yield)
151
+
}
152
+
}
153
+
154
+
func yieldConjuncts(it *itemAllOf, yield func(item) bool) bool {
155
+
for _, e := range it.elems {
156
+
if ae, ok := e.(*itemAllOf); ok {
157
+
if !yieldConjuncts(ae, yield) {
158
+
return false
159
+
}
160
+
} else {
161
+
if !yield(e) {
162
+
return false
163
+
}
164
+
}
165
+
}
166
+
return true
140
167
}
141
168
142
169
// enumFromConst returns the item with disjunctive
···
560
587
}
561
588
return xs1
562
589
}
563
-
564
-
// dedupe returns xs with any items considered equal
565
-
// according to eq de-duplicated.
566
-
func dedupe[T any](xs []T, eq func(T, T) bool) []T {
567
-
if len(xs) < 2 {
568
-
return xs
569
-
}
570
-
571
-
w := 1
572
-
outer:
573
-
for i := 1; i < len(xs); i++ {
574
-
x := xs[i]
575
-
for j := 0; j < w; j++ {
576
-
if eq(x, xs[j]) {
577
-
continue outer
578
-
}
579
-
}
580
-
if w != i {
581
-
xs[w] = x
582
-
}
583
-
w++
584
-
}
585
-
586
-
return xs[:w]
587
-
}
+6
-7
encoding/jsonschema/generate_items.go
+6
-7
encoding/jsonschema/generate_items.go
···
81
81
82
82
for _, e := range i.elems {
83
83
expr := e.generate(g)
84
-
85
84
if lit, ok := expr.(*ast.BasicLit); ok {
86
85
switch lit.Kind {
87
86
case token.TRUE:
···
197
196
}
198
197
199
198
func (i *itemNot) apply(f func(item) item) item {
200
-
elem := i.elem.apply(f)
199
+
elem := f(i.elem)
201
200
if elem == i.elem {
202
201
return i
203
202
}
···
455
454
}
456
455
457
456
func (i *itemContains) apply(f func(item) item) item {
458
-
elem := i.elem.apply(f)
457
+
elem := f(i.elem)
459
458
if elem == i.elem {
460
459
return i
461
460
}
···
495
494
changed := false
496
495
elems := i.elems
497
496
for j, prop := range elems {
498
-
if it := prop.item.apply(f); it != prop.item {
497
+
if it := f(prop.item); it != prop.item {
499
498
if !changed {
500
499
elems = slices.Clone(elems)
501
500
changed = true
···
550
549
}
551
550
552
551
func (i *itemIfThenElse) apply(f func(item) item) item {
553
-
ifElem := i.ifElem.apply(f)
552
+
ifElem := f(i.ifElem)
554
553
var thenElem, elseElem item
555
554
if i.thenElem != nil {
556
-
thenElem = i.thenElem.apply(f)
555
+
thenElem = f(i.thenElem)
557
556
}
558
557
if i.elseElem != nil {
559
-
elseElem = i.elseElem.apply(f)
558
+
elseElem = f(i.elseElem)
560
559
}
561
560
562
561
if ifElem == i.ifElem && thenElem == i.thenElem && elseElem == i.elseElem {
+260
encoding/jsonschema/generate_mergeallof_test.go
+260
encoding/jsonschema/generate_mergeallof_test.go
···
1
+
// Copyright 2025 CUE Authors
2
+
//
3
+
// Licensed under the Apache License, Version 2.0 (the "License");
4
+
// you may not use this file except in compliance with the License.
5
+
// You may obtain a copy of the License at
6
+
//
7
+
// http://www.apache.org/licenses/LICENSE-2.0
8
+
//
9
+
// Unless required by applicable law or agreed to in writing, software
10
+
// distributed under the License is distributed on an "AS IS" BASIS,
11
+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12
+
// See the License for the specific language governing permissions and
13
+
// limitations under the License.
14
+
15
+
package jsonschema
16
+
17
+
import (
18
+
"testing"
19
+
20
+
"github.com/go-quicktest/qt"
21
+
"github.com/google/go-cmp/cmp"
22
+
)
23
+
24
+
func TestMergeAllOf(t *testing.T) {
25
+
itemString := &itemType{kinds: []string{"string"}}
26
+
itemNumber := &itemType{kinds: []string{"number"}}
27
+
itemBool := &itemType{kinds: []string{"boolean"}}
28
+
29
+
tests := []struct {
30
+
name string
31
+
item item
32
+
want item
33
+
}{
34
+
{
35
+
name: "NonAllOfItemReturnsAsIs",
36
+
item: itemString,
37
+
want: itemString,
38
+
},
39
+
{
40
+
name: "AllOfWithSingleElementReturnsThatElement",
41
+
item: &itemAllOf{
42
+
elems: []item{itemString},
43
+
},
44
+
want: itemString,
45
+
},
46
+
{
47
+
name: "AllOfWithMultipleElementsStaysAsAllOf",
48
+
item: &itemAllOf{
49
+
elems: []item{itemString, itemNumber},
50
+
},
51
+
want: &itemAllOf{
52
+
elems: []item{itemString, itemNumber},
53
+
},
54
+
},
55
+
{
56
+
name: "NestedAllOfIsFlattened",
57
+
item: &itemAllOf{
58
+
elems: []item{
59
+
itemString,
60
+
&itemAllOf{
61
+
elems: []item{itemNumber, itemBool},
62
+
},
63
+
},
64
+
},
65
+
want: &itemAllOf{
66
+
elems: []item{itemString, itemNumber, itemBool},
67
+
},
68
+
},
69
+
{
70
+
name: "MultipleNestedAllOfAreAllFlattened",
71
+
item: &itemAllOf{
72
+
elems: []item{
73
+
&itemAllOf{
74
+
elems: []item{itemString},
75
+
},
76
+
&itemAllOf{
77
+
elems: []item{itemNumber},
78
+
},
79
+
&itemAllOf{
80
+
elems: []item{itemBool},
81
+
},
82
+
},
83
+
},
84
+
want: &itemAllOf{
85
+
elems: []item{itemString, itemNumber, itemBool},
86
+
},
87
+
},
88
+
{
89
+
name: "DeeplyNestedAllOfIsFullyFlattened",
90
+
item: &itemAllOf{
91
+
elems: []item{
92
+
itemString,
93
+
&itemAllOf{
94
+
elems: []item{
95
+
itemNumber,
96
+
&itemAllOf{
97
+
elems: []item{itemBool},
98
+
},
99
+
},
100
+
},
101
+
},
102
+
},
103
+
want: &itemAllOf{
104
+
elems: []item{itemString, itemNumber, itemBool},
105
+
},
106
+
},
107
+
{
108
+
name: "DuplicateItemsAreRemoved",
109
+
item: &itemAllOf{
110
+
elems: []item{itemString, itemString, itemNumber, itemString},
111
+
},
112
+
want: &itemAllOf{
113
+
elems: []item{itemString, itemNumber},
114
+
},
115
+
},
116
+
{
117
+
name: "DuplicateItemsAfterFlatteningAreRemoved",
118
+
item: &itemAllOf{
119
+
elems: []item{
120
+
itemString,
121
+
&itemAllOf{
122
+
elems: []item{itemString, itemNumber},
123
+
},
124
+
itemString,
125
+
},
126
+
},
127
+
want: &itemAllOf{
128
+
elems: []item{itemString, itemNumber},
129
+
},
130
+
},
131
+
{
132
+
name: "AllOfNestedInOtherItemTypesHasChildrenMerged",
133
+
item: &itemNot{
134
+
elem: &itemAllOf{
135
+
elems: []item{
136
+
&itemAllOf{
137
+
elems: []item{
138
+
&itemAllOf{
139
+
elems: []item{itemString},
140
+
},
141
+
itemNumber,
142
+
},
143
+
},
144
+
},
145
+
},
146
+
},
147
+
want: &itemNot{
148
+
elem: &itemAllOf{
149
+
elems: []item{itemString, itemNumber},
150
+
},
151
+
},
152
+
},
153
+
{
154
+
name: "AllOfNestedInAnyOfIsRecursivelyMerged",
155
+
item: &itemAnyOf{
156
+
elems: []item{
157
+
&itemAllOf{
158
+
elems: []item{
159
+
&itemAllOf{
160
+
elems: []item{itemString},
161
+
},
162
+
itemNumber,
163
+
},
164
+
},
165
+
itemBool,
166
+
},
167
+
},
168
+
want: &itemAnyOf{
169
+
elems: []item{
170
+
&itemAllOf{
171
+
elems: []item{itemString, itemNumber},
172
+
},
173
+
itemBool,
174
+
},
175
+
},
176
+
},
177
+
{
178
+
name: "SingleElementAfterFlatteningAndDeduplication",
179
+
item: &itemAllOf{
180
+
elems: []item{
181
+
&itemAllOf{
182
+
elems: []item{itemString},
183
+
},
184
+
&itemAllOf{
185
+
elems: []item{itemString},
186
+
},
187
+
},
188
+
},
189
+
want: itemString,
190
+
},
191
+
{
192
+
name: "EmptyAllOfBecomesSingleElementAndIsUnwrapped",
193
+
item: &itemAllOf{
194
+
elems: []item{
195
+
&itemAllOf{
196
+
elems: []item{itemString},
197
+
},
198
+
},
199
+
},
200
+
want: itemString,
201
+
},
202
+
{
203
+
name: "ComplexNestedStructureWithMixedTypes",
204
+
item: &itemAllOf{
205
+
elems: []item{
206
+
&itemAllOf{
207
+
elems: []item{
208
+
itemString,
209
+
&itemAllOf{
210
+
elems: []item{itemNumber},
211
+
},
212
+
},
213
+
},
214
+
&itemNot{
215
+
elem: &itemAllOf{
216
+
elems: []item{
217
+
itemBool,
218
+
&itemAllOf{
219
+
elems: []item{&itemFormat{format: "date"}},
220
+
},
221
+
},
222
+
},
223
+
},
224
+
itemString, // Duplicate, should be removed
225
+
},
226
+
},
227
+
want: &itemAllOf{
228
+
elems: []item{
229
+
itemString,
230
+
itemNumber,
231
+
&itemNot{
232
+
elem: &itemAllOf{
233
+
elems: []item{
234
+
itemBool,
235
+
&itemFormat{format: "date"},
236
+
},
237
+
},
238
+
},
239
+
},
240
+
},
241
+
},
242
+
}
243
+
244
+
// Define comparison options for unexported fields
245
+
cmpOpt := cmp.AllowUnexported(
246
+
itemAllOf{},
247
+
itemAnyOf{},
248
+
itemFormat{},
249
+
itemNot{},
250
+
itemType{},
251
+
property{},
252
+
)
253
+
254
+
for _, tt := range tests {
255
+
t.Run(tt.name, func(t *testing.T) {
256
+
got := mergeAllOf(tt.item)
257
+
qt.Assert(t, qt.CmpEquals(got, tt.want, cmpOpt))
258
+
})
259
+
}
260
+
}
+10
-20
encoding/jsonschema/testdata/generate/callop.txtar
+10
-20
encoding/jsonschema/testdata/generate/callop.txtar
···
90
90
}
91
91
score: {
92
92
allOf: [{
93
-
type: "number"
94
-
multipleOf: 10
93
+
type: "number"
95
94
}, {
96
-
type: "integer"
95
+
type: "integer"
96
+
multipleOf: 10
97
97
}]
98
98
}
99
99
shortString: {
···
105
105
format: "time"
106
106
}
107
107
userName: {
108
-
allOf: [{
109
-
type: "string"
110
-
maxLength: 20
111
-
}, {
112
-
type: "string"
113
-
minLength: 3
114
-
}]
108
+
type: "string"
109
+
maxLength: 20
110
+
minLength: 3
115
111
}
116
112
}
117
113
}
···
124
120
1:158
125
121
./datatest/tests.cue:26:18
126
122
-- out/generate-v3/badScore2 --
127
-
badScore2.data.score: invalid value 5 (does not satisfy matchN): 1 matched, expected 2:
128
-
1:278
129
-
./datatest/tests.cue:42:15
130
123
badScore2.data.score: invalid value 5 (does not satisfy math.MultipleOf(10)):
131
-
1:304
124
+
1:323
132
125
1:278
133
126
./datatest/tests.cue:42:15
134
127
-- out/generate-v3/badTime --
···
136
129
1:113
137
130
./datatest/tests.cue:34:14
138
131
-- out/generate-v3/badUserName --
139
-
badUserName.data.userName: invalid value "x" (does not satisfy matchN): 1 matched, expected 2:
140
-
1:446
141
-
./datatest/tests.cue:38:18
142
132
badUserName.data.userName: invalid value "x" (does not satisfy strings.MinRunes(3)):
143
-
1:505
144
-
1:446
145
-
1:505
133
+
1:477
134
+
1:462
135
+
1:477
146
136
./datatest/tests.cue:38:18
147
137
-- out/generate-v3/notMultiple --
148
138
notMultiple.data.multiple: invalid value 1 (does not satisfy math.MultipleOf(5)):