Merge remote-tracking branch 'origin/feat/type-patch-origins' into feat/type-patch-origins

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);

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<string, string>();
 
+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. */

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));

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 };

tests/fixtures/type-patches/node_modules/gridctx/package.json

@@ -0,0 +1 @@
+{ "name": "gridctx", "version": "4.1.0", "main": "index.js" }

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 };

tests/fixtures/type-patches/node_modules/panectx/package.json

@@ -0,0 +1 @@
+{ "name": "panectx", "version": "2.0.0", "main": "index.js" }

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);
+  }
+}

tests/fixtures/type-patches/same-name/package.json

@@ -0,0 +1 @@
+{ "name": "type-patches-same-name", "private": true, "type": "module" }

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 },