From 5acf3cd09ba204bb32a6cb6a71f73ed6b405924e Mon Sep 17 00:00:00 2001 From: creatixchu Date: Mon, 7 Sep 2026 18:00:39 +0800 Subject: [PATCH] fix(typert): try explicit forwarding edges before star edges and reuse the resolution cache Review follow-up. A forwarding module's explicit re-exports are tried before its star re-exports and an entered module is skipped on later edges, so a star edge that loops back no longer hides the explicit re-export beside it. Import resolution outside the checker goes through the face's shared compiler host and module-resolution cache, and an import naming an unregistered package fails the public-export check instead of throwing a TypeError. Tests pin the looping star, the relative-only cycle exit, namespace re-exports, and re-exported namespace imports. --- ...package-local-forwarding-imports.i18n.yaml | 4 +- ...typert-package-local-forwarding-imports.md | 8 +- ...ert-package-local-forwarding-imports.zh.md | 8 +- packages/typert/generator/src/analyzer.ts | 83 +++++++++++-------- .../typert/generator/tests/type-model.spec.ts | 58 +++++++++++++ 5 files changed, 117 insertions(+), 44 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.i18n.yaml index 5af6b050c5..e5cdb40eca 100644 --- a/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.md -2026-09-07-typert-package-local-forwarding-imports.md: bcae7e6c6e3ccca9c1b67d7d925501199dce4854 -2026-09-07-typert-package-local-forwarding-imports.zh.md: 828343af3f7c0b45257248c7e678ef912bbaa10f +2026-09-07-typert-package-local-forwarding-imports.md: 35780d5fe9e6c1f7a2ff82d7157b98f163a11a34 +2026-09-07-typert-package-local-forwarding-imports.zh.md: 52bb0e025d3374289eb15ad97339d2b92a5ab6ef diff --git a/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.md b/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.md index bcae7e6c6e..35780d5fe9 100644 --- a/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.md +++ b/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.md @@ -10,9 +10,9 @@ English | [中文](2026-09-07-typert-package-local-forwarding-imports.zh.md) ## Decision -[`targetForReference`](../../../../packages/typert/generator/src/analyzer.ts) resolves a relative specifier with the program's module resolution and follows it only while the resolved file stays inside the referencing package. In each forwarding module it locates the `export` edge that carries the requested name: a named re-export with a specifier, a star re-export whose module exports the same symbol, or an `export { local }` backed by that module's `import`. The walk stops at the first package specifier and feeds that identity and export name to the existing `packageExportName` check, so a forwarded type must still be public at the package subpath the forwarding module names. The reference model is unchanged: the target remains `declaration` for a same-face owner and `cross-face` for another face. +[`targetForReference`](../../../../packages/typert/generator/src/analyzer.ts) resolves a relative specifier through the face's shared compiler host and module-resolution cache and follows it only while the resolved file stays inside the referencing package. In each forwarding module it collects the `export` edges that carry the requested name: a named re-export with a specifier, an `export { local }` backed by that module's `import`, and star re-exports whose module exports the same symbol. Explicit edges are tried before star edges, matching TypeScript's shadowing of star exports, and a module already entered on the current walk is not entered again, so circular star re-exports terminate. The walk stops at the first package specifier and feeds that identity and export name to the existing `packageExportName` check, so a forwarded type must still be public at the package subpath the forwarding module names, and a package name without a registration is refused there. The reference model is unchanged: the target remains `declaration` for a same-face owner and `cross-face` for another face. -The walk yields no package import, and the reference fails as before, when a relative specifier resolves outside the referencing package, when a forwarding module re-exports a namespace, or when a forwarding chain revisits a file. +The walk yields no package import, and the reference fails as before, when a relative specifier resolves outside the referencing package, when the only edge carrying the name is a namespace re-export or a re-exported namespace import, or when every edge loops back to a module already entered. ## Alternatives considered @@ -20,8 +20,10 @@ The walk yields no package import, and the reference fails as before, when a rel **Record the forwarding module as the reference target.** Rejected: emitters and cross-face links need the original declaration's package and public subpath; a package-local module has no public identity of its own. +**Select edges in source order without symbol checks.** Rejected: a star re-export that loops back to an earlier module can precede the explicit re-export that actually carries the type, and TypeScript itself lets explicit exports shadow star exports; ordering explicit edges first and continuing past an entered module keeps such modules accepted without an unbounded walk. + **Make batched and whole-workspace analysis select the same roots.** Rejected as a fix: root selection does not change the verdict on a reference, only whether the reference is visited, so aligning the callers would hide the incorrect classification rather than remove it. ## Consequences -Packages may keep one forwarding module for foreign types and import it relatively, matching how their own modules are organized. Each cross-package relative reference costs one module resolution per hop, cached by the shared compiler host. [`type-model.spec.ts`](../../../../packages/typert/generator/tests/type-model.spec.ts) pins named, renamed multi-hop, import-then-export, star, and namespace-import forwarding, a forwarded private export, a forwarding module that crosses by relative path, cross-face forwarding, and equality of whole and batched analysis for the forwarding fixture across batch sizes and package orders. +Packages may keep one forwarding module for foreign types and import it relatively, matching how their own modules are organized. Each cross-package relative reference costs one module resolution per hop through the face's shared resolution cache; `reachableFiles` now resolves through the same cache. [`type-model.spec.ts`](../../../../packages/typert/generator/tests/type-model.spec.ts) pins named, renamed multi-hop, import-then-export, star, and namespace-import forwarding, an explicit re-export beside a looping star edge, a forwarded private export, a forwarding module that crosses by relative path, a cycle whose only exit crosses by relative path, a namespace re-export, a re-exported namespace import, cross-face forwarding, and equality of whole and batched analysis for the forwarding fixture across batch sizes and package orders. diff --git a/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.zh.md b/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.zh.md index 828343af3f..52bb0e025d 100644 --- a/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-09-07-typert-package-local-forwarding-imports.zh.md @@ -10,9 +10,9 @@ Status: implemented ## Decision -[`targetForReference`](../../../../packages/typert/generator/src/analyzer.ts) 用程序的模块解析来解析相对说明符，且只在解析到的文件仍位于引用方包内时继续追踪。在每个转发模块里，它找出承载所请求名字的那条 `export` 边：带说明符的具名重新导出、导出同一符号的星号重新导出，或由该模块自身 `import` 支撑的 `export { local }`。追踪在遇到第一个包说明符时停止，并把该包身份和导出名交给现有的 `packageExportName` 检查，因此被转发的类型仍必须在转发模块所写的包子路径上公开。引用模型不变：同 face 的所有者仍是 `declaration`，另一 face 仍是 `cross-face`。 +[`targetForReference`](../../../../packages/typert/generator/src/analyzer.ts) 通过该 face 共享的编译器宿主及其模块解析缓存来解析相对说明符，且只在解析到的文件仍位于引用方包内时继续追踪。在每个转发模块里，它收集承载所请求名字的 `export` 边：带说明符的具名重新导出、由该模块自身 `import` 支撑的 `export { local }`，以及导出同一符号的星号重新导出。显式边先于星号边尝试，与 TypeScript 中显式导出遮蔽星号导出的规则一致；本次追踪已进入过的模块不会再次进入，因此循环的星号重新导出能够终止。追踪在遇到第一个包说明符时停止，并把该包身份和导出名交给现有的 `packageExportName` 检查，因此被转发的类型仍必须在转发模块所写的包子路径上公开，没有登记的包名也在此被拒绝。引用模型不变：同 face 的所有者仍是 `declaration`，另一 face 仍是 `cross-face`。 -当相对说明符解析到引用方包之外、转发模块重新导出命名空间，或转发链重复访问同一文件时，追踪得不到包导入，引用照旧失败。 +当相对说明符解析到引用方包之外、承载该名字的唯一边是命名空间重新导出或被重新导出的命名空间导入，或所有边都回到已进入的模块时，追踪得不到包导入，引用照旧失败。 ## Alternatives considered @@ -20,8 +20,10 @@ Status: implemented **把转发模块记为引用目标。** 已拒绝：发射器和跨 face 链接需要原始声明的包和公开子路径，包内模块没有自己的公开身份。 +**按源码顺序选边且不校验符号。** 已拒绝：回到更早模块的星号重新导出可能排在真正承载该类型的显式重新导出之前，而 TypeScript 本身允许显式导出遮蔽星号导出；显式边优先并跳过已进入的模块，既能接受这类模块，又不会无限追踪。 + **让分批分析与全工作区分析选择相同的根。** 作为修复方案已拒绝：根的选择不改变对一个引用的判定，只决定该引用是否被访问，对齐调用方只会掩盖错误分类，不能消除它。 ## Consequences -包可以为外部类型保留一个转发模块并用相对路径导入它，与自身模块的组织方式一致。每个跨包相对引用每跳付出一次模块解析，由共享编译器宿主缓存。[`type-model.spec.ts`](../../../../packages/typert/generator/tests/type-model.spec.ts) 固定了具名、改名多跳、先导入再导出、星号和命名空间导入这几种转发，被转发的私有导出，用相对路径跨包的转发模块，跨 face 转发，以及转发 fixture 在不同批次大小和包顺序下全量分析与分批分析相等。 +包可以为外部类型保留一个转发模块并用相对路径导入它，与自身模块的组织方式一致。每个跨包相对引用每跳付出一次经该 face 共享解析缓存的模块解析；`reachableFiles` 现在也通过同一缓存解析。[`type-model.spec.ts`](../../../../packages/typert/generator/tests/type-model.spec.ts) 固定了具名、改名多跳、先导入再导出、星号和命名空间导入这几种转发，与回环星号边并存的显式重新导出，被转发的私有导出，用相对路径跨包的转发模块，唯一出口用相对路径跨包的循环，命名空间重新导出，被重新导出的命名空间导入，跨 face 转发，以及转发 fixture 在不同批次大小和包顺序下全量分析与分批分析相等。 diff --git a/packages/typert/generator/src/analyzer.ts b/packages/typert/generator/src/analyzer.ts index 406d7d929d..d00fcae534 100644 --- a/packages/typert/generator/src/analyzer.ts +++ b/packages/typert/generator/src/analyzer.ts @@ -336,15 +336,13 @@ export class WorkspaceAnalyzer { incremental: false, noEmit: true, } - const program = ts.createProgram({ - rootNames, - options, - host: this.caches.programHost(face, options), - }) + const host = this.caches.programHost(face, options) + const program = ts.createProgram({ rootNames, options, host }) faces.push(new FaceAnalyzer({ root: this.options.root, face, program, + host, registrations, allRegistrations: this.registrations, mode: this.options.mode, @@ -607,6 +605,8 @@ interface FaceAnalyzerOptions { readonly root: string readonly face: TypertFace readonly program: ts.Program + /** Program host whose module-resolution cache serves import resolution outside the checker. */ + readonly host: ts.CompilerHost readonly registrations: readonly PackageRegistration[] readonly allRegistrations: readonly PackageRegistration[] readonly mode: AnalysisMode @@ -618,6 +618,7 @@ class FaceAnalyzer { private readonly root: string private readonly face: TypertFace private readonly program: ts.Program + private readonly host: ts.CompilerHost private readonly checker: ts.TypeChecker private readonly registrations: readonly PackageRegistration[] private readonly allRegistrations: readonly PackageRegistration[] @@ -637,6 +638,7 @@ class FaceAnalyzer { this.root = options.root this.face = options.face this.program = options.program + this.host = options.host this.checker = options.program.getTypeChecker() this.registrations = options.registrations this.allRegistrations = options.allRegistrations @@ -862,15 +864,23 @@ class FaceAnalyzer { } private resolveImport(specifier: string, fromFile: string): string | undefined { - const resolved = ts.resolveModuleName(specifier, fromFile, this.program.getCompilerOptions(), ts.sys).resolvedModule + const resolved = ts.resolveModuleName( + specifier, + fromFile, + this.program.getCompilerOptions(), + this.host, + this.host.getModuleResolutionCache?.(), + ).resolvedModule return resolved === undefined ? undefined : realPath(resolved.resolvedFileName) } /** * Follow the import that names `symbol` at `site` through modules of the - * referencing package until a package specifier appears. A relative - * specifier that resolves outside `from`, a forwarding module that does not - * re-export the name from a module, or a namespace hop yields undefined. + * referencing package until a package specifier appears. Each forwarding + * module is entered once; its explicit export edges are tried before its + * star edges. A relative specifier that resolves outside `from`, a namespace + * hop, or a module with no edge leading to a package specifier yields + * undefined. */ private packageImportOf( site: ReferenceSite, @@ -878,48 +888,48 @@ class FaceAnalyzer { symbol: ts.Symbol, from: PackageRegistration, ): PackageImport | undefined { - let sourceFile = site.getSourceFile() - let specifier = moduleSpecifier - let name = authoredExportName(site, moduleSpecifier) const visited = new Set() - for (;;) { + const walk = (sourceFile: ts.SourceFile, specifier: string, name: string): PackageImport | undefined => { const module = moduleIdentity(specifier) if (module !== undefined) return { module, name } const resolvedPath = this.resolveImport(specifier, sourceFile.fileName) if (resolvedPath === undefined || !isWithin(resolvedPath, from.root) || visited.has(resolvedPath)) return undefined visited.add(resolvedPath) - sourceFile = this.sourceFiles.get(resolvedPath) as ts.SourceFile - const forwarded = this.forwardedExport(sourceFile, name, symbol) - if (forwarded === undefined) return undefined - specifier = forwarded.specifier - name = forwarded.name + const forward = this.sourceFiles.get(resolvedPath) as ts.SourceFile + for (const edge of this.forwardedExports(forward, name, symbol)) { + const found = walk(forward, edge.specifier, edge.name) + if (found !== undefined) return found + } + return undefined } + return walk(site.getSourceFile(), moduleSpecifier, authoredExportName(site, moduleSpecifier)) } - private forwardedExport(sourceFile: ts.SourceFile, name: string, symbol: ts.Symbol): ForwardedExport | undefined { + /** Export edges of `sourceFile` that carry `name`: explicit edges in source order, then star edges exporting `symbol`. */ + private forwardedExports(sourceFile: ts.SourceFile, name: string, symbol: ts.Symbol): ForwardedExport[] { + const explicit: ForwardedExport[] = [] + const stars: ForwardedExport[] = [] for (const statement of sourceFile.statements) { - if (!ts.isExportDeclaration(statement)) continue - if (statement.moduleSpecifier !== undefined && ts.isStringLiteral(statement.moduleSpecifier)) { - const specifier = statement.moduleSpecifier.text - if (statement.exportClause === undefined) { - const exported = this.moduleExports(statement.moduleSpecifier) - .find(candidate => candidate.name === name && this.resolveSymbol(candidate) === symbol) - if (exported !== undefined) return { specifier, name } - continue - } - if (ts.isNamespaceExport(statement.exportClause)) continue - const element = statement.exportClause.elements.find(candidate => candidate.name.text === name) - if (element !== undefined) return { specifier, name: element.propertyName?.text ?? name } + if (!ts.isExportDeclaration(statement) + || (statement.exportClause === undefined && statement.moduleSpecifier === undefined) + || (statement.exportClause !== undefined && ts.isNamespaceExport(statement.exportClause))) continue + if (statement.exportClause === undefined) { + const specifier = (statement.moduleSpecifier as ts.StringLiteral).text + const exported = this.moduleExports(statement.moduleSpecifier as ts.StringLiteral) + .find(candidate => candidate.name === name && this.resolveSymbol(candidate) === symbol) + if (exported !== undefined) stars.push({ specifier, name }) continue } - if (statement.exportClause === undefined || ts.isNamespaceExport(statement.exportClause)) continue const element = statement.exportClause.elements.find(candidate => candidate.name.text === name) if (element === undefined) continue + if (statement.moduleSpecifier !== undefined) { + explicit.push({ specifier: (statement.moduleSpecifier as ts.StringLiteral).text, name: element.propertyName?.text ?? name }) + continue + } const binding = importBindingOf(sourceFile, element.propertyName?.text ?? name) - if (binding?.name === undefined) return undefined - return { specifier: binding.specifier, name: binding.name } + if (binding?.name !== undefined) explicit.push({ specifier: binding.specifier, name: binding.name }) } - return undefined + return [...explicit, ...stars] } private collectServices( @@ -2563,7 +2573,8 @@ class FaceAnalyzer { requestedName: string, ): string | undefined { const registration = this.allRegistrations.find(candidate => - candidate.face === face && candidate.name === module.package) as PackageRegistration + candidate.face === face && candidate.name === module.package) + if (registration === undefined) return undefined const target = packageExportTargets(registration.manifest) .find(([subpath]) => subpath === module.subpath)?.[1] if (target === undefined) return undefined diff --git a/packages/typert/generator/tests/type-model.spec.ts b/packages/typert/generator/tests/type-model.spec.ts index f2f33ed917..5fb3233719 100644 --- a/packages/typert/generator/tests/type-model.spec.ts +++ b/packages/typert/generator/tests/type-model.spec.ts @@ -754,6 +754,64 @@ describe('WorkspaceAnalyzer', { timeout: 60_000 }, () => { }) }) + it('prefers an explicit re-export over a star edge that loops back', () => { + const root = copyFixture('typert-forward-cycle-explicit-') + addSameFacePackage(root, './outer.ts', 'Payload', { + 'outer.ts': "export * from './inner.ts'\n", + 'inner.ts': "export * from './outer.ts'\nexport type { Payload } from '@fixture/host/models'\n", + }) + + expect(consumerPayloadTarget(root)).toEqual({ kind: 'declaration', symbol: hostPayload }) + }) + + it('rejects a forwarding cycle whose only exit crosses a package by relative path', () => { + const root = copyFixture('typert-forward-cycle-relative-') + addSameFacePackage(root, './outer.ts', 'Payload', { + 'outer.ts': "export * from './inner.ts'\n", + 'inner.ts': "export * from './outer.ts'\nexport type { Payload } from '../../host/src/models.ts'\n", + }) + + expect(() => new WorkspaceAnalyzer({ root }).analyze()).toThrow( + 'reference to Payload crosses a package without an explicit package import', + ) + }) + + it('rejects a namespace re-export in a forwarding module', () => { + const root = copyFixture('typert-forward-namespace-export-') + addSameFacePackage(root, '@fixture/host/models', 'Payload', { + 'forward.ts': "export * as models from '@fixture/host/models'\n", + }) + const sourcePath = join(root, 'packages/consumer/src/index.ts') + writeFileSync( + sourcePath, + readFileSync(sourcePath, 'utf8') + .replace("import type { Payload } from '@fixture/host/models'", "import type * as Forward from './forward.ts'") + .replace('readonly value: Payload', 'readonly value: Forward.models.Payload'), + ) + + expect(() => new WorkspaceAnalyzer({ root }).analyze()).toThrow( + 'reference to Payload crosses a package without an explicit package import', + ) + }) + + it('rejects a forwarding module that exports a namespace import binding', () => { + const root = copyFixture('typert-forward-namespace-binding-') + addSameFacePackage(root, '@fixture/host/models', 'Payload', { + 'forward.ts': "import type * as Models from '@fixture/host/models'\nexport type { Models }\n", + }) + const sourcePath = join(root, 'packages/consumer/src/index.ts') + writeFileSync( + sourcePath, + readFileSync(sourcePath, 'utf8') + .replace("import type { Payload } from '@fixture/host/models'", "import type { Models } from './forward.ts'") + .replace('readonly value: Payload', 'readonly value: Models.Payload'), + ) + + expect(() => new WorkspaceAnalyzer({ root }).analyze()).toThrow( + 'reference to Payload crosses a package without an explicit package import', + ) + }) + it('produces one model regardless of batch size or package order', () => { const root = copyFixture('typert-forward-batches-') addSameFacePackage(root, './forward.ts', 'Payload', {