Skip to content

Commit 9dd5b47

Browse files
swallezpquentin
swallez
and
pquentin
authored
Fix spec validator bugs (#3295)
* Fix open generics definition in type alias verification * Add void builtin type * A tagged union can have generic parameters * Add binary builtin type * Fix lint --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
1 parent fb25929 commit 9dd5b47

File tree

2 files changed

+18
-68
lines changed

2 files changed

+18
-68
lines changed

‎compiler/src/steps/validate-model.ts

+16-20
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
149149

150150
// Register builtin types
151151
for (const name of [
152-
'string', 'boolean', 'number', 'null'
152+
'string', 'boolean', 'number', 'null', 'void', 'binary'
153153
]) {
154154
const typeName = {
155155
namespace: '_builtins',
@@ -541,33 +541,29 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
541541
}
542542

543543
function validateTypeAlias (typeDef: model.TypeAlias): void {
544-
const openGenerics = new Set(typeDef.generics?.map(t => t.name))
544+
const openGenerics = openGenericSet(typeDef)
545545

546546
if (typeDef.variants != null) {
547-
if (typeDef.generics != null && typeDef.generics.length !== 0) {
548-
modelError('A tagged union should not have generic parameters')
549-
}
550-
551547
if (typeDef.type.kind !== 'union_of') {
552548
modelError('The "variants" tag only applies to unions')
553549
} else {
554-
validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants)
550+
validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants, openGenerics)
555551
}
556552
} else {
557553
validateValueOf(typeDef.type, openGenerics)
558554
}
559555
}
560556

561-
function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged): void {
557+
function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged, openGenerics: Set<string>): void {
562558
if (variants.kind === 'external_tag') {
563559
// All items must have a 'variant' attribute
564-
const items = flattenUnionMembers(valueOf)
560+
const items = flattenUnionMembers(valueOf, openGenerics)
565561

566562
for (const item of items) {
567563
if (item.kind !== 'instance_of') {
568564
modelError('Items of externally tagged unions must be types with a "variant_tag" annotation')
569565
} else {
570-
validateTypeRef(item.type, undefined, new Set<string>())
566+
validateTypeRef(item.type, item.generics, openGenerics)
571567
const type = getTypeDef(item.type)
572568
if (type == null) {
573569
modelError(`Type ${fqn(item.type)} not found`)
@@ -582,13 +578,13 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
582578
}
583579
} else if (variants.kind === 'internal_tag') {
584580
const tagName = variants.tag
585-
const items = flattenUnionMembers(valueOf)
581+
const items = flattenUnionMembers(valueOf, openGenerics)
586582

587583
for (const item of items) {
588584
if (item.kind !== 'instance_of') {
589585
modelError('Items of internally tagged unions must be type references')
590586
} else {
591-
validateTypeRef(item.type, undefined, new Set<string>())
587+
validateTypeRef(item.type, item.generics, openGenerics)
592588
const type = getTypeDef(item.type)
593589
if (type == null) {
594590
modelError(`Type ${fqn(item.type)} not found`)
@@ -601,7 +597,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
601597
}
602598
}
603599

604-
validateValueOf(valueOf, new Set())
600+
validateValueOf(valueOf, openGenerics)
605601
} else if (variants.kind === 'untagged') {
606602
if (fqn(parentName) !== '_types.query_dsl:DecayFunction' &&
607603
fqn(parentName) !== '_types.query_dsl:DistanceFeatureQuery' &&
@@ -614,15 +610,15 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
614610
modelError(`Type ${fqn(variants.untypedVariant)} not found`)
615611
}
616612

617-
const items = flattenUnionMembers(valueOf)
613+
const items = flattenUnionMembers(valueOf, openGenerics)
618614
const baseTypes = new Set<string>()
619615
let foundUntyped = false
620616

621617
for (const item of items) {
622618
if (item.kind !== 'instance_of') {
623619
modelError('Items of type untagged unions must be type references')
624620
} else {
625-
validateTypeRef(item.type, undefined, new Set<string>())
621+
validateTypeRef(item.type, item.generics, openGenerics)
626622
const type = getTypeDef(item.type)
627623
if (type == null) {
628624
modelError(`Type ${fqn(item.type)} not found`)
@@ -674,7 +670,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
674670
// -----------------------------------------------------------------------------------------------
675671
// Constituents of type definitions
676672

677-
function openGenericSet (typeDef: model.Request | model.Response | model.Interface): Set<string> {
673+
function openGenericSet (typeDef: model.Request | model.Response | model.Interface | model.TypeAlias): Set<string> {
678674
return new Set((typeDef.generics ?? []).map(name => fqn(name)))
679675
}
680676

@@ -879,7 +875,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
879875
} else {
880876
// tagged union: the discriminant tells us what to look for, check each member in isolation
881877
assert(typeDef.type.kind === 'union_of', 'Variants are only allowed on union_of type aliases')
882-
for (const item of flattenUnionMembers(typeDef.type)) {
878+
for (const item of flattenUnionMembers(typeDef.type, new Set())) {
883879
validateValueOfJsonEvents(item)
884880
}
885881

@@ -894,13 +890,13 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
894890
}
895891

896892
/** Build the flattened item list of potentially nested unions (this is used for large unions) */
897-
function flattenUnionMembers (union: model.UnionOf): model.ValueOf[] {
893+
function flattenUnionMembers (union: model.UnionOf, openGenerics: Set<string>): model.ValueOf[] {
898894
const allItems = new Array<model.ValueOf>()
899895

900896
function collectItems (items: model.ValueOf[]): void {
901897
for (const item of items) {
902898
if (item.kind !== 'instance_of') {
903-
validateValueOf(item, new Set<string>())
899+
validateValueOf(item, openGenerics)
904900
allItems.push(item)
905901
} else {
906902
const itemType = getTypeDef(item.type)
@@ -910,7 +906,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma
910906
// Recurse in nested union
911907
collectItems(itemType.type.items)
912908
} else {
913-
validateValueOf(item, new Set<string>())
909+
validateValueOf(item, openGenerics)
914910
allItems.push(item)
915911
}
916912
}

‎output/schema/validation-errors.json

+2-48
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,9 @@
3636
],
3737
"response": []
3838
},
39-
"async_search.get": {
40-
"request": [],
41-
"response": [
42-
"type_alias definition _types:EpochTime / instance_of - No type definition for '_types.EpochTime:Unit'",
43-
"type_alias definition _types.aggregations:Buckets / union_of / dictionary_of / instance_of - No type definition for '_types.aggregations.Buckets:TBucket'",
44-
"type_alias definition _types.aggregations:Buckets / union_of / array_of / instance_of - No type definition for '_types.aggregations.Buckets:TBucket'",
45-
"type_alias definition _spec_utils:Void / instance_of - No type definition for '_builtins:void'",
46-
"type_alias definition _types:DurationValue / instance_of - No type definition for '_types.DurationValue:Unit'",
47-
"type_alias definition _global.search._types:Suggest - A tagged union should not have generic parameters",
48-
"type_alias definition _global.search._types:Suggest / instance_of / Generics / instance_of - No type definition for '_global.search._types.Suggest:TDocument'",
49-
"type_alias definition _global.search._types:Suggest - Expected 1 generic parameters but got 0"
50-
]
51-
},
5239
"async_search.submit": {
5340
"request": [
54-
"interface definition _types:QueryVectorBuilder - Property text_embedding is a single-variant and must be required",
55-
"type_alias definition _spec_utils:PipeSeparatedFlags / union_of / instance_of - No type definition for '_spec_utils.PipeSeparatedFlags:T'"
41+
"interface definition _types:QueryVectorBuilder - Property text_embedding is a single-variant and must be required"
5642
],
5743
"response": []
5844
},
@@ -62,12 +48,6 @@
6248
],
6349
"response": []
6450
},
65-
"cat.allocation": {
66-
"request": [],
67-
"response": [
68-
"type_alias definition _spec_utils:Stringified / union_of / instance_of - No type definition for '_spec_utils.Stringified:T'"
69-
]
70-
},
7151
"cat.count": {
7252
"request": [
7353
"Request: query parameter 'master_timeout' does not exist in the json spec"
@@ -353,12 +333,6 @@
353333
],
354334
"response": []
355335
},
356-
"connector.update_error": {
357-
"request": [
358-
"type_alias definition _spec_utils:WithNullValue / union_of / instance_of - No type definition for '_spec_utils.WithNullValue:T'"
359-
],
360-
"response": []
361-
},
362336
"connector.update_features": {
363337
"request": [
364338
"Missing request & response"
@@ -415,12 +389,6 @@
415389
],
416390
"response": []
417391
},
418-
"esql.query": {
419-
"request": [],
420-
"response": [
421-
"type_alias definition _types:EsqlColumns / instance_of - No type definition for '_builtins:binary'"
422-
]
423-
},
424392
"features.get_features": {
425393
"request": [
426394
"Request: missing json spec query parameter 'master_timeout'"
@@ -445,12 +413,6 @@
445413
],
446414
"response": []
447415
},
448-
"fleet.msearch": {
449-
"request": [],
450-
"response": [
451-
"type_alias definition _global.msearch:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.msearch.ResponseItem:TDocument'"
452-
]
453-
},
454416
"fleet.post_secret": {
455417
"request": [
456418
"Missing request & response"
@@ -650,12 +612,6 @@
650612
],
651613
"response": []
652614
},
653-
"mget": {
654-
"request": [],
655-
"response": [
656-
"type_alias definition _global.mget:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.mget.ResponseItem:TDocument'"
657-
]
658-
},
659615
"ml.delete_trained_model": {
660616
"request": [
661617
"Request: missing json spec query parameter 'timeout'"
@@ -764,9 +720,7 @@
764720
"Request: query parameter 'grid_agg' does not exist in the json spec",
765721
"Request: missing json spec query parameter 'track_total_hits'"
766722
],
767-
"response": [
768-
"type_alias definition _types:MapboxVectorTiles / instance_of - No type definition for '_builtins:binary'"
769-
]
723+
"response": []
770724
},
771725
"search_shards": {
772726
"request": [

0 commit comments

Comments
 (0)