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

fix  microsoft#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);
   }