From 41529181be5b0fc9d23cc3316030006c385be34a Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Tue, 19 May 2026 12:42:31 -0400 Subject: [PATCH] fix(writer): don't splice `import type {` as a Python simple import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isPythonSimpleImport` matched any line starting with `import` followed by a word character and lacking a `from` clause. That correctly catches Python's `import hashlib` and rejects TS's `import X from '…'`, but it also matched the bare first line of a TS multi-line type import: import type { WebhookEndpoint, } from './interfaces/webhook-endpoint.interface'; The opener `import type {` has no `from` on its own line, so the detector pulled it into `existingSimple` and the splice step injected the bare opener into the regenerated file — producing an orphan `import type {` that broke TypeScript parsing. Tighten the regex to exclude lines containing `{` or `}`. Multi-line TS type imports are already covered by the dedicated TS collector below. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/engine/writer.ts | 20 ++++++++--- test/engine/writer-ignore.test.ts | 57 +++++++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/engine/writer.ts b/src/engine/writer.ts index 76371f3..01f2ec7 100644 --- a/src/engine/writer.ts +++ b/src/engine/writer.ts @@ -36,7 +36,13 @@ export async function writeFiles( outputDir: string, options?: WriteOptions, ): Promise { - const result: WriteResult = { written: [], merged: [], skipped: [], identical: [], ignored: [] }; + const result: WriteResult = { + written: [], + merged: [], + skipped: [], + identical: [], + ignored: [], + }; const sorted = [...files].sort((a, b) => a.path.localeCompare(b.path)); const language = options?.language; const header = options?.header ?? ''; @@ -275,7 +281,11 @@ export async function overwriteWithPreservedRegions( // a class matching one in the generated content. const replacedClasses = new Set(); const handledBlockKeys = new Set(); - const replacements: { startLine: number; endLine: number; content: string[] }[] = []; + const replacements: { + startLine: number; + endLine: number; + content: string[]; + }[] = []; for (const [className, classBlocks] of blocksByClass) { // Determine the target class for replacement. When the block has a // containing class, we check if the block redefines that class. When @@ -544,8 +554,10 @@ function collectTsImports(lines: string[]): TsImports { */ function spliceExtraImports(existingLines: string[], resultLines: string[]): void { // Python-style `import hashlib`. Excludes TS forms like `import X from '…'` - // which have a `from` clause and are handled by the TS collector below. - const isPythonSimpleImport = (l: string) => /^import\s+\w/.test(l) && !/\sfrom\s/.test(l); + // (have a `from` clause) and TS multi-line type imports whose first line is + // `import type {` (have a brace) — both are handled by the TS collector + // below. + const isPythonSimpleImport = (l: string) => /^import\s+\w/.test(l) && !/\sfrom\s/.test(l) && !/[{}]/.test(l); // Simple imports (import hashlib, import json, …) const existingSimple = new Set(existingLines.filter(isPythonSimpleImport).map((l) => l.trim())); diff --git a/test/engine/writer-ignore.test.ts b/test/engine/writer-ignore.test.ts index 7b29abb..05269fa 100644 --- a/test/engine/writer-ignore.test.ts +++ b/test/engine/writer-ignore.test.ts @@ -14,7 +14,12 @@ describe('@oagen-ignore-file', () => { await fs.writeFile(filePath, '// @oagen-ignore-file\nexport class Client {}', 'utf-8'); const result = await writeFiles( - [{ path: 'client.ts', content: 'export class Client { newMethod() {} }' }], + [ + { + path: 'client.ts', + content: 'export class Client { newMethod() {} }', + }, + ], tmpDir, { language: 'node', header }, ); @@ -31,7 +36,12 @@ describe('@oagen-ignore-file', () => { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'oagen-test-')); try { const result = await writeFiles( - [{ path: 'client.ts', content: '// @oagen-ignore-file\nexport class Client {}' }], + [ + { + path: 'client.ts', + content: '// @oagen-ignore-file\nexport class Client {}', + }, + ], tmpDir, { language: 'node', header }, ); @@ -51,7 +61,10 @@ describe('@oagen-ignore-file', () => { const result = await writeFiles( [ - { path: 'ignored.ts', content: 'export class Ignored { updated() {} }' }, + { + path: 'ignored.ts', + content: 'export class Ignored { updated() {} }', + }, { path: 'normal.ts', content: 'export class Normal {}' }, ], tmpDir, @@ -607,4 +620,42 @@ describe('@oagen-ignore-start/end with overwriteExisting', () => { await fs.rm(tmpDir, { recursive: true }); } }); + + it('does not splice `import type {` as a Python simple import when existing has multi-line TS type imports', async () => { + // Regression: the Python simple-import detector matched the first line + // of a multi-line TS `import type { … }` block (`import type {`), so the + // bare opener got spliced into the result as if it were a `import json` + // line. Result: a stranded `import type {` orphan before the value + // imports, breaking TypeScript parsing. + const existing = [ + "import type { WorkOS } from '../workos';", + 'import type {', + ' WebhookEndpoint,', + ' WebhookEndpointResponse,', + "} from './interfaces/webhook-endpoint.interface';", + "import { deserializeEvent } from '../common/serializers';", + '', + 'export class Webhooks {', + ' // @oagen-ignore-start', + ' hand() { deserializeEvent({} as any); }', + ' // @oagen-ignore-end', + '}', + ].join('\n'); + + const generated = [ + "import type { WorkOS } from '../workos';", + "import type { WebhookEndpoint, WebhookEndpointResponse } from './interfaces/webhook-endpoint.interface';", + '', + 'export class Webhooks {', + ' list() { return [] as WebhookEndpoint[]; }', + '}', + ].join('\n'); + + const result = await overwriteWithPreservedRegions(existing, generated, 'node'); + + // The bare `import type {` from existing must NOT appear in the result. + expect(result).not.toMatch(/^\s*import type \{\s*$/m); + // The hand-written value import survives, on its own line. + expect(result).toContain("import { deserializeEvent } from '../common/serializers';"); + }); });