diff --git a/packages/compiler/src/frontend/lowering/coercions/structural.ts b/packages/compiler/src/frontend/lowering/coercions/structural.ts index 76c6c9c3b..863472910 100644 --- a/packages/compiler/src/frontend/lowering/coercions/structural.ts +++ b/packages/compiler/src/frontend/lowering/coercions/structural.ts @@ -398,10 +398,12 @@ export function recordWidthHelper( // Package code converting into a package-declared record type (an // object literal returned under `@returns {HelpContext}`): its field // extractions name the declared fields (decl-origin.ts), so the helper - // is keyed by that type too. Every other projection is shared as before. + // is keyed by that type's declaration too (two packages may declare + // records of the same name and shape). Every other projection is shared + // as before. const declared = lowerer.coercionRecordOrigin(); const key = - declared === null ? `rec:${fromId}:${toId}` : `rec:${fromId}:${toId}:origin:${declared.name}`; + declared === null ? `rec:${fromId}:${toId}` : `rec:${fromId}:${toId}:origin:${declared.key}`; const existing = lowerer.valueHelpers.get(key); if (existing) return existing; const name = keyedHelperName(lowerer, "%rec.width", key); diff --git a/packages/compiler/src/frontend/lowering/decl-origin.ts b/packages/compiler/src/frontend/lowering/decl-origin.ts index 60510ecec..72a135b67 100644 --- a/packages/compiler/src/frontend/lowering/decl-origin.ts +++ b/packages/compiler/src/frontend/lowering/decl-origin.ts @@ -40,8 +40,15 @@ interface PackageFile { rel: string; } +/** Installed package versions by package root, read once per program load + * (resetDeclOriginVersions): a dependency reinstalled at another version + * between two builds of one process is read again. */ const versions = new Map(); +export function resetDeclOriginVersions(): void { + versions.clear(); +} + function packageFileOf(fileName: string): PackageFile | null { const path = fileName.split("\\").join("/"); const marker = "/node_modules/"; @@ -268,6 +275,9 @@ export function slotOrigin(checker: ts.TypeChecker, node: ts.Node): string | nul export interface RecordOrigin { type: ts.Type; name: string; + /** The declaration's identity (package, version, file, line and name): + * records of the same shape and name declared by two packages differ. */ + key: string; } export function recordOrigin(checker: ts.TypeChecker, node: ts.Node): RecordOrigin | null { @@ -277,13 +287,11 @@ export function recordOrigin(checker: ts.TypeChecker, node: ts.Node): RecordOrig const name = symbol === undefined ? "" : ts.symbolName(symbol); if (!/^[A-Za-z_$][\w$]*$/.test(name)) return null; const decl = symbol?.declarations[0]; - if ( - decl === undefined || - genericDeclaration(decl) || - packageFileOf(ts.getSourceFileOfNode(decl).fileName) === null - ) - return null; - return { type: contextual, name }; + if (decl === undefined || genericDeclaration(decl)) return null; + const file = packageFileOf(ts.getSourceFileOfNode(decl).fileName); + if (file === null) return null; + const key = [file.pkg, file.version, file.rel, lineOf(decl), name].join(ORIGIN_SEPARATOR); + return { type: contextual, name, key }; } /** The origin of one field of a package-declared record type. */ diff --git a/packages/compiler/src/frontend/program.ts b/packages/compiler/src/frontend/program.ts index 44e048916..44979ca68 100644 --- a/packages/compiler/src/frontend/program.ts +++ b/packages/compiler/src/frontend/program.ts @@ -145,6 +145,7 @@ import { type DeclProjection, } from "./decl-projection.js"; import { configureTypePatches, packageSourceText } from "./type-patches/layer.js"; +import { resetDeclOriginVersions } from "./lowering/decl-origin.js"; import { reportShapes } from "./shape-inference/index.js"; import { inferredJsDiagnosticSuppressed } from "./inferred-js-diagnostics.js"; import { @@ -1082,6 +1083,7 @@ export function loadProgram( // (chalk 6) compile from their own source. configurePackageShims(entryPath); resetDeclBoundaryOrigins(); + resetDeclOriginVersions(); // The project's type patches (type-patches/layer.ts), read by every host // this load creates; nothing at all without `scriptc.typePatches`. configureTypePatches(entryPath, (path, text, kind) => services.parse(path, text, kind)); diff --git a/tests/fixtures/type-patches/node_modules/gridctx/index.js b/tests/fixtures/type-patches/node_modules/gridctx/index.js new file mode 100644 index 000000000..44c282ad7 --- /dev/null +++ b/tests/fixtures/type-patches/node_modules/gridctx/index.js @@ -0,0 +1,43 @@ +"use strict"; +// termctx's Terminal under another name: panectx and gridctx declare the +// same OutputContext (one name, one shape) in two packages. + +class Terminal { + constructor(tty) { + this.tty = tty; + this.settings = { + columns: () => (this.tty ? 80 : undefined), + }; + } + + /** + * @returns {number} + */ + columns() { + return this.settings.columns(); + } + + /** + * @typedef OutputContext + * @type {object} + * @property {boolean} error + * @property {number} width + * + * @param {boolean} error + * @returns {OutputContext} + */ + outputContext(error) { + const width = this.settings.columns(); + return { error, width }; + } + + /** + * @param {OutputContext} context + * @returns {string} + */ + describe(context) { + return `width ${context.width}${context.error ? " (stderr)" : ""}`; + } +} + +module.exports = { Terminal }; diff --git a/tests/fixtures/type-patches/node_modules/gridctx/package.json b/tests/fixtures/type-patches/node_modules/gridctx/package.json new file mode 100644 index 000000000..36f6aba11 --- /dev/null +++ b/tests/fixtures/type-patches/node_modules/gridctx/package.json @@ -0,0 +1 @@ +{ "name": "gridctx", "version": "4.1.0", "main": "index.js" } diff --git a/tests/fixtures/type-patches/node_modules/panectx/index.js b/tests/fixtures/type-patches/node_modules/panectx/index.js new file mode 100644 index 000000000..44c282ad7 --- /dev/null +++ b/tests/fixtures/type-patches/node_modules/panectx/index.js @@ -0,0 +1,43 @@ +"use strict"; +// termctx's Terminal under another name: panectx and gridctx declare the +// same OutputContext (one name, one shape) in two packages. + +class Terminal { + constructor(tty) { + this.tty = tty; + this.settings = { + columns: () => (this.tty ? 80 : undefined), + }; + } + + /** + * @returns {number} + */ + columns() { + return this.settings.columns(); + } + + /** + * @typedef OutputContext + * @type {object} + * @property {boolean} error + * @property {number} width + * + * @param {boolean} error + * @returns {OutputContext} + */ + outputContext(error) { + const width = this.settings.columns(); + return { error, width }; + } + + /** + * @param {OutputContext} context + * @returns {string} + */ + describe(context) { + return `width ${context.width}${context.error ? " (stderr)" : ""}`; + } +} + +module.exports = { Terminal }; diff --git a/tests/fixtures/type-patches/node_modules/panectx/package.json b/tests/fixtures/type-patches/node_modules/panectx/package.json new file mode 100644 index 000000000..e73708257 --- /dev/null +++ b/tests/fixtures/type-patches/node_modules/panectx/package.json @@ -0,0 +1 @@ +{ "name": "panectx", "version": "2.0.0", "main": "index.js" } diff --git a/tests/fixtures/type-patches/same-name/main.ts b/tests/fixtures/type-patches/same-name/main.ts new file mode 100644 index 000000000..3cbfffc57 --- /dev/null +++ b/tests/fixtures/type-patches/same-name/main.ts @@ -0,0 +1,15 @@ +// panectx and gridctx each declare an OutputContext of the same shape: +// each failing check's TypeError names its own package's declaration +// (native only; Node never checks). +import { Terminal as PaneTerminal } from "panectx"; +import { Terminal as GridTerminal } from "gridctx"; + +const pane = new PaneTerminal(false); +const grid = new GridTerminal(false); +for (const read of [() => pane.outputContext(false).width, () => grid.outputContext(false).width]) { + try { + console.log(read()); + } catch (e) { + console.log((e as Error).message); + } +} diff --git a/tests/fixtures/type-patches/same-name/package.json b/tests/fixtures/type-patches/same-name/package.json new file mode 100644 index 000000000..fdf221cba --- /dev/null +++ b/tests/fixtures/type-patches/same-name/package.json @@ -0,0 +1 @@ +{ "name": "type-patches-same-name", "private": true, "type": "module" } diff --git a/tests/harness/type-patches.test.ts b/tests/harness/type-patches.test.ts index cfd8b4dd6..7afb4433d 100644 --- a/tests/harness/type-patches.test.ts +++ b/tests/harness/type-patches.test.ts @@ -9,7 +9,8 @@ * type-patch-types-twin, type-patch-files, type-patch-types-entry); here * their unpatched copies show the patches are what makes them compile, and * a failing check names the declaration and the patch (scr_type_origin.c, - * frontend/lowering/decl-origin.ts). And + * frontend/lowering/decl-origin.ts), its own package's also when two + * packages declare a record of one name and shape. And * commander-calc carries the HelpContext.helpWidth patch. */ import { execFile } from "node:child_process"; import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; @@ -193,6 +194,56 @@ describe("type patches", () => { }, ); + test( + "records of one name and shape from two packages name their own declarations", + { timeout: 600_000 }, + async () => { + const result = await build(join(fixture, "same-name/main.ts"), "same-name", [ + "panectx", + "gridctx", + ]); + const native = await run(result.binaryPath, []); + const failure = (pkg: string, version: string) => [ + "undefined is not representable in the target union (a value narrowed or asserted past it still held it)", + ` ${pkg}@${version} declares OutputContext.width as number (index.js:24)`, + ` if that declaration is wrong, patch it in package.json: "scriptc": { "typePatches": { "${pkg}@^${version}": { "declarations": { "OutputContext.width": "| undefined" } } } }`, + ]; + expect(native.stdout).toBe( + [...failure("panectx", "2.0.0"), ...failure("gridctx", "4.1.0"), ""].join("\n"), + ); + }, + ); + + test( + "a rebuild after the package's version changes names the installed version", + { timeout: 600_000 }, + async () => { + const copy = mkdtempSync(join(tmpdir(), "scriptc-reinstalled-")); + try { + cpSync(fixture, copy, { recursive: true }); + const entry = join(copy, "unpatched/main.ts"); + const declares = async (name: string) => { + const native = await run((await build(entry, name, ["termctx"])).binaryPath, []); + return native.stdout.split("\n").filter((line) => line.includes(" declares ")); + }; + expect(await declares("reinstalled-1.2.0")).toEqual([ + " termctx@1.2.0 declares Terminal.columns() as number (index.js:16)", + " termctx@1.2.0 declares OutputContext.width as number (index.js:24)", + ]); + writeFileSync( + join(copy, "node_modules/termctx/package.json"), + '{ "name": "termctx", "version": "1.3.0", "main": "index.js" }\n', + ); + expect(await declares("reinstalled-1.3.0")).toEqual([ + " termctx@1.3.0 declares Terminal.columns() as number (index.js:16)", + " termctx@1.3.0 declares OutputContext.width as number (index.js:24)", + ]); + } finally { + rmSync(copy, { recursive: true, force: true }); + } + }, + ); + test( "patches that do not apply as written warn and the build goes on", { timeout: 600_000 },