diff --git a/encoding/jsonschema/constraints.go b/encoding/jsonschema/constraints.go index f6c5bc246a5..f7fe70c4c6c 100644 --- a/encoding/jsonschema/constraints.go +++ b/encoding/jsonschema/constraints.go @@ -192,9 +192,18 @@ var constraints = []*constraint{ p1("enum", func(key string, n cue.Value, s *state) { var a []ast.Expr + var types cue.Kind for _, x := range s.listItems("enum", n, true) { + if (s.allowedTypes & x.Kind()) == 0 { + // Enum value is redundant because it's + // not in the allowed type set. + continue + } a = append(a, s.value(x)) + types |= x.Kind() } + s.knownTypes &= types + s.allowedTypes &= types s.all.add(n, ast.NewBinExpr(token.OR, a...)) }), @@ -207,6 +216,8 @@ var constraints = []*constraint{ p1d("const", 6, func(key string, n cue.Value, s *state) { s.all.add(n, s.value(n)) + s.allowedTypes &= n.Kind() + s.knownTypes &= n.Kind() }), p1("default", func(key string, n cue.Value, s *state) { @@ -249,8 +260,6 @@ var constraints = []*constraint{ p1("$defs", addDefinitions), p1("definitions", addDefinitions), p1("$ref", func(key string, n cue.Value, s *state) { - s.usedTypes = allTypes - u := s.resolveURI(n) fragmentParts, err := splitFragment(u) @@ -288,15 +297,26 @@ var constraints = []*constraint{ p2("allOf", func(key string, n cue.Value, s *state) { var a []ast.Expr + var knownTypes cue.Kind for _, v := range s.listItems("allOf", n, false) { x, sub := s.schemaState(v, s.allowedTypes, nil, true) s.allowedTypes &= sub.allowedTypes - s.usedTypes |= sub.usedTypes if sub.hasConstraints() { + // This might seem a little odd, since the actual + // types are the intersection of the known types + // of the allOf members. However, knownTypes + // is really there to avoid adding redundant disjunctions. + // So if we have (int & string) & (disjunction) + // we definitely don't have to add int or string to + // disjunction. + knownTypes |= sub.knownTypes a = append(a, x) } } + // TODO maybe give an error/warning if s.allowedTypes == 0 + // as that's a known-impossible assertion? if len(a) > 0 { + s.knownTypes &= knownTypes s.all.add(n, ast.NewBinExpr(token.AND, a...)) } }), @@ -304,19 +324,23 @@ var constraints = []*constraint{ p2("anyOf", func(key string, n cue.Value, s *state) { var types cue.Kind var a []ast.Expr + var knownTypes cue.Kind for _, v := range s.listItems("anyOf", n, false) { x, sub := s.schemaState(v, s.allowedTypes, nil, true) types |= sub.allowedTypes + knownTypes |= sub.knownTypes a = append(a, x) } s.allowedTypes &= types if len(a) > 0 { + s.knownTypes &= knownTypes s.all.add(n, ast.NewBinExpr(token.OR, a...)) } }), p2("oneOf", func(key string, n cue.Value, s *state) { var types cue.Kind + var knownTypes cue.Kind var a []ast.Expr hasSome := false for _, v := range s.listItems("oneOf", n, false) { @@ -329,12 +353,15 @@ var constraints = []*constraint{ } if !isAny(x) { + knownTypes |= sub.knownTypes a = append(a, x) } } + // TODO if there are no elements in the oneOf, validation + // should fail. s.allowedTypes &= types if len(a) > 0 && hasSome { - s.usedTypes = allTypes + s.knownTypes &= knownTypes s.all.add(n, ast.NewBinExpr(token.OR, a...)) } @@ -352,19 +379,16 @@ var constraints = []*constraint{ } return } - s.usedTypes |= cue.StringKind s.add(n, stringType, &ast.UnaryExpr{Op: token.MAT, X: s.string(n)}) }), p1("minLength", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.StringKind min := s.number(n) strings := s.addImport(n, "strings") s.add(n, stringType, ast.NewCall(ast.NewSel(strings, "MinRunes"), min)) }), p1("maxLength", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.StringKind max := s.number(n) strings := s.addImport(n, "strings") s.add(n, stringType, ast.NewCall(ast.NewSel(strings, "MaxRunes"), max)) @@ -372,12 +396,10 @@ var constraints = []*constraint{ p1d("contentMediaType", 7, func(key string, n cue.Value, s *state) { // TODO: only mark as used if it generates something. - // s.usedTypes |= cue.StringKind }), p1d("contentEncoding", 7, func(key string, n cue.Value, s *state) { // TODO: only mark as used if it generates something. - // s.usedTypes |= cue.StringKind // 7bit, 8bit, binary, quoted-printable and base64. // RFC 2054, part 6.1. // https://tools.ietf.org/html/rfc2045 @@ -387,7 +409,6 @@ var constraints = []*constraint{ // Number constraints p2("minimum", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.NumberKind op := token.GEQ if s.exclusiveMin { op = token.GTR @@ -400,12 +421,10 @@ var constraints = []*constraint{ s.exclusiveMin = true return } - s.usedTypes |= cue.NumberKind s.add(n, numType, &ast.UnaryExpr{Op: token.GTR, X: s.number(n)}) }), p2("maximum", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.NumberKind op := token.LEQ if s.exclusiveMax { op = token.LSS @@ -418,12 +437,10 @@ var constraints = []*constraint{ s.exclusiveMax = true return } - s.usedTypes |= cue.NumberKind s.add(n, numType, &ast.UnaryExpr{Op: token.LSS, X: s.number(n)}) }), p1("multipleOf", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.NumberKind multiple := s.number(n) var x big.Int _, _ = n.MantExp(&x) @@ -437,7 +454,6 @@ var constraints = []*constraint{ // Object constraints p1("properties", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.StructKind obj := s.object(n) if n.Kind() != cue.StructKind { @@ -475,8 +491,6 @@ var constraints = []*constraint{ return } - s.usedTypes |= cue.StructKind - // TODO: detect that properties is defined somewhere. // s.errf(n, `"required" without a "properties" field`) obj := s.object(n) @@ -518,7 +532,6 @@ var constraints = []*constraint{ p1d("propertyNames", 6, func(key string, n cue.Value, s *state) { // [=~pattern]: _ if names, _ := s.schemaState(n, cue.StringKind, nil, false); !isAny(names) { - s.usedTypes |= cue.StructKind x := ast.NewStruct(ast.NewList(names), ast.NewIdent("_")) s.add(n, objectType, x) } @@ -526,23 +539,18 @@ var constraints = []*constraint{ // TODO: reenable when we have proper non-monotonic contraint validation. // p1("minProperties", func(key string, n cue.Value, s *state) { - // s.usedTypes |= cue.StructKind // pkg := s.addImport(n, "struct") // s.addConjunct(n, ast.NewCall(ast.NewSel(pkg, "MinFields"), s.uint(n))) // }), p1("maxProperties", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.StructKind - pkg := s.addImport(n, "struct") x := ast.NewCall(ast.NewSel(pkg, "MaxFields"), s.uint(n)) s.add(n, objectType, x) }), p1("dependencies", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.StructKind - // Schema and property dependencies. // TODO: the easiest implementation is with comprehensions. // The nicer implementation is with disjunctions. This has to be done @@ -556,7 +564,6 @@ var constraints = []*constraint{ }), p2("patternProperties", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.StructKind if n.Kind() != cue.StructKind { s.errf(n, `value of "patternProperties" must be an object, found %v`, n.Kind()) } @@ -583,7 +590,6 @@ var constraints = []*constraint{ s.closeStruct = !s.boolValue(n) case cue.StructKind: - s.usedTypes |= cue.StructKind s.closeStruct = true obj := s.object(n) if len(obj.Elts) == 0 { @@ -609,7 +615,6 @@ var constraints = []*constraint{ // Array constraints. p1("items", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.ListKind switch n.Kind() { case cue.StructKind: elem := s.schema(n) @@ -638,7 +643,6 @@ var constraints = []*constraint{ case cue.StructKind: if s.list != nil { - s.usedTypes |= cue.ListKind elem := s.schema(n) s.list.Elts = append(s.list.Elts, &ast.Ellipsis{Type: elem}) } @@ -649,7 +653,6 @@ var constraints = []*constraint{ }), p1("contains", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.ListKind list := s.addImport(n, "list") // TODO: Passing non-concrete values is not yet supported in CUE. if x := s.schema(n); !isAny(x) { @@ -661,7 +664,6 @@ var constraints = []*constraint{ // TODO: min/maxContains p1("minItems", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.ListKind a := []ast.Expr{} p, err := n.Uint64() if err != nil { @@ -678,14 +680,12 @@ var constraints = []*constraint{ }), p1("maxItems", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.ListKind list := s.addImport(n, "list") x := ast.NewCall(ast.NewSel(list, "MaxItems"), clearPos(s.uint(n))) s.add(n, arrayType, x) }), p1("uniqueItems", func(key string, n cue.Value, s *state) { - s.usedTypes |= cue.ListKind if s.boolValue(n) { list := s.addImport(n, "list") s.add(n, arrayType, ast.NewCall(ast.NewSel(list, "UniqueItems"))) diff --git a/encoding/jsonschema/decode.go b/encoding/jsonschema/decode.go index 4f8fed06a70..1fe6e1cbab4 100644 --- a/encoding/jsonschema/decode.go +++ b/encoding/jsonschema/decode.go @@ -333,9 +333,16 @@ type state struct { all constraintInfo // values and oneOf etc. nullable *ast.BasicLit // nullable - usedTypes cue.Kind + // allowedTypes holds the set of types that + // this node is allowed to be. allowedTypes cue.Kind + // knownTypes holds the set of types that this node + // is known to be one of by virtue of the constraints inside + // all. This is used to avoid adding redundant elements + // to the disjunction created by [state.finalize]. + knownTypes cue.Kind + default_ ast.Expr examples []ast.Expr title string @@ -414,15 +421,15 @@ const allTypes = cue.NullKind | cue.BoolKind | cue.NumberKind | cue.IntKind | // finalize constructs a CUE type from the collected constraints. func (s *state) finalize() (e ast.Expr) { + if s.allowedTypes == 0 { + // Nothing is possible. + s.addErr(errors.Newf(s.pos.Pos(), "constraints are not possible to satisfy")) + return &ast.BottomLit{} + } + conjuncts := []ast.Expr{} disjuncts := []ast.Expr{} - types := s.allowedTypes &^ s.usedTypes - if types == allTypes { - disjuncts = append(disjuncts, ast.NewIdent("_")) - types = 0 - } - // Sort literal structs and list last for nicer formatting. sort.SliceStable(s.types[arrayType].constraints, func(i, j int) bool { _, ok := s.types[arrayType].constraints[i].(*ast.ListLit) @@ -438,59 +445,57 @@ func (s *state) finalize() (e ast.Expr) { typIndex int } var excluded []excludeInfo - npossible := 0 - nexcluded := 0 - for i, t := range s.types { - k := coreToCUE[i] - isAllowed := s.allowedTypes&k != 0 - if len(t.constraints) > 0 { - npossible++ - if t.typ == nil && !isAllowed { - // TODO this implies a redundant constraint, which is technically allowed, - // but in practice probably represents a mistake. We could provide - // a mode that warns about such likely errors. - nexcluded++ - for _, c := range t.constraints { - excluded = append(excluded, excludeInfo{c.Pos(), i}) - } - continue - } - x := ast.NewBinExpr(token.AND, t.constraints...) - disjuncts = append(disjuncts, x) - } else if s.usedTypes&k != 0 { - npossible++ - continue - } else if t.typ != nil { - npossible++ - if !isAllowed { - // TODO this implies a redundant type constraint, which is technically allowed, - // but in practice probably represents a mistake. We could provide - // a mode that warns about such likely errors. - nexcluded++ - excluded = append(excluded, excludeInfo{t.typ.Pos(), i}) - continue + + needsTypeDisjunction := s.allowedTypes != s.knownTypes + if !needsTypeDisjunction { + for i, t := range s.types { + k := coreToCUE[i] + if len(t.constraints) > 0 && s.allowedTypes&k != 0 { + // We need to include at least one type-specific + // constraint in the disjunction. + needsTypeDisjunction = true + break } - disjuncts = append(disjuncts, t.typ) - } else if types&k != 0 { - npossible++ - x := kindToAST(k) - if x != nil { + } + } + + if needsTypeDisjunction { + npossible := 0 + nexcluded := 0 + for i, t := range s.types { + k := coreToCUE[i] + allowed := s.allowedTypes&k != 0 + switch { + case len(t.constraints) > 0: + npossible++ + if !allowed { + nexcluded++ + for _, c := range t.constraints { + excluded = append(excluded, excludeInfo{c.Pos(), i}) + } + continue + } + x := ast.NewBinExpr(token.AND, t.constraints...) disjuncts = append(disjuncts, x) + case allowed: + npossible++ + if s.knownTypes&k != 0 { + disjuncts = append(disjuncts, kindToAST(k)) + } } } - } - if nexcluded == npossible { - // All possibilities have been excluded: this is an impossible - // schema. - for _, e := range excluded { - s.addErr(errors.Newf(e.pos, - "constraint not allowed because type %s is excluded", - coreTypeName[e.typIndex], - )) + if nexcluded == npossible { + // All possibilities have been excluded: this is an impossible + // schema. + for _, e := range excluded { + s.addErr(errors.Newf(e.pos, + "constraint not allowed because type %s is excluded", + coreTypeName[e.typIndex], + )) + } } } conjuncts = append(conjuncts, s.all.constraints...) - obj := s.obj if obj == nil { obj, _ = s.types[objectType].typ.(*ast.StructLit) @@ -507,7 +512,14 @@ func (s *state) finalize() (e ast.Expr) { } if len(conjuncts) == 0 { - e = &ast.BottomLit{} + // There are no conjuncts, which can only happen when there + // are no disjuncts, which can only happen when the entire + // set of disjuncts is redundant with respect to the types + // already implied by s.all. As we've already checked that + // s.allowedTypes is non-zero (so we know that + // it's not bottom) and we need _some_ expression + // to be part of the subequent syntax, we use top. + e = ast.NewIdent("_") } else { e = ast.NewBinExpr(token.AND, conjuncts...) } @@ -523,7 +535,7 @@ outer: // check conditions where default can be skipped. switch x := s.default_.(type) { case *ast.ListLit: - if s.usedTypes == cue.ListKind && len(x.Elts) == 0 { + if s.allowedTypes == cue.ListKind && len(x.Elts) == 0 { break outer } } @@ -556,6 +568,10 @@ outer: } } + // Now that we've expressed the schema as actual syntax, + // all the allowed types are actually explicit and will not + // need to be mentioned again. + s.knownTypes = s.allowedTypes return e } @@ -594,15 +610,19 @@ func (s *state) schema(n cue.Value, idRef ...label) ast.Expr { return expr } -// schemaState is a low-level API for schema. isLogical specifies whether the -// caller is a logical operator like anyOf, allOf, oneOf, or not. -func (s *state) schemaState(n cue.Value, types cue.Kind, idRef []label, isLogical bool) (ast.Expr, *state) { +// schemaState returns a new state value derived from s. +// n holds the JSONSchema node to translate to a schema. +// types holds the set of possible types that the value can hold. +// idRef holds the path to the value. +// isLogical specifies whether the caller is a logical operator like anyOf, allOf, oneOf, or not. +func (s *state) schemaState(n cue.Value, types cue.Kind, idRef []label, isLogical bool) (_e ast.Expr, _ *state) { state := &state{ up: s, schemaVersion: s.schemaVersion, isSchema: s.isSchema, decoder: s.decoder, allowedTypes: types, + knownTypes: allTypes, path: s.path, idRef: idRef, pos: n, @@ -638,8 +658,6 @@ func (s *state) schemaState(n cue.Value, types cue.Kind, idRef []label, isLogica func (s *state) value(n cue.Value) ast.Expr { k := n.Kind() - s.usedTypes |= k - s.allowedTypes &= k switch k { case cue.ListKind: a := []ast.Expr{} diff --git a/encoding/jsonschema/testdata/emptyobjinanyof.txtar b/encoding/jsonschema/testdata/emptyobjinanyof.txtar index c32734221d8..abfb318ff37 100644 --- a/encoding/jsonschema/testdata/emptyobjinanyof.txtar +++ b/encoding/jsonschema/testdata/emptyobjinanyof.txtar @@ -23,4 +23,4 @@ -- out/decode/cue -- _ -#shell: (string | ("bash" | "sh" | "cmd" | "powershell")) & string +#shell: string | ("bash" | "sh" | "cmd" | "powershell") diff --git a/encoding/jsonschema/testdata/impossibleallof.txtar b/encoding/jsonschema/testdata/impossibleallof.txtar index 544c88da441..5e9b8905d08 100644 --- a/encoding/jsonschema/testdata/impossibleallof.txtar +++ b/encoding/jsonschema/testdata/impossibleallof.txtar @@ -6,5 +6,7 @@ ] } -- out/decode/err -- -constraint not allowed because type string is excluded: - type.json:4:11 +constraints are not possible to satisfy: + type.json:1:1 +constraints are not possible to satisfy: + type.json:4:9 diff --git a/encoding/jsonschema/testdata/issue3176.txtar b/encoding/jsonschema/testdata/issue3176.txtar index 8a4c8d28e7c..51884836449 100644 --- a/encoding/jsonschema/testdata/issue3176.txtar +++ b/encoding/jsonschema/testdata/issue3176.txtar @@ -20,7 +20,7 @@ import "strings" ({ ... -} | strings.MaxRunes(3)) & { +} | strings.MaxRunes(3)) & (string | { {[=~"^x-" & !~"^()$"]: string} ... -} +}) diff --git a/encoding/jsonschema/testdata/issue3351.txtar b/encoding/jsonschema/testdata/issue3351.txtar index edff31f9531..4424fef30d5 100644 --- a/encoding/jsonschema/testdata/issue3351.txtar +++ b/encoding/jsonschema/testdata/issue3351.txtar @@ -72,7 +72,7 @@ _schema: { $let?: [string]: null | bool | number | string | [...] | { [string]: _schema_1 } - $sort?: (_schema_5 | [...number]) & _ + $sort?: _schema_5 | [...number] {[!~"^($else|$let|$sort)$"]: #["jsone-value"]} #: "jsone-value": _schema_A | [..._schema_8] diff --git a/encoding/jsonschema/testdata/typedis.txtar b/encoding/jsonschema/testdata/typedis.txtar index 94fa42cccd5..faf9e8c1313 100644 --- a/encoding/jsonschema/testdata/typedis.txtar +++ b/encoding/jsonschema/testdata/typedis.txtar @@ -40,6 +40,6 @@ // Main schema intOrString1?: int | string intOrString2?: int | string -intOrString3?: (int | string) & (number | string) -disjunction?: (int | string) & (number | string) | int & >=3 +intOrString3?: int | string +disjunction?: int | string | int & >=3 ... diff --git a/encoding/jsonschema/testdata/typeexcluded.txtar b/encoding/jsonschema/testdata/typeexcluded.txtar index 2de2374fda1..8f4a92706f1 100644 --- a/encoding/jsonschema/testdata/typeexcluded.txtar +++ b/encoding/jsonschema/testdata/typeexcluded.txtar @@ -120,15 +120,11 @@ e3?: list.UniqueItems() & [_, ...] & [...strings.MinRunes(1)] e4?: [...string] e5?: int & >=0 e6?: [...=~"^[A-Za-z0-9 _.-]+$"] -e7?: (true | { - ... -} | { +e7?: true | { disableFix?: bool ... } & { ignoreMediaFeatureNames?: list.UniqueItems() & [_, ...] & [...string] ... -}) & { - ... } ...