x-ms-identifiers alternatives #1983
Conversation
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
AlitzelMendez
changed the title
AlitzelMendez
requested review from
bterlson,
markcowl,
allenjzhang and
timotheeguerin
as code owners
| } | ||
| }, | ||
| "x-ms-identifiers": [ | ||
| "id" |
There was a problem hiding this comment.
We do not need to write x-ms-identifiers if there is an "id" or "name" property
| @@ -1210,7 +1213,9 @@ | |||
| "items": { | |||
| "$ref": "#/definitions/Widget" | |||
| }, | |||
| "x-ms-identifiers": [] | |||
| "x-ms-identifiers": [ | |||
There was a problem hiding this comment.
See above. if the property is named "id" or "name", we should not add x-ms-identifiers
| if (!ifArrayItemContainsIdentifier(program, typespecType as any)) { | ||
|
|
||
| const armIdentifiers = getArmIdentifiers(program, typespecType); | ||
| if (armIdentifiers !== undefined && armIdentifiers.length > 0) { |
There was a problem hiding this comment.
Here you should filter out "name" and "id" and ensure that no extension is written if one of those key properties exist
| if (element === undefined || element.kind !== "Model") { | ||
| context.reportDiagnostic({ | ||
| messageId: "missingProperty", | ||
| format: { propertyName: prop, targetModelName: element?.name }, |
There was a problem hiding this comment.
"name" and "id" properties shoudll not cause this to execute, but should cause x-ms-identifiers to be omitted.
| ); | ||
| }; | ||
|
|
||
| export function getArmIdentifiers(program: Program, entity: ArrayModelType): string[] | undefined { |
There was a problem hiding this comment.
Need documentation comments to generate docs for this accessor, and describe the parameters and return type
| @@ -373,6 +376,93 @@ export const $armProviderNameValue: ArmProviderNameValueDecorator = ( | |||
| } | |||
| }; | |||
|
|
|||
| export const $identifiers: IdentifiersDecorator = ( | |||
There was a problem hiding this comment.
Need documentation comments to describe the use of the decorator and parameters
| armIdentifiersIncorrectEntity: | ||
| "The @identifiers decorator must be applied to a property that is an array of objects", | ||
| armIdentifiersProperties: | ||
| "The @identifiers decorator expects a parameter that is an array of strings or an empty array as a parameter.", |
There was a problem hiding this comment.
| "The @identifiers decorator expects a parameter that is an array of strings or an empty array as a parameter.", | |
| "The @identifiers decorator expects a parameter that is an array of strings or an empty array.", |
| @@ -353,6 +354,22 @@ See more details on [different Azure Resource Manager resource type here.](https | |||
|
|
|||
| None | |||
|
|
|||
| #### `@identifiers` | |||
|
|
|||
There was a problem hiding this comment.
See comments below - need jsdoc comment to generate this data
| @@ -823,6 +823,161 @@ describe("typespec-autorest: extension decorator", () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe("typespec-azure: identifiers decorator", () => { | |||
There was a problem hiding this comment.
Should add a test to verify that no extension is created when the object type contains a 'name' or 'id' property.
issue: #1473