From f642d74d7450e9e91c1014031ca9163d8bfe7e0e Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Fri, 13 Dec 2024 08:34:28 +0100 Subject: [PATCH] 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 (cherry picked from commit 9dd5b4760e80bba78839a3845a4ee682fb92efd8) # Conflicts: # output/schema/validation-errors.json --- compiler/src/steps/validate-model.ts | 36 ++++++++++------------ output/schema/validation-errors.json | 46 +++------------------------- 2 files changed, 20 insertions(+), 62 deletions(-) diff --git a/compiler/src/steps/validate-model.ts b/compiler/src/steps/validate-model.ts index 86e7530f81..0f3ee1fccc 100644 --- a/compiler/src/steps/validate-model.ts +++ b/compiler/src/steps/validate-model.ts @@ -149,7 +149,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma // Register builtin types for (const name of [ - 'string', 'boolean', 'number', 'null' + 'string', 'boolean', 'number', 'null', 'void', 'binary' ]) { const typeName = { namespace: '_builtins', @@ -546,33 +546,29 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma } function validateTypeAlias (typeDef: model.TypeAlias): void { - const openGenerics = new Set(typeDef.generics?.map(t => t.name)) + const openGenerics = openGenericSet(typeDef) if (typeDef.variants != null) { - if (typeDef.generics != null && typeDef.generics.length !== 0) { - modelError('A tagged union should not have generic parameters') - } - if (typeDef.type.kind !== 'union_of') { modelError('The "variants" tag only applies to unions') } else { - validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants) + validateTaggedUnion(typeDef.name, typeDef.type, typeDef.variants, openGenerics) } } else { validateValueOf(typeDef.type, openGenerics) } } - function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged): void { + function validateTaggedUnion (parentName: TypeName, valueOf: model.UnionOf, variants: model.InternalTag | model.ExternalTag | model.Untagged, openGenerics: Set): void { if (variants.kind === 'external_tag') { // All items must have a 'variant' attribute - const items = flattenUnionMembers(valueOf) + const items = flattenUnionMembers(valueOf, openGenerics) for (const item of items) { if (item.kind !== 'instance_of') { modelError('Items of externally tagged unions must be types with a "variant_tag" annotation') } else { - validateTypeRef(item.type, undefined, new Set()) + validateTypeRef(item.type, item.generics, openGenerics) const type = getTypeDef(item.type) if (type == null) { modelError(`Type ${fqn(item.type)} not found`) @@ -587,13 +583,13 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma } } else if (variants.kind === 'internal_tag') { const tagName = variants.tag - const items = flattenUnionMembers(valueOf) + const items = flattenUnionMembers(valueOf, openGenerics) for (const item of items) { if (item.kind !== 'instance_of') { modelError('Items of internally tagged unions must be type references') } else { - validateTypeRef(item.type, undefined, new Set()) + validateTypeRef(item.type, item.generics, openGenerics) const type = getTypeDef(item.type) if (type == null) { modelError(`Type ${fqn(item.type)} not found`) @@ -606,7 +602,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma } } - validateValueOf(valueOf, new Set()) + validateValueOf(valueOf, openGenerics) } else if (variants.kind === 'untagged') { if (fqn(parentName) !== '_types.query_dsl:DecayFunction' && fqn(parentName) !== '_types.query_dsl:DistanceFeatureQuery' && @@ -619,7 +615,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma modelError(`Type ${fqn(variants.untypedVariant)} not found`) } - const items = flattenUnionMembers(valueOf) + const items = flattenUnionMembers(valueOf, openGenerics) const baseTypes = new Set() let foundUntyped = false @@ -627,7 +623,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma if (item.kind !== 'instance_of') { modelError('Items of type untagged unions must be type references') } else { - validateTypeRef(item.type, undefined, new Set()) + validateTypeRef(item.type, item.generics, openGenerics) const type = getTypeDef(item.type) if (type == null) { modelError(`Type ${fqn(item.type)} not found`) @@ -679,7 +675,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma // ----------------------------------------------------------------------------------------------- // Constituents of type definitions - function openGenericSet (typeDef: model.Request | model.Response | model.Interface): Set { + function openGenericSet (typeDef: model.Request | model.Response | model.Interface | model.TypeAlias): Set { return new Set((typeDef.generics ?? []).map(name => fqn(name))) } @@ -884,7 +880,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma } else { // tagged union: the discriminant tells us what to look for, check each member in isolation assert(typeDef.type.kind === 'union_of', 'Variants are only allowed on union_of type aliases') - for (const item of flattenUnionMembers(typeDef.type)) { + for (const item of flattenUnionMembers(typeDef.type, new Set())) { validateValueOfJsonEvents(item) } @@ -899,13 +895,13 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma } /** Build the flattened item list of potentially nested unions (this is used for large unions) */ - function flattenUnionMembers (union: model.UnionOf): model.ValueOf[] { + function flattenUnionMembers (union: model.UnionOf, openGenerics: Set): model.ValueOf[] { const allItems = new Array() function collectItems (items: model.ValueOf[]): void { for (const item of items) { if (item.kind !== 'instance_of') { - validateValueOf(item, new Set()) + validateValueOf(item, openGenerics) allItems.push(item) } else { const itemType = getTypeDef(item.type) @@ -915,7 +911,7 @@ export default async function validateModel (apiModel: model.Model, restSpec: Ma // Recurse in nested union collectItems(itemType.type.items) } else { - validateValueOf(item, new Set()) + validateValueOf(item, openGenerics) allItems.push(item) } } diff --git a/output/schema/validation-errors.json b/output/schema/validation-errors.json index 6adb0b9b32..3ab40076c8 100644 --- a/output/schema/validation-errors.json +++ b/output/schema/validation-errors.json @@ -36,24 +36,10 @@ ], "response": [] }, - "async_search.get": { - "request": [], - "response": [ - "type_alias definition _types:EpochTime / instance_of - No type definition for '_types.EpochTime:Unit'", - "type_alias definition _types.aggregations:Buckets / union_of / dictionary_of / instance_of - No type definition for '_types.aggregations.Buckets:TBucket'", - "type_alias definition _types.aggregations:Buckets / union_of / array_of / instance_of - No type definition for '_types.aggregations.Buckets:TBucket'", - "type_alias definition _spec_utils:Void / instance_of - No type definition for '_builtins:void'", - "type_alias definition _types:DurationValue / instance_of - No type definition for '_types.DurationValue:Unit'", - "type_alias definition _global.search._types:Suggest - A tagged union should not have generic parameters", - "type_alias definition _global.search._types:Suggest / instance_of / Generics / instance_of - No type definition for '_global.search._types.Suggest:TDocument'", - "type_alias definition _global.search._types:Suggest - Expected 1 generic parameters but got 0" - ] - }, "async_search.submit": { "request": [ "Request: query parameter 'min_compatible_shard_node' does not exist in the json spec", - "interface definition _types:QueryVectorBuilder - Property text_embedding is a single-variant and must be required", - "type_alias definition _spec_utils:PipeSeparatedFlags / union_of / instance_of - No type definition for '_spec_utils.PipeSeparatedFlags:T'" + "interface definition _types:QueryVectorBuilder - Property text_embedding is a single-variant and must be required" ], "response": [] }, @@ -86,9 +72,7 @@ "Request: missing json spec query parameter 'v'", "request definition cat.allocation:Request / body - A request with inherited properties must have a PropertyBody" ], - "response": [ - "type_alias definition _spec_utils:Stringified / union_of / instance_of - No type definition for '_spec_utils.Stringified:T'" - ] + "response": [] }, "cat.component_templates": { "request": [ @@ -539,12 +523,6 @@ ], "response": [] }, - "connector.update_error": { - "request": [ - "type_alias definition _spec_utils:WithNullValue / union_of / instance_of - No type definition for '_spec_utils.WithNullValue:T'" - ], - "response": [] - }, "connector.update_features": { "request": [ "Missing request & response" @@ -601,12 +579,6 @@ ], "response": [] }, - "esql.query": { - "request": [], - "response": [ - "type_alias definition _types:EsqlColumns / instance_of - No type definition for '_builtins:binary'" - ] - }, "features.get_features": { "request": [ "Request: missing json spec query parameter 'master_timeout'" @@ -647,9 +619,7 @@ "Request: query parameter 'wait_for_checkpoints' does not exist in the json spec", "Request: query parameter 'allow_partial_search_results' does not exist in the json spec" ], - "response": [ - "type_alias definition _global.msearch:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.msearch.ResponseItem:TDocument'" - ] + "response": [] }, "fleet.post_secret": { "request": [ @@ -898,12 +868,6 @@ ], "response": [] }, - "mget": { - "request": [], - "response": [ - "type_alias definition _global.mget:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.mget.ResponseItem:TDocument'" - ] - }, "ml.delete_trained_model": { "request": [ "Request: missing json spec query parameter 'timeout'" @@ -1012,9 +976,7 @@ "Request: query parameter 'grid_agg' does not exist in the json spec", "Request: missing json spec query parameter 'track_total_hits'" ], - "response": [ - "type_alias definition _types:MapboxVectorTiles / instance_of - No type definition for '_builtins:binary'" - ] + "response": [] }, "search_shards": { "request": [