Add madeRequired decorator and tests (#3292)

Hopefully I got all the places I needed to get and added enough tests.
My methodology was pretty much "Look at what madeOptional does, and do
the opposite"

Closes #2731

---------

Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
Co-authored-by: Timothee Guerin <timothee.guerin@outlook.com>

.chronus/changes/add_madeRequired-2024-4-9-21-13-8.md

@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: fix
+packages:
+  - "@typespec/versioning"
+---
+
+Add `@madeRequired` decorator

docs/libraries/versioning/reference/decorators.md

@@ -72,6 +72,35 @@ model Foo {
 }
 ```
 
+### `@madeRequired` {#@TypeSpec.Versioning.madeRequired}
+
+Identifies when a target was made required.
+
+```typespec
+@TypeSpec.Versioning.madeRequired(version: EnumMember)
+```
+
+#### Target
+
+`ModelProperty`
+
+#### Parameters
+
+| Name    | Type         | Description                                       |
+| ------- | ------------ | ------------------------------------------------- |
+| version | `EnumMember` | The version that the target was made required in. |
+
+#### Examples
+
+```tsp
+model Foo {
+  name: string;
+
+  @madeRequired(Versions.v2)
+  nickname: string;
+}
+```
+
 ### `@removed` {#@TypeSpec.Versioning.removed}
 
 Identifies when the target was removed.

docs/libraries/versioning/reference/index.mdx

@@ -37,6 +37,7 @@ npm install --save-peer @typespec/versioning
 
 - [`@added`](./decorators.md#@TypeSpec.Versioning.added)
 - [`@madeOptional`](./decorators.md#@TypeSpec.Versioning.madeOptional)
+- [`@madeRequired`](./decorators.md#@TypeSpec.Versioning.madeRequired)
 - [`@removed`](./decorators.md#@TypeSpec.Versioning.removed)
 - [`@renamedFrom`](./decorators.md#@TypeSpec.Versioning.renamedFrom)
 - [`@returnTypeChangedFrom`](./decorators.md#@TypeSpec.Versioning.returnTypeChangedFrom)

packages/versioning/README.md

@@ -50,6 +50,7 @@ If the emitter needs to have the whole picture of the service evolution across t
 
 - [`@added`](#@added)
 - [`@madeOptional`](#@madeoptional)
+- [`@madeRequired`](#@maderequired)
 - [`@removed`](#@removed)
 - [`@renamedFrom`](#@renamedfrom)
 - [`@returnTypeChangedFrom`](#@returntypechangedfrom)
@@ -121,6 +122,35 @@ model Foo {
 }
 ```
 
+#### `@madeRequired`
+
+Identifies when a target was made required.
+
+```typespec
+@TypeSpec.Versioning.madeRequired(version: EnumMember)
+```
+
+##### Target
+
+`ModelProperty`
+
+##### Parameters
+
+| Name    | Type         | Description                                       |
+| ------- | ------------ | ------------------------------------------------- |
+| version | `EnumMember` | The version that the target was made required in. |
+
+##### Examples
+
+```tsp
+model Foo {
+  name: string;
+
+  @madeRequired(Versions.v2)
+  nickname: string;
+}
+```
+
 #### `@removed`
 
 Identifies when the target was removed.

packages/versioning/generated-defs/TypeSpec.Versioning.ts

@@ -181,6 +181,25 @@ export type MadeOptionalDecorator = (
   version: EnumMember
 ) => void;
 
+/**
+ * Identifies when a target was made required.
+ *
+ * @param version The version that the target was made required in.
+ * @example
+ * ```tsp
+ * model Foo {
+ * name: string;
+ * @madeRequired(Versions.v2)
+ * nickname: string;
+ * }
+ * ```
+ */
+export type MadeRequiredDecorator = (
+  context: DecoratorContext,
+  target: ModelProperty,
+  version: EnumMember
+) => void;
+
 /**
  * Identifies when the target type changed.
  *

packages/versioning/generated-defs/TypeSpec.Versioning.ts-test.ts

@@ -2,6 +2,7 @@
 import {
   $added,
   $madeOptional,
+  $madeRequired,
   $removed,
   $renamedFrom,
   $returnTypeChangedFrom,
@@ -12,6 +13,7 @@ import {
 import type {
   AddedDecorator,
   MadeOptionalDecorator,
+  MadeRequiredDecorator,
   RemovedDecorator,
   RenamedFromDecorator,
   ReturnTypeChangedFromDecorator,
@@ -27,6 +29,7 @@ type Decorators = {
   $removed: RemovedDecorator;
   $renamedFrom: RenamedFromDecorator;
   $madeOptional: MadeOptionalDecorator;
+  $madeRequired: MadeRequiredDecorator;
   $typeChangedFrom: TypeChangedFromDecorator;
   $returnTypeChangedFrom: ReturnTypeChangedFromDecorator;
 };
@@ -39,6 +42,7 @@ const _: Decorators = {
   $removed,
   $renamedFrom,
   $madeOptional,
+  $madeRequired,
   $typeChangedFrom,
   $returnTypeChangedFrom,
 };

packages/versioning/lib/decorators.tsp

@@ -161,6 +161,22 @@ namespace TypeSpec {
      */
     extern dec madeOptional(target: ModelProperty, version: EnumMember);
 
+    /**
+     * Identifies when a target was made required.
+     * @param version The version that the target was made required in.
+     *
+     * @example
+     *
+     * ```tsp
+     * model Foo {
+     *   name: string;
+     *   @madeRequired(Versions.v2)
+     *   nickname: string;
+     * }
+     * ```
+     */
+    extern dec madeRequired(target: ModelProperty, version: EnumMember);
+
     /**
      * Identifies when the target type changed.
      * @param version The version that the target type changed in.
@@ -193,6 +209,12 @@ namespace TypeSpec {
      */
     extern fn madeOptionalAfter(target: unknown, version: EnumMember): boolean;
 
+    /**
+     * Returns whether the target was made required after the given version.
+     * @param version The version to check.
+     */
+    extern fn madeRequiredAfter(target: unknown, version: EnumMember): boolean;
+
     /**
      * Returns whether the version exists for the provided enum member.
      * @param version The version to check.

packages/versioning/lib/versioning.tsp

@@ -135,6 +135,10 @@ projection model#v {
         p::setOptional(false);
       };
 
+      if madeRequiredAfter(p, version) {
+        p::setOptional(true);
+      };
+
       if hasDifferentTypeAtVersion(p, version) {
         self::changePropertyType(p::name, getTypeBeforeVersion(p, version));
       };
@@ -163,6 +167,10 @@ projection model#v {
         p::setOptional(true);
       };
 
+      if madeRequiredAfter(p, version) {
+        p::setOptional(false);
+      };
+
       if hasDifferentTypeAtVersion(p, version) {
         self::changePropertyType(p::name, p::type);
       };

packages/versioning/src/lib.ts

@@ -89,6 +89,12 @@ const libDef = {
         default: paramMessage`Property '${"name"}' marked with @madeOptional but is required. Should be '${"name"}?'`,
       },
     },
+    "made-required-optional": {
+      severity: "error",
+      messages: {
+        default: paramMessage`Property '${"name"}?' marked with @madeRequired but is optional. Should be '${"name"}'`,
+      },
+    },
     "renamed-duplicate-property": {
       severity: "error",
       messages: {

packages/versioning/src/validate.ts

@@ -18,6 +18,7 @@ import {
   findVersionedNamespace,
   getAvailabilityMap,
   getMadeOptionalOn,
+  getMadeRequiredOn,
   getRenamedFrom,
   getReturnTypeChangedFrom,
   getTypeChangedFrom,
@@ -69,6 +70,9 @@ export function $onValidate(program: Program) {
 
           // Validate model property type is correct when madeOptional
           validateMadeOptional(program, prop);
+
+          // Validate model property type is correct when madeRequired
+          validateMadeRequired(program, prop);
         }
         validateVersionedPropertyNames(program, model);
       },
@@ -458,6 +462,26 @@ function validateMadeOptional(program: Program, target: Type) {
   }
 }
 
+function validateMadeRequired(program: Program, target: Type) {
+  if (target.kind === "ModelProperty") {
+    const madeRequiredOn = getMadeRequiredOn(program, target);
+    if (!madeRequiredOn) {
+      return;
+    }
+    // if the @madeRequired decorator is on a property, it MUST NOT be optional
+    if (target.optional) {
+      reportDiagnostic(program, {
+        code: "made-required-optional",
+        format: {
+          name: target.name,
+        },
+        target: target,
+      });
+      return;
+    }
+  }
+}
+
 interface IncompatibleVersionValidateOptions {
   isTargetADependent?: boolean;
 }

packages/versioning/src/versioning.ts

@@ -20,6 +20,7 @@ import {
 import {
   AddedDecorator,
   MadeOptionalDecorator,
+  MadeRequiredDecorator,
   RenamedFromDecorator,
   ReturnTypeChangedFromDecorator,
   TypeChangedFromDecorator,
@@ -37,6 +38,7 @@ const useDependencyNamespaceKey = createStateSymbol("useDependencyNamespace");
 const useDependencyEnumKey = createStateSymbol("useDependencyEnum");
 const renamedFromKey = createStateSymbol("renamedFrom");
 const madeOptionalKey = createStateSymbol("madeOptional");
+const madeRequiredKey = createStateSymbol("madeRequired");
 const typeChangedFromKey = createStateSymbol("typeChangedFrom");
 const returnTypeChangedFromKey = createStateSymbol("returnTypeChangedFrom");
 
@@ -236,6 +238,19 @@ export const $madeOptional: MadeOptionalDecorator = (
   program.stateMap(madeOptionalKey).set(t, version);
 };
 
+export const $madeRequired: MadeRequiredDecorator = (
+  context: DecoratorContext,
+  t: ModelProperty,
+  v: EnumMember
+) => {
+  const { program } = context;
+  const version = checkIsVersion(context.program, v, context.getArgumentTarget(0)!);
+  if (!version) {
+    return;
+  }
+  program.stateMap(madeRequiredKey).set(t, version);
+};
+
 /**
  * @returns the array of RenamedFrom metadata if applicable.
  */
@@ -320,6 +335,13 @@ export function getMadeOptionalOn(p: Program, t: Type): Version | undefined {
   return p.stateMap(madeOptionalKey).get(t);
 }
 
+/**
+ * @returns version when the given type was made required if applicable.
+ */
+export function getMadeRequiredOn(p: Program, t: Type): Version | undefined {
+  return p.stateMap(madeRequiredKey).get(t);
+}
+
 export class VersionMap {
   private map = new Map<EnumMember, Version>();
 
@@ -882,6 +904,17 @@ export function madeOptionalAfter(program: Program, type: Type, versionKey: Obje
   return versioningState.timeline.isBefore(versioningState.projectingMoment, madeOptionalAtVersion);
 }
 
+export function madeRequiredAfter(program: Program, type: Type, versionKey: ObjectType): boolean {
+  const versioningState = getVersioningState(program, versionKey);
+
+  const madeRequiredAtVersion = getMadeRequiredOn(program, type);
+  if (madeRequiredAtVersion === undefined) {
+    return false;
+  }
+
+  return versioningState.timeline.isBefore(versioningState.projectingMoment, madeRequiredAtVersion);
+}
+
 export function hasDifferentTypeAtVersion(p: Program, type: Type, version: ObjectType): boolean {
   return getTypeBeforeVersion(p, type, version) !== undefined;
 }

packages/versioning/test/incompatible-versioning.test.ts

@@ -479,6 +479,19 @@ describe("versioning: validate incompatible references", () => {
         message: "Property 'name' marked with @madeOptional but is required. Should be 'name?'",
       });
     });
+
+    it("emit diagnostic when property marked @madeRequired but is optional", async () => {
+      const diagnostics = await runner.diagnose(`
+        model Foo {
+          @madeRequired(Versions.v2)
+          name?: string;
+        }
+      `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/made-required-optional",
+        message: "Property 'name?' marked with @madeRequired but is optional. Should be 'name'",
+      });
+    });
   });
 
   describe("complex type references", () => {

packages/versioning/test/versioning.test.ts

@@ -418,6 +418,23 @@ describe("versioning: logic", () => {
       ok(v2.properties.get("b")!.optional === true);
     });
 
+    it("can be made required", async () => {
+      const {
+        projections: [v1, v2],
+      } = await versionedModel(
+        ["v1", "v2"],
+        `model Test {
+          a: int32;
+          @madeRequired(Versions.v2) b: int32;
+        }`
+      );
+
+      ok(v1.properties.get("a")!.optional === false);
+      ok(v1.properties.get("b")!.optional === true);
+      ok(v2.properties.get("a")!.optional === false);
+      ok(v2.properties.get("b")!.optional === false);
+    });
+
     it("can change type to versioned models", async () => {
       const {
         projections: [v1, v2, v3],