From 9dd5b4760e80bba78839a3845a4ee682fb92efd8 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 --- compiler/src/steps/validate-model.ts | 36 +++++++++----------- output/schema/validation-errors.json | 50 ++-------------------------- 2 files changed, 18 insertions(+), 68 deletions(-) diff --git a/compiler/src/steps/validate-model.ts b/compiler/src/steps/validate-model.ts index 22b47b1ca1..a50cf9f43e 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', @@ -541,33 +541,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`) @@ -582,13 +578,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`) @@ -601,7 +597,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' && @@ -614,7 +610,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 @@ -622,7 +618,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`) @@ -674,7 +670,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))) } @@ -879,7 +875,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) } @@ -894,13 +890,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) @@ -910,7 +906,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 9bc2f2649d..18f4b43d11 100644 --- a/output/schema/validation-errors.json +++ b/output/schema/validation-errors.json @@ -36,23 +36,9 @@ ], "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": [ - "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": [] }, @@ -62,12 +48,6 @@ ], "response": [] }, - "cat.allocation": { - "request": [], - "response": [ - "type_alias definition _spec_utils:Stringified / union_of / instance_of - No type definition for '_spec_utils.Stringified:T'" - ] - }, "cat.count": { "request": [ "Request: query parameter 'master_timeout' does not exist in the json spec" @@ -353,12 +333,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" @@ -415,12 +389,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'" @@ -445,12 +413,6 @@ ], "response": [] }, - "fleet.msearch": { - "request": [], - "response": [ - "type_alias definition _global.msearch:ResponseItem / union_of / instance_of / Generics / instance_of - No type definition for '_global.msearch.ResponseItem:TDocument'" - ] - }, "fleet.post_secret": { "request": [ "Missing request & response" @@ -650,12 +612,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'" @@ -764,9 +720,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": [