Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add madeRequired decorator and tests #3292

Merged
merged 4 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .chronus/changes/add_madeRequired-2024-4-9-21-13-8.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/versioning"
---

Add `@madeRequired` decorator
29 changes: 29 additions & 0 deletions docs/libraries/versioning/reference/decorators.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions docs/libraries/versioning/reference/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions packages/versioning/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
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 @@ -18,6 +18,7 @@ import {
findVersionedNamespace,
getAvailabilityMap,
getMadeOptionalOn,
getMadeRequiredOn,
getRenamedFrom,
getReturnTypeChangedFrom,
getTypeChangedFrom,
Expand Down Expand Up @@ -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);
},
Expand Down Expand Up @@ -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;
}
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
Loading