Skip to content

Commit

Permalink
Fix spec validator bugs (#3295)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
(cherry picked from commit 9dd5b47)

# Conflicts:
#	output/schema/validation-errors.json
  • Loading branch information
swallez committed Dec 13, 2024
1 parent dc1aa45 commit f642d74
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 62 deletions.
36 changes: 16 additions & 20 deletions compiler/src/steps/validate-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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<string>): 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<string>())
validateTypeRef(item.type, item.generics, openGenerics)
const type = getTypeDef(item.type)
if (type == null) {
modelError(`Type ${fqn(item.type)} not found`)
Expand All @@ -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<string>())
validateTypeRef(item.type, item.generics, openGenerics)
const type = getTypeDef(item.type)
if (type == null) {
modelError(`Type ${fqn(item.type)} not found`)
Expand All @@ -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' &&
Expand All @@ -619,15 +615,15 @@ 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<string>()
let foundUntyped = false

for (const item of items) {
if (item.kind !== 'instance_of') {
modelError('Items of type untagged unions must be type references')
} else {
validateTypeRef(item.type, undefined, new Set<string>())
validateTypeRef(item.type, item.generics, openGenerics)
const type = getTypeDef(item.type)
if (type == null) {
modelError(`Type ${fqn(item.type)} not found`)
Expand Down Expand Up @@ -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<string> {
function openGenericSet (typeDef: model.Request | model.Response | model.Interface | model.TypeAlias): Set<string> {
return new Set((typeDef.generics ?? []).map(name => fqn(name)))
}

Expand Down Expand Up @@ -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)
}

Expand All @@ -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<string>): model.ValueOf[] {
const allItems = new Array<model.ValueOf>()

function collectItems (items: model.ValueOf[]): void {
for (const item of items) {
if (item.kind !== 'instance_of') {
validateValueOf(item, new Set<string>())
validateValueOf(item, openGenerics)
allItems.push(item)
} else {
const itemType = getTypeDef(item.type)
Expand All @@ -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<string>())
validateValueOf(item, openGenerics)
allItems.push(item)
}
}
Expand Down
46 changes: 4 additions & 42 deletions output/schema/validation-errors.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f642d74

Please sign in to comment.