From 4afd56e0a740d7cee65e7822ad7c1b31f4c70144 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Fri, 19 Apr 2024 21:52:21 -0400 Subject: [PATCH] feat: improve literal union type handling (#1927) * feat: improve literal union type handling * Kinda working * make it work * Ignore hidden types * Fix null handling * Fix build --- src/NodeParser/IntersectionNodeParser.ts | 12 ++ src/NodeParser/MappedTypeNodeParser.ts | 2 +- src/Type/LiteralType.ts | 4 + src/Type/StringType.ts | 8 ++ src/TypeFormatter/AnnotatedTypeFormatter.ts | 4 +- .../LiteralUnionTypeFormatter.ts | 92 +++++++++++--- src/TypeFormatter/UnionTypeFormatter.ts | 36 +----- src/Utils/derefType.ts | 20 +++ src/Utils/typeKeys.ts | 2 +- test/invalid-data.test.ts | 14 +-- test/valid-data-other.test.ts | 1 + test/valid-data/string-literals-hack/main.ts | 14 +++ .../string-literals-hack/schema.json | 114 ++++++++++++++++++ test/vega-lite/schema.json | 97 ++++++--------- 14 files changed, 286 insertions(+), 134 deletions(-) create mode 100644 test/valid-data/string-literals-hack/main.ts create mode 100644 test/valid-data/string-literals-hack/schema.json diff --git a/src/NodeParser/IntersectionNodeParser.ts b/src/NodeParser/IntersectionNodeParser.ts index 317018b9..da200a97 100644 --- a/src/NodeParser/IntersectionNodeParser.ts +++ b/src/NodeParser/IntersectionNodeParser.ts @@ -9,6 +9,8 @@ import { derefType } from "../Utils/derefType.js"; import { uniqueTypeArray } from "../Utils/uniqueTypeArray.js"; import { UndefinedType } from "../Type/UndefinedType.js"; import { NeverType } from "../Type/NeverType.js"; +import { ObjectType } from "../Type/ObjectType.js"; +import { StringType } from "../Type/StringType.js"; export class IntersectionNodeParser implements SubNodeParser { public constructor( @@ -28,10 +30,20 @@ export class IntersectionNodeParser implements SubNodeParser { return new NeverType(); } + // handle autocomplete hacks like `string & {}` + if (types.length === 2 && types.some((t) => t instanceof StringType) && types.some((t) => isEmptyObject(t))) { + return new StringType(true); + } + return translate(types); } } +function isEmptyObject(x: BaseType) { + const t = derefType(x); + return t instanceof ObjectType && !t.getAdditionalProperties() && !t.getProperties().length; +} + function derefAndFlattenUnions(type: BaseType): BaseType[] { const derefed = derefType(type); return derefed instanceof UnionType diff --git a/src/NodeParser/MappedTypeNodeParser.ts b/src/NodeParser/MappedTypeNodeParser.ts index cba96694..62561819 100644 --- a/src/NodeParser/MappedTypeNodeParser.ts +++ b/src/NodeParser/MappedTypeNodeParser.ts @@ -164,7 +164,7 @@ export class MappedTypeNodeParser implements SubNodeParser { protected createSubContext( node: ts.MappedTypeNode, - key: LiteralType | StringType, + key: LiteralType | StringType | NumberType, parentContext: Context, ): Context { const subContext = new Context(node); diff --git a/src/Type/LiteralType.ts b/src/Type/LiteralType.ts index e520d3c3..0b784030 100644 --- a/src/Type/LiteralType.ts +++ b/src/Type/LiteralType.ts @@ -14,4 +14,8 @@ export class LiteralType extends BaseType { public getValue(): LiteralValue { return this.value; } + + public isString(): boolean { + return typeof this.value === "string"; + } } diff --git a/src/Type/StringType.ts b/src/Type/StringType.ts index 6ff9a5d2..bc73cc1c 100644 --- a/src/Type/StringType.ts +++ b/src/Type/StringType.ts @@ -1,7 +1,15 @@ import { PrimitiveType } from "./PrimitiveType.js"; export class StringType extends PrimitiveType { + constructor(protected preserveLiterals = false) { + super(); + } + public getId(): string { return "string"; } + + public getPreserveLiterals(): boolean { + return this.preserveLiterals; + } } diff --git a/src/TypeFormatter/AnnotatedTypeFormatter.ts b/src/TypeFormatter/AnnotatedTypeFormatter.ts index 9012cc15..06c18c8f 100644 --- a/src/TypeFormatter/AnnotatedTypeFormatter.ts +++ b/src/TypeFormatter/AnnotatedTypeFormatter.ts @@ -61,9 +61,7 @@ export class AnnotatedTypeFormatter implements SubTypeFormatter { delete annotations.discriminator; } else { throw new Error( - `Cannot assign discriminator tag to type: ${JSON.stringify( - derefed, - )}. This tag can only be assigned to union types.`, + `Cannot assign discriminator tag to type: ${derefed.getName()}. This tag can only be assigned to union types.`, ); } } diff --git a/src/TypeFormatter/LiteralUnionTypeFormatter.ts b/src/TypeFormatter/LiteralUnionTypeFormatter.ts index 58d90453..51935c98 100644 --- a/src/TypeFormatter/LiteralUnionTypeFormatter.ts +++ b/src/TypeFormatter/LiteralUnionTypeFormatter.ts @@ -2,43 +2,97 @@ import { Definition } from "../Schema/Definition.js"; import { RawTypeName } from "../Schema/RawType.js"; import { SubTypeFormatter } from "../SubTypeFormatter.js"; import { BaseType } from "../Type/BaseType.js"; -import { LiteralType } from "../Type/LiteralType.js"; +import { LiteralType, LiteralValue } from "../Type/LiteralType.js"; import { NullType } from "../Type/NullType.js"; +import { StringType } from "../Type/StringType.js"; import { UnionType } from "../Type/UnionType.js"; +import { derefAliasedType, isHiddenType } from "../Utils/derefType.js"; import { typeName } from "../Utils/typeName.js"; import { uniqueArray } from "../Utils/uniqueArray.js"; export class LiteralUnionTypeFormatter implements SubTypeFormatter { public supportsType(type: BaseType): boolean { - return type instanceof UnionType && type.getTypes().length > 0 && this.isLiteralUnion(type); + return type instanceof UnionType && type.getTypes().length > 0 && isLiteralUnion(type); } public getDefinition(type: UnionType): Definition { - const values = uniqueArray(type.getTypes().map((item: LiteralType | NullType) => this.getLiteralValue(item))); - const types = uniqueArray(type.getTypes().map((item: LiteralType | NullType) => this.getLiteralType(item))); + let hasString = false; + let preserveLiterals = false; + let allStrings = true; + let hasNull = false; - if (types.length === 1) { + const flattenedTypes = flattenTypes(type); + + // filter out String types since we need to be more careful about them + const types = flattenedTypes.filter((t) => { + if (t instanceof StringType) { + hasString = true; + preserveLiterals = preserveLiterals || t.getPreserveLiterals(); + return false; + } else if (t instanceof NullType) { + hasNull = true; + return true; + } else if (t instanceof LiteralType && !t.isString()) { + allStrings = false; + } + + return true; + }); + + if (allStrings && hasString && !preserveLiterals) { return { - type: types[0], - enum: values, + type: hasNull ? ["string", "null"] : "string", }; - } else { + } + + const values = uniqueArray(types.map(getLiteralValue)); + const typeNames = uniqueArray(types.map(getLiteralType)); + + const ret = { + type: typeNames.length === 1 ? typeNames[0] : typeNames, + enum: values, + }; + + if (preserveLiterals) { return { - type: types, - enum: values, + anyOf: [ + { + type: "string", + }, + ret, + ], }; } + + return ret; } public getChildren(type: UnionType): BaseType[] { return []; } +} - protected isLiteralUnion(type: UnionType): boolean { - return type.getTypes().every((item) => item instanceof LiteralType || item instanceof NullType); - } - protected getLiteralValue(value: LiteralType | NullType): string | number | boolean | null { - return value instanceof LiteralType ? value.getValue() : null; - } - protected getLiteralType(value: LiteralType | NullType): RawTypeName { - return value instanceof LiteralType ? typeName(value.getValue()) : "null"; - } +function flattenTypes(type: UnionType): (StringType | LiteralType | NullType)[] { + return type + .getTypes() + .filter((t) => !isHiddenType(t)) + .map(derefAliasedType) + .flatMap((t) => { + if (t instanceof UnionType) { + return flattenTypes(t); + } + return t as StringType | LiteralType | NullType; + }); +} + +function isLiteralUnion(type: UnionType): boolean { + return flattenTypes(type).every( + (item) => item instanceof LiteralType || item instanceof NullType || item instanceof StringType, + ); +} + +function getLiteralValue(value: LiteralType | NullType): LiteralValue | null { + return value instanceof LiteralType ? value.getValue() : null; +} + +function getLiteralType(value: LiteralType | NullType): RawTypeName { + return value instanceof LiteralType ? typeName(value.getValue()) : "null"; } diff --git a/src/TypeFormatter/UnionTypeFormatter.ts b/src/TypeFormatter/UnionTypeFormatter.ts index 66fcfda0..790f5743 100644 --- a/src/TypeFormatter/UnionTypeFormatter.ts +++ b/src/TypeFormatter/UnionTypeFormatter.ts @@ -40,9 +40,7 @@ export class UnionTypeFormatter implements SubTypeFormatter { if (undefinedIndex != -1) { throw new Error( - `Cannot find discriminator keyword "${discriminator}" in type ${JSON.stringify( - type.getTypes()[undefinedIndex], - )}.`, + `Cannot find discriminator keyword "${discriminator}" in type ${type.getTypes()[undefinedIndex].getName()}.`, ); } @@ -98,38 +96,6 @@ export class UnionTypeFormatter implements SubTypeFormatter { const definitions = this.getTypeDefinitions(type); - // TODO: why is this not covered by LiteralUnionTypeFormatter? - // special case for string literals | string -> string - let stringType = true; - let oneNotEnum = false; - for (const def of definitions) { - if (def.type !== "string") { - stringType = false; - break; - } - if (def.enum === undefined) { - oneNotEnum = true; - } - } - if (stringType && oneNotEnum) { - const values = []; - for (const def of definitions) { - if (def.enum) { - values.push(...def.enum); - } else if (def.const) { - values.push(def.const); - } else { - return { - type: "string", - }; - } - } - return { - type: "string", - enum: values, - }; - } - const flattenedDefinitions: JSONSchema7[] = []; // Flatten anyOf inside anyOf unless the anyOf has an annotation diff --git a/src/Utils/derefType.ts b/src/Utils/derefType.ts index c73a7699..0c68d93e 100644 --- a/src/Utils/derefType.ts +++ b/src/Utils/derefType.ts @@ -2,6 +2,8 @@ import { AliasType } from "../Type/AliasType.js"; import { AnnotatedType } from "../Type/AnnotatedType.js"; import { BaseType } from "../Type/BaseType.js"; import { DefinitionType } from "../Type/DefinitionType.js"; +import { HiddenType } from "../Type/HiddenType.js"; +import { NeverType } from "../Type/NeverType.js"; import { ReferenceType } from "../Type/ReferenceType.js"; /** @@ -25,3 +27,21 @@ export function derefAnnotatedType(type: BaseType): BaseType { return type; } + +export function isHiddenType(type: BaseType): boolean { + if (type instanceof HiddenType || type instanceof NeverType) { + return true; + } else if (type instanceof DefinitionType || type instanceof AliasType || type instanceof AnnotatedType) { + return isHiddenType(type.getType()); + } + + return false; +} + +export function derefAliasedType(type: BaseType): BaseType { + if (type instanceof AliasType) { + return derefAliasedType(type.getType()); + } + + return type; +} diff --git a/src/Utils/typeKeys.ts b/src/Utils/typeKeys.ts index 1053f1ed..d73b2a5b 100644 --- a/src/Utils/typeKeys.ts +++ b/src/Utils/typeKeys.ts @@ -47,7 +47,7 @@ export function getTypeKeys(type: BaseType): LiteralType[] { return []; } -export function getTypeByKey(type: BaseType, index: LiteralType | StringType): BaseType | undefined { +export function getTypeByKey(type: BaseType, index: LiteralType | StringType | NumberType): BaseType | undefined { type = derefType(type); if (type instanceof IntersectionType || type instanceof UnionType) { diff --git a/test/invalid-data.test.ts b/test/invalid-data.test.ts index aa82d2d6..5c743737 100644 --- a/test/invalid-data.test.ts +++ b/test/invalid-data.test.ts @@ -36,24 +36,14 @@ describe("invalid-data", () => { 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}}.', - ), + assertSchema("missing-discriminator", "MyType", 'Cannot find discriminator keyword "type" in type B.'), ); 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.", + "Cannot assign discriminator tag to type: interface-2103469249-0-76-2103469249-0-77. This tag can only be assigned to union types.", ), ); it( diff --git a/test/valid-data-other.test.ts b/test/valid-data-other.test.ts index 309c1c18..e54f499f 100644 --- a/test/valid-data-other.test.ts +++ b/test/valid-data-other.test.ts @@ -25,6 +25,7 @@ describe("valid-data-other", () => { it("string-literals-inline", assertValidSchema("string-literals-inline", "MyObject")); it("string-literals-intrinsic", assertValidSchema("string-literals-intrinsic", "MyObject")); it("string-literals-null", assertValidSchema("string-literals-null", "MyObject")); + it("string-literals-hack", assertValidSchema("string-literals-hack", "MyObject")); it("string-template-literals", assertValidSchema("string-template-literals", "MyObject")); it("string-template-expression-literals", assertValidSchema("string-template-expression-literals", "MyObject")); it( diff --git a/test/valid-data/string-literals-hack/main.ts b/test/valid-data/string-literals-hack/main.ts new file mode 100644 index 00000000..57489b47 --- /dev/null +++ b/test/valid-data/string-literals-hack/main.ts @@ -0,0 +1,14 @@ +type Union = "a" | "b"; + +export type MyObject = { + literals: "foo" | "bar"; + stringWithNull: string | null; + literalWithNull: "foo" | "bar" | null; + literalWithString: "foo" | "bar" | string; + literalWithStringAndNull: "foo" | "bar" | string | null; + withRef: "foo" | Union; + withRefWithString: Union | string; + withHack: "foo" | "bar" | (string & {}); + withHackRecord: "foo" | "bar" | (string & Record); + withHackNull: "foo" | "bar" | null | (string & Record); +}; diff --git a/test/valid-data/string-literals-hack/schema.json b/test/valid-data/string-literals-hack/schema.json new file mode 100644 index 00000000..388eceac --- /dev/null +++ b/test/valid-data/string-literals-hack/schema.json @@ -0,0 +1,114 @@ +{ + "$ref": "#/definitions/MyObject", + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "MyObject": { + "additionalProperties": false, + "properties": { + "literalWithNull": { + "enum": [ + "foo", + "bar", + null + ], + "type": [ + "string", + "null" + ] + }, + "literalWithString": { + "type": "string" + }, + "literalWithStringAndNull": { + "type": [ + "string", + "null" + ] + }, + "literals": { + "enum": [ + "foo", + "bar" + ], + "type": "string" + }, + "stringWithNull": { + "type": [ + "string", + "null" + ] + }, + "withHack": { + "anyOf": [ + { + "type": "string" + }, + { + "enum": [ + "foo", + "bar" + ], + "type": "string" + } + ] + }, + "withHackNull": { + "anyOf": [ + { + "type": "string" + }, + { + "enum": [ + "foo", + "bar", + null + ], + "type": [ + "string", + "null" + ] + } + ] + }, + "withHackRecord": { + "anyOf": [ + { + "type": "string" + }, + { + "enum": [ + "foo", + "bar" + ], + "type": "string" + } + ] + }, + "withRef": { + "enum": [ + "foo", + "a", + "b" + ], + "type": "string" + }, + "withRefWithString": { + "type": "string" + } + }, + "required": [ + "literals", + "stringWithNull", + "literalWithNull", + "literalWithString", + "literalWithStringAndNull", + "withRef", + "withRefWithString", + "withHack", + "withHackRecord", + "withHackNull" + ], + "type": "object" + } + } +} diff --git a/test/vega-lite/schema.json b/test/vega-lite/schema.json index a0a64b44..b5aef98f 100644 --- a/test/vega-lite/schema.json +++ b/test/vega-lite/schema.json @@ -4428,46 +4428,37 @@ ] }, "BinnedTimeUnit": { - "anyOf": [ - { - "enum": [ - "binnedyear", - "binnedyearquarter", - "binnedyearquartermonth", - "binnedyearmonth", - "binnedyearmonthdate", - "binnedyearmonthdatehours", - "binnedyearmonthdatehoursminutes", - "binnedyearmonthdatehoursminutesseconds", - "binnedyearweek", - "binnedyearweekday", - "binnedyearweekdayhours", - "binnedyearweekdayhoursminutes", - "binnedyearweekdayhoursminutesseconds", - "binnedyeardayofyear" - ], - "type": "string" - }, - { - "enum": [ - "binnedutcyear", - "binnedutcyearquarter", - "binnedutcyearquartermonth", - "binnedutcyearmonth", - "binnedutcyearmonthdate", - "binnedutcyearmonthdatehours", - "binnedutcyearmonthdatehoursminutes", - "binnedutcyearmonthdatehoursminutesseconds", - "binnedutcyearweek", - "binnedutcyearweekday", - "binnedutcyearweekdayhours", - "binnedutcyearweekdayhoursminutes", - "binnedutcyearweekdayhoursminutesseconds", - "binnedutcyeardayofyear" - ], - "type": "string" - } - ] + "enum": [ + "binnedyear", + "binnedyearquarter", + "binnedyearquartermonth", + "binnedyearmonth", + "binnedyearmonthdate", + "binnedyearmonthdatehours", + "binnedyearmonthdatehoursminutes", + "binnedyearmonthdatehoursminutesseconds", + "binnedyearweek", + "binnedyearweekday", + "binnedyearweekdayhours", + "binnedyearweekdayhoursminutes", + "binnedyearweekdayhoursminutesseconds", + "binnedyeardayofyear", + "binnedutcyear", + "binnedutcyearquarter", + "binnedutcyearquartermonth", + "binnedutcyearmonth", + "binnedutcyearmonthdate", + "binnedutcyearmonthdatehours", + "binnedutcyearmonthdatehoursminutes", + "binnedutcyearmonthdatehoursminutesseconds", + "binnedutcyearweek", + "binnedutcyearweekday", + "binnedutcyearweekdayhours", + "binnedutcyearweekdayhoursminutes", + "binnedutcyearweekdayhoursminutesseconds", + "binnedutcyeardayofyear" + ], + "type": "string" }, "Blend": { "enum": [ @@ -19228,29 +19219,9 @@ "type": "object" }, "ParseValue": { - "anyOf": [ - { - "type": "null" - }, - { - "type": "string" - }, - { - "const": "string", - "type": "string" - }, - { - "const": "boolean", - "type": "string" - }, - { - "const": "date", - "type": "string" - }, - { - "const": "number", - "type": "string" - } + "type": [ + "string", + "null" ] }, "PivotTransform": {