From bb168a052c9aa8beb8ca5c934bdb972c5c911660 Mon Sep 17 00:00:00 2001 From: Patrick McLaughlin Date: Tue, 12 Sep 2023 10:39:48 -0400 Subject: [PATCH 1/2] chore: remove distinct 'literal' ir type --- packages/openapi-generator/src/codec.ts | 8 ++++---- packages/openapi-generator/src/ir.ts | 8 +------- .../openapi-generator/src/knownImports.ts | 8 ++++---- packages/openapi-generator/src/openapi.ts | 8 +------- packages/openapi-generator/src/route.ts | 14 +++++++------ packages/openapi-generator/test/codec.test.ts | 20 +++++++++---------- .../openapi-generator/test/resolve.test.ts | 6 +++--- 7 files changed, 31 insertions(+), 41 deletions(-) diff --git a/packages/openapi-generator/src/codec.ts b/packages/openapi-generator/src/codec.ts index 0c6b672c..6f49dc6e 100644 --- a/packages/openapi-generator/src/codec.ts +++ b/packages/openapi-generator/src/codec.ts @@ -227,13 +227,13 @@ export function parsePlainInitializer( } else if (init.type === 'ArrayExpression') { return parseArrayExpression(project, source, init); } else if (init.type === 'StringLiteral') { - return E.right({ type: 'literal', kind: 'string', value: init.value }); + return E.right({ type: 'primitive', value: 'string', enum: [init.value] }); } else if (init.type === 'NumericLiteral') { - return E.right({ type: 'literal', kind: 'number', value: init.value }); + return E.right({ type: 'primitive', value: 'number', enum: [init.value] }); } else if (init.type === 'BooleanLiteral') { - return E.right({ type: 'literal', kind: 'boolean', value: init.value }); + return E.right({ type: 'primitive', value: 'boolean', enum: [init.value] }); } else if (init.type === 'NullLiteral') { - return E.right({ type: 'literal', kind: 'null', value: null }); + return E.right({ type: 'primitive', value: 'null', enum: [null] }); } else if (init.type === 'Identifier' && init.value === 'undefined') { return E.right({ type: 'undefined' }); } else if (init.type === 'TsConstAssertion' || init.type === 'TsAsExpression') { diff --git a/packages/openapi-generator/src/ir.ts b/packages/openapi-generator/src/ir.ts index fefa7c3b..ec1886a8 100644 --- a/packages/openapi-generator/src/ir.ts +++ b/packages/openapi-generator/src/ir.ts @@ -12,12 +12,7 @@ export type UndefinedValue = { export type Primitive = { type: 'primitive'; value: 'string' | 'number' | 'integer' | 'boolean' | 'null'; -}; - -export type Literal = { - type: 'literal'; - kind: 'string' | 'number' | 'integer' | 'boolean' | 'null'; - value: string | number | boolean | null | PseudoBigInt; + enum?: (string | number | boolean | null | PseudoBigInt)[]; }; export type Array = { @@ -49,7 +44,6 @@ export type Reference = { export type BaseSchema = | Primitive - | Literal | Array | Object | RecordObject diff --git a/packages/openapi-generator/src/knownImports.ts b/packages/openapi-generator/src/knownImports.ts index 9dfeaa42..bed032f4 100644 --- a/packages/openapi-generator/src/knownImports.ts +++ b/packages/openapi-generator/src/knownImports.ts @@ -89,7 +89,7 @@ export const KNOWN_IMPORTS: KnownImports = { return E.right({ type: 'intersection', schemas: schema.schemas }); }, literal: (_, arg) => { - if (arg.type !== 'literal') { + if (arg.type !== 'primitive' || arg.enum === undefined) { return E.left(`Unimplemented literal type ${arg.type}`); } else { return E.right(arg); @@ -100,9 +100,9 @@ export const KNOWN_IMPORTS: KnownImports = { return E.left(`Unimplemented keyof type ${arg.type}`); } const schemas: Schema[] = Object.keys(arg.properties).map((prop) => ({ - type: 'literal', - kind: 'string', - value: prop, + type: 'primitive', + value: 'string', + enum: [prop], })); return E.right({ type: 'union', diff --git a/packages/openapi-generator/src/openapi.ts b/packages/openapi-generator/src/openapi.ts index 53de3cc1..9e3162a6 100644 --- a/packages/openapi-generator/src/openapi.ts +++ b/packages/openapi-generator/src/openapi.ts @@ -17,13 +17,7 @@ function schemaToOpenAPI( // Or should we just conflate explicit null and undefined properties? return { nullable: true, enum: [] }; } else { - return { type: schema.value }; - } - case 'literal': - if (schema.kind === 'null') { - return { nullable: true, enum: [] }; - } else { - return { type: schema.kind, enum: [schema.value] }; + return { type: schema.value, ...(schema.enum ?? {}) }; } case 'ref': return { $ref: `#/components/schemas/${schema.name}` }; diff --git a/packages/openapi-generator/src/route.ts b/packages/openapi-generator/src/route.ts index b233b1a2..1e873e9a 100644 --- a/packages/openapi-generator/src/route.ts +++ b/packages/openapi-generator/src/route.ts @@ -210,8 +210,9 @@ export function parseRoute(project: Project, schema: Schema): E.Either Date: Tue, 12 Sep 2023 17:50:46 -0400 Subject: [PATCH 2/2] feat: add optimization pass to OpenAPI conversion --- packages/openapi-generator/src/index.ts | 1 + packages/openapi-generator/src/openapi.ts | 7 +- packages/openapi-generator/src/optimize.ts | 132 ++++++++++++++++++ .../openapi-generator/test/optimize.test.ts | 109 +++++++++++++++ 4 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 packages/openapi-generator/src/optimize.ts create mode 100644 packages/openapi-generator/test/optimize.test.ts diff --git a/packages/openapi-generator/src/index.ts b/packages/openapi-generator/src/index.ts index 6ccb8dee..a6598c81 100644 --- a/packages/openapi-generator/src/index.ts +++ b/packages/openapi-generator/src/index.ts @@ -2,6 +2,7 @@ export { parseApiSpec } from './apiSpec'; export { parseCodecInitializer, parsePlainInitializer } from './codec'; export { parseCommentBlock, type JSDoc } from './jsdoc'; export { convertRoutesToOpenAPI } from './openapi'; +export { optimize } from './optimize'; export { Project } from './project'; export { getRefs } from './ref'; export { parseRoute, type Route } from './route'; diff --git a/packages/openapi-generator/src/openapi.ts b/packages/openapi-generator/src/openapi.ts index 9e3162a6..23cb80c7 100644 --- a/packages/openapi-generator/src/openapi.ts +++ b/packages/openapi-generator/src/openapi.ts @@ -2,22 +2,25 @@ import { OpenAPIV3 } from 'openapi-types'; import { STATUS_CODES } from 'http'; import { parseCommentBlock } from './jsdoc'; +import { optimize } from './optimize'; import type { Route } from './route'; import type { Schema } from './ir'; function schemaToOpenAPI( schema: Schema, ): OpenAPIV3.SchemaObject | OpenAPIV3.ReferenceObject | undefined { + schema = optimize(schema); + switch (schema.type) { case 'primitive': if (schema.value === 'integer') { - return { type: 'number' }; + return { type: 'number', ...(schema.enum ? { enum: schema.enum } : {}) }; } else if (schema.value === 'null') { // TODO: OpenAPI v3 does not have an explicit null type, is there a better way to represent this? // Or should we just conflate explicit null and undefined properties? return { nullable: true, enum: [] }; } else { - return { type: schema.value, ...(schema.enum ?? {}) }; + return { type: schema.value, ...(schema.enum ? { enum: schema.enum } : {}) }; } case 'ref': return { $ref: `#/components/schemas/${schema.name}` }; diff --git a/packages/openapi-generator/src/optimize.ts b/packages/openapi-generator/src/optimize.ts new file mode 100644 index 00000000..cd8914f9 --- /dev/null +++ b/packages/openapi-generator/src/optimize.ts @@ -0,0 +1,132 @@ +import type { Schema } from './ir'; + +export type OptimizeFn = (schema: Schema) => Schema; + +export function foldIntersection(schema: Schema, optimize: OptimizeFn): Schema { + if (schema.type !== 'intersection') { + return schema; + } + + const innerSchemas = schema.schemas.map(optimize); + let combinedObject: Schema & { type: 'object' } = { + type: 'object', + properties: {}, + required: [], + }; + let result: Schema = combinedObject; + innerSchemas.forEach((innerSchema) => { + if (innerSchema.type === 'object') { + Object.assign(combinedObject.properties, innerSchema.properties); + combinedObject.required.push(...innerSchema.required); + } else if (result.type === 'intersection') { + result.schemas.push(innerSchema); + } else { + result = { + type: 'intersection', + schemas: [combinedObject, innerSchema], + }; + } + }); + + return result; +} + +export function simplifyUnion(schema: Schema, optimize: OptimizeFn): Schema { + if (schema.type !== 'union') { + return schema; + } else if (schema.schemas.length === 1) { + return schema.schemas[0]!; + } else if (schema.schemas.length === 0) { + return { type: 'undefined' }; + } + + const innerSchemas = schema.schemas.map(optimize); + + const literals: Record<(Schema & { type: 'primitive' })['value'], any[]> = { + string: [], + number: [], + integer: [], + boolean: [], + null: [], + }; + const remainder: Schema[] = []; + innerSchemas.forEach((innerSchema) => { + if (innerSchema.type === 'primitive' && innerSchema.enum !== undefined) { + literals[innerSchema.value].push(...innerSchema.enum); + } else { + remainder.push(innerSchema); + } + }); + const result: Schema = { + type: 'union', + schemas: remainder, + }; + for (const [key, value] of Object.entries(literals)) { + if (value.length > 0) { + result.schemas.push({ + type: 'primitive', + value: key as any, + enum: value, + }); + } + } + + if (result.schemas.length === 1) { + return result.schemas[0]!; + } else { + return result; + } +} + +export function filterUndefinedUnion(schema: Schema): [boolean, Schema] { + if (schema.type !== 'union') { + return [false, schema]; + } + + const undefinedIndex = schema.schemas.findIndex((s) => s.type === 'undefined'); + if (undefinedIndex < 0) { + return [false, schema]; + } + + const schemas = schema.schemas.filter((s) => s.type !== 'undefined'); + if (schemas.length === 0) { + return [true, { type: 'undefined' }]; + } else if (schemas.length === 1) { + return [true, schemas[0]!]; + } else { + return [true, { type: 'union', schemas }]; + } +} + +export function optimize(schema: Schema): Schema { + if (schema.type === 'object') { + const properties: Record = {}; + const required: string[] = []; + for (const [key, prop] of Object.entries(schema.properties)) { + const optimized = optimize(prop); + if (optimized.type === 'undefined') { + continue; + } + const [isOptional, filteredSchema] = filterUndefinedUnion(optimized); + properties[key] = filteredSchema; + if (schema.required.indexOf(key) >= 0 && !isOptional) { + required.push(key); + } + } + return { type: 'object', properties, required }; + } else if (schema.type === 'intersection') { + return foldIntersection(schema, optimize); + } else if (schema.type === 'union') { + return simplifyUnion(schema, optimize); + } else if (schema.type === 'array') { + const optimized = optimize(schema.items); + return { type: 'array', items: optimized }; + } else if (schema.type === 'record') { + return { type: 'record', codomain: optimize(schema.codomain) }; + } else if (schema.type === 'tuple') { + const schemas = schema.schemas.map(optimize); + return { type: 'tuple', schemas }; + } else { + return schema; + } +} diff --git a/packages/openapi-generator/test/optimize.test.ts b/packages/openapi-generator/test/optimize.test.ts new file mode 100644 index 00000000..592311ce --- /dev/null +++ b/packages/openapi-generator/test/optimize.test.ts @@ -0,0 +1,109 @@ +import assert from 'node:assert'; +import test from 'node:test'; + +import { optimize, type Schema } from '../src'; + +test('intersections are simplified', () => { + const input: Schema = { + type: 'intersection', + schemas: [ + { + type: 'object', + properties: { + foo: { type: 'primitive', value: 'string' }, + }, + required: ['foo'], + }, + { + type: 'object', + properties: { + bar: { type: 'primitive', value: 'string' }, + }, + required: [], + }, + ], + }; + + const expected: Schema = { + type: 'object', + properties: { + foo: { type: 'primitive', value: 'string' }, + bar: { type: 'primitive', value: 'string' }, + }, + required: ['foo'], + }; + + assert.deepStrictEqual(optimize(input), expected); +}); + +test('unions are combined', () => { + const input: Schema = { + type: 'union', + schemas: [ + { + type: 'primitive', + value: 'string', + enum: ['foo'], + }, + { + type: 'primitive', + value: 'string', + enum: ['bar'], + }, + ], + }; + + const expected: Schema = { + type: 'primitive', + value: 'string', + enum: ['foo', 'bar'], + }; + + assert.deepStrictEqual(optimize(input), expected); +}); + +test('undefined properties are simplified', () => { + const input: Schema = { + type: 'object', + properties: { + foo: { type: 'undefined' }, + bar: { type: 'primitive', value: 'string' }, + }, + required: ['foo'], + }; + + const expected: Schema = { + type: 'object', + properties: { + bar: { type: 'primitive', value: 'string' }, + }, + required: [], + }; + + assert.deepStrictEqual(optimize(input), expected); +}); + +test('undefined property unions are simplified', () => { + const input: Schema = { + type: 'object', + properties: { + foo: { + type: 'union', + schemas: [{ type: 'undefined' }, { type: 'primitive', value: 'string' }], + }, + bar: { type: 'primitive', value: 'string' }, + }, + required: ['foo', 'bar'], + }; + + const expected: Schema = { + type: 'object', + properties: { + foo: { type: 'primitive', value: 'string' }, + bar: { type: 'primitive', value: 'string' }, + }, + required: ['bar'], + }; + + assert.deepStrictEqual(optimize(input), expected); +});