Skip to content

Commit 2f9a2c2

Browse files
timotheeguerinCopilot
andauthored
Warn usage of implicitOptionality in @patch with deprecation (#9884)
fix #7235 --------- Co-authored-by: Copilot <copilot@github.com>
1 parent ecf574f commit 2f9a2c2

12 files changed

Lines changed: 117 additions & 20 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
3+
changeKind: internal
4+
packages:
5+
- "@typespec/http-specs"
6+
- "@typespec/rest"
7+
---
8+
9+
Add suppressions for deprecated behavior
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
3+
changeKind: deprecation
4+
packages:
5+
- "@typespec/http"
6+
---
7+
8+
Deprecate use of `@patch(#{implicitOptionality: true})`.
9+
10+
Migrate using one of the following patterns depending on intended semantics:
11+
12+
1. Preserve previous behavior with an explicit patch model (optional properties)
13+
14+
```diff lang=typespec
15+
model Pet {
16+
name: string;
17+
age: int32;
18+
}
19+
20+
+ model PetPatch {
21+
+ name?: string;
22+
+ age?: int32;
23+
+ }
24+
25+
26+
- @patch(#{implicitOptionality: true}) op updatePet(@body patch: Pet): void;
27+
+ @patch op updatePet(@body patch: PetPatch): void;
28+
```
29+
30+
2. Use merge-patch semantics explicitly with `MergePatchUpdate<T>`
31+
32+
```typespec
33+
model Pet {
34+
name: string;
35+
age: int32;
36+
}
37+
38+
@patch op updatePet(@body patch: MergePatchUpdate<Pet>): void;
39+
```
40+
41+
Use `MergePatchCreateOrUpdate<T>` when the operation supports create-or-update behavior.

packages/http-specs/specs/type/model/visibility/main.tsp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ op headModel(@bodyRoot input: VisibilityModel): OkResponse;
8888
@put
8989
op putModel(@body input: VisibilityModel): void;
9090

91+
#suppress "@typespec/http/deprecated-implicit-optionality" "for legacy test"
9192
@scenario
9293
@scenarioDoc("""
9394
Generate abd send put model with write/update properties.

packages/http/README.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,10 @@ Specify the HTTP verb for the target operation to be `PATCH`.
344344
@patch op update(pet: Pet): void;
345345
```
346346

347+
###### Using MergePatch template for proper merge-patch semantics
348+
347349
```typespec
348-
// Disable implicit optionality, making the body of the PATCH operation use the
349-
// optionality as defined in the `Pet` model.
350-
@patch(#{ implicitOptionality: false })
351-
op update(pet: Pet): void;
350+
@patch op update(@body pet: MergePatchUpdate<Pet>): void;
352351
```
353352

354353
#### `@path`

packages/http/generated-defs/TypeSpec.Http.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,9 @@ export type PostDecorator = (
244244
* ```typespec
245245
* @patch op update(pet: Pet): void;
246246
* ```
247-
* @example
247+
* @example Using MergePatch template for proper merge-patch semantics
248248
* ```typespec
249-
* // Disable implicit optionality, making the body of the PATCH operation use the
250-
* // optionality as defined in the `Pet` model.
251-
* @patch(#{ implicitOptionality: false })
252-
* op update(pet: Pet): void;
249+
* @patch op update(@body pet: MergePatchUpdate<Pet>): void;
253250
* ```
254251
*/
255252
export type PatchDecorator = (

packages/http/lib/decorators.tsp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,12 @@ model PatchOptions {
266266
/**
267267
* If set to `false`, disables the implicit transform that makes the body of a
268268
* PATCH operation deeply optional.
269+
*
270+
* @deprecated `implicitOptionality` is deprecated and will be removed.
271+
* To preserve the previous behavior, define and use an explicit patch model
272+
* with optional properties for your `@body` parameter.
273+
* For actual JSON Merge Patch behavior, use `MergePatchUpdate<T>` as the
274+
* `@body` type (for example: `@patch op update(@body pet: MergePatchUpdate<Pet>): void;`).
269275
*/
270276
implicitOptionality?: boolean;
271277
}
@@ -281,12 +287,9 @@ model PatchOptions {
281287
* @patch op update(pet: Pet): void;
282288
* ```
283289
*
284-
* @example
290+
* @example Using MergePatch template for proper merge-patch semantics
285291
* ```typespec
286-
* // Disable implicit optionality, making the body of the PATCH operation use the
287-
* // optionality as defined in the `Pet` model.
288-
* @patch(#{ implicitOptionality: false })
289-
* op update(pet: Pet): void;
292+
* @patch op update(@body pet: MergePatchUpdate<Pet>): void;
290293
* ```
291294
*/
292295
extern dec patch(target: Operation, options?: valueof PatchOptions);

packages/http/src/decorators.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,15 @@ export const $patch: PatchDecorator = (
416416
) => {
417417
_patch(context, entity);
418418

419-
if (options) setPatchOptions(context.program, entity, options);
419+
if (options) {
420+
if (options.implicitOptionality === true) {
421+
reportDiagnostic(context.program, {
422+
code: "deprecated-implicit-optionality",
423+
target: entity,
424+
});
425+
}
426+
setPatchOptions(context.program, entity, options);
427+
}
420428
};
421429

422430
/**

packages/http/src/lib.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,12 @@ export const $lib = createTypeSpecLibrary({
195195
default: paramMessage`The 'contents' property of the file model must be a scalar type that extends 'string' or 'bytes'. Found '${"type"}'.`,
196196
},
197197
},
198+
"deprecated-implicit-optionality": {
199+
severity: "warning",
200+
messages: {
201+
default: `The implicitOptionality option is deprecated. To preserve previous behavior, use an explicit patch model with optional properties. For actual merge-patch semantics, use MergePatchUpdate<T> for the @body type.`,
202+
},
203+
},
198204
"merge-patch-contains-null": {
199205
severity: "error",
200206
messages: {

packages/http/test/http-decorators.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,26 @@ describe("http: decorators", () => {
5050
message: `Argument of type '"/test"' is not assignable to parameter of type 'valueof TypeSpec.Http.PatchOptions'`,
5151
});
5252
});
53+
54+
it(`@patch emits deprecation warning when implicitOptionality: true`, async () => {
55+
const diagnostics = await Tester.diagnose(`
56+
@patch(#{ implicitOptionality: true }) op test(): string;
57+
`);
58+
59+
expectDiagnostics(diagnostics, {
60+
code: "@typespec/http/deprecated-implicit-optionality",
61+
message:
62+
"The implicitOptionality option is deprecated. To preserve previous behavior, use an explicit patch model with optional properties. For actual merge-patch semantics, use MergePatchUpdate<T> for the @body type.",
63+
});
64+
});
65+
66+
it(`@patch does not emit deprecation warning when implicitOptionality: false`, async () => {
67+
const diagnostics = await Tester.diagnose(`
68+
@patch(#{ implicitOptionality: false }) op test(): string;
69+
`);
70+
71+
expectDiagnosticEmpty(diagnostics);
72+
});
5373
});
5474

5575
describe("@header", () => {

packages/openapi3/test/metadata.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
9999
@visibility(Lifecycle.Read, Lifecycle.Update, Lifecycle.Create) ruc?: string;
100100
}
101101
@parameterVisibility(Lifecycle.Create, Lifecycle.Update)
102+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
102103
@route("/") @patch(#{implicitOptionality: true}) op createOrUpdate(...M): M;
103104
`);
104105

@@ -176,7 +177,8 @@ worksFor(supportedVersions, ({ openApiFor }) => {
176177
person: Person;
177178
relationship: string;
178179
}
179-
@route("/") @patch(#{implicitOptionality: true}) op update(...Person): Person;
180+
@route("/") #suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
181+
@patch(#{implicitOptionality: true}) op update(...Person): Person;
180182
`);
181183

182184
const response = res.paths["/"].patch.responses["200"].content["application/json"].schema;
@@ -217,6 +219,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
217219
weight: float64;
218220
}
219221
@post op create(...Widget): void;
222+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
220223
@patch(#{implicitOptionality: true}) op update(...Widget): void;
221224
`);
222225

@@ -358,6 +361,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
358361
@get get(...M): M;
359362
@post create(...M): M;
360363
@put createOrUpdate(...M): M;
364+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
361365
@patch(#{implicitOptionality: true}) update(...M): M;
362366
@delete delete(...M): void;
363367
}
@@ -367,6 +371,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
367371
@get get(...D): D;
368372
@post create(...D): D;
369373
@put createOrUpdate(...D): D;
374+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
370375
@patch(#{implicitOptionality: true}) update(...D): D;
371376
@delete delete(...D): void;
372377
}
@@ -376,6 +381,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
376381
@get op get(id: string): R;
377382
@post op create(...R): R;
378383
@put op createOrUpdate(...R): R;
384+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
379385
@patch(#{implicitOptionality: true}) op update(...R): R;
380386
@delete op delete(...D): void;
381387
}
@@ -384,6 +390,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
384390
@get op get(id: string): U;
385391
@post op create(...U): U;
386392
@put op createOrUpdate(...U): U;
393+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
387394
@patch(#{implicitOptionality: true}) op update(...U): U;
388395
@delete op delete(...U): void;
389396
}
@@ -1087,6 +1094,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
10871094
id: uuid;
10881095
}
10891096
1097+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
10901098
@patch(#{implicitOptionality: true}) op test(...Bar): Bar;
10911099
`);
10921100

@@ -1102,6 +1110,7 @@ worksFor(supportedVersions, ({ openApiFor }) => {
11021110
id: string;
11031111
}
11041112
1113+
#suppress "@typespec/http/deprecated-implicit-optionality" "testing legacy behavior"
11051114
@patch(#{implicitOptionality: true}) op test(bar: Bar): void;
11061115
11071116
model Foo {

0 commit comments

Comments
 (0)