Feature: Spread `Record<T>` (#2920)

fix  #2785

.chronus/changes/feature-spread-record-2024-1-16-1-8-24.md

@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: breaking
+packages:
+  - "@typespec/compiler"
+---
+
+Intersecting Record<T> with incompatible properties will now emit an error

.chronus/changes/feature-spread-record-2024-1-16-1-8-25.md

@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: feature
+packages:
+  - "@typespec/compiler"
+---
+
+Add support for `...Record<T>` to define the type of remaining properties

.chronus/changes/feature-spread-record-2024-1-16-3-42-36.md

@@ -0,0 +1,6 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: internal
+packages:
+  - "@typespec/openapi3"
+---

docs/language-basics/models.md

@@ -75,6 +75,62 @@ model Cat is Pet {
 // name, age, meow, address, furColor
 ```
 
+### Additional properties
+
+The `Record<T>` model can be used to define a model with an arbitrary number of properties of type T. It can be combined with a named model to provide some known properties.
+
+There is 3 ways this can be done which all have slightly different semantics:
+
+- Using the `...` operator
+- Using `is` operator
+- Using `extends` operator
+
+#### Using `...` operator
+
+Spreading a Record into your model means that your model has all the properties you have explicitly defined plus any additional properties defined by the Record.
+This means that the property in the model could be of a different and incompatible type with the Record value type.
+
+```tsp
+// Here we are saying the Person model has a property `age` that is an int32 but has some other properties that are all string.
+model Person {
+  age: int32;
+  ...Record<string>;
+}
+```
+
+#### Using `is` operator
+
+When using `is Record<T>` it is now saying that all properties of this model are of type T. This means that each property explicitly defined in the model must be also be of type T.
+
+The example above would be invalid
+
+```tsp
+model Person is Record<string> {
+  age: int32;
+  //   ^ int32 is not assignable to string
+}
+```
+
+But the following would be valid
+
+```tsp
+model Person is Record<string> {
+  name: string;
+}
+```
+
+#### Using `extends` operator
+
+`extends` is going to have similar semantics to `is` but is going to define the relationship between the 2 models.
+
+In many languages this would probably result in the same emitted code as `is` and is recommended to just use `is Record<T>` instead.
+
+```tsp
+model Person extends Record<string> {
+  name: string;
+}
+```
+
 ### Special property types
 
 #### `never`

packages/compiler/src/core/checker.ts

@@ -1,4 +1,4 @@
-import { $docFromComment, getIndexer } from "../lib/decorators.js";
+import { $docFromComment, getIndexer, isArrayModelType } from "../lib/decorators.js";
 import { MultiKeyMap, Mutable, createRekeyableMap, isArray, mutate } from "../utils/misc.js";
 import { createSymbol, createSymbolTable } from "./binder.js";
 import { getDeprecationDetails, markDeprecated } from "./deprecation.js";
@@ -75,7 +75,6 @@ import {
   ModelIndexer,
   ModelProperty,
   ModelPropertyNode,
-  ModelSpreadPropertyNode,
   ModelStatementNode,
   ModifierFlags,
   Namespace,
@@ -1588,17 +1587,20 @@ export function createChecker(program: Program): Checker {
     });
 
     const indexers: ModelIndexer[] = [];
-    for (const [optionNode, option] of options) {
+    const modelOptions: [Node, Model][] = options.filter((entry): entry is [Node, Model] => {
+      const [optionNode, option] = entry;
       if (option.kind === "TemplateParameter") {
-        continue;
+        return false;
       }
       if (option.kind !== "Model") {
         reportCheckerDiagnostic(
           createDiagnostic({ code: "intersect-non-model", target: optionNode })
         );
-        continue;
+        return false;
       }
-
+      return true;
+    });
+    for (const [optionNode, option] of modelOptions) {
       if (option.indexer) {
         if (option.indexer.key.name === "integer") {
           reportCheckerDiagnostic(
@@ -1612,19 +1614,8 @@ export function createChecker(program: Program): Checker {
           indexers.push(option.indexer);
         }
       }
-      if (indexers.length === 1) {
-        intersection.indexer = indexers[0];
-      } else if (indexers.length > 1) {
-        intersection.indexer = {
-          key: indexers[0].key,
-          value: mergeModelTypes(
-            node,
-            indexers.map((x) => [x.value.node!, x.value]),
-            mapper
-          ),
-        };
-      }
-
+    }
+    for (const [_, option] of modelOptions) {
       const allProps = walkPropertiesInherited(option);
       for (const prop of allProps) {
         if (properties.has(prop.name)) {
@@ -1643,8 +1634,24 @@ export function createChecker(program: Program): Checker {
           model: intersection,
         });
         properties.set(prop.name, newPropType);
+        for (const indexer of indexers.filter((x) => x !== option.indexer)) {
+          checkPropertyCompatibleWithIndexer(indexer, prop, node);
+        }
       }
     }
+
+    if (indexers.length === 1) {
+      intersection.indexer = indexers[0];
+    } else if (indexers.length > 1) {
+      intersection.indexer = {
+        key: indexers[0].key,
+        value: mergeModelTypes(
+          node,
+          indexers.map((x) => [x.value.node!, x.value]),
+          mapper
+        ),
+      };
+    }
     linkMapper(intersection, mapper);
     return finishType(intersection);
   }
@@ -2836,15 +2843,23 @@ export function createChecker(program: Program): Checker {
     return undefined;
   }
 
-  function checkPropertyCompatibleWithIndexer(
+  function checkPropertyCompatibleWithModelIndexer(
     parentModel: Model,
     property: ModelProperty,
-    diagnosticTarget: ModelPropertyNode | ModelSpreadPropertyNode
+    diagnosticTarget: Node
   ) {
     const indexer = findIndexer(parentModel);
     if (indexer === undefined) {
       return;
     }
+    return checkPropertyCompatibleWithIndexer(indexer, property, diagnosticTarget);
+  }
+
+  function checkPropertyCompatibleWithIndexer(
+    indexer: ModelIndexer,
+    property: ModelProperty,
+    diagnosticTarget: Node
+  ) {
     if (indexer.key.name === "integer") {
       reportCheckerDiagnostics([
         createDiagnostic({
@@ -2855,14 +2870,18 @@ export function createChecker(program: Program): Checker {
       return;
     }
 
-    const [valid, diagnostics] = isTypeAssignableTo(
-      property.type,
-      indexer.value,
-      diagnosticTarget.kind === SyntaxKind.ModelSpreadProperty
-        ? diagnosticTarget
-        : diagnosticTarget.value
-    );
-    if (!valid) reportCheckerDiagnostics(diagnostics);
+    const [valid, diagnostics] = isTypeAssignableTo(property.type, indexer.value, diagnosticTarget);
+    if (!valid)
+      reportCheckerDiagnostic(
+        createDiagnostic({
+          code: "incompatible-indexer",
+          format: { message: diagnostics.map((x) => `  ${x.message}`).join("\n") },
+          target:
+            diagnosticTarget.kind === SyntaxKind.ModelProperty
+              ? diagnosticTarget.value
+              : diagnosticTarget,
+        })
+      );
   }
 
   function checkModelProperties(
@@ -2871,23 +2890,74 @@ export function createChecker(program: Program): Checker {
     parentModel: Model,
     mapper: TypeMapper | undefined
   ) {
+    let spreadIndexers: ModelIndexer[] | undefined;
     for (const prop of node.properties!) {
       if ("id" in prop) {
         const newProp = checkModelProperty(prop, mapper);
         newProp.model = parentModel;
-        checkPropertyCompatibleWithIndexer(parentModel, newProp, prop);
+        checkPropertyCompatibleWithModelIndexer(parentModel, newProp, prop);
         defineProperty(properties, newProp);
       } else {
         // spread property
-        const newProperties = checkSpreadProperty(node.symbol, prop.target, parentModel, mapper);
-
+        const [newProperties, additionalIndexer] = checkSpreadProperty(
+          node.symbol,
+          prop.target,
+          parentModel,
+          mapper
+        );
+        if (additionalIndexer) {
+          if (spreadIndexers) {
+            spreadIndexers.push(additionalIndexer);
+          } else {
+            spreadIndexers = [additionalIndexer];
+          }
+        }
         for (const newProp of newProperties) {
           linkIndirectMember(node, newProp, mapper);
-          checkPropertyCompatibleWithIndexer(parentModel, newProp, prop);
+          checkPropertyCompatibleWithModelIndexer(parentModel, newProp, prop);
           defineProperty(properties, newProp, prop);
         }
       }
     }
+
+    if (spreadIndexers) {
+      const value =
+        spreadIndexers.length === 1
+          ? spreadIndexers[0].value
+          : createUnion(spreadIndexers.map((i) => i.value));
+      parentModel.indexer = {
+        key: spreadIndexers[0].key,
+        value: value,
+      };
+    }
+  }
+
+  function createUnion(options: Type[]): Union {
+    const variants = createRekeyableMap<string | symbol, UnionVariant>();
+    const union: Union = createAndFinishType({
+      kind: "Union",
+      node: undefined!,
+      options,
+      decorators: [],
+      variants,
+      expression: true,
+    });
+
+    for (const option of options) {
+      const name = Symbol("indexer-union-variant");
+      variants.set(
+        name,
+        createAndFinishType({
+          kind: "UnionVariant",
+          node: undefined!,
+          type: option,
+          name,
+          union,
+          decorators: [],
+        })
+      );
+    }
+    return union;
   }
 
   function defineProperty(
@@ -3342,16 +3412,21 @@ export function createChecker(program: Program): Checker {
     targetNode: TypeReferenceNode,
     parentModel: Model,
     mapper: TypeMapper | undefined
-  ): ModelProperty[] {
+  ): [ModelProperty[], ModelIndexer | undefined] {
     const targetType = getTypeForNode(targetNode, mapper);
 
     if (targetType.kind === "TemplateParameter" || isErrorType(targetType)) {
-      return [];
+      return [[], undefined];
     }
     if (targetType.kind !== "Model") {
       reportCheckerDiagnostic(createDiagnostic({ code: "spread-model", target: targetNode }));
-      return [];
+      return [[], undefined];
     }
+    if (isArrayModelType(program, targetType)) {
+      reportCheckerDiagnostic(createDiagnostic({ code: "spread-model", target: targetNode }));
+      return [[], undefined];
+    }
+
     if (parentModel === targetType) {
       reportCheckerDiagnostic(
         createDiagnostic({
@@ -3373,7 +3448,8 @@ export function createChecker(program: Program): Checker {
         })
       );
     }
-    return props;
+
+    return [props, targetType.indexer];
   }
 
   /**

packages/compiler/src/core/messages.ts

@@ -298,6 +298,12 @@ const diagnostics = {
       array: "Cannot intersect an array model.",
     },
   },
+  "incompatible-indexer": {
+    severity: "error",
+    messages: {
+      default: paramMessage`Property is incompatible with indexer:\n${"message"}`,
+    },
+  },
   "no-array-properties": {
     severity: "error",
     messages: {

packages/compiler/src/core/projector.ts

@@ -284,9 +284,10 @@ export function createProjector(
     }
 
     if (model.indexer) {
+      const projectedValue = projectType(model.indexer.value);
       projectedModel.indexer = {
         key: projectType(model.indexer.key) as Scalar,
-        value: projectType(model.indexer.value),
+        value: projectedValue,
       };
     }
 
@@ -368,7 +369,7 @@ export function createProjector(
    * a template type, because we don't want to run decorators for templates.
    */
   function shouldFinishType(type: Type) {
-    const parentTemplate = getParentTemplateNode(type.node!);
+    const parentTemplate = type.node && getParentTemplateNode(type.node);
     return !parentTemplate || isTemplateInstance(type);
   }
 

packages/compiler/test/checker/model.test.ts

@@ -2,7 +2,7 @@ import { deepStrictEqual, match, ok, strictEqual } from "assert";
 import { beforeEach, describe, it } from "vitest";
 import { isTemplateDeclaration } from "../../src/core/type-utils.js";
 import { Model, ModelProperty, Type } from "../../src/core/types.js";
-import { Operation, getDoc, isArrayModelType } from "../../src/index.js";
+import { Operation, getDoc, isArrayModelType, isRecordModelType } from "../../src/index.js";
 import {
   TestHost,
   createTestHost,
@@ -399,24 +399,6 @@ describe("compiler: models", () => {
       strictEqual((Spread.properties.get("h1")!.type as any)!.value, "test");
     });
 
-    it("can decorate spread properties independently", async () => {
-      testHost.addTypeSpecFile(
-        "main.tsp",
-        `
-        @test model Base {@doc("base doc") one: string}
-        @test model Spread {...Base}
-
-        @@doc(Spread.one, "override for spread");
-        `
-      );
-      const { Base, Spread } = (await testHost.compile("main.tsp")) as {
-        Base: Model;
-        Spread: Model;
-      };
-      strictEqual(getDoc(testHost.program, Spread.properties.get("one")!), "override for spread");
-      strictEqual(getDoc(testHost.program, Base.properties.get("one")!), "base doc");
-    });
-
     it("keeps reference of children", async () => {
       testHost.addTypeSpecFile(
         "main.tsp",
@@ -850,6 +832,102 @@ describe("compiler: models", () => {
     });
   });
 
+  describe("spread", () => {
+    it("can decorate spread properties independently", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        @test model Base {@doc("base doc") one: string}
+        @test model Spread {...Base}
+
+        @@doc(Spread.one, "override for spread");
+        `
+      );
+      const { Base, Spread } = (await testHost.compile("main.tsp")) as {
+        Base: Model;
+        Spread: Model;
+      };
+      strictEqual(getDoc(testHost.program, Spread.properties.get("one")!), "override for spread");
+      strictEqual(getDoc(testHost.program, Base.properties.get("one")!), "base doc");
+    });
+
+    it("can spread a Record<T>", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        @test model Test {...Record<int32>;}
+        `
+      );
+      const { Test } = (await testHost.compile("main.tsp")) as {
+        Test: Model;
+      };
+      ok(isRecordModelType(testHost.program, Test));
+      strictEqual(Test.indexer?.key.name, "string");
+      strictEqual(Test.indexer?.value.kind, "Scalar");
+      strictEqual(Test.indexer?.value.name, "int32");
+    });
+
+    it("can spread a Record<T> with different value than existing props", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        @test model Test {
+          name: string;
+          ...Record<int32>;
+        }
+        `
+      );
+      const { Test } = (await testHost.compile("main.tsp")) as {
+        Test: Model;
+      };
+      ok(isRecordModelType(testHost.program, Test));
+      const nameProp = Test.properties.get("name");
+      strictEqual(nameProp?.type.kind, "Scalar");
+      strictEqual(nameProp?.type.name, "string");
+      strictEqual(Test.indexer?.key.name, "string");
+      strictEqual(Test.indexer?.value.kind, "Scalar");
+      strictEqual(Test.indexer?.value.name, "int32");
+    });
+
+    it("can spread different records", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        @test model Test {
+          ...Record<int32>;
+          ...Record<string>;
+        }
+        `
+      );
+      const { Test } = (await testHost.compile("main.tsp")) as {
+        Test: Model;
+      };
+      ok(isRecordModelType(testHost.program, Test));
+      strictEqual(Test.indexer?.key.name, "string");
+      const indexerValue = Test.indexer?.value;
+      strictEqual(indexerValue.kind, "Union");
+      const options = [...indexerValue.variants.values()].map((x) => x.type);
+      strictEqual(options[0].kind, "Scalar");
+      strictEqual(options[0].name, "int32");
+      strictEqual(options[1].kind, "Scalar");
+      strictEqual(options[1].name, "string");
+    });
+
+    it("emit diagnostic if spreading an T[]", async () => {
+      testHost.addTypeSpecFile(
+        "main.tsp",
+        `
+        @test model Test {...Array<int32>;}
+        `
+      );
+      const diagnostics = await testHost.diagnose("main.tsp");
+      expectDiagnostics(diagnostics, {
+        code: "spread-model",
+        message: "Cannot spread properties of non-model type.",
+      });
+    });
+  });
+
   describe("property circular references", () => {
     it("emit diagnostics if property reference itself", async () => {
       testHost.addTypeSpecFile(

packages/compiler/test/checker/relation.test.ts

@@ -78,8 +78,11 @@ describe("compiler: checker: type relations", () => {
           prop1: string;
         }`);
       expectDiagnostics(diagnostics, {
-        code: "unassignable",
-        message: "Type 'string' is not assignable to type 'int32'",
+        code: "incompatible-indexer",
+        message: [
+          "Property is incompatible with indexer:",
+          "  Type 'string' is not assignable to type 'int32'",
+        ].join("\n"),
       });
     });
 
@@ -89,8 +92,11 @@ describe("compiler: checker: type relations", () => {
           prop1: string;
         }`);
       expectDiagnostics(diagnostics, {
-        code: "unassignable",
-        message: "Type 'string' is not assignable to type 'int32'",
+        code: "incompatible-indexer",
+        message: [
+          "Property is incompatible with indexer:",
+          "  Type 'string' is not assignable to type 'int32'",
+        ].join("\n"),
       });
     });
 
@@ -106,6 +112,19 @@ describe("compiler: checker: type relations", () => {
       deepStrictEqual([...indexValue.properties.keys()], ["foo", "bar"]);
     });
 
+    it("cannot intersect model with property incompatible with record", async () => {
+      const diagnostics = await runner.diagnose(`
+        alias A = Record<int32> & {prop1: string};
+      `);
+      expectDiagnostics(diagnostics, {
+        code: "incompatible-indexer",
+        message: [
+          "Property is incompatible with indexer:",
+          "  Type 'string' is not assignable to type 'int32'",
+        ].join("\n"),
+      });
+    });
+
     it("cannot intersect model with a scalar", async () => {
       const diagnostics = await runner.diagnose(`
         alias A = string & {prop1: string};
@@ -135,6 +154,38 @@ describe("compiler: checker: type relations", () => {
         message: "Cannot intersect an array model.",
       });
     });
+
+    it("spread Record<string> lets other property be non string", async () => {
+      const diagnostics = await runner.diagnose(`
+        model Foo {
+          age: int32;
+          enabled: boolean;
+          ...Record<string>;
+        }
+      `);
+      expectDiagnosticEmpty(diagnostics);
+    });
+
+    it("model is a model that spread record does need to respect indexer", async () => {
+      const diagnostics = await runner.diagnose(`
+        model Foo {
+          age: int32;
+          enabled: boolean;
+          ...Record<string>;
+        }
+
+        model Bar is Foo {
+          thisNeedsToBeString: int32;
+        }
+      `);
+      expectDiagnostics(diagnostics, {
+        code: "incompatible-indexer",
+        message: [
+          "Property is incompatible with indexer:",
+          "  Type 'int32' is not assignable to type 'string'",
+        ].join("\n"),
+      });
+    });
   });
 
   describe("unknown target", () => {

packages/openapi3/test/additional-properties.test.ts

@@ -1,7 +1,6 @@
-import { expectDiagnostics } from "@typespec/compiler/testing";
 import { deepStrictEqual, ok } from "assert";
 import { describe, it } from "vitest";
-import { diagnoseOpenApiFor, oapiForModel } from "./test-host.js";
+import { oapiForModel } from "./test-host.js";
 
 describe("openapi3: Additional properties", () => {
   describe("extends Record<T>", () => {
@@ -62,20 +61,6 @@ describe("openapi3: Additional properties", () => {
     });
   });
 
-  it("emits error if model extends record with incompatible value type", async () => {
-    const diagnostics = await diagnoseOpenApiFor(
-      `
-      model Pet extends Record<string> { age: int16 };
-      `
-    );
-    expectDiagnostics(diagnostics, [
-      {
-        code: "unassignable",
-        message: "Type 'int16' is not assignable to type 'string'",
-      },
-    ]);
-  });
-
   it("set additionalProperties if model extends Record with leaf type", async () => {
     const res = await oapiForModel(
       "Pet",

packages/openapi3/test/record.test.ts

@@ -36,4 +36,36 @@ describe("openapi3: Record", () => {
       additionalProperties: { type: "integer", format: "int32" },
     });
   });
+
+  it("specify additionalProperties when `...Record<T>`", async () => {
+    const res = await oapiForModel(
+      "Person",
+      `
+      model Person {age: int32, ...Record<string>}
+      `
+    );
+
+    deepStrictEqual(res.schemas.Person, {
+      type: "object",
+      properties: { age: { type: "integer", format: "int32" } },
+      additionalProperties: { type: "string" },
+      required: ["age"],
+    });
+  });
+
+  it("specify additionalProperties of anyOf when multiple `...Record<T>`", async () => {
+    const res = await oapiForModel(
+      "Person",
+      `
+      model Person {age: int32, ...Record<string>, ...Record<boolean>}
+      `
+    );
+
+    deepStrictEqual(res.schemas.Person, {
+      type: "object",
+      properties: { age: { type: "integer", format: "int32" } },
+      additionalProperties: { anyOf: [{ type: "string" }, { type: "boolean" }] },
+      required: ["age"],
+    });
+  });
 });