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

ARM: Add put-operation-response-codes and post-operation-response-codes rules to allow disabling LintDiff rules #369

Merged
merged 6 commits into from
Mar 20, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-resource-manager"
---

Add `arm-put-operation-response-codes` and `arm-post-operation-response-codes` rules.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Available ruleSets:
| [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](/libraries/azure-resource-manager/rules/no-record.md) | Don't use Record types for ARM resources. |
| `@azure-tools/typespec-azure-resource-manager/arm-common-types-version` | Specify the ARM common-types version using @armCommonTypesVersion. |
| [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](/libraries/azure-resource-manager/rules/delete-operation-response-codes.md) | Ensure delete operations have the appropriate status codes. |
| [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](/libraries/azure-resource-manager/rules/put-operation-response-codes.md) | Ensure put operations have the appropriate status codes. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment` | `@armResourceAction` should not be used with `@segment`. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property` | Warn about duplicate properties in resources. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
title: post-operation-response-codes
---

```text title=- Full name-
@azure-tools/typespec-azure-resource-manager/post-operation-response-codes
```

## Synchronous

Synchronous post operations should have one of the following combinations of responses - 200 and default, or 204 and default. They must have no other responses.

#### ❌ Incorrect

```tsp
@armResourceOperations
interface Employees {
@armResourceDelete(Employee)
delete(...ApiVersionParameter): {
@statusCode _: 200;
result: boolean;
};
}
```

#### ✅ Correct

```tsp
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteSync<Employee>;
}
```

## Asynchronous

Long-running (LRO) post operations should have 202 and default responses. The must also have a 200 response only if the final response is intended to have a schema. They must have no other responses. The 202 response must not have a response schema specified.

#### ❌ Incorrect

```tsp
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteAsync<Employee>;
}
```

#### ✅ Correct

```tsp
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteWithoutOkAsync<Employee>;
}
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
title: put-operation-response-codes
---

```text title=- Full name-
@azure-tools/typespec-azure-resource-manager/put-operation-response-codes
```

Put operations should use the `ArmResourceCreateOrUpdateAsync`, `ArmResourceCreateOrReplaceAsync` or `ArmResourceCreateOrReplaceSync` template. They must have 200, 201, default and no other responses.
tjprescott marked this conversation as resolved.
Show resolved Hide resolved

#### ❌ Incorrect

```tsp
@armResourceOperations
interface Employees {
@armResourceCreateOrUpdate(Employee)
createOrUpdate(...ApiVersionParameter): {
@statusCode _: 200;
result: boolean;
};
}
```

#### ✅ Correct

```tsp
@armResourceOperations
interface Employees {
createOrUpdate is ArmResourceCreateOrUpdateAsync<Employee>;
}
```
1 change: 1 addition & 0 deletions packages/typespec-azure-resource-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Available ruleSets:
| [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-record) | Don't use Record types for ARM resources. |
| `@azure-tools/typespec-azure-resource-manager/arm-common-types-version` | Specify the ARM common-types version using @armCommonTypesVersion. |
| [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/delete-operation-response-codes) | Ensure delete operations have the appropriate status codes. |
| [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/put-operation-response-codes) | Ensure put operations have the appropriate status codes. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment` | `@armResourceAction` should not be used with `@segment`. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property` | Warn about duplicate properties in resources. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. |
Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-azure-resource-manager/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { defineLinter } from "@typespec/compiler";
import { armCommonTypesVersionRule } from "./rules/arm-common-types-version.js";
import { armDeleteResponseCodesRule } from "./rules/arm-delete-response-codes.js";
import { armNoRecordRule } from "./rules/arm-no-record.js";
import { armPutResponseCodesRule } from "./rules/arm-put-response-codes.js";
import { armResourceActionNoSegmentRule } from "./rules/arm-resource-action-no-segment.js";
import { armResourceDuplicatePropertiesRule } from "./rules/arm-resource-duplicate-property.js";
import { interfacesRule } from "./rules/arm-resource-interfaces.js";
Expand Down Expand Up @@ -30,6 +31,7 @@ const rules = [
armNoRecordRule,
armCommonTypesVersionRule,
armDeleteResponseCodesRule,
armPutResponseCodesRule,
armResourceActionNoSegmentRule,
armResourceDuplicatePropertiesRule,
armResourceEnvelopeProperties,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Model, createRule } from "@typespec/compiler";

import { getLroMetadata } from "@azure-tools/typespec-azure-core";
import { getHttpOperation } from "@typespec/http";
import { getArmResources } from "../resource.js";

/**
Expand All @@ -24,7 +23,7 @@ export const armDeleteResponseCodesRule = createRule({
if (armResource && armResource.operations.lifecycle.delete) {
const deleteOperation = armResource.operations.lifecycle.delete;
const isAsync = getLroMetadata(context.program, deleteOperation.operation) !== undefined;
const [httpOp, _] = getHttpOperation(context.program, deleteOperation.operation);
const httpOp = deleteOperation.httpOperation;
const statusCodes = new Set([...httpOp.responses.map((r) => r.statusCodes.toString())]);
const expected = new Set(["204", "*"]);
expected.add(isAsync ? "202" : "200");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { Program, createRule } from "@typespec/compiler";

import { getLroMetadata } from "@azure-tools/typespec-azure-core";
import { HttpOperationBody, HttpOperationResponse } from "@typespec/http";
import { ArmResourceOperation } from "../operations.js";
import { getArmResources } from "../resource.js";

/**
* Verify that a post operation has the correct response codes.
*/
export const armPostResponseCodesRule = createRule({
name: "arm-post-operation-response-codes",
severity: "warning",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/post-operation-response-codes",
description: "Ensure post operations have the appropriate status codes.",
messages: {
sync: `Synchronous post operations must have a 200 or 204 response and a default response. They must not have any other responses.`,
async: `Long-running post operations must have 202 and default responses. They must also have a 200 response if the final response has a schema. They must not have any other responses.`,
},
create(context) {
function getResponseBody(
response: HttpOperationResponse | undefined
): HttpOperationBody | undefined {
if (response === undefined) return undefined;
if (response.responses.length > 1) {
throw new Error("Multiple responses are not supported.");
}
if (response.responses[0].body !== undefined) {
return response.responses[0].body;
}
return undefined;
}

function validateAsyncPost(op: ArmResourceOperation) {
const statusCodes = new Set([
...op.httpOperation.responses.map((r) => r.statusCodes.toString()),
]);
// validate that there are 202 and * status codes, and maybe 200
const expected =
statusCodes.size === 2 ? new Set(["202", "*"]) : new Set(["202", "200", "*"]);
if (statusCodes.size !== expected.size || ![...statusCodes].every((v) => expected.has(v))) {
context.reportDiagnostic({
target: op.operation,
messageId: "async",
});
}
// validate that 202 does not have a schema
const response202 = op.httpOperation.responses.find((r) => r.statusCodes === 202);
const body202 = getResponseBody(response202);
if (body202 !== undefined) {
context.reportDiagnostic({
target: op.operation.returnType,
messageId: "async",
});
}
// validate that a 200 response does have a schema
const response200 = op.httpOperation.responses.find((r) => r.statusCodes === 200);
const body200 = getResponseBody(response200);
if (response200 && body200 === undefined) {
context.reportDiagnostic({
target: op.operation.returnType,
messageId: "async",
});
}
}

function validateSyncPost(op: ArmResourceOperation) {
const allowed = [new Set(["200", "*"]), new Set(["204", "*"])];
const statusCodes = new Set([
...op.httpOperation.responses.map((r) => r.statusCodes.toString()),
]);
if (
!allowed.some(
(expected) =>
statusCodes.size === expected.size && [...statusCodes].every((v) => expected.has(v))
)
) {
context.reportDiagnostic({
target: op.operation,
messageId: "sync",
});
}
}

return {
root: (program: Program) => {
const resources = getArmResources(program);
for (const resource of resources) {
const operations = [
resource.operations.lifecycle.createOrUpdate,
resource.operations.lifecycle.update,
...Object.values(resource.operations.actions),
];
for (const op of operations) {
if (op === undefined || op.httpOperation.verb !== "post") {
continue;
}
const isAsync = getLroMetadata(context.program, op.operation) !== undefined;
if (isAsync) {
validateAsyncPost(op);
} else {
validateSyncPost(op);
}
}
}
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { Program, createRule } from "@typespec/compiler";

import { getArmResources } from "../resource.js";

/**
* Verify that a put operation has the correct response codes.
*/
export const armPutResponseCodesRule = createRule({
name: "arm-put-operation-response-codes",
severity: "warning",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/put-operation-response-codes",
description: "Ensure put operations have the appropriate status codes.",
messages: {
default: `Put operations must have 200, 201 and default responses. They must not have any other responses.`,
},
create(context) {
return {
root: (program: Program) => {
const resources = getArmResources(program);
const expected = new Set(["200", "201", "*"]);
for (const resource of resources) {
const operations = [
resource.operations.lifecycle.createOrUpdate,
resource.operations.lifecycle.update,
...Object.values(resource.operations.actions),
];
for (const op of operations) {
if (op === undefined) {
continue;
}
if (op.httpOperation.verb === "put") {
const statusCodes = new Set([
...op.httpOperation.responses.map((r) => r.statusCodes.toString()),
]);
if (
statusCodes.size !== expected.size ||
![...statusCodes].every((v) => expected.has(v))
) {
context.reportDiagnostic({
target: op.operation,
});
}
}
}
}
},
};
},
});
Loading
Loading