From c436d93c55519ec5a07317565c7efa8b902e83c2 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Mon, 18 May 2026 19:24:42 -0400 Subject: [PATCH 1/2] fix(writer): splice hand-written TypeScript imports during overwrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `spliceExtraImports` understood Python `import X` and `from X import Y` but not TypeScript `import { X } from 'module'`, `import type { X } from 'module'`, `import X from 'module'`, `import * as X from 'module'`, or `import 'module'`. When a TypeScript file with hand-written `@oagen-ignore-start/end` regions was overwritten via `overwriteWithPreservedRegions`, the hand-written imports those regions depended on were dropped — landing in the output only because top-level ignore blocks get appended to the end of the file by the placement logic (which compiles but reads as garbage and trips eslint `import/first`). Add `collectTsImports` covering all five TS import forms (including multi-line `{ … }` named imports) and wire it into `spliceExtraImports` alongside the existing Python collectors. Also widen the `lastImportIdx` loop to recognize TS import lines so the spliced imports land in the import block rather than wherever the loop happened to stop. The existing Python-style detector is unchanged in semantics — its `isSimpleImport` regex is tightened to exclude lines containing `from` so TS default imports (`import X from 'module'`) flow through the new TS handler instead of being added verbatim. Verified end-to-end against a hand-written `Webhooks` class with hand-written named imports referenced from inside an `@oagen-ignore-start/end` block: imports now land at the top of the file alongside the generated imports. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/engine/writer.ts | 155 +++++++++++++++++++++++++++--- test/engine/writer-ignore.test.ts | 56 +++++++++++ 2 files changed, 196 insertions(+), 15 deletions(-) diff --git a/src/engine/writer.ts b/src/engine/writer.ts index a7b6684..83b6bba 100644 --- a/src/engine/writer.ts +++ b/src/engine/writer.ts @@ -440,17 +440,111 @@ function collectFromImports(lines: string[]): Map> { return byModule; } +/** + * Collected TypeScript imports grouped by form. Each map keys on the module + * specifier (e.g. `./foo`, `node:fs`). + */ +interface TsImports { + /** `import { X, Y } from 'module'` — module → identifier names. */ + named: Map>; + /** `import type { X } from 'module'` — module → identifier names. */ + typeOnly: Map>; + /** `import X from 'module'` or `import type X from 'module'` — module → name. */ + defaults: Map; + /** `import * as X from 'module'` — module → namespace alias name. */ + namespaces: Map; + /** `import 'module'` — modules imported for side effects only. */ + sideEffects: Set; +} + +/** + * Collect TypeScript imports, grouped by form. Handles single-line and + * multi-line `{ … }` named imports. Disjoint from Python's `from X import Y` + * syntax so this can be called in parallel with `collectFromImports`. + */ +function collectTsImports(lines: string[]): TsImports { + const out: TsImports = { + named: new Map(), + typeOnly: new Map(), + defaults: new Map(), + namespaces: new Map(), + sideEffects: new Set(), + }; + + for (let i = 0; i < lines.length; i++) { + const trimmed = lines[i].trim(); + if (!/^import(\s|$)/.test(trimmed)) continue; + + // Accumulate multi-line `{ … }` imports. + let fullText = lines[i]; + if (fullText.includes('{') && !fullText.includes('}')) { + while (i + 1 < lines.length) { + i++; + fullText += ' ' + lines[i].trim(); + if (lines[i].includes('}')) break; + } + } + + const normalized = fullText.replace(/\s+/g, ' ').trim().replace(/;\s*$/, ''); + + // `import 'module'` + let m = normalized.match(/^import\s+['"]([^'"]+)['"]$/); + if (m) { + out.sideEffects.add(m[1]); + continue; + } + + // `import * as X from 'module'` + m = normalized.match(/^import\s+\*\s+as\s+(\w+)\s+from\s+['"]([^'"]+)['"]$/); + if (m) { + out.namespaces.set(m[2], m[1]); + continue; + } + + // `import [Default,] { X, Y as Z } from 'module'` or `import type { X } from 'module'` + m = normalized.match(/^import\s+(type\s+)?(?:(\w+)\s*,\s*)?\{\s*([^}]*)\s*\}\s+from\s+['"]([^'"]+)['"]$/); + if (m) { + const isTypeOnly = !!m[1]; + const defaultName = m[2]; + const namesRaw = m[3]; + const mod = m[4]; + if (defaultName) out.defaults.set(mod, defaultName); + const names = namesRaw + .split(',') + .map((s) => s.trim().replace(/\s+as\s+\w+$/, '')) + .filter(Boolean); + const target = isTypeOnly ? out.typeOnly : out.named; + if (!target.has(mod)) target.set(mod, new Set()); + for (const n of names) target.get(mod)!.add(n); + continue; + } + + // `import X from 'module'` or `import type X from 'module'` + m = normalized.match(/^import\s+(?:type\s+)?(\w+)\s+from\s+['"]([^'"]+)['"]$/); + if (m) { + out.defaults.set(m[2], m[1]); + continue; + } + } + + return out; +} + /** * Add imports from the existing file that are absent from the generated - * result. Handles both `import X` and `from X import Y` forms. - * Mutates `resultLines` in place. + * result. Handles Python `import X` and `from X import Y` forms, plus + * TypeScript `import { X } from 'module'`, `import type { X } from 'module'`, + * `import X from 'module'`, `import * as X from 'module'`, and side-effect + * `import 'module'` forms. Mutates `resultLines` in place. */ function spliceExtraImports(existingLines: string[], resultLines: string[]): void { - const isSimpleImport = (l: string) => /^import\s+\w/.test(l); + // 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); // Simple imports (import hashlib, import json, …) - const existingSimple = new Set(existingLines.filter(isSimpleImport).map((l) => l.trim())); - const generatedSimple = new Set(resultLines.filter(isSimpleImport).map((l) => l.trim())); + const existingSimple = new Set(existingLines.filter(isPythonSimpleImport).map((l) => l.trim())); + const generatedSimple = new Set(resultLines.filter(isPythonSimpleImport).map((l) => l.trim())); const extraSimple = [...existingSimple].filter((l) => !generatedSimple.has(l)); // From-imports — compare by module and find extra identifiers @@ -483,20 +577,51 @@ function spliceExtraImports(existingLines: string[], resultLines: string[]): voi } } - const allExtra = [...extraSimple, ...extraFromLines]; + // TypeScript imports. + const existingTs = collectTsImports(existingLines); + const generatedTs = collectTsImports(resultLines); + const extraTsLines: string[] = []; + + for (const mod of existingTs.sideEffects) { + if (!generatedTs.sideEffects.has(mod)) extraTsLines.push(`import '${mod}';`); + } + for (const [mod, name] of existingTs.defaults) { + if (generatedTs.defaults.has(mod)) continue; + extraTsLines.push(`import ${name} from '${mod}';`); + } + for (const [mod, name] of existingTs.namespaces) { + if (!generatedTs.namespaces.has(mod)) extraTsLines.push(`import * as ${name} from '${mod}';`); + } + for (const [mod, existIds] of existingTs.named) { + const genIds = generatedTs.named.get(mod) ?? new Set(); + const genTypeIds = generatedTs.typeOnly.get(mod) ?? new Set(); + const extra = [...existIds].filter((id) => !genIds.has(id) && !genTypeIds.has(id)); + if (extra.length > 0) extraTsLines.push(`import { ${extra.join(', ')} } from '${mod}';`); + } + for (const [mod, existIds] of existingTs.typeOnly) { + const genIds = generatedTs.typeOnly.get(mod) ?? new Set(); + const genNamedIds = generatedTs.named.get(mod) ?? new Set(); + const extra = [...existIds].filter((id) => !genIds.has(id) && !genNamedIds.has(id)); + if (extra.length > 0) extraTsLines.push(`import type { ${extra.join(', ')} } from '${mod}';`); + } + + const allExtra = [...extraSimple, ...extraFromLines, ...extraTsLines]; if (allExtra.length === 0) return; - // Find last import line in resultLines (accounting for multiline imports) + // Find last import line in resultLines (accounting for multiline imports). + // Recognizes Python (`from X import Y`, `import X`) and TS (`import …`). + const isImportStart = (l: string) => /^\s*import(\s|$)/.test(l) || /^from\s+/.test(l); let lastImportIdx = -1; for (let i = 0; i < resultLines.length; i++) { - if (isSimpleImport(resultLines[i]) || /^from\s+/.test(resultLines[i])) { - lastImportIdx = i; - if (resultLines[i].includes('(') && !resultLines[i].includes(')')) { - while (i + 1 < resultLines.length) { - i++; - lastImportIdx = i; - if (resultLines[i].includes(')')) break; - } + if (!isImportStart(resultLines[i])) continue; + lastImportIdx = i; + const opens = (resultLines[i].match(/[({]/g) ?? []).length; + const closes = (resultLines[i].match(/[)}]/g) ?? []).length; + if (opens > closes) { + while (i + 1 < resultLines.length) { + i++; + lastImportIdx = i; + if ((resultLines[i].match(/[)}]/g) ?? []).length > 0) break; } } } diff --git a/test/engine/writer-ignore.test.ts b/test/engine/writer-ignore.test.ts index 1f335dc..2245c2f 100644 --- a/test/engine/writer-ignore.test.ts +++ b/test/engine/writer-ignore.test.ts @@ -172,6 +172,62 @@ describe('@oagen-ignore-start/end with overwriteExisting', () => { expect(ignoreStart).toBeLessThan(asyncClass); }); + it('preserves hand-written TypeScript imports during overwrite', async () => { + // When a hand-owned TS file is partially regenerated, named/type-only + // imports referenced inside @oagen-ignore-start/end regions must be + // brought along at the top of the file, not appended at the end. + const existing = [ + 'import { deserializeEvent } from "../common/serializers";', + 'import { Event, EventResponse } from "../common/interfaces";', + 'import { SignatureProvider } from "../common/crypto/signature-provider";', + 'import type { WorkOS } from "../workos";', + '', + 'export class Webhooks {', + ' constructor(private readonly workos: WorkOS) {}', + '', + ' // @oagen-ignore-start', + ' private _sp?: SignatureProvider;', + ' async constructEvent(): Promise {', + ' const payload: EventResponse = {} as EventResponse;', + ' return deserializeEvent(payload);', + ' }', + ' // @oagen-ignore-end', + '}', + ].join('\n'); + + const generated = [ + 'import type { WorkOS } from "../workos";', + 'import type { Foo } from "./interfaces/foo.interface";', + '', + 'export class Webhooks {', + ' constructor(private readonly workos: WorkOS) {}', + '', + ' async listFoo(): Promise { return {} as Foo; }', + '}', + ].join('\n'); + + const result = await overwriteWithPreservedRegions(existing, generated, 'node'); + const lines = result.split('\n'); + + // Hand-written imports survive. + expect(result).toContain("import { deserializeEvent } from '../common/serializers';"); + expect(result).toContain("import { Event, EventResponse } from '../common/interfaces';"); + expect(result).toContain("import { SignatureProvider } from '../common/crypto/signature-provider';"); + + // …and they land before the class declaration, not after it. + let lastImportLine = -1; + for (let i = 0; i < lines.length; i++) { + if (/^import\b/.test(lines[i].trim())) lastImportLine = i; + } + const classLine = lines.findIndex((l: string) => /^export class\b/.test(l.trim())); + expect(lastImportLine).toBeGreaterThan(-1); + expect(classLine).toBeGreaterThan(lastImportLine); + + // Generated method and ignore-region body both survive. + expect(result).toContain('async listFoo()'); + expect(result).toContain('constructEvent()'); + }); + it('preserves multiple ignore regions across different classes', async () => { const existing = [ 'class SSO:', From debf0209860b302d8736da5893614d12b90318ea Mon Sep 17 00:00:00 2001 From: Garen Torikian Date: Mon, 18 May 2026 19:58:47 -0400 Subject: [PATCH 2/2] fix(writer): splice hand-written PHP and Ruby imports during overwrite (#81) Co-authored-by: Claude Opus 4.7 (1M context) --- src/engine/writer.ts | 38 +++++++++-- test/engine/writer-ignore.test.ts | 105 ++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 7 deletions(-) diff --git a/src/engine/writer.ts b/src/engine/writer.ts index 83b6bba..76371f3 100644 --- a/src/engine/writer.ts +++ b/src/engine/writer.ts @@ -532,10 +532,15 @@ function collectTsImports(lines: string[]): TsImports { /** * Add imports from the existing file that are absent from the generated - * result. Handles Python `import X` and `from X import Y` forms, plus - * TypeScript `import { X } from 'module'`, `import type { X } from 'module'`, - * `import X from 'module'`, `import * as X from 'module'`, and side-effect - * `import 'module'` forms. Mutates `resultLines` in place. + * result. Handles: + * + * - Python: `import X` and `from X import Y` + * - TypeScript: `import { X } from 'module'`, `import type { X } from 'module'`, + * `import X from 'module'`, `import * as X from 'module'`, `import 'module'` + * - PHP: `use Foo\Bar;` and `use Foo\Bar as Baz;` (namespace-qualified only) + * - Ruby: `require '…'` and `require_relative '…'` + * + * Mutates `resultLines` in place. */ function spliceExtraImports(existingLines: string[], resultLines: string[]): void { // Python-style `import hashlib`. Excludes TS forms like `import X from '…'` @@ -605,12 +610,31 @@ function spliceExtraImports(existingLines: string[], resultLines: string[]): voi if (extra.length > 0) extraTsLines.push(`import type { ${extra.join(', ')} } from '${mod}';`); } - const allExtra = [...extraSimple, ...extraFromLines, ...extraTsLines]; + // PHP `use Foo\Bar;` and `use Foo\Bar as Baz;` — namespace-qualified only + // (a backslash anywhere in the path) so this doesn't mistakenly hoist + // single-segment in-class trait references like `use TraitName;`. + const isPhpUseImport = (l: string) => /^use\s+[\w\\]+\\[\w\\]*(\s+as\s+\w+)?\s*;?$/.test(l); + const existingPhpUse = new Set(existingLines.filter(isPhpUseImport).map((l) => l.trim())); + const generatedPhpUse = new Set(resultLines.filter(isPhpUseImport).map((l) => l.trim())); + const extraPhpUse = [...existingPhpUse].filter((l) => !generatedPhpUse.has(l)); + + // Ruby `require '...'` and `require_relative '...'`. + const isRubyRequire = (l: string) => /^require(_relative)?\s+['"][^'"]+['"]\s*$/.test(l); + const existingRubyRequire = new Set(existingLines.filter(isRubyRequire).map((l) => l.trim())); + const generatedRubyRequire = new Set(resultLines.filter(isRubyRequire).map((l) => l.trim())); + const extraRubyRequire = [...existingRubyRequire].filter((l) => !generatedRubyRequire.has(l)); + + const allExtra = [...extraSimple, ...extraFromLines, ...extraTsLines, ...extraPhpUse, ...extraRubyRequire]; if (allExtra.length === 0) return; // Find last import line in resultLines (accounting for multiline imports). - // Recognizes Python (`from X import Y`, `import X`) and TS (`import …`). - const isImportStart = (l: string) => /^\s*import(\s|$)/.test(l) || /^from\s+/.test(l); + // Recognizes Python (`from X import Y`, `import X`), TS (`import …`), PHP + // (`use …`), and Ruby (`require`, `require_relative`). + const isImportStart = (l: string) => + /^\s*import(\s|$)/.test(l) || + /^from\s+/.test(l) || + /^\s*use\s+[\w\\]/.test(l) || + /^\s*require(_relative)?\s+['"]/.test(l); let lastImportIdx = -1; for (let i = 0; i < resultLines.length; i++) { if (!isImportStart(resultLines[i])) continue; diff --git a/test/engine/writer-ignore.test.ts b/test/engine/writer-ignore.test.ts index 2245c2f..7b29abb 100644 --- a/test/engine/writer-ignore.test.ts +++ b/test/engine/writer-ignore.test.ts @@ -228,6 +228,111 @@ describe('@oagen-ignore-start/end with overwriteExisting', () => { expect(result).toContain('constructEvent()'); }); + it('preserves hand-written PHP `use` imports during overwrite', async () => { + // PHP `use Foo\Bar;` and `use Foo\Bar as Baz;` referenced from inside an + // @oagen-ignore-start/end block must land alongside the other top-level + // use statements, not be dropped or appended to the end of the file. + const existing = [ + ' /^class\b/.test(l.trim())); + expect(lastUseLine).toBeGreaterThan(-1); + expect(classLine).toBeGreaterThan(lastUseLine); + + // Generated method/property and ignore-region body both survive. + expect(result).toContain('private Foo $foo;'); + expect(result).toContain('private ?Passwordless $passwordless'); + }); + + it('preserves hand-written Ruby `require`/`require_relative` during overwrite', async () => { + const existing = [ + "require 'openssl'", + "require 'workos/version'", + "require_relative 'helpers/crypto'", + '', + 'module WorkOS', + ' class Webhooks', + ' # @oagen-ignore-start', + ' def verify_event(sig:)', + ' OpenSSL::HMAC.hexdigest("SHA256", "k", sig)', + ' end', + ' # @oagen-ignore-end', + ' end', + 'end', + ].join('\n'); + + const generated = [ + "require 'workos/version'", + "require 'workos/http_client'", + '', + 'module WorkOS', + ' class Webhooks', + ' def list_endpoints; end', + ' end', + 'end', + ].join('\n'); + + const result = await overwriteWithPreservedRegions(existing, generated, 'ruby'); + const lines = result.split('\n'); + + // Hand-written requires survive. + expect(result).toContain("require 'openssl'"); + expect(result).toContain("require_relative 'helpers/crypto'"); + + // …and they land before the module declaration. + let lastRequireLine = -1; + for (let i = 0; i < lines.length; i++) { + if (/^require(_relative)?\b/.test(lines[i].trim())) lastRequireLine = i; + } + const moduleLine = lines.findIndex((l: string) => /^module\b/.test(l.trim())); + expect(lastRequireLine).toBeGreaterThan(-1); + expect(moduleLine).toBeGreaterThan(lastRequireLine); + + // Generated method and ignore-region body both survive. + expect(result).toContain('def list_endpoints'); + expect(result).toContain('def verify_event'); + }); + it('preserves multiple ignore regions across different classes', async () => { const existing = [ 'class SSO:',