From 167ddf44f954d16dd151ea235ff07d5056293751 Mon Sep 17 00:00:00 2001 From: Daan Boer Date: Fri, 26 Aug 2022 16:54:04 +0200 Subject: [PATCH 1/5] Added support for discriminator tag This tag allows for generating if-then, in favor of anyOf clauses. --- .../BasicAnnotationsReader.ts | 3 ++ src/Type/UnionType.ts | 9 ++++++ src/TypeFormatter/AnnotatedTypeFormatter.ts | 14 ++++++++ src/TypeFormatter/UnionTypeFormatter.ts | 32 +++++++++++++++++++ src/Utils/removeUnreachable.ts | 2 ++ 5 files changed, 60 insertions(+) diff --git a/src/AnnotationsReader/BasicAnnotationsReader.ts b/src/AnnotationsReader/BasicAnnotationsReader.ts index 568a26189..62fc705cf 100644 --- a/src/AnnotationsReader/BasicAnnotationsReader.ts +++ b/src/AnnotationsReader/BasicAnnotationsReader.ts @@ -19,6 +19,9 @@ export class BasicAnnotationsReader implements AnnotationsReader { "comment", "contentMediaType", "contentEncoding", + + // Custom tag for if-then-else support. + "discriminator", ]); private static jsonTags = new Set([ "minimum", diff --git a/src/Type/UnionType.ts b/src/Type/UnionType.ts index 3f28023d3..99426e4e0 100644 --- a/src/Type/UnionType.ts +++ b/src/Type/UnionType.ts @@ -5,6 +5,7 @@ import { derefType } from "../Utils/derefType"; export class UnionType extends BaseType { private readonly types: BaseType[]; + private discriminator?: string = undefined; public constructor(types: readonly BaseType[]) { super(); @@ -20,6 +21,14 @@ export class UnionType extends BaseType { ); } + public setDiscriminator(discriminator: string) { + this.discriminator = discriminator; + } + + public getDiscriminator() { + return this.discriminator; + } + public getId(): string { return `(${this.types.map((type) => type.getId()).join("|")})`; } diff --git a/src/TypeFormatter/AnnotatedTypeFormatter.ts b/src/TypeFormatter/AnnotatedTypeFormatter.ts index 854c876e5..0b0f7b9c9 100644 --- a/src/TypeFormatter/AnnotatedTypeFormatter.ts +++ b/src/TypeFormatter/AnnotatedTypeFormatter.ts @@ -2,7 +2,9 @@ import { Definition } from "../Schema/Definition"; import { SubTypeFormatter } from "../SubTypeFormatter"; import { AnnotatedType } from "../Type/AnnotatedType"; import { BaseType } from "../Type/BaseType"; +import { UnionType } from "../Type/UnionType"; import { TypeFormatter } from "../TypeFormatter"; +import { derefType } from "../Utils/derefType"; export function makeNullable(def: Definition): Definition { const union: Definition[] | undefined = (def.oneOf as Definition[]) || def.anyOf; @@ -50,6 +52,18 @@ export class AnnotatedTypeFormatter implements SubTypeFormatter { return type instanceof AnnotatedType; } public getDefinition(type: AnnotatedType): Definition { + let annotations = type.getAnnotations(); + + if ("discriminator" in annotations) { + const derefed = derefType(type.getType()); + if (derefed instanceof UnionType) { + derefed.setDiscriminator(annotations.discriminator); + delete annotations.discriminator; + } else { + throw new Error(`Cannot assign discriminator tag to type: ${derefed.constructor.name}.`); + } + } + const def: Definition = { ...this.childTypeFormatter.getDefinition(type.getType()), ...type.getAnnotations(), diff --git a/src/TypeFormatter/UnionTypeFormatter.ts b/src/TypeFormatter/UnionTypeFormatter.ts index 2e4d1ef22..f261b552c 100644 --- a/src/TypeFormatter/UnionTypeFormatter.ts +++ b/src/TypeFormatter/UnionTypeFormatter.ts @@ -2,10 +2,12 @@ import { JSONSchema7 } from "json-schema"; import { Definition } from "../Schema/Definition"; import { SubTypeFormatter } from "../SubTypeFormatter"; import { BaseType } from "../Type/BaseType"; +import { LiteralType } from "../Type/LiteralType"; import { NeverType } from "../Type/NeverType"; import { UnionType } from "../Type/UnionType"; import { TypeFormatter } from "../TypeFormatter"; import { derefType } from "../Utils/derefType"; +import { getTypeByKey } from "../Utils/typeKeys"; import { uniqueArray } from "../Utils/uniqueArray"; export class UnionTypeFormatter implements SubTypeFormatter { @@ -20,6 +22,36 @@ export class UnionTypeFormatter implements SubTypeFormatter { .filter((item) => !(derefType(item) instanceof NeverType)) .map((item) => this.childTypeFormatter.getDefinition(item)); + const discriminator = type.getDiscriminator(); + if (discriminator !== undefined) { + if (definitions.length === 1) { + return definitions[0]; + } + + let kindTypes = type.getTypes().map((type) => getTypeByKey(type, new LiteralType(discriminator))); + + const undefinedIndex = kindTypes.findIndex((type) => type === undefined); + + if (undefinedIndex != -1) { + throw new Error(`Cannot apply discriminator keyword ${discriminator}. To type ${type}.`); + } + + const kindDefinitions = kindTypes.map((type) => this.childTypeFormatter.getDefinition(type as BaseType)); + + let allOf = []; + + for (let i = 0; i < definitions.length; i++) { + allOf.push({ + if: { + properties: { [discriminator]: kindDefinitions[i] }, + }, + then: definitions[i], + }); + } + + return { allOf }; + } + // TODO: why is this not covered by LiteralUnionTypeFormatter? // special case for string literals | string -> string let stringType = true; diff --git a/src/Utils/removeUnreachable.ts b/src/Utils/removeUnreachable.ts index 537cf38bb..430ccb1ed 100644 --- a/src/Utils/removeUnreachable.ts +++ b/src/Utils/removeUnreachable.ts @@ -58,6 +58,8 @@ function addReachable( } else if (items) { addReachable(items, definitions, reachable); } + } else if (definition.then) { + addReachable(definition.then, definitions, reachable); } } From 1dd156ea15da76bdaf02c6c8150c443cc2ef0be2 Mon Sep 17 00:00:00 2001 From: Daan Boer Date: Fri, 26 Aug 2022 17:03:48 +0200 Subject: [PATCH 2/5] Added test --- test/valid-data-annotations.test.ts | 2 + .../annotation-union-if-then/main.ts | 14 ++++ .../annotation-union-if-then/schema.json | 65 +++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 test/valid-data/annotation-union-if-then/main.ts create mode 100644 test/valid-data/annotation-union-if-then/schema.json diff --git a/test/valid-data-annotations.test.ts b/test/valid-data-annotations.test.ts index 182b3da16..81134a95a 100644 --- a/test/valid-data-annotations.test.ts +++ b/test/valid-data-annotations.test.ts @@ -42,4 +42,6 @@ describe("valid-data-annotations", () => { it("annotation-ref", assertValidSchema("annotation-ref", "MyObject", "extended")); it("annotation-writeOnly", assertValidSchema("annotation-writeOnly", "MyObject", "basic")); + + it("annotation-union-if-then", assertValidSchema("annotation-union-if-then", "Animal", "basic")); }); diff --git a/test/valid-data/annotation-union-if-then/main.ts b/test/valid-data/annotation-union-if-then/main.ts new file mode 100644 index 000000000..e46709edd --- /dev/null +++ b/test/valid-data/annotation-union-if-then/main.ts @@ -0,0 +1,14 @@ +export type Fish = { + animal_type: "fish"; + found_in: "ocean" | "river"; +}; + +export type Bird = { + animal_type: "bird"; + can_fly: boolean; +}; + +/** + * @discriminator animal_type + */ +export type Animal = Bird | Fish; diff --git a/test/valid-data/annotation-union-if-then/schema.json b/test/valid-data/annotation-union-if-then/schema.json new file mode 100644 index 000000000..aa5e893ac --- /dev/null +++ b/test/valid-data/annotation-union-if-then/schema.json @@ -0,0 +1,65 @@ +{ + "$ref": "#/definitions/Animal", + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "Animal": { + "allOf": [ + { + "if": { + "properties": { + "animal_type": { + "const": "bird", + "type": "string" + } + } + }, + "then": { + "$ref": "#/definitions/Bird" + } + }, + { + "if": { + "properties": { + "animal_type": { + "const": "fish", + "type": "string" + } + } + }, + "then": { + "$ref": "#/definitions/Fish" + } + } + ] + }, + "Bird": { + "additionalProperties": false, + "properties": { + "animal_type": { + "const": "bird", + "type": "string" + }, + "can_fly": { + "type": "boolean" + } + }, + "required": ["animal_type", "can_fly"], + "type": "object" + }, + "Fish": { + "additionalProperties": false, + "properties": { + "animal_type": { + "const": "fish", + "type": "string" + }, + "found_in": { + "enum": ["ocean", "river"], + "type": "string" + } + }, + "required": ["animal_type", "found_in"], + "type": "object" + } + } +} From d83c7979ea04a17999c1a6806127b61d7876a27f Mon Sep 17 00:00:00 2001 From: Daan Boer Date: Fri, 26 Aug 2022 18:45:20 +0200 Subject: [PATCH 3/5] Added tests for cases that should fail --- src/TypeFormatter/AnnotatedTypeFormatter.ts | 6 +++++- src/TypeFormatter/UnionTypeFormatter.ts | 14 ++++++++++++-- test/invalid-data.test.ts | 18 +++++++++++++++++- .../invalid-data/missing-discriminator/main.ts | 10 ++++++++++ .../non-union-discriminator/main.ts | 6 ++++++ 5 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 test/invalid-data/missing-discriminator/main.ts create mode 100644 test/invalid-data/non-union-discriminator/main.ts diff --git a/src/TypeFormatter/AnnotatedTypeFormatter.ts b/src/TypeFormatter/AnnotatedTypeFormatter.ts index 0b0f7b9c9..30f871154 100644 --- a/src/TypeFormatter/AnnotatedTypeFormatter.ts +++ b/src/TypeFormatter/AnnotatedTypeFormatter.ts @@ -60,7 +60,11 @@ export class AnnotatedTypeFormatter implements SubTypeFormatter { derefed.setDiscriminator(annotations.discriminator); delete annotations.discriminator; } else { - throw new Error(`Cannot assign discriminator tag to type: ${derefed.constructor.name}.`); + throw new Error( + `Cannot assign discriminator tag to type: ${JSON.stringify( + derefed + )}. This tag can only be assigned to union types.` + ); } } diff --git a/src/TypeFormatter/UnionTypeFormatter.ts b/src/TypeFormatter/UnionTypeFormatter.ts index f261b552c..aaac57f4c 100644 --- a/src/TypeFormatter/UnionTypeFormatter.ts +++ b/src/TypeFormatter/UnionTypeFormatter.ts @@ -17,6 +17,9 @@ export class UnionTypeFormatter implements SubTypeFormatter { return type instanceof UnionType; } public getDefinition(type: UnionType): Definition { + // FIXME: Filtering never types from union types is wrong. However, + // disabling this line will result in regressions of tests using the + // `hidden` tag. const definitions = type .getTypes() .filter((item) => !(derefType(item) instanceof NeverType)) @@ -28,12 +31,19 @@ export class UnionTypeFormatter implements SubTypeFormatter { return definitions[0]; } - let kindTypes = type.getTypes().map((type) => getTypeByKey(type, new LiteralType(discriminator))); + let kindTypes = type + .getTypes() + .filter((item) => !(derefType(item) instanceof NeverType)) + .map((type) => getTypeByKey(type, new LiteralType(discriminator))); const undefinedIndex = kindTypes.findIndex((type) => type === undefined); if (undefinedIndex != -1) { - throw new Error(`Cannot apply discriminator keyword ${discriminator}. To type ${type}.`); + throw new Error( + `Cannot find discriminator keyword "${discriminator}" in type ${JSON.stringify( + type.getTypes()[undefinedIndex] + )}.` + ); } const kindDefinitions = kindTypes.map((type) => this.childTypeFormatter.getDefinition(type as BaseType)); diff --git a/test/invalid-data.test.ts b/test/invalid-data.test.ts index ef38f4ae3..b31936f1b 100644 --- a/test/invalid-data.test.ts +++ b/test/invalid-data.test.ts @@ -13,7 +13,7 @@ function assertSchema(name: string, type: string, message: string) { type: type, expose: "export", topRef: true, - jsDoc: "none", + jsDoc: "basic", skipTypeCheck: !!process.env.FAST_TEST, }; @@ -33,6 +33,22 @@ describe("invalid-data", () => { it("script-empty", assertSchema("script-empty", "MyType", `No root type "MyType" found`)); it("duplicates", assertSchema("duplicates", "MyType", `Type "A" has multiple definitions.`)); + it( + "missing-discriminator", + assertSchema( + "missing-discriminator", + "MyType", + `Cannot find discriminator keyword "type" in type {"name":"B","type":{"id":"interface-1119825560-40-63-1119825560-0-124","baseTypes":[],"properties":[],"additionalProperties":false,"nonPrimitive":false}}.` + ) + ); + it( + "non-union-discriminator", + assertSchema( + "non-union-discriminator", + "MyType", + `Cannot assign discriminator tag to type: {"id":"interface-2103469249-0-76-2103469249-0-77","baseTypes":[],"properties":[{"name":"name","type":{},"required":true}],"additionalProperties":false,"nonPrimitive":false}. This tag can only be assigned to union types.` + ) + ); it( "no-function-name", assertSchema( diff --git a/test/invalid-data/missing-discriminator/main.ts b/test/invalid-data/missing-discriminator/main.ts new file mode 100644 index 000000000..7e79ba67b --- /dev/null +++ b/test/invalid-data/missing-discriminator/main.ts @@ -0,0 +1,10 @@ +export interface A { + type: string; +} + +export interface B {} + +/** + * @discriminator type + */ +export type MyType = A | B; diff --git a/test/invalid-data/non-union-discriminator/main.ts b/test/invalid-data/non-union-discriminator/main.ts new file mode 100644 index 000000000..9fd78cf4f --- /dev/null +++ b/test/invalid-data/non-union-discriminator/main.ts @@ -0,0 +1,6 @@ +/** + * @discriminator name + */ +export interface MyType { + name: string; +} From 28f6494c752e82892277ea0c455c51b8a950727a Mon Sep 17 00:00:00 2001 From: Daan Boer Date: Fri, 26 Aug 2022 20:45:15 +0200 Subject: [PATCH 4/5] Fixed linting errors --- src/TypeFormatter/AnnotatedTypeFormatter.ts | 2 +- src/TypeFormatter/UnionTypeFormatter.ts | 10 +++++----- test/invalid-data.test.ts | 10 ++++++++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/TypeFormatter/AnnotatedTypeFormatter.ts b/src/TypeFormatter/AnnotatedTypeFormatter.ts index 30f871154..e197e051c 100644 --- a/src/TypeFormatter/AnnotatedTypeFormatter.ts +++ b/src/TypeFormatter/AnnotatedTypeFormatter.ts @@ -52,7 +52,7 @@ export class AnnotatedTypeFormatter implements SubTypeFormatter { return type instanceof AnnotatedType; } public getDefinition(type: AnnotatedType): Definition { - let annotations = type.getAnnotations(); + const annotations = type.getAnnotations(); if ("discriminator" in annotations) { const derefed = derefType(type.getType()); diff --git a/src/TypeFormatter/UnionTypeFormatter.ts b/src/TypeFormatter/UnionTypeFormatter.ts index aaac57f4c..36b664bc1 100644 --- a/src/TypeFormatter/UnionTypeFormatter.ts +++ b/src/TypeFormatter/UnionTypeFormatter.ts @@ -31,12 +31,12 @@ export class UnionTypeFormatter implements SubTypeFormatter { return definitions[0]; } - let kindTypes = type + const kindTypes = type .getTypes() .filter((item) => !(derefType(item) instanceof NeverType)) - .map((type) => getTypeByKey(type, new LiteralType(discriminator))); + .map((item) => getTypeByKey(item, new LiteralType(discriminator))); - const undefinedIndex = kindTypes.findIndex((type) => type === undefined); + const undefinedIndex = kindTypes.findIndex((item) => item === undefined); if (undefinedIndex != -1) { throw new Error( @@ -46,9 +46,9 @@ export class UnionTypeFormatter implements SubTypeFormatter { ); } - const kindDefinitions = kindTypes.map((type) => this.childTypeFormatter.getDefinition(type as BaseType)); + const kindDefinitions = kindTypes.map((item) => this.childTypeFormatter.getDefinition(item as BaseType)); - let allOf = []; + const allOf = []; for (let i = 0; i < definitions.length; i++) { allOf.push({ diff --git a/test/invalid-data.test.ts b/test/invalid-data.test.ts index b31936f1b..f2bc84f7d 100644 --- a/test/invalid-data.test.ts +++ b/test/invalid-data.test.ts @@ -38,7 +38,9 @@ describe("invalid-data", () => { assertSchema( "missing-discriminator", "MyType", - `Cannot find discriminator keyword "type" in type {"name":"B","type":{"id":"interface-1119825560-40-63-1119825560-0-124","baseTypes":[],"properties":[],"additionalProperties":false,"nonPrimitive":false}}.` + 'Cannot find discriminator keyword "type" in type ' + + '{"name":"B","type":{"id":"interface-1119825560-40-63-1119825560-0-124",' + + '"baseTypes":[],"properties":[],"additionalProperties":false,"nonPrimitive":false}}.' ) ); it( @@ -46,7 +48,11 @@ describe("invalid-data", () => { assertSchema( "non-union-discriminator", "MyType", - `Cannot assign discriminator tag to type: {"id":"interface-2103469249-0-76-2103469249-0-77","baseTypes":[],"properties":[{"name":"name","type":{},"required":true}],"additionalProperties":false,"nonPrimitive":false}. This tag can only be assigned to union types.` + "Cannot assign discriminator tag to type: " + + '{"id":"interface-2103469249-0-76-2103469249-0-77","baseTypes":[],' + + '"properties":[{"name":"name","type":{},"required":true}],' + + '"additionalProperties":false,"nonPrimitive":false}. ' + + "This tag can only be assigned to union types." ) ); it( From eaec70a7d2a67f8dcaec01b9cb21059e60b095eb Mon Sep 17 00:00:00 2001 From: Daan Boer Date: Mon, 5 Sep 2022 12:10:41 +0200 Subject: [PATCH 5/5] Cleanup Removed incorrect FIXME. Removed unreachable code. --- src/TypeFormatter/UnionTypeFormatter.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/TypeFormatter/UnionTypeFormatter.ts b/src/TypeFormatter/UnionTypeFormatter.ts index 36b664bc1..18538dd63 100644 --- a/src/TypeFormatter/UnionTypeFormatter.ts +++ b/src/TypeFormatter/UnionTypeFormatter.ts @@ -17,9 +17,6 @@ export class UnionTypeFormatter implements SubTypeFormatter { return type instanceof UnionType; } public getDefinition(type: UnionType): Definition { - // FIXME: Filtering never types from union types is wrong. However, - // disabling this line will result in regressions of tests using the - // `hidden` tag. const definitions = type .getTypes() .filter((item) => !(derefType(item) instanceof NeverType)) @@ -27,10 +24,6 @@ export class UnionTypeFormatter implements SubTypeFormatter { const discriminator = type.getDiscriminator(); if (discriminator !== undefined) { - if (definitions.length === 1) { - return definitions[0]; - } - const kindTypes = type .getTypes() .filter((item) => !(derefType(item) instanceof NeverType))