Skip to content

Commit

Permalink
Add madeRequired decorator and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
borrrden committed May 7, 2024
1 parent 4b3489f commit 26f3a87
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 0 deletions.
19 changes: 19 additions & 0 deletions packages/versioning/generated-defs/TypeSpec.Versioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import {
$added,
$madeOptional,
$madeRequired,
$removed,
$renamedFrom,
$returnTypeChangedFrom,
Expand All @@ -12,6 +13,7 @@ import {
import type {
AddedDecorator,
MadeOptionalDecorator,
MadeRequiredDecorator,
RemovedDecorator,
RenamedFromDecorator,
ReturnTypeChangedFromDecorator,
Expand All @@ -27,6 +29,7 @@ type Decorators = {
$removed: RemovedDecorator;
$renamedFrom: RenamedFromDecorator;
$madeOptional: MadeOptionalDecorator;
$madeRequired: MadeRequiredDecorator;
$typeChangedFrom: TypeChangedFromDecorator;
$returnTypeChangedFrom: ReturnTypeChangedFromDecorator;
};
Expand All @@ -39,6 +42,7 @@ const _: Decorators = {
$removed,
$renamedFrom,
$madeOptional,
$madeRequired,
$typeChangedFrom,
$returnTypeChangedFrom,
};
22 changes: 22 additions & 0 deletions packages/versioning/lib/decorators.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions packages/versioning/lib/versioning.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
};
Expand Down Expand Up @@ -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);
};
Expand Down
6 changes: 6 additions & 0 deletions packages/versioning/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
24 changes: 24 additions & 0 deletions packages/versioning/src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
findVersionedNamespace,
getAvailabilityMap,
getMadeOptionalOn,
getMadeRequiredOn,
getRenamedFrom,
getReturnTypeChangedFrom,
getTypeChangedFrom,
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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;
}
Expand Down
33 changes: 33 additions & 0 deletions packages/versioning/src/versioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import {
AddedDecorator,
MadeOptionalDecorator,
MadeRequiredDecorator,
RenamedFromDecorator,
ReturnTypeChangedFromDecorator,
TypeChangedFromDecorator,
Expand All @@ -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");

Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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>();

Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions packages/versioning/test/incompatible-versioning.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
17 changes: 17 additions & 0 deletions packages/versioning/test/versioning.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down

0 comments on commit 26f3a87

Please sign in to comment.