Report dictionary copies of {} views, and keep method calls direct after writes into dictionaries of dictionaries

docs/content/limitations/record-identity-and-property-order.mdx

@@ -4,6 +4,8 @@ Empty record layouts are also snapshots because they have no property storage.
 `scriptc coverage` and `scriptc build` report `SC6001` where a copied value is then written or compared, whether the record was passed as an argument, stored in a typed binding, asserted with `as`, assigned to a typed binding later, or stored as an element of a typed array literal.
 
 Typed records with declared fields, indexed records, and arrays retain identity and mutations through native checked storage.
+A record or class instance converted to a string-keyed dictionary type such as `Record<string, string>`, directly or through a `{}`, `object` or `unknown` binding or parameter, keeps its identity when the program writes through, compares or inspects values of that dictionary type.
+Every dictionary type with the same value type then uses checked dynamic storage, which is slower than a native dictionary.
 Class instances retain identity at their exact type or a base class, and at any interface or object type that a class instance is passed, stored, returned, or asserted as when the program could tell a copy from the instance: values of such a type are checked views of the original object, so writes, identity comparisons, Set and Map keys, `util.inspect`, and `JSON.stringify` through them reach the instance, as in Node.js.
 These types use checked dynamic storage, which is slower than native record fields, for all of their values, including plain objects.
 A type whose values are only read field by field, from fields the class does not write after construction and does not implement as accessors, stays a native record and holds a copy.

packages/compiler/src/frontend/lowering/class-method-mutations.ts

@@ -170,14 +170,32 @@ export function collectClassMethodMutations(
   // A string index whose values cannot be functions excludes callable
   // method slots. Class instances have no implicit index signature, so such
   // a dictionary is only a class view when the class declares that index
-  // signature, and then every method must be assignable to its value type.
-  const functionFreeDictionary = (value: ts.Expression): boolean =>
-    lowerer.checker
-      .getIndexInfosOfType(lowerer.typeOf(value))
+  // signature, and then every method must be assignable to its value type,
+  // or when the dictionary keeps checked identity: it then holds the
+  // existing objects it receives, class instances included.
+  // A string-keyed dictionary that keeps checked identity holds the
+  // existing objects it receives, class instances included, so any write
+  // through it can replace a method.
+  const dictionaryView = (value: ts.Expression): boolean => {
+    const type = lowerer.typeOf(value);
+    return (
+      lowerer.checker.getPropertiesOfType(type).length === 0 &&
+      lowerer.checker
+        .getIndexInfosOfType(type)
+        .some((index) => (index.keyType.flags & ts.TypeFlags.StringLike) !== 0) &&
+      lowerer.classImplementedProtocol(type)
+    );
+  };
+  const functionFreeDictionary = (value: ts.Expression): boolean => {
+    const type = lowerer.typeOf(value);
+    if (lowerer.classImplementedProtocol(type)) return false;
+    return lowerer.checker
+      .getIndexInfosOfType(type)
       .some(
         (index) =>
           (index.keyType.flags & ts.TypeFlags.StringLike) !== 0 && functionFree(index.type),
       );
+  };
   const literalLoopKeys = (key: ts.Expression): (string | number)[] | null => {
     if (!ts.isIdentifier(key)) return null;
     const symbol = lowerer.checker.getSymbolAtLocation(key);
@@ -577,7 +595,10 @@ export function collectClassMethodMutations(
   );
   for (const { target, site } of propertyWrites) {
     const name = target.name.text;
-    if (functionFree(lowerer.typeOf(target)) || !methodOwnerCanInhabit(target.expression, name))
+    if (
+      !dictionaryView(target.expression) &&
+      (functionFree(lowerer.typeOf(target)) || !methodOwnerCanInhabit(target.expression, name))
+    )
       continue;
     if (lowerer.prototypeMethodAccesses.has(name)) lowerer.noteMethodSite(name, site);
     else lowerer.noteMethodAccess(name, site);

packages/compiler/src/frontend/lowering/class-protocols.ts

@@ -10,6 +10,9 @@ export class ClassProtocols {
   private readonly accepted = new Map<ts.Type, ts.Symbol[]>();
   /** What markIdentityFlows learned about the program, while it runs. */
   private facts: FlowFacts | null = null;
+  /** The value types of the string-keyed dictionaries that keep checked
+   * identity (see markDictionaryViews). */
+  private readonly dictionaryViews = new Set<ts.Type>();
 
   constructor(
     private readonly checker: ts.TypeChecker,
@@ -323,6 +326,7 @@ export class ClassProtocols {
         else if (names !== "all") names.add(name);
       }
     }
+    this.markDictionaryViews(sites, sourceNodes, observedNodes, writeNodes);
     for (let i = 0; i < looseArguments.length; i++) {
       const type = typeOf(looseNodes[i]!);
       if (type === undefined) continue;
@@ -697,6 +701,163 @@ export class ClassProtocols {
     return source !== undefined && this.isProgramClassInstance(source);
   }
 
+  /** String-keyed dictionaries (`Record<string, V>`, `{ [key: string]: V }`)
+   * that receive an existing record or class instance. Natively a
+   * dictionary is a key/value record, and converting another object into
+   * one copies its fields, so writes through the dictionary and identity
+   * checks would miss the original. When the program writes through, or
+   * observes, a value of such a dictionary type, every dictionary with that
+   * value type keeps checked identity instead (records with the same value
+   * type share one native layout, so values pass between them). A source
+   * counts when it is an existing object (a binding, member, element or
+   * `this`, not a fresh literal or call result) whose type is a program
+   * class instance or a record with declared properties, or a constant
+   * `{}`, `object` or `unknown` binding initialized with one. */
+  private markDictionaryViews(
+    sites: readonly {
+      source: ts.Expression;
+      target: () => ts.Type | undefined;
+      contextual?: ts.Expression;
+    }[],
+    sourceNodes: readonly ts.Expression[],
+    observedNodes: readonly ts.Expression[],
+    writeNodes: readonly ts.Expression[],
+  ): void {
+    // The calls the sites' arguments belong to, by callee name, built on
+    // first use.
+    let calls: Map<string, (ts.CallExpression | ts.NewExpression)[]> | null = null;
+    const callsNamed = (name: string): readonly (ts.CallExpression | ts.NewExpression)[] => {
+      if (calls === null) {
+        calls = new Map();
+        for (const site of sites) {
+          const call = site.contextual?.parent;
+          if (call === undefined || (!ts.isCallExpression(call) && !ts.isNewExpression(call)))
+            continue;
+          const callee = call.expression;
+          const named = ts.isIdentifier(callee)
+            ? callee.text
+            : ts.isPropertyAccessExpression(callee)
+              ? callee.name.text
+              : undefined;
+          if (named === undefined) continue;
+          const list = calls.get(named) ?? [];
+          if (!list.includes(call)) list.push(call);
+          calls.set(named, list);
+        }
+      }
+      return calls.get(name) ?? [];
+    };
+    const candidates: { index: number; sourceType: ts.Type }[] = [];
+    for (let i = 0; i < sites.length; i++) {
+      const source = sourceNodes[i]!;
+      if (!isExistingObject(source)) continue;
+      const sourceType = attempt(() => this.checker.getTypeAtLocation(source));
+      if (sourceType !== undefined && this.dictionarySource(source, sourceType, callsNamed))
+        candidates.push({ index: i, sourceType });
+    }
+    if (candidates.length === 0) return;
+    // One batch for the contextual targets of the candidate sites.
+    attempt(() =>
+      this.checker.prefetchContextualTypes(
+        candidates.flatMap(({ index }) => {
+          const contextual = sites[index]!.contextual;
+          return contextual === undefined ? [] : [contextual];
+        }),
+      ),
+    );
+    const received = new Set<ts.Type>();
+    for (const { index, sourceType } of candidates) {
+      const target = attempt(sites[index]!.target);
+      const parts =
+        target === undefined ? [] : target.isUnionType() ? ts.constituentTypes(target) : [target];
+      for (const part of parts) {
+        const value = this.dictionaryValueType(part);
+        // A dictionary of `unknown` or `any` values is checked-dynamic already.
+        if (
+          value !== undefined &&
+          (value.flags & (ts.TypeFlags.Unknown | ts.TypeFlags.Any)) === 0 &&
+          this.dictionaryValueType(sourceType) !== value
+        )
+          received.add(value);
+      }
+    }
+    if (received.size === 0) return;
+    // The dictionary values the program writes through or observes.
+    const seen = new Set<ts.Type>();
+    for (const node of [...writeNodes, ...observedNodes]) {
+      const type = attempt(() => this.checker.getTypeAtLocation(node));
+      const parts =
+        type === undefined ? [] : type.isUnionType() ? ts.constituentTypes(type) : [type];
+      for (const part of parts) {
+        const value = this.dictionaryValueType(part);
+        if (value !== undefined) seen.add(value);
+      }
+    }
+    for (const value of received) if (seen.has(value)) this.dictionaryViews.add(value);
+  }
+
+  /** Whether `source` holds an existing object that a dictionary
+   * conversion would copy. */
+  private dictionarySource(
+    source: ts.Expression,
+    type: ts.Type,
+    callsNamed: (name: string) => readonly (ts.CallExpression | ts.NewExpression)[],
+  ): boolean {
+    if (this.isProgramClassInstance(type) || this.isRecordWithFields(type)) return true;
+    if (!isDynamicView(this.checker, type) || !ts.isIdentifier(source)) return false;
+    const decl = attempt(() => this.checker.getSymbolAtLocation(source))?.valueDeclaration;
+    if (decl === undefined) return false;
+    const holdsObject = (expr: ts.Expression): boolean => {
+      const held = attempt(() => this.checker.getTypeAtLocation(this.flowSourceNode(expr)));
+      return (
+        held !== undefined && (this.isProgramClassInstance(held) || this.isRecordWithFields(held))
+      );
+    };
+    if (ts.isParameter(decl)) {
+      // A parameter holds what its callers pass: the calls of the function
+      // by name whose signature resolves to it.
+      const fn = decl.parent;
+      if (fn === undefined || !ts.isFunctionLike(fn)) return false;
+      const index = fn.parameters.nodes.indexOf(decl);
+      const name = functionName(fn);
+      if (name === undefined || index < 0) return false;
+      return callsNamed(name).some((call) => {
+        const argument = call.arguments?.nodes[index];
+        return (
+          argument !== undefined &&
+          !ts.isSpreadElement(argument) &&
+          attempt(() => this.checker.getResolvedSignature(call)?.declaration) === fn &&
+          holdsObject(argument)
+        );
+      });
+    }
+    return (
+      ts.isVariableDeclaration(decl) &&
+      decl.initializer !== undefined &&
+      ts.isVariableDeclarationList(decl.parent) &&
+      (decl.parent.flags & ts.NodeFlags.Const) !== 0 &&
+      holdsObject(decl.initializer)
+    );
+  }
+
+  /** A program object type with declared properties that is not a class
+   * instance, array, tuple, function or library type. */
+  private isRecordWithFields(type: ts.Type): boolean {
+    return this.isStructuralTarget(type) && this.checker.getPropertiesOfType(type).length > 0;
+  }
+
+  /** The value type of a string-keyed dictionary with no declared
+   * properties, or undefined for any other type. */
+  private dictionaryValueType(type: ts.Type): ts.Type | undefined {
+    if ((type.flags & ts.TypeFlags.Object) === 0) return undefined;
+    if (this.checker.isArrayType(type) || this.checker.isTupleType(type)) return undefined;
+    if (this.checker.getCallSignatures(type).length > 0) return undefined;
+    if (this.checker.getPropertiesOfType(type).length > 0) return undefined;
+    return this.checker
+      .getIndexInfosOfType(type)
+      .find((index) => (index.keyType.flags & ts.TypeFlags.String) !== 0)?.type;
+  }
+
   private isProgramClassInstance(type: ts.Type): boolean {
     const symbol = type.getSymbol();
     return (
@@ -822,7 +983,13 @@ export class ClassProtocols {
     if ((type.flags & ts.TypeFlags.Object) === 0) return false;
     const previous = this.answers.get(type);
     if (previous !== undefined) return previous;
-    const answer = this.protocol(type);
+    // A dictionary keeps checked identity when the protocols already mark
+    // it (a literal whose methods read `this`) or when it receives existing
+    // objects (markDictionaryViews).
+    const dictionaryValue = this.dictionaryValueType(type);
+    const answer =
+      this.protocol(type) ||
+      (dictionaryValue !== undefined && this.dictionaryViews.has(dictionaryValue));
     this.answers.set(type, answer);
     return answer;
   }
@@ -922,6 +1089,44 @@ const IDENTITY_METHODS = new Set([
   "includes",
 ]);
 
+/** An expression that denotes an object that already exists, not a fresh
+ * literal or call result. */
+function isExistingObject(node: ts.Expression): boolean {
+  return (
+    ts.isIdentifier(node) ||
+    ts.isPropertyAccessExpression(node) ||
+    ts.isElementAccessExpression(node) ||
+    node.kind === ts.SyntaxKind.ThisKeyword
+  );
+}
+
+/** `{}`, `object` and `unknown`: types whose values are checked-dynamic
+ * and keep the original object. */
+function isDynamicView(checker: ts.TypeChecker, type: ts.Type): boolean {
+  if (type.flags & (ts.TypeFlags.Unknown | ts.TypeFlags.NonPrimitive)) return true;
+  return (
+    (type.flags & ts.TypeFlags.Object) !== 0 &&
+    checker.getPropertiesOfType(type).length === 0 &&
+    checker.getIndexInfosOfType(type).length === 0 &&
+    checker.getCallSignatures(type).length === 0 &&
+    checker.getConstructSignatures(type).length === 0
+  );
+}
+
+/** The name calls use for a function, method or function-valued binding. */
+function functionName(fn: ts.Node): string | undefined {
+  if ((ts.isFunctionDeclaration(fn) || ts.isMethodDeclaration(fn)) && fn.name !== undefined)
+    return ts.isIdentifier(fn.name) ? fn.name.text : undefined;
+  if (
+    (ts.isArrowFunction(fn) || ts.isFunctionExpression(fn)) &&
+    fn.parent !== undefined &&
+    ts.isVariableDeclaration(fn.parent) &&
+    ts.isIdentifier(fn.parent.name)
+  )
+    return fn.parent.name.text;
+  return undefined;
+}
+
 function isNullishLiteral(node: ts.Expression): boolean {
   while (ts.isParenthesizedExpression(node)) node = node.expression;
   return (

packages/compiler/test/class-method-mutations.test.ts

@@ -167,3 +167,47 @@ test("a store into a dictionary whose values can be functions keeps checked disp
   );
   expect(await callersOf(source, "%dyn.class.properties")).not.toEqual([]);
 });
+
+test("a store into a generic dictionary of dictionaries keeps checked dispatch", async () => {
+  const source = dictionaries
+    .replace(
+      "interface Settings { groups: Record<string, Record<string, string>> }",
+      "interface Settings<T> { groups: Record<string, Record<string, T>> }",
+    )
+    .replace("function addGroup(settings: Settings,", "function addGroup<T>(settings: Settings<T>,")
+    .replace("const settings: Settings =", "const settings: Settings<string> =");
+  expect(await callersOf(source, "%dyn.class.properties")).not.toEqual([]);
+});
+
+test("a runtime-key store into a dictionary view of a class instance keeps checked dispatch", async () => {
+  const source = `
+class Counter {
+  n = 0;
+  tick(): number { return ++this.n; }
+}
+const counter = new Counter();
+const view: {} = counter;
+const slots: Record<string, string> = view;
+const key: string = String(process.argv.length);
+slots[key] = "replaced";
+console.log(counter.tick());
+`;
+  // The method has a slot the write can replace, which calls consult.
+  expect(await callersOf(source, "%Counter.tick")).toContain("%method.value:%Counter.tick");
+});
+
+test("a dot store into a dictionary view of a class instance keeps checked dispatch", async () => {
+  const source = `
+class Lamp {
+  on = false;
+  toggle(): boolean { this.on = !this.on; return this.on; }
+}
+const lamp = new Lamp();
+const view: {} = lamp;
+const slots: Record<string, string> = view;
+slots.toggle = "replaced";
+console.log(lamp.toggle());
+`;
+  // The method has a slot the write can replace, which calls consult.
+  expect(await callersOf(source, "%Lamp.toggle")).toContain("%method.value:%Lamp.toggle");
+});