Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 85 additions & 2 deletions src/parser/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ function extractNestedSchema(name: string, schema: SchemaObject): Model[] {
// Handle oneOf: extract each object variant as its own model
if (schema.oneOf) {
const models: Model[] = [];

// Collect the inline object variants up front. When every variant pins
// the same property to a const value, we can derive meaningful names
// from the const (e.g. `UserApiKeyOwner` instead of `ApiKeyOwner2`)
// — much friendlier for SDK consumers than numeric suffixes.
const inlineObjectVariants: SchemaObject[] = schema.oneOf.filter(
(v) => !v.$ref && v.properties && (v.type === 'object' || !v.type),
);
const constNamingDiscriminator = deriveConstNamingDiscriminator(inlineObjectVariants);

for (const variant of schema.oneOf) {
if (variant.$ref) continue;
if (variant.properties && (variant.type === 'object' || !variant.type)) {
Expand All @@ -278,8 +288,7 @@ function extractNestedSchema(name: string, schema: SchemaObject): Model[] {
if (!fs) continue;
fields.push(buildFieldFromSchema(fn, fs, name, requiredSet));
}
// Use the name for the first variant, suffix for subsequent
const variantName = models.length === 0 ? name : `${name}${models.length + 1}`;
const variantName = nameVariantModel(variant, name, models, constNamingDiscriminator);
models.push({ name: variantName, description: variant.description, fields });
}
}
Expand Down Expand Up @@ -311,6 +320,80 @@ function extractNestedSchema(name: string, schema: SchemaObject): Model[] {
return [];
}

/**
* Derive a meaningful name for a oneOf object variant past the first,
* falling back to the numeric-suffix scheme (`Foo2`, `Foo3`) when no
* better signal is available. The "better signal" is a const-valued
* discriminator property that distinguishes one variant from the others.
*
* For a schema like `ApiKey.owner` whose oneOf has two variants — one
* with `type: { const: "organization" }` and one with `type: { const:
* "user" }` — this returns `ApiKeyOwner` for the first variant (the
* baseline name that the union TypeRef will reference, so consumer-
* visible typed fields stay backward-compatible) and `UserApiKeyOwner`
* for the second instead of `ApiKeyOwner2`. SDK consumers reading the
* generated types can tell the variants apart at a glance.
*
* The first variant always keeps the bare parent name because
* `schemaToTypeRef` builds union variants whose model refs collapse to
* `qualifyInlineModelName(field, parent)` — so the first model name
* must match for the union's degenerate-collapse to land on a valid
* type. Renaming variant 0 would orphan it.
*
* Falls back to numeric suffix when:
* - The variant has no discriminator property (no shared const
* property across all object variants), OR
* - The const value PascalCases to an empty/whitespace name, OR
* - The derived name collides with the parent name or an already-
* emitted variant name (numeric keeps uniqueness).
*/
function nameVariantModel(
variant: SchemaObject,
parentName: string,
alreadyEmitted: Model[],
discriminator: { property: string } | null,
): string {
// Variant 0 keeps the bare parent name so the union TypeRef's degenerate
// collapse target exists in the model registry.
if (alreadyEmitted.length === 0) return parentName;

if (discriminator) {
const constValue = getConstPropertyValue(variant, discriminator.property);
if (constValue) {
const prefix = toPascalCase(constValue);
if (prefix) {
const candidate = parentName.startsWith(prefix) ? parentName : `${prefix}${parentName}`;
const collision = candidate === parentName || alreadyEmitted.some((m) => m.name === candidate);
if (!collision) return candidate;
}
}
}
return `${parentName}${alreadyEmitted.length + 1}`;
}

/**
* Returns a discriminator descriptor when every inline object variant in a
* oneOf has a string-const value on the same property — the signature of a
* discriminated union without an explicit `discriminator:` keyword. Used by
* `nameVariantModel` to derive variant names from the const values.
*
* Distinct from `detectConstPropertyDiscriminator` (which requires every
* variant to carry a `$ref` or `title` so a name mapping can be built):
* this pass operates on inline anonymous variants, where naming is exactly
* what we're trying to derive.
*/
function deriveConstNamingDiscriminator(variants: SchemaObject[]): { property: string } | null {
if (variants.length < 2) return null;
const candidates = Object.keys(variants[0]?.properties ?? {});
for (const propName of candidates) {
const values = variants.map((v) => getConstPropertyValue(v, propName));
if (values.some((v) => v === null)) continue;
if (new Set(values).size !== values.length) continue; // collisions = no signal
return { property: propName };
}
return null;
}

/** Build a single Field from a schema property entry. Shared across all extraction sites. */
export function buildFieldFromSchema(
fieldName: string,
Expand Down
234 changes: 234 additions & 0 deletions test/parser/oneof-variant-naming.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
import { describe, it, expect } from 'vitest';
import { parseSpec } from '../../src/parser/parse.js';
import * as fs from 'node:fs/promises';
import * as os from 'node:os';
import * as path from 'node:path';

/**
* Helper: write a spec to a temp file, parse it, return the IR. Cleans up
* the temp dir on completion.
*/
async function parseInline<T>(specYaml: string, fn: (ir: Awaited<ReturnType<typeof parseSpec>>) => T): Promise<T> {
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'oagen-oneof-naming-'));
const specFile = path.join(tmpDir, 'spec.yml');
try {
await fs.writeFile(specFile, specYaml, 'utf-8');
const ir = await parseSpec(specFile);
return fn(ir);
} finally {
await fs.rm(tmpDir, { recursive: true });
}
}

describe('oneOf variant naming', () => {
it('derives names from a const-property discriminator instead of using a numeric suffix', async () => {
// Mirrors the workos/openapi-spec ApiKey.owner shape: an inline oneOf
// whose object variants pin the same `type` property to distinct const
// values (`organization`, `user`). Without the const-naming pass the
// emitter would produce `ApiKeyOwner` and `ApiKeyOwner2` — informative
// for a robot, opaque for a human reading the generated SDK. With the
// pass it produces `ApiKeyOwner` (the first variant, kept as the bare
// parent name so the union TypeRef's degenerate-collapse target stays
// valid) and `UserApiKeyOwner` (the second variant, named after its
// discriminator value).
await parseInline(
`
openapi: 3.1.0
info:
title: Test
version: 1.0.0
servers:
- url: https://api.example.com
paths:
/api_keys/{id}:
get:
operationId: getApiKey
parameters:
- name: id
in: path
required: true
schema: { type: string }
responses:
'200':
description: ok
content:
application/json:
schema:
$ref: '#/components/schemas/ApiKey'
components:
schemas:
ApiKey:
type: object
required: [object, id, owner]
properties:
object: { type: string, const: api_key }
id: { type: string }
owner:
oneOf:
- type: object
required: [type, id]
properties:
type: { type: string, const: organization }
id: { type: string }
- type: object
required: [type, id, organization_id]
properties:
type: { type: string, const: user }
id: { type: string }
organization_id: { type: string }
`,
(ir) => {
const ownerModels = ir.models.filter((m) => m.name.endsWith('ApiKeyOwner'));
const ownerNames = ir.models.map((m) => m.name).filter((n) => n.toLowerCase().includes('apikeyowner'));

// First variant keeps the parent name so the union TypeRef has a
// valid collapse target.
expect(ownerNames).toContain('ApiKeyOwner');
// Second variant gets a const-derived prefix instead of a `2` suffix.
expect(ownerNames).toContain('UserApiKeyOwner');
expect(ownerNames).not.toContain('ApiKeyOwner2');
expect(ownerModels.length).toBeGreaterThanOrEqual(1);
},
);
});

it('falls back to numeric suffix when no shared const property distinguishes the variants', async () => {
// No discriminator-style property — variants differ structurally
// (different field sets) but neither has a `const` value pinning a
// shared property. Numeric-suffix scheme remains the safe fallback.
await parseInline(
`
openapi: 3.1.0
info: { title: Test, version: 1.0.0 }
servers: [{ url: https://api.example.com }]
paths:
/thing:
get:
operationId: getThing
responses:
'200':
description: ok
content:
application/json:
schema:
$ref: '#/components/schemas/Thing'
components:
schemas:
Thing:
type: object
required: [data]
properties:
data:
oneOf:
- type: object
required: [a]
properties:
a: { type: string }
- type: object
required: [b]
properties:
b: { type: integer }
`,
(ir) => {
const dataModels = ir.models.filter((m) => m.name.startsWith('ThingData'));
const names = dataModels.map((m) => m.name);
expect(names).toContain('ThingData');
// Second variant has no discriminator, so falls back to numeric suffix.
expect(names).toContain('ThingData2');
},
);
});

it('falls back to numeric suffix when two variants have identical const values on the same property', async () => {
// The const-naming heuristic requires distinct values per variant — if
// two variants both pin `type: organization`, naming both
// `OrganizationApiKeyOwner` would collide. Numeric suffix is safer.
await parseInline(
`
openapi: 3.1.0
info: { title: Test, version: 1.0.0 }
servers: [{ url: https://api.example.com }]
paths:
/resource:
get:
operationId: getResource
responses:
'200':
description: ok
content:
application/json:
schema:
$ref: '#/components/schemas/Resource'
components:
schemas:
Resource:
type: object
required: [owner]
properties:
owner:
oneOf:
- type: object
required: [type, id]
properties:
type: { type: string, const: organization }
id: { type: string }
- type: object
required: [type, slug]
properties:
type: { type: string, const: organization }
slug: { type: string }
`,
(ir) => {
const ownerModels = ir.models.filter((m) => m.name.includes('ResourceOwner'));
const names = ownerModels.map((m) => m.name);
expect(names).toContain('ResourceOwner');
// Same const value on both — we can't derive distinct names, so
// the fallback numeric suffix kicks in.
expect(names).toContain('ResourceOwner2');
},
);
});

it('preserves single-variant inline schemas (one object, one null) as nullable, no naming work needed', async () => {
// Make sure the const-naming pass doesn't accidentally fire on the
// common `oneOf: [{ type: object … }, { type: null }]` nullable
// pattern — there's only one *object* variant so the multi-variant
// logic shouldn't run.
await parseInline(
`
openapi: 3.1.0
info: { title: Test, version: 1.0.0 }
servers: [{ url: https://api.example.com }]
paths:
/x:
get:
operationId: getX
responses:
'200':
description: ok
content:
application/json:
schema:
$ref: '#/components/schemas/X'
components:
schemas:
X:
type: object
required: [box]
properties:
box:
oneOf:
- type: object
required: [name]
properties:
name: { type: string }
- type: 'null'
`,
(ir) => {
const names = ir.models.map((m) => m.name);
expect(names).toContain('XBox');
expect(names).not.toContain('XBox2');
},
);
});
});
Loading