From 26f3a878a03672311b9e70720e2f8c0872d593bd Mon Sep 17 00:00:00 2001 From: Jim Borden Date: Tue, 7 May 2024 11:15:00 +0900 Subject: [PATCH] Add madeRequired decorator and tests --- .../generated-defs/TypeSpec.Versioning.ts | 19 +++++++++++ .../TypeSpec.Versioning.ts-test.ts | 4 +++ packages/versioning/lib/decorators.tsp | 22 +++++++++++++ packages/versioning/lib/versioning.tsp | 8 +++++ packages/versioning/src/lib.ts | 6 ++++ packages/versioning/src/validate.ts | 24 ++++++++++++++ packages/versioning/src/versioning.ts | 33 +++++++++++++++++++ .../test/incompatible-versioning.test.ts | 13 ++++++++ packages/versioning/test/versioning.test.ts | 17 ++++++++++ 9 files changed, 146 insertions(+) diff --git a/packages/versioning/generated-defs/TypeSpec.Versioning.ts b/packages/versioning/generated-defs/TypeSpec.Versioning.ts index 7ef9ef7d472..1048c630048 100644 --- a/packages/versioning/generated-defs/TypeSpec.Versioning.ts +++ b/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. * diff --git a/packages/versioning/generated-defs/TypeSpec.Versioning.ts-test.ts b/packages/versioning/generated-defs/TypeSpec.Versioning.ts-test.ts index f9fa0b2ed26..c09f96075c1 100644 --- a/packages/versioning/generated-defs/TypeSpec.Versioning.ts-test.ts +++ b/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, }; diff --git a/packages/versioning/lib/decorators.tsp b/packages/versioning/lib/decorators.tsp index e8b8d429294..042e2cb31f7 100644 --- a/packages/versioning/lib/decorators.tsp +++ b/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. diff --git a/packages/versioning/lib/versioning.tsp b/packages/versioning/lib/versioning.tsp index e9a0c5f1cb9..be8234aee05 100644 --- a/packages/versioning/lib/versioning.tsp +++ b/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); }; diff --git a/packages/versioning/src/lib.ts b/packages/versioning/src/lib.ts index 9a4d74df891..5696d0b5a8f 100644 --- a/packages/versioning/src/lib.ts +++ b/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: { diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index d3dd6f4e598..73f0c88fdfb 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -17,6 +17,7 @@ import { findVersionedNamespace, getAvailabilityMap, getMadeOptionalOn, + getMadeRequiredOn, getRenamedFrom, getReturnTypeChangedFrom, getTypeChangedFrom, @@ -68,6 +69,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); }, @@ -457,6 +461,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; } diff --git a/packages/versioning/src/versioning.ts b/packages/versioning/src/versioning.ts index 68846d60a7b..63ff9970a36 100644 --- a/packages/versioning/src/versioning.ts +++ b/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(); @@ -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; } diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts index f6789778eb3..3b775bfb530 100644 --- a/packages/versioning/test/incompatible-versioning.test.ts +++ b/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", () => { diff --git a/packages/versioning/test/versioning.test.ts b/packages/versioning/test/versioning.test.ts index f955bc44304..5378243760a 100644 --- a/packages/versioning/test/versioning.test.ts +++ b/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],