Skip to content

Commit 7ca1a14

Browse files
committed
encoding/jsonschema: better "type excluded" logic
Currently, the error "constraint not allowed because type xxx is excluded" is produced when there is a constraint for a given kind and the type is known to be disallowed. However, types are not exclusive in this way in JSONSchema. Instead, produce an error only when _all_ types are known to be excluded which, although technically still not an error, is much less likely to result in a real-world schema being rejected. Fixes #393. Signed-off-by: Roger Peppe <[email protected]> Change-Id: I45b4a7fa540ba537f2e2dbfe973782474be280d5 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199308 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
1 parent 5fde360 commit 7ca1a14

File tree

2 files changed

+60
-32
lines changed

2 files changed

+60
-32
lines changed

encoding/jsonschema/decode.go

+31-9
Original file line numberDiff line numberDiff line change
@@ -433,40 +433,62 @@ func (s *state) finalize() (e ast.Expr) {
433433
return !ok
434434
})
435435

436+
type excludeInfo struct {
437+
pos token.Pos
438+
typIndex int
439+
}
440+
var excluded []excludeInfo
441+
npossible := 0
442+
nexcluded := 0
436443
for i, t := range s.types {
437444
k := coreToCUE[i]
438445
isAllowed := s.allowedTypes&k != 0
439446
if len(t.constraints) > 0 {
447+
npossible++
440448
if t.typ == nil && !isAllowed {
449+
// TODO this implies a redundant constraint, which is technically allowed,
450+
// but in practice probably represents a mistake. We could provide
451+
// a mode that warns about such likely errors.
452+
nexcluded++
441453
for _, c := range t.constraints {
442-
s.addErr(errors.Newf(c.Pos(),
443-
"constraint not allowed because type %s is excluded",
444-
coreTypeName[i],
445-
))
454+
excluded = append(excluded, excludeInfo{c.Pos(), i})
446455
}
447456
continue
448457
}
449458
x := ast.NewBinExpr(token.AND, t.constraints...)
450459
disjuncts = append(disjuncts, x)
451460
} else if s.usedTypes&k != 0 {
461+
npossible++
452462
continue
453463
} else if t.typ != nil {
464+
npossible++
454465
if !isAllowed {
455-
s.addErr(errors.Newf(t.typ.Pos(),
456-
"constraint not allowed because type %s is excluded",
457-
coreTypeName[i],
458-
))
466+
// TODO this implies a redundant type constraint, which is technically allowed,
467+
// but in practice probably represents a mistake. We could provide
468+
// a mode that warns about such likely errors.
469+
nexcluded++
470+
excluded = append(excluded, excludeInfo{t.typ.Pos(), i})
459471
continue
460472
}
461473
disjuncts = append(disjuncts, t.typ)
462474
} else if types&k != 0 {
475+
npossible++
463476
x := kindToAST(k)
464477
if x != nil {
465478
disjuncts = append(disjuncts, x)
466479
}
467480
}
468481
}
469-
482+
if nexcluded == npossible {
483+
// All possibilities have been excluded: this is an impossible
484+
// schema.
485+
for _, e := range excluded {
486+
s.addErr(errors.Newf(e.pos,
487+
"constraint not allowed because type %s is excluded",
488+
coreTypeName[e.typIndex],
489+
))
490+
}
491+
}
470492
conjuncts = append(conjuncts, s.all.constraints...)
471493

472494
obj := s.obj

encoding/jsonschema/testdata/typeexcluded.txtar

+29-23
Original file line numberDiff line numberDiff line change
@@ -103,26 +103,32 @@
103103
}
104104
}
105105
}
106-
-- out/decode/err --
107-
constraint not allowed because type string is excluded:
108-
type.json:10:7
109-
constraint not allowed because type array is excluded:
110-
type.json:18:9
111-
constraint not allowed because type bool is excluded:
112-
type.json:19:9
113-
constraint not allowed because type number is excluded:
114-
type.json:20:9
115-
constraint not allowed because type object is excluded:
116-
type.json:21:9
117-
constraint not allowed because type null is excluded:
118-
type.json:23:9
119-
constraint not allowed because type object is excluded:
120-
type.json:32:7
121-
constraint not allowed because type string is excluded:
122-
type.json:44:7
123-
constraint not allowed because type array is excluded:
124-
type.json:51:9
125-
constraint not allowed because type string is excluded:
126-
type.json:64:7
127-
constraint not allowed because type bool is excluded:
128-
type.json:68:9
106+
-- out/decode/cue --
107+
import (
108+
"list"
109+
"strings"
110+
)
111+
112+
e0?: int & >=2 & <=3
113+
e1?: "cover" | "contain"
114+
115+
// none
116+
e2?: "none"
117+
e3?: list.UniqueItems() & [_, ...] & [...strings.MinRunes(1)]
118+
119+
// Main authors of the plugin
120+
e4?: [...string]
121+
e5?: int & >=0
122+
e6?: [...=~"^[A-Za-z0-9 _.-]+$"]
123+
e7?: (true | {
124+
...
125+
} | {
126+
disableFix?: bool
127+
...
128+
} & {
129+
ignoreMediaFeatureNames?: list.UniqueItems() & [_, ...] & [...string]
130+
...
131+
}) & {
132+
...
133+
}
134+
...

0 commit comments

Comments
 (0)