Skip to content

Commit

Permalink
Adding sections for common lintdiff errors
Browse files Browse the repository at this point in the history
  • Loading branch information
markcowl committed Jan 30, 2024
2 parents 930fd46 + b0cb2f3 commit c92b8a2
Show file tree
Hide file tree
Showing 24 changed files with 328 additions and 1,288 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilly-foxes-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@azure-tools/typespec-client-generator-core": patch
---

add MultipartFile type
48 changes: 46 additions & 2 deletions docs/howtos/migrate-swagger/migrate-arm-tips.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Migrating Azure Resource Manager Swagger to TypeSpec
# Migrate ARm Specs

The Swagger Converter will not be able to accurately represent every part of every API in TypeSpec. This document
outlines some common changes you might need to make to a converted TypeSpec to make it conform to your existing service API and
Expand Down Expand Up @@ -174,11 +174,55 @@ You can generally choose an asynchronous operation template that matches your op

## Resolving Swagger LintDiff Violations

### VisibilityChanged for `nextLink` and `value` properties
### `VisibilityChanged` for `nextLink` and `value` properties

The issue is that some older specifications marked these values as read only. This has no real impact on the API or client generation, but it is easy to mitigate for the whole specification. To fix, simply add the following augment decorator statements to the `main.tsp` file.

```tsp
@@visibility(Azure.Core.Page.value, "read");
@@visibility(Azure.Core.Page.nextLink, "read");
```

### `ProvisioningStateMustBeReadOnly`

This violation is caused by a problem with the mechanism that ARM Api validation uses to determine if a [property is read-only. You can work around the issue by setting the `use-read-only-status-schema` configuration setting in `azure/tools/typespec-autorest` options to `true` in your `tspConfig.yaml` configuration file:

```yml
emit:
- "@azure-tools/typespec-autorest"
options:
"@azure-tools/typespec-autorest":
use-read-only-status-schema: true
```
### `LroLocationHeader`

This violation occurs when your spec uses an LRO operation template that follows the older version of LRO standards. Tof fix the issue, you would change the operation template to match the latest recommendation.

#### PUT Operations

```tsp
// LRO PUT template with required headers and no 200 response
createOrUpdate is ArmResourceCreateOrReplaceAsync<MyResource>;
```

#### PATCH Operations

```tsp
// LRO PATCH template with required headers, response codes, and lro options
update is ArmResourcePatchAsync<MyResource, MyResourceProperties>;
```

### POST(Action) Operations

```tsp
// LRO POST (Action) template with required headers, response codes, and lro options
doAction is ArmResourceActionAsync<MyResource, RequestModel, ResponseModel>;
```

### DELETE Operations

```tsp
// LRO delete template with required headers and no 200 response
delete is ArmResourceDeleteWithoutOKAsync<MyResource>;
```
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"key2913": "urperxmkkhhkp"
},
"location": "itajgxyqozseoygnl",
"id": "dnkyotqlrefuwxribpzbl",
"id": "/subscriptions/11809CA1-E126-4017-945E-AA795CD5C5A9/resourceGroups/rgopenapi/providers/Microsoft.Contoso/employees/le-8MU--J3W6q8D386p3-iT3",
"name": "xepyxhpb",
"type": "svvamxrdnnv",
"systemData": {
Expand Down Expand Up @@ -59,7 +59,7 @@
"key2913": "urperxmkkhhkp"
},
"location": "itajgxyqozseoygnl",
"id": "dnkyotqlrefuwxribpzbl",
"id": "/subscriptions/11809CA1-E126-4017-945E-AA795CD5C5A9/resourceGroups/rgopenapi/providers/Microsoft.Contoso/employees/9KF-f-8b",
"name": "xepyxhpb",
"type": "svvamxrdnnv",
"systemData": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"key2913": "urperxmkkhhkp"
},
"location": "itajgxyqozseoygnl",
"id": "dnkyotqlrefuwxribpzbl",
"id": "/subscriptions/11809CA1-E126-4017-945E-AA795CD5C5A9/resourceGroups/rgopenapi/providers/Microsoft.Contoso/employees/le-8MU--J3W6q8D386p3-iT3",
"name": "xepyxhpb",
"type": "svvamxrdnnv",
"systemData": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"key2913": "urperxmkkhhkp"
},
"location": "itajgxyqozseoygnl",
"id": "dnkyotqlrefuwxribpzbl",
"id": "/subscriptions/11809CA1-E126-4017-945E-AA795CD5C5A9/resourceGroups/rgopenapi/providers/Microsoft.Contoso/employees/test",
"name": "xepyxhpb",
"type": "svvamxrdnnv",
"systemData": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"key2913": "urperxmkkhhkp"
},
"location": "itajgxyqozseoygnl",
"id": "dnkyotqlrefuwxribpzbl",
"id": "/subscriptions/11809CA1-E126-4017-945E-AA795CD5C5A9/resourceGroups/rgopenapi/providers/Microsoft.Contoso/employees/test",
"name": "xepyxhpb",
"type": "svvamxrdnnv",
"systemData": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"key2913": "urperxmkkhhkp"
},
"location": "itajgxyqozseoygnl",
"id": "dnkyotqlrefuwxribpzbl",
"id": "/subscriptions/11809CA1-E126-4017-945E-AA795CD5C5A9/resourceGroups/contoso/providers/Microsoft.Contoso/employees/test",
"name": "xepyxhpb",
"type": "svvamxrdnnv",
"systemData": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"actionType": "Internal"
}
],
"nextLink": "bamebrbqkebjwevbq"
"nextLink": "https://sample.com/nextLink"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ jobs:
nodeVersion: "20.x"
- template: ./templates/build.yml

- script: node ./core/packages/internal-build-utils/cmd/cli.js bump-version-preview . ./core
- script: node ./core/packages/internal-build-utils/cmd/cli.js bump-version-preview .
displayName: Bump version to prerelease targets

- script: |
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/cadl-ranch-specs/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@azure-tools/typespec-e2e-cadl-ranch-specs",
"dependencies": {
"@azure-tools/cadl-ranch-specs": "0.28.7"
"@azure-tools/cadl-ranch-specs": "0.29.0"
},
"type": "module",
"private": true
Expand Down
2 changes: 1 addition & 1 deletion packages/samples/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"@typespec/http": "workspace:~0.52.0",
"@typespec/rest": "workspace:~0.52.0",
"@azure-tools/typespec-azure-core": "workspace:~0.38.0",
"@azure-tools/typespec-autorest": "workspace:~0.38.0",
"@azure-tools/typespec-autorest": "workspace:~0.38.1",
"@azure-tools/typespec-azure-resource-manager": "workspace:~0.38.0",
"@azure-tools/typespec-client-generator-core": "workspace:~0.38.0"
},
Expand Down
12 changes: 12 additions & 0 deletions packages/typespec-autorest/CHANGELOG.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
{
"name": "@azure-tools/typespec-autorest",
"entries": [
{
"version": "0.38.1",
"tag": "@azure-tools/typespec-autorest_v0.38.1",
"date": "Mon, 29 Jan 2024 22:16:39 GMT",
"comments": {
"patch": [
{
"comment": "Fix: Autorest emitter should generated format: decimal"
}
]
}
},
{
"version": "0.38.0",
"tag": "@azure-tools/typespec-autorest_v0.38.0",
Expand Down
9 changes: 8 additions & 1 deletion packages/typespec-autorest/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
# Change Log - @azure-tools/typespec-autorest

This log was last generated on Wed, 24 Jan 2024 05:47:18 GMT and should not be manually modified.
This log was last generated on Mon, 29 Jan 2024 22:16:39 GMT and should not be manually modified.

## 0.38.1
Mon, 29 Jan 2024 22:16:39 GMT

### Patches

- Fix: Autorest emitter should generated format: decimal

## 0.38.0
Wed, 24 Jan 2024 05:47:18 GMT
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-autorest/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@azure-tools/typespec-autorest",
"version": "0.38.0",
"version": "0.38.1",
"author": "Microsoft Corporation",
"description": "TypeSpec library for emitting openapi from the TypeSpec REST protocol binding",
"homepage": "https://azure.github.io/typespec-azure",
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-autorest/src/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2111,7 +2111,7 @@ function createOAPIEmitter(program: Program, options: ResolvedAutorestEmitterOpt
case "decimal":
return { type: "number", format: "decimal" };
case "decimal128":
return { type: "number", format: "decimal128" };
return { type: "number", format: "decimal" };
case "string":
return { type: "string" };
case "boolean":
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-autorest/test/primitive-types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe("typespec-autorest: primitives", () => {
["duration", { type: "string", format: "duration" }],
["bytes", { type: "string", format: "byte" }],
["decimal", { type: "number", format: "decimal" }],
["decimal128", { type: "number", format: "decimal128" }],
["decimal128", { type: "number", format: "decimal" }],
];

for (const test of cases) {
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-azure-playground-website/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"@typespec/http": "workspace:~0.52.0",
"@typespec/openapi": "workspace:~0.52.0",
"@typespec/openapi3": "workspace:~0.52.0",
"@azure-tools/typespec-autorest": "workspace:~0.38.0",
"@azure-tools/typespec-autorest": "workspace:~0.38.1",
"@azure-tools/typespec-azure-core": "workspace:~0.38.0",
"@azure-tools/typespec-azure-resource-manager": "workspace:~0.38.0",
"@azure-tools/typespec-client-generator-core": "workspace:~0.38.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/typespec-azure-resource-manager/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"peerDependencies": {
"@typespec/compiler": "workspace:~0.52.0",
"@azure-tools/typespec-azure-core": "~0.38.0",
"@azure-tools/typespec-autorest": "~0.38.0",
"@azure-tools/typespec-autorest": "~0.38.1",
"@typespec/openapi": "workspace:~0.52.0",
"@typespec/rest": "workspace:~0.52.0",
"@typespec/http": "workspace:~0.52.0",
Expand All @@ -64,7 +64,7 @@
"@typespec/compiler": "workspace:~0.52.0",
"@typespec/openapi": "workspace:~0.52.0",
"@azure-tools/typespec-azure-core": "workspace:~0.38.0",
"@azure-tools/typespec-autorest": "workspace:~0.38.0",
"@azure-tools/typespec-autorest": "workspace:~0.38.1",
"@typespec/rest": "workspace:~0.52.0",
"@typespec/http": "workspace:~0.52.0",
"@typespec/versioning": "workspace:~0.52.0",
Expand Down
12 changes: 10 additions & 2 deletions packages/typespec-client-generator-core/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ export type SdkType =
| SdkEnumValueType
| SdkConstantType
| SdkUnionType
| SdkModelType;
| SdkModelType
| SdkMultipartFileType;

export interface SdkBuiltInType extends SdkTypeBase {
kind: SdkBuiltInKinds;
Expand All @@ -110,7 +111,8 @@ export type SdkBuiltInKinds =
| "armId"
| "ipAddress"
| "azureLocation"
| "etag";
| "etag"
| "multipartFile";

const SdkDatetimeEncodingsConst = ["rfc3339", "rfc7231", "unixTimestamp"] as const;

Expand All @@ -130,6 +132,11 @@ export interface SdkDurationType extends SdkTypeBase {
wireType: SdkBuiltInType;
}

export interface SdkMultipartFileType extends SdkTypeBase {
kind: "multipartFile";
encode: "binary";
}

export interface SdkArrayType extends SdkTypeBase {
kind: "array";
valueType: SdkType;
Expand Down Expand Up @@ -188,6 +195,7 @@ export interface SdkModelType extends SdkTypeBase {
kind: "model";
properties: SdkModelPropertyType[];
name: string;
isFormDataType: boolean;
generatedName?: string;
description?: string;
details?: string;
Expand Down
7 changes: 7 additions & 0 deletions packages/typespec-client-generator-core/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ export const $lib = createTypeSpecLibrary({
wrongType: paramMessage`Encoding '${"encoding"}' cannot be used on type '${"type"}'`,
},
},
"conflicting-multipart-model-usage": {
severity: "error",
messages: {
default: "Invalid encoding",
wrongType: paramMessage`Model '${"modelName"}' cannot be used as both multipart/form-data input and regular body input. You can create a separate model with name 'model ${"modelName"}FormData' extends ${"modelName"} {}`,
},
},
"discriminator-not-constant": {
severity: "error",
messages: {
Expand Down
34 changes: 34 additions & 0 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import {
SdkEnumValueType,
SdkModelPropertyTypeBase,
SdkModelType,
SdkMultipartFileType,
SdkTupleType,
SdkType,
} from "./interfaces.js";
Expand Down Expand Up @@ -266,6 +267,13 @@ export function getSdkDurationType(context: SdkContext, type: Scalar): SdkDurati
};
}

function getSdkMultipartFileType(context: SdkContext, type: Scalar): SdkMultipartFileType {
return {
...getSdkTypeBaseHelper(context, type, "multipartFile"),
encode: "binary",
};
}

export function getSdkArrayOrDict(
context: SdkContext,
type: Model,
Expand Down Expand Up @@ -433,8 +441,24 @@ function addDiscriminatorToModelType(
export function getSdkModel(context: SdkContext, type: Model, operation?: Operation): SdkModelType {
type = getEffectivePayloadType(context, type);
let sdkType = context.modelsMap?.get(type) as SdkModelType | undefined;
const httpOperation = operation
? ignoreDiagnostics(getHttpOperation(context.program, operation))
: undefined;
const isFormDataType = httpOperation
? Boolean(httpOperation.parameters.body?.contentTypes.includes("multipart/form-data"))
: false;
if (sdkType) {
updateModelsMap(context, type, sdkType, operation);
if (isFormDataType !== sdkType.isFormDataType) {
// This means we have a model that is used both for formdata input and for regular body input
reportDiagnostic(context.program, {
code: "conflicting-multipart-model-usage",
target: type,
format: {
modelName: sdkType.name,
},
});
}
} else {
const docWrapper = getDocHelper(context, type);
sdkType = {
Expand All @@ -448,6 +472,7 @@ export function getSdkModel(context: SdkContext, type: Model, operation?: Operat
access: undefined, // dummy value since we need to update models map before we can set this
usage: UsageFlags.None, // dummy value since we need to update models map before we can set this
crossLanguageDefinitionId: getCrossLanguageDefinitionId(type),
isFormDataType,
};

updateModelsMap(context, type, sdkType, operation);
Expand Down Expand Up @@ -650,6 +675,15 @@ export function getClientType(context: SdkContext, type: Type, operation?: Opera
if (type.name === "duration") {
return getSdkDurationType(context, type);
}
const httpOperation = operation
? ignoreDiagnostics(getHttpOperation(context.program, operation))
: undefined;
const hasMultipartInput =
httpOperation &&
httpOperation.parameters.body?.contentTypes.includes("multipart/form-data");
if (type.name === "bytes" && hasMultipartInput) {
return getSdkMultipartFileType(context, type);
}
const scalarType = getSdkBuiltInType(context, type);
// just add default encode, normally encode is on extended scalar and model property
addEncodeInfo(context, type, scalarType);
Expand Down
Loading

0 comments on commit c92b8a2

Please sign in to comment.