diff --git a/src/cli/generate.ts b/src/cli/generate.ts index 6a4f3a6..044a34e 100644 --- a/src/cli/generate.ts +++ b/src/cli/generate.ts @@ -15,6 +15,8 @@ export async function generateCommand(opts: { apiSurface?: string; manifest?: string; compatCheck?: boolean; + /** From `--no-prune`. When false, manifest-driven stale-file pruning is skipped. */ + prune?: boolean; operationIdTransform?: (id: string) => string; schemaNameTransform?: (name: string) => string; docUrl?: string; @@ -55,6 +57,7 @@ export async function generateCommand(opts: { overlayLookup, operationHints: opts.operationHints, mountRules: opts.mountRules, + noPrune: opts.prune === false, }); if (opts.dryRun) { diff --git a/src/cli/index.ts b/src/cli/index.ts index b7856db..4421f8d 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -69,6 +69,7 @@ program .option('--api-surface ', 'Path to baseline API surface JSON for compat overlay') .option('--manifest ', 'Path to smoke-manifest.json for method overlay') .option('--no-compat-check', 'Skip compat overlay even if --api-surface is provided') + .option('--no-prune', 'Skip deletion of stale files recorded in the previous .oagen-manifest.json') .action((opts) => { opts.spec ??= process.env.OPENAPI_SPEC_PATH; if (!opts.spec) { diff --git a/src/compat/extractors/node.ts b/src/compat/extractors/node.ts index 4e9e83c..650c7cd 100644 --- a/src/compat/extractors/node.ts +++ b/src/compat/extractors/node.ts @@ -210,12 +210,31 @@ function extractClass(sym: ts.Symbol, checker: ts.TypeChecker): ApiClass { const methods: Record = {}; const properties: Record = {}; const constructorParams: ApiParam[] = []; + let deprecationMessage: string | undefined; // Extract constructor params const declarations = sym.getDeclarations(); if (declarations) { for (const decl of declarations) { if (ts.isClassDeclaration(decl)) { + // Look for `@deprecated` in the class's JSDoc. JSDoc nodes are + // attached to the declaration's parent range; TS exposes them via + // ts.getJSDocDeprecatedTag. + const deprecatedTag = ts.getJSDocDeprecatedTag(decl); + if (deprecatedTag) { + const comment = deprecatedTag.comment; + if (typeof comment === 'string') { + deprecationMessage = comment.trim() || ''; + } else if (Array.isArray(comment)) { + deprecationMessage = + comment + .map((c) => c.text) + .join('') + .trim() || ''; + } else { + deprecationMessage = ''; + } + } for (const member of decl.members) { if (ts.isConstructorDeclaration(member)) { for (const param of member.parameters) { @@ -280,6 +299,7 @@ function extractClass(sym: ts.Symbol, checker: ts.TypeChecker): ApiClass { methods: sortRecord(methods), properties: sortRecord(properties), constructorParams, + ...(deprecationMessage !== undefined && { deprecationMessage }), }; } diff --git a/src/compat/types.ts b/src/compat/types.ts index a05722c..2e497ef 100644 --- a/src/compat/types.ts +++ b/src/compat/types.ts @@ -15,6 +15,11 @@ export interface ApiClass { methods: Record; properties: Record; constructorParams: ApiParam[]; + /** Present when the class JSDoc carries `@deprecated`. Emitters use this + * to propagate deprecation to the service property on the client class + * so IDEs surface the strikethrough at the access site, not just when + * users instantiate the class directly. */ + deprecationMessage?: string; } export interface ApiMethod { diff --git a/src/engine/generate-files.ts b/src/engine/generate-files.ts index 918481b..53f3e32 100644 --- a/src/engine/generate-files.ts +++ b/src/engine/generate-files.ts @@ -15,6 +15,8 @@ export function buildEmitterContext( overlayLookup?: OverlayLookup; operationHints?: Record; mountRules?: Record; + target?: string; + priorTargetManifestPaths?: Set; }, ): EmitterContext { return { @@ -25,6 +27,8 @@ export function buildEmitterContext( apiSurface: options.apiSurface, overlayLookup: options.overlayLookup, resolvedOperations: resolveOperations(spec, options.operationHints, options.mountRules), + targetDir: options.target, + priorTargetManifestPaths: options.priorTargetManifestPaths, }; } @@ -145,6 +149,8 @@ export function generateFiles( overlayLookup?: OverlayLookup; operationHints?: Record; mountRules?: Record; + target?: string; + priorTargetManifestPaths?: Set; }, ): { files: GeneratedFile[]; ctx: EmitterContext; header: string } { const ctx = buildEmitterContext(spec, options); diff --git a/src/engine/integrate.ts b/src/engine/integrate.ts index a2d0284..0b61d21 100644 --- a/src/engine/integrate.ts +++ b/src/engine/integrate.ts @@ -40,8 +40,9 @@ function resolveImportPath( function isRootFile(filePath: string): boolean { // Fixtures are never roots (and should be filtered by integrateTarget already) if (filePath.includes('/fixtures/')) return false; - // Test files - if (filePath.endsWith('.spec.ts') || filePath.endsWith('.test.ts')) return false; + // Test files are roots — they are standalone entry points that belong in the + // target repo even though nothing imports them. + if (filePath.endsWith('.spec.ts') || filePath.endsWith('.test.ts')) return true; // Serializer files if (filePath.includes('/serializers/')) return false; // Standalone interface files (not barrel/index) @@ -208,12 +209,17 @@ async function resolveFileToDirectoryConflicts( return { files: result, removals }; } +export interface IntegrateResult extends WriteResult { + /** Final set of paths this run claims ownership of in the target (post tree-shake + conflict resolution). */ + emittedPaths: string[]; +} + export async function integrateGeneratedFiles(opts: { files: GeneratedFile[]; language: string; targetDir: string; header: string; -}): Promise { +}): Promise { const mapped = mapFilesForTargetIntegration(opts.files, opts.language); const shaken = await treeShakeFiles(mapped, opts.language); @@ -234,5 +240,5 @@ export async function integrateGeneratedFiles(opts: { } } - return result; + return { ...result, emittedPaths: resolved.map((f) => f.path) }; } diff --git a/src/engine/manifest.ts b/src/engine/manifest.ts new file mode 100644 index 0000000..ee2c54b --- /dev/null +++ b/src/engine/manifest.ts @@ -0,0 +1,167 @@ +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; + +/** + * Prune-manifest support for `oagen generate`. + * + * The manifest records every auto-generated file emitted on the previous run, + * relative to the directory it lives in (either the `--output` standalone dir + * or the `--target` integration dir). On the next run we diff the previous + * list against the current emission set and delete anything that's no longer + * produced — preventing stale-file accumulation across regens. + * + * Design notes: + * - The manifest is versioned so future format changes can be handled safely. + * - Paths are stored sorted for deterministic diffs in git. + * - Deletion is gated on a header guard when one is available: we only remove + * files whose contents start with the auto-generated header. This protects + * a hand-maintained file that happens to share a name with a previously + * generated one (extremely rare, but worth catching). + * - On first adoption (no previous manifest), pruning is skipped and a + * one-line notice is printed — no surprise deletions. + */ + +export const MANIFEST_FILENAME = '.oagen-manifest.json'; +const MANIFEST_VERSION = 1; + +export interface Manifest { + /** Schema version. Bump when the format changes incompatibly. */ + version: number; + /** Emitter language (e.g. "python"). Used only as a consistency hint. */ + language: string; + /** ISO-8601 timestamp of the run that produced this manifest. */ + generatedAt: string; + /** Sorted list of paths, relative to the manifest's containing directory. */ + files: string[]; +} + +export interface PruneResult { + /** Paths actually deleted. */ + pruned: string[]; + /** Paths skipped because the header guard didn't match (preserved). */ + preserved: string[]; + /** Paths already absent on disk (nothing to do). */ + missing: string[]; +} + +/** Read `.oagen-manifest.json` from a directory. Returns null if absent or malformed. */ +export async function readManifest(dir: string): Promise { + const manifestPath = path.join(dir, MANIFEST_FILENAME); + let raw: string; + try { + raw = await fs.readFile(manifestPath, 'utf-8'); + } catch { + return null; + } + try { + const parsed = JSON.parse(raw) as Partial; + if ( + typeof parsed.version !== 'number' || + typeof parsed.language !== 'string' || + typeof parsed.generatedAt !== 'string' || + !Array.isArray(parsed.files) || + !parsed.files.every((p): p is string => typeof p === 'string') + ) { + return null; + } + if (parsed.version > MANIFEST_VERSION) { + console.warn( + `[oagen] ${MANIFEST_FILENAME} schema version ${parsed.version} is newer than supported (${MANIFEST_VERSION}); ignoring for pruning.`, + ); + return null; + } + return parsed as Manifest; + } catch { + return null; + } +} + +/** Write `.oagen-manifest.json` to a directory with sorted paths. */ +export async function writeManifest(dir: string, opts: { language: string; files: Iterable }): Promise { + const manifest: Manifest = { + version: MANIFEST_VERSION, + language: opts.language, + generatedAt: new Date().toISOString(), + files: [...new Set(opts.files)].sort(), + }; + const manifestPath = path.join(dir, MANIFEST_FILENAME); + await fs.mkdir(dir, { recursive: true }); + await fs.writeFile(manifestPath, JSON.stringify(manifest, null, 2) + '\n', 'utf-8'); +} + +/** Return the set of previous paths no longer present in the current emission. */ +export function computeStalePaths(prev: Manifest, currentPaths: Iterable): string[] { + const current = new Set(currentPaths); + return prev.files.filter((p) => !current.has(p)).sort(); +} + +/** + * Delete stale files from `dir`. + * + * When `header` is provided, a file is only deleted if its contents start with + * that header — so hand-maintained files that somehow collide with a previously + * generated path can't be clobbered. When `header` is omitted, the guard is + * skipped (useful for non-source artifacts like fixture JSON). + * + * Returns lists for reporting. Empty parent directories are removed after + * deletion, up to (but not including) `dir` itself. + */ +export async function pruneStaleFiles( + dir: string, + paths: string[], + opts: { header?: string } = {}, +): Promise { + const pruned: string[] = []; + const preserved: string[] = []; + const missing: string[] = []; + + for (const relPath of paths) { + const fullPath = path.join(dir, relPath); + let content: string; + try { + content = await fs.readFile(fullPath, 'utf-8'); + } catch { + missing.push(relPath); + continue; + } + if (opts.header && !content.startsWith(opts.header)) { + preserved.push(relPath); + continue; + } + try { + await fs.unlink(fullPath); + pruned.push(relPath); + } catch { + // Racing deletion or permission issue — treat as missing rather than fatal. + missing.push(relPath); + continue; + } + await removeEmptyParents(path.dirname(fullPath), dir); + } + + return { pruned, preserved, missing }; +} + +/** + * Walk up from `startDir` removing empty directories until we hit `stopDir` + * (exclusive) or a non-empty directory. Silently ignores errors. + */ +async function removeEmptyParents(startDir: string, stopDir: string): Promise { + let current = path.resolve(startDir); + const stop = path.resolve(stopDir); + while (current.startsWith(stop + path.sep) && current !== stop) { + let entries: string[]; + try { + entries = await fs.readdir(current); + } catch { + return; + } + if (entries.length > 0) return; + try { + await fs.rmdir(current); + } catch { + return; + } + current = path.dirname(current); + } +} diff --git a/src/engine/merge-adapters/kotlin.ts b/src/engine/merge-adapters/kotlin.ts index 97ec511..c32a6c1 100644 --- a/src/engine/merge-adapters/kotlin.ts +++ b/src/engine/merge-adapters/kotlin.ts @@ -3,6 +3,8 @@ import type { MergeAdapter, MergeStatement, MergeImport, + MergeMember, + DeepMergeSymbol, DocstringInfo, SymbolDocstrings, MemberDocstrings, @@ -59,6 +61,59 @@ function findPrecedingKdoc(children: Parser.SyntaxNode[], index: number, source: return null; } +function extractKotlinClassMembers(classBody: Parser.SyntaxNode, source: string): MergeMember[] { + const members: MergeMember[] = []; + const children = classBody.namedChildren; + + for (let i = 0; i < children.length; i++) { + const child = children[i]; + let memberName: string | null = null; + + if (child.type === 'property_declaration') { + const varDecl = child.children.find((c) => c.type === 'variable_declaration'); + memberName = varDecl?.children.find((c) => c.type === 'simple_identifier')?.text ?? null; + } else if (child.type === 'function_declaration') { + memberName = child.children.find((c) => c.type === 'simple_identifier')?.text ?? null; + } else if (child.type === 'companion_object') { + memberName = 'companion'; + } + + if (!memberName) continue; + + // Fold trailing getter/setter into the property's text span + let endIdx = child.endIndex; + if (child.type === 'property_declaration') { + while (i + 1 < children.length) { + const next = children[i + 1]; + if (next.type === 'getter' || next.type === 'setter') { + endIdx = next.endIndex; + i++; + } else { + break; + } + } + } + + // Include preceding KDoc comment in the text span so deep merge + // inserts the member with its documentation intact. + let startIdx = child.startIndex; + for (let k = i - 1; k >= 0; k--) { + const prev = children[k]; + if (prev.type === 'multiline_comment') { + const text = source.slice(prev.startIndex, prev.endIndex); + if (text.startsWith('/**')) startIdx = prev.startIndex; + break; + } + if (prev.type === 'line_comment') continue; + break; + } + + members.push({ key: memberName, text: source.slice(startIdx, endIdx) }); + } + + return members; +} + export const kotlinMergeAdapter: MergeAdapter = { language: 'kotlin', grammarModule: 'tree-sitter-kotlin', @@ -100,6 +155,33 @@ export const kotlinMergeAdapter: MergeAdapter = { renderImports(imports) { return imports.map((entry) => entry.text); }, + shouldSkipDeepMerge(_symbolName, existingMemberKeys, newMembers) { + // Skip if new members reference instance properties that don't exist in the target + const newText = newMembers.map((m) => m.text).join('\n'); + for (const match of newText.matchAll(/this\.(\w+)/g)) { + const propName = match[1]; + if (propName && !existingMemberKeys.has(propName)) return true; + } + return false; + }, + extractMembers(tree, source) { + const result = new Map(); + + for (const child of tree.rootNode.children) { + if (child.type !== 'class_declaration') continue; + const nameNode = child.children.find((c) => c.type === 'type_identifier'); + if (!nameNode) continue; + const body = child.children.find((c) => c.type === 'class_body'); + if (!body) continue; + + const members = extractKotlinClassMembers(body, source); + const firstMember = body.firstNamedChild; + const memberIndent = firstMember ? ' '.repeat(firstMember.startPosition.column) : undefined; + result.set(nameNode.text, { members, bodyEndLine: body.endPosition.row, memberIndent }); + } + + return result; + }, extractDocstrings(tree, source) { const result = new Map(); const rootChildren = tree.rootNode.children; diff --git a/src/engine/merger.ts b/src/engine/merger.ts index 8eccf89..7ff84e8 100644 --- a/src/engine/merger.ts +++ b/src/engine/merger.ts @@ -490,8 +490,9 @@ export async function mergeIntoExisting( } const indent = existSymbol.memberIndent ?? ' '; - const insertText = newMembers.map((m) => indent + m.text).join('\n'); - insertions.push({ line: existSymbol.bodyEndLine, text: insertText }); + const insertText = newMembers.map((m) => indent + m.text).join('\n\n'); + // Leading blank line separates inserted members from existing ones. + insertions.push({ line: existSymbol.bodyEndLine, text: '\n' + insertText }); deepAdded += newMembers.length; } } diff --git a/src/engine/operation-plan.ts b/src/engine/operation-plan.ts index 89d6d6c..282ada3 100644 --- a/src/engine/operation-plan.ts +++ b/src/engine/operation-plan.ts @@ -13,6 +13,10 @@ export interface OperationPlan { * (unwrapped from the list wrapper). Null for non-paginated. */ paginatedItemModelName: string | null; isModelResponse: boolean; + /** True when the response is an unpaginated `type: array` of models. + * Emitters should use this to return `Model[]` (or the language's list + * equivalent) and map the deserializer over each element. */ + isArrayResponse: boolean; hasQueryParams: boolean; isAsync: boolean; } @@ -28,6 +32,9 @@ export function planOperation(op: Operation): OperationPlan { const paginatedItemModelName = isPaginated && op.pagination?.itemType.kind === 'model' ? op.pagination.itemType.name : null; const isModelResponse = responseModelName !== null; + // Unpaginated `type: array` of models — needs `Model[]` return + elementwise + // deserialization. Paginated operations use the pagination wrapper instead. + const isArrayResponse = !isPaginated && op.response.kind === 'array' && op.response.items.kind === 'model'; const isAsync = op.async ?? true; return { @@ -40,6 +47,7 @@ export function planOperation(op: Operation): OperationPlan { responseModelName, paginatedItemModelName, isModelResponse, + isArrayResponse, hasQueryParams, isAsync, }; diff --git a/src/engine/orchestrator.ts b/src/engine/orchestrator.ts index 1b5bdfa..8337363 100644 --- a/src/engine/orchestrator.ts +++ b/src/engine/orchestrator.ts @@ -6,6 +6,7 @@ import { writeFiles } from './writer.js'; import { generateFiles } from './generate-files.js'; import { integrateGeneratedFiles } from './integrate.js'; import { formatTargetFiles } from './formatter.js'; +import { computeStalePaths, MANIFEST_FILENAME, pruneStaleFiles, readManifest, writeManifest } from './manifest.js'; export async function generate( spec: ApiSpec, @@ -19,9 +20,21 @@ export async function generate( overlayLookup?: OverlayLookup; operationHints?: Record; mountRules?: Record; + /** When true, skip deletion of files recorded in the previous manifest but not in the current emission. */ + noPrune?: boolean; }, ): Promise { - const { files: withHeaders, header } = generateFiles(spec, emitter, options); + // Read the target's previous manifest up front so emitters can mark files + // the prior run wrote as safe-to-overwrite (vs files a human hand-maintains). + // Skipped when --no-prune is set so users opting out of lifecycle tracking + // also opt out of overwrite-on-regen behavior. + const targetManifestForCtx = options.target && !options.noPrune ? await readManifest(options.target) : null; + const priorTargetManifestPaths = targetManifestForCtx ? new Set(targetManifestForCtx.files) : undefined; + + const { files: withHeaders, header } = generateFiles(spec, emitter, { + ...options, + priorTargetManifestPaths, + }); if (options.dryRun) { if (options.target) { @@ -33,6 +46,9 @@ export async function generate( return withHeaders; } + // Read previous manifest BEFORE writing so a manifest overwrite can't alter the diff. + const outputPrevManifest = options.noPrune ? null : await readManifest(options.outputDir); + const writeResult = await writeFiles(withHeaders, options.outputDir, { language: emitter.language, header, @@ -45,8 +61,23 @@ export async function generate( console.log(`Ignored ${writeResult.ignored.length} files (@oagen-ignore-file)`); } + const outputEmittedPaths = withHeaders.map((f) => f.path); + await applyManifestPrune({ + dir: options.outputDir, + label: 'Output', + prevManifest: outputPrevManifest, + currentPaths: outputEmittedPaths, + language: emitter.language, + header, + noPrune: options.noPrune, + }); + // Target integration pass if (options.target) { + // Reuse the manifest we already read to build the emitter context. This + // is correct because nothing between the two reads writes the manifest. + const targetPrevManifest = targetManifestForCtx; + const targetResult = await integrateGeneratedFiles({ files: withHeaders, language: emitter.language, @@ -64,6 +95,16 @@ export async function generate( console.log(`Target: skipped ${targetResult.skipped.length} files (excluded or no grammar)`); } + await applyManifestPrune({ + dir: options.target, + label: 'Target', + prevManifest: targetPrevManifest, + currentPaths: targetResult.emittedPaths, + language: emitter.language, + header, + noPrune: options.noPrune, + }); + // Run the emitter's formatter on all written/merged/identical files const allTargetFiles = [...targetResult.written, ...targetResult.merged, ...targetResult.identical]; if (allTargetFiles.length > 0) { @@ -73,3 +114,47 @@ export async function generate( return withHeaders; } + +/** + * Prune files recorded in the previous manifest but absent from the current emission, + * then write the fresh manifest. Safe on first adoption (no previous manifest). + */ +async function applyManifestPrune(opts: { + dir: string; + label: string; + prevManifest: Awaited>; + currentPaths: string[]; + language: string; + header: string; + noPrune?: boolean; +}): Promise { + if (opts.noPrune) { + // Still refresh the manifest so future runs with pruning enabled have a baseline. + await writeManifest(opts.dir, { language: opts.language, files: opts.currentPaths }); + return; + } + + if (!opts.prevManifest) { + console.log( + `${opts.label}: no ${MANIFEST_FILENAME} found — skipping prune (this baseline manifest will be written now).`, + ); + await writeManifest(opts.dir, { language: opts.language, files: opts.currentPaths }); + return; + } + + const stale = computeStalePaths(opts.prevManifest, opts.currentPaths); + if (stale.length > 0) { + const { pruned, preserved } = await pruneStaleFiles(opts.dir, stale, { header: opts.header }); + if (pruned.length > 0) { + console.log(`${opts.label}: pruned ${pruned.length} stale file${pruned.length === 1 ? '' : 's'}`); + } + if (preserved.length > 0) { + console.log( + `${opts.label}: preserved ${preserved.length} file${preserved.length === 1 ? '' : 's'} in previous manifest but lacking the auto-generated header (possibly hand-edited).`, + ); + for (const p of preserved) console.log(` ${p}`); + } + } + + await writeManifest(opts.dir, { language: opts.language, files: opts.currentPaths }); +} diff --git a/src/engine/types.ts b/src/engine/types.ts index 7ab19fd..1e9bdad 100644 --- a/src/engine/types.ts +++ b/src/engine/types.ts @@ -22,6 +22,16 @@ export interface EmitterContext { overlayLookup?: OverlayLookup; /** Resolved operations from the hint-aware resolver. Populated by buildEmitterContext(). */ resolvedOperations?: ResolvedOperation[]; + /** Absolute path to the integration target directory (when --target is used). */ + targetDir?: string; + /** + * Paths (relative to the target dir) that the previous run wrote into + * `--target`, loaded from that directory's `.oagen-manifest.json`. Emitters + * can use this to distinguish "file exists because oagen wrote it last time" + * from "file exists because a human wrote it" — the former is safe to + * overwrite, the latter should be merged. + */ + priorTargetManifestPaths?: Set; } export interface FormatCommand { diff --git a/src/ir/operation-hints.ts b/src/ir/operation-hints.ts index 2642233..e9c5614 100644 --- a/src/ir/operation-hints.ts +++ b/src/ir/operation-hints.ts @@ -24,6 +24,13 @@ export interface OperationHint { defaults?: Record; /** Fields the SDK reads from client config at runtime (e.g. ['client_id']). */ inferFromClient?: string[]; + /** + * Marks this operation as a URL builder rather than a real HTTP call. Emitters + * should generate a method that returns the constructed URL (typically as a + * string) without performing any I/O. Used for OAuth-style redirect endpoints + * such as /sso/authorize and /user_management/sessions/logout. + */ + urlBuilder?: boolean; } export interface SplitHint { @@ -37,6 +44,12 @@ export interface SplitHint { inferFromClient?: string[]; /** Only these body fields are exposed as method params. If omitted, all non-default/non-inferred fields are exposed. */ exposedParams?: string[]; + /** + * Subset of exposedParams that should be emitted as optional even when the + * variant model would otherwise mark them required (or when the variant has + * no addressable model, as happens for inline oneOf branches). + */ + optionalParams?: string[]; } // --------------------------------------------------------------------------- @@ -58,6 +71,8 @@ export interface ResolvedOperation { defaults: Record; /** Fields the SDK reads from client config at runtime (from operation-level hint). */ inferFromClient: string[]; + /** Marks this operation as a URL builder; emitters should not generate an HTTP call. */ + urlBuilder: boolean; } export interface ResolvedWrapper { @@ -208,9 +223,11 @@ export function deriveMethodName(op: Operation, _service: Service): string { } } - // Singularize only when there is a trailing path param (single-resource operation). - // POST/PUT/DELETE to collection endpoints keep plural nouns. - const isSingleResource = trailingParam; + // Singularize when: + // * there's a trailing path param (single-resource read/update/delete), or + // * the verb itself implies a single-resource operation (e.g. POST → create + // always creates one item even when posted to a collection endpoint). + const isSingleResource = trailingParam || verb === 'create'; const noun = isSingleResource ? singularize(resource) : resource; // For "list" verb, keep plural @@ -256,7 +273,7 @@ export function resolveOperations( defaults: sh.defaults ?? {}, inferFromClient: sh.inferFromClient ?? [], exposedParams: sh.exposedParams ?? [], - optionalParams: [], + optionalParams: sh.optionalParams ?? [], responseModelName: resolveResponseModelName(op), })); } @@ -269,6 +286,7 @@ export function resolveOperations( wrappers, defaults: hint?.defaults ?? {}, inferFromClient: hint?.inferFromClient ?? [], + urlBuilder: hint?.urlBuilder ?? false, }); } } diff --git a/src/parser/schemas.ts b/src/parser/schemas.ts index bc245dd..cc11f34 100644 --- a/src/parser/schemas.ts +++ b/src/parser/schemas.ts @@ -318,6 +318,104 @@ export function buildFieldFromSchema( }; } +/** + * If every non-null oneOf/anyOf variant is a string-valued const, return the + * list of those const values so the union can collapse into a single enum. + * Returns null when any variant isn't a pure string-const (model ref, + * number const, nullable, etc.). + */ +function collectLiteralStringConsts(variants: SchemaObject[]): string[] | null { + if (variants.length === 0) return null; + const values: string[] = []; + for (const v of variants) { + // A single-element enum is semantically identical to const. + if (typeof v.const === 'string') { + values.push(v.const); + continue; + } + if (Array.isArray(v.enum) && v.enum.length === 1 && typeof v.enum[0] === 'string') { + values.push(v.enum[0]); + continue; + } + return null; + } + return values; +} + +/** Build an EnumRef the same way the regular enum path does for naming. */ +function buildSyntheticEnumRef( + values: string[], + contextName: string | undefined, + parentModelName: string | undefined, +): TypeRef { + const baseName = toPascalCase(contextName ?? 'UnknownEnum'); + const cleanParent = parentModelName ? stripListItemMarkers(parentModelName) : undefined; + const qualifiedName = cleanParent && !baseName.startsWith(cleanParent) ? `${cleanParent}${baseName}` : baseName; + return { kind: 'enum', name: qualifiedName, values }; +} + +/** + * Detect an implicit discriminator on an oneOf where every variant is an + * object schema that pins the same property to a const value. Returns + * { property, mapping } so the IR can represent the union as discriminated + * without needing an explicit `discriminator:` key in the spec. + * + * Both shapes are supported: + * { properties: { event: { const: "x" } }, required: ["event"] } + * { properties: { event: { type: "string", enum: ["x"] } } } + * + * Returns null when any variant doesn't fit the pattern or when variants + * disagree on which property carries the discriminator. + */ +function detectConstPropertyDiscriminator( + variants: SchemaObject[], +): { property: string; mapping: Record } | null { + if (variants.length < 2) return null; + + // Find properties whose const value is present on every variant. + let candidateProperty: string | null = null; + const candidates = Object.keys(variants[0]?.properties ?? {}); + for (const propName of candidates) { + if (variants.every((v) => getConstPropertyValue(v, propName) !== null)) { + candidateProperty = propName; + break; + } + } + + if (!candidateProperty) return null; + + const mapping: Record = {}; + for (const v of variants) { + const value = getConstPropertyValue(v, candidateProperty); + if (value === null) return null; + // Require a $ref or title so we have a concrete model name to map to. + const variantName = resolveVariantModelName(v); + if (!variantName) return null; + mapping[value] = variantName; + } + + return { property: candidateProperty, mapping }; +} + +function getConstPropertyValue(schema: SchemaObject, property: string): string | null { + const prop = schema.properties?.[property]; + if (!prop) return null; + if (typeof prop.const === 'string') return prop.const; + if (Array.isArray(prop.enum) && prop.enum.length === 1 && typeof prop.enum[0] === 'string') { + return prop.enum[0]; + } + return null; +} + +function resolveVariantModelName(schema: SchemaObject): string | null { + if (schema.$ref) { + return resolveSchemaName(schema.$ref.replace(/^#\/components\/schemas\//, '')); + } + const title = typeof schema['title'] === 'string' ? (schema['title'] as string) : null; + if (title) return resolveSchemaName(title); + return null; +} + function extractEnum(name: string, schema: SchemaObject): Enum { const values: EnumValue[] = (schema.enum ?? []).map((v) => ({ name: toUpperSnakeCase(String(v)), @@ -609,6 +707,24 @@ export function schemaToTypeRef(schema: SchemaObject, contextName?: string, pare }; } + // Collapse an oneOf/anyOf whose non-null variants are all string-const + // schemas into a single enum. This turns patterns like + // provider: { oneOf: [{ const: "AppleOAuth" }, { const: "GitHubOAuth" }, ...] } + // into a proper enum type with one member per variant instead of a + // structurally-opaque union of literal refs. + const literalStrings = collectLiteralStringConsts(nonNullVariants); + if (literalStrings !== null && literalStrings.length >= 2) { + const enumRef = buildSyntheticEnumRef(literalStrings, contextName, parentModelName); + return nullVariant ? { kind: 'nullable', inner: enumRef } : enumRef; + } + + // Synthesize a discriminator when all non-null variants are objects that + // share a property whose schema carries a `const` value. Covers the + // EventSchema-style pattern where each oneOf variant pins `event: + // const: "..."` instead of the spec using an explicit `discriminator:`. + const syntheticDiscriminator = + !schema.discriminator && compositionKind === 'oneOf' ? detectConstPropertyDiscriminator(nonNullVariants) : null; + // General union const variants = rawVariants .filter((v: SchemaObject) => v.type !== 'null') @@ -630,7 +746,9 @@ export function schemaToTypeRef(schema: SchemaObject, contextName?: string, pare ), }, } - : {}), + : syntheticDiscriminator + ? { discriminator: syntheticDiscriminator } + : {}), }; return hasNull ? { kind: 'nullable', inner: union } : union; } diff --git a/test/engine/manifest.test.ts b/test/engine/manifest.test.ts new file mode 100644 index 0000000..9af5733 --- /dev/null +++ b/test/engine/manifest.test.ts @@ -0,0 +1,187 @@ +import { describe, it, expect } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + computeStalePaths, + MANIFEST_FILENAME, + pruneStaleFiles, + readManifest, + writeManifest, +} from '../../src/engine/manifest.js'; + +async function tmp(): Promise { + return fs.mkdtemp(path.join(os.tmpdir(), 'oagen-manifest-')); +} + +describe('manifest read/write', () => { + it('writes a manifest with sorted, deduped paths', async () => { + const dir = await tmp(); + try { + await writeManifest(dir, { language: 'python', files: ['b.py', 'a.py', 'b.py', 'c.py'] }); + const raw = await fs.readFile(path.join(dir, MANIFEST_FILENAME), 'utf-8'); + const parsed = JSON.parse(raw); + expect(parsed.files).toEqual(['a.py', 'b.py', 'c.py']); + expect(parsed.language).toBe('python'); + expect(parsed.version).toBe(1); + expect(typeof parsed.generatedAt).toBe('string'); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('readManifest returns null when file is absent', async () => { + const dir = await tmp(); + try { + expect(await readManifest(dir)).toBeNull(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('readManifest returns null on malformed JSON', async () => { + const dir = await tmp(); + try { + await fs.writeFile(path.join(dir, MANIFEST_FILENAME), '{ not json'); + expect(await readManifest(dir)).toBeNull(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('readManifest returns null when required fields are missing', async () => { + const dir = await tmp(); + try { + await fs.writeFile(path.join(dir, MANIFEST_FILENAME), JSON.stringify({ version: 1, files: ['a.py'] })); + expect(await readManifest(dir)).toBeNull(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('round-trips language and files list', async () => { + const dir = await tmp(); + try { + await writeManifest(dir, { language: 'node', files: ['src/index.ts', 'src/client.ts'] }); + const got = await readManifest(dir); + expect(got).not.toBeNull(); + expect(got!.language).toBe('node'); + expect(got!.files).toEqual(['src/client.ts', 'src/index.ts']); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); +}); + +describe('computeStalePaths', () => { + it('returns files present in prev but absent from current', () => { + const prev = { + version: 1, + language: 'python', + generatedAt: '2026-01-01T00:00:00.000Z', + files: ['a.py', 'b.py', 'c.py'], + }; + expect(computeStalePaths(prev, ['a.py', 'c.py', 'd.py'])).toEqual(['b.py']); + }); + + it('returns empty when nothing is stale', () => { + const prev = { + version: 1, + language: 'python', + generatedAt: '2026-01-01T00:00:00.000Z', + files: ['a.py'], + }; + expect(computeStalePaths(prev, ['a.py', 'b.py'])).toEqual([]); + }); + + it('sorts the result', () => { + const prev = { + version: 1, + language: 'python', + generatedAt: '2026-01-01T00:00:00.000Z', + files: ['zeta.py', 'alpha.py', 'mu.py'], + }; + expect(computeStalePaths(prev, [])).toEqual(['alpha.py', 'mu.py', 'zeta.py']); + }); +}); + +describe('pruneStaleFiles', () => { + const header = '# auto-generated by oagen'; + + it('deletes files that start with the header', async () => { + const dir = await tmp(); + try { + await fs.writeFile(path.join(dir, 'gen.py'), `${header}\n\nclass Foo: pass\n`); + const result = await pruneStaleFiles(dir, ['gen.py'], { header }); + expect(result.pruned).toEqual(['gen.py']); + expect(result.preserved).toEqual([]); + await expect(fs.stat(path.join(dir, 'gen.py'))).rejects.toThrow(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('preserves files missing the header', async () => { + const dir = await tmp(); + try { + await fs.writeFile(path.join(dir, 'hand.py'), 'class HandWritten: pass\n'); + const result = await pruneStaleFiles(dir, ['hand.py'], { header }); + expect(result.pruned).toEqual([]); + expect(result.preserved).toEqual(['hand.py']); + await expect(fs.stat(path.join(dir, 'hand.py'))).resolves.toBeTruthy(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('reports paths that do not exist on disk', async () => { + const dir = await tmp(); + try { + const result = await pruneStaleFiles(dir, ['missing.py'], { header }); + expect(result.missing).toEqual(['missing.py']); + expect(result.pruned).toEqual([]); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('removes empty parent directories after deletion', async () => { + const dir = await tmp(); + try { + const nested = path.join(dir, 'deep', 'nest'); + await fs.mkdir(nested, { recursive: true }); + await fs.writeFile(path.join(nested, 'gen.py'), `${header}\n`); + const result = await pruneStaleFiles(dir, ['deep/nest/gen.py'], { header }); + expect(result.pruned).toEqual(['deep/nest/gen.py']); + await expect(fs.stat(path.join(dir, 'deep'))).rejects.toThrow(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('leaves parent directory alone when siblings remain', async () => { + const dir = await tmp(); + try { + const nest = path.join(dir, 'models'); + await fs.mkdir(nest, { recursive: true }); + await fs.writeFile(path.join(nest, 'gen.py'), `${header}\n`); + await fs.writeFile(path.join(nest, 'keep.py'), 'keep\n'); + await pruneStaleFiles(dir, ['models/gen.py'], { header }); + await expect(fs.stat(nest)).resolves.toBeTruthy(); + await expect(fs.stat(path.join(nest, 'keep.py'))).resolves.toBeTruthy(); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); + + it('deletes without guard when header is omitted', async () => { + const dir = await tmp(); + try { + await fs.writeFile(path.join(dir, 'fixture.json'), '{"foo": 1}'); + const result = await pruneStaleFiles(dir, ['fixture.json']); + expect(result.pruned).toEqual(['fixture.json']); + } finally { + await fs.rm(dir, { recursive: true }); + } + }); +}); diff --git a/test/engine/operation-plan.test.ts b/test/engine/operation-plan.test.ts index 3ea9818..a77403d 100644 --- a/test/engine/operation-plan.test.ts +++ b/test/engine/operation-plan.test.ts @@ -24,6 +24,7 @@ describe('planOperation', () => { { "hasBody": false, "hasQueryParams": false, + "isArrayResponse": false, "isAsync": true, "isDelete": true, "isIdempotentPost": false, @@ -56,6 +57,7 @@ describe('planOperation', () => { { "hasBody": false, "hasQueryParams": false, + "isArrayResponse": false, "isAsync": true, "isDelete": false, "isIdempotentPost": false, @@ -95,6 +97,7 @@ describe('planOperation', () => { { "hasBody": true, "hasQueryParams": false, + "isArrayResponse": false, "isAsync": true, "isDelete": false, "isIdempotentPost": true, @@ -213,6 +216,48 @@ describe('planOperation', () => { ); expect(plan.responseModelName).toBe('User'); expect(plan.isModelResponse).toBe(true); + expect(plan.isArrayResponse).toBe(true); + }); + + it('unpaginated array-of-model response → isArrayResponse true', () => { + const plan = planOperation( + makeOp({ + response: { kind: 'array', items: { kind: 'model', name: 'Secret' } }, + }), + ); + expect(plan.isArrayResponse).toBe(true); + expect(plan.isPaginated).toBe(false); + }); + + it('paginated operation with array response → isArrayResponse false', () => { + const plan = planOperation( + makeOp({ + pagination: { + strategy: 'cursor', + param: 'after', + dataPath: 'data', + itemType: { kind: 'model', name: 'Organization' }, + }, + response: { kind: 'array', items: { kind: 'model', name: 'Organization' } }, + }), + ); + expect(plan.isPaginated).toBe(true); + // Paginated responses use the pagination wrapper, not raw arrays. + expect(plan.isArrayResponse).toBe(false); + }); + + it('single-model response → isArrayResponse false', () => { + const plan = planOperation(makeOp({ response: { kind: 'model', name: 'Organization' } })); + expect(plan.isArrayResponse).toBe(false); + }); + + it('array of primitives → isArrayResponse false (no model to deserialize)', () => { + const plan = planOperation( + makeOp({ + response: { kind: 'array', items: { kind: 'primitive', type: 'string' } }, + }), + ); + expect(plan.isArrayResponse).toBe(false); }); it('nullable response extracts inner model name', () => { diff --git a/test/engine/orchestrator-prune.test.ts b/test/engine/orchestrator-prune.test.ts new file mode 100644 index 0000000..fb120a7 --- /dev/null +++ b/test/engine/orchestrator-prune.test.ts @@ -0,0 +1,183 @@ +import { describe, it, expect } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { generate } from '../../src/engine/orchestrator.js'; +import { readManifest, MANIFEST_FILENAME } from '../../src/engine/manifest.js'; +import type { Emitter } from '../../src/engine/types.js'; +import type { ApiSpec } from '../../src/ir/types.js'; +import { defaultSdkBehavior } from '../../src/ir/sdk-behavior.js'; + +const HEADER = '# Auto-generated'; + +function mockEmitter(files: { path: string; content: string }[]): Emitter { + return { + language: 'mock', + generateModels: () => files.filter((f) => f.path.startsWith('models/')), + generateEnums: () => [], + generateResources: () => [], + generateClient: () => files.filter((f) => !f.path.startsWith('models/')), + generateErrors: () => [], + generateTypeSignatures: () => [], + generateTests: () => [], + fileHeader: () => HEADER, + }; +} + +const minimalSpec: ApiSpec = { + name: 'Test API', + version: '1.0.0', + baseUrl: 'https://api.test.com', + services: [], + models: [], + enums: [], + sdk: defaultSdkBehavior(), +}; + +async function tmp(): Promise { + return fs.mkdtemp(path.join(os.tmpdir(), 'oagen-prune-')); +} + +describe('orchestrator — manifest-based pruning', () => { + it('writes a manifest on first run (no prior manifest present)', async () => { + const outputDir = await tmp(); + try { + const emitter = mockEmitter([ + { path: 'models/user.rb', content: 'class User; end' }, + { path: 'client.rb', content: 'class Client; end' }, + ]); + + await generate(minimalSpec, emitter, { namespace: 'test', outputDir }); + + const manifest = await readManifest(outputDir); + expect(manifest).not.toBeNull(); + expect(manifest!.language).toBe('mock'); + expect(manifest!.files).toEqual(['client.rb', 'models/user.rb']); + } finally { + await fs.rm(outputDir, { recursive: true }); + } + }); + + it('prunes files recorded in prior manifest but absent in new run', async () => { + const outputDir = await tmp(); + try { + // Run 1: emits user.rb + client.rb + await generate( + minimalSpec, + mockEmitter([ + { path: 'models/user.rb', content: 'class User; end' }, + { path: 'client.rb', content: 'class Client; end' }, + ]), + { namespace: 'test', outputDir }, + ); + + expect(await fileExists(path.join(outputDir, 'models/user.rb'))).toBe(true); + + // Run 2: emits only client.rb — user.rb should be pruned + await generate(minimalSpec, mockEmitter([{ path: 'client.rb', content: 'class Client; end' }]), { + namespace: 'test', + outputDir, + }); + + expect(await fileExists(path.join(outputDir, 'models/user.rb'))).toBe(false); + expect(await fileExists(path.join(outputDir, 'client.rb'))).toBe(true); + + const manifest = await readManifest(outputDir); + expect(manifest!.files).toEqual(['client.rb']); + } finally { + await fs.rm(outputDir, { recursive: true }); + } + }); + + it('preserves files in prior manifest that no longer have the auto-generated header', async () => { + const outputDir = await tmp(); + try { + await generate(minimalSpec, mockEmitter([{ path: 'models/user.rb', content: 'class User; end' }]), { + namespace: 'test', + outputDir, + }); + + // Simulate hand-edit: header removed + await fs.writeFile(path.join(outputDir, 'models/user.rb'), 'class User\n def hand_written; end\nend\n'); + + // Run 2: emitter no longer emits user.rb + await generate(minimalSpec, mockEmitter([{ path: 'client.rb', content: 'class Client; end' }]), { + namespace: 'test', + outputDir, + }); + + // File should be preserved because it's missing the header + expect(await fileExists(path.join(outputDir, 'models/user.rb'))).toBe(true); + } finally { + await fs.rm(outputDir, { recursive: true }); + } + }); + + it('skips pruning entirely when --no-prune is set, but still refreshes the manifest', async () => { + const outputDir = await tmp(); + try { + await generate(minimalSpec, mockEmitter([{ path: 'models/user.rb', content: 'class User; end' }]), { + namespace: 'test', + outputDir, + }); + + await generate(minimalSpec, mockEmitter([{ path: 'client.rb', content: 'class Client; end' }]), { + namespace: 'test', + outputDir, + noPrune: true, + }); + + // user.rb should still be present because pruning was skipped + expect(await fileExists(path.join(outputDir, 'models/user.rb'))).toBe(true); + + // Manifest should reflect the latest emission (baseline for future prune-enabled runs) + const manifest = await readManifest(outputDir); + expect(manifest!.files).toEqual(['client.rb']); + } finally { + await fs.rm(outputDir, { recursive: true }); + } + }); + + it('does not prune on first adoption even when previous files exist (no manifest → no baseline)', async () => { + const outputDir = await tmp(); + try { + // Pre-seed a file that would look stale if there were a manifest pointing to it + await fs.writeFile(path.join(outputDir, 'legacy.rb'), `${HEADER}\nclass Legacy; end\n`); + + await generate(minimalSpec, mockEmitter([{ path: 'client.rb', content: 'class Client; end' }]), { + namespace: 'test', + outputDir, + }); + + // Without a previous manifest, legacy.rb is not claimed by the emitter and must not be touched + expect(await fileExists(path.join(outputDir, 'legacy.rb'))).toBe(true); + // Manifest should now exist and list only the current emission + const manifest = await readManifest(outputDir); + expect(manifest!.files).toEqual(['client.rb']); + } finally { + await fs.rm(outputDir, { recursive: true }); + } + }); + + it('writes the manifest at the MANIFEST_FILENAME path', async () => { + const outputDir = await tmp(); + try { + await generate(minimalSpec, mockEmitter([{ path: 'client.rb', content: 'class Client; end' }]), { + namespace: 'test', + outputDir, + }); + expect(await fileExists(path.join(outputDir, MANIFEST_FILENAME))).toBe(true); + } finally { + await fs.rm(outputDir, { recursive: true }); + } + }); +}); + +async function fileExists(p: string): Promise { + try { + await fs.stat(p); + return true; + } catch { + return false; + } +} diff --git a/test/ir/operation-hints.test.ts b/test/ir/operation-hints.test.ts index e93d942..5235bf9 100644 --- a/test/ir/operation-hints.test.ts +++ b/test/ir/operation-hints.test.ts @@ -57,8 +57,8 @@ describe('deriveMethodName', () => { expect(deriveMethodName(op('get', '/users/{id}'), service)).toBe('get_user'); }); - it('POST on collection → create_', () => { - expect(deriveMethodName(op('post', '/users'), service)).toBe('create_users'); + it('POST on collection → create_ (creating one item)', () => { + expect(deriveMethodName(op('post', '/users'), service)).toBe('create_user'); }); it('PUT on resource → update_', () => { @@ -137,8 +137,8 @@ describe('deriveMethodName', () => { }); describe('singularization', () => { - it('keeps plural for collection POST', () => { - expect(deriveMethodName(op('post', '/organizations'), service)).toBe('create_organizations'); + it('singularizes collection POST (a create operation makes one item)', () => { + expect(deriveMethodName(op('post', '/organizations'), service)).toBe('create_organization'); }); it('singularizes with trailing param', () => { @@ -200,7 +200,7 @@ describe('resolveOperations', () => { expect(resolved).toHaveLength(3); expect(resolved[0].methodName).toBe('list_users'); expect(resolved[1].methodName).toBe('get_user'); - expect(resolved[2].methodName).toBe('create_users'); + expect(resolved[2].methodName).toBe('create_user'); // No mount override — stays on original service expect(resolved[0].mountOn).toBe('Users'); });