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

Fix linting rule post operation #1690

Closed
Closed
Show file tree
Hide file tree
Changes from 11 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: fix
packages:
- "@azure-tools/typespec-azure-resource-manager"
---

Fix `arm-resource-invalid-action-verb` rule to trigger on `@get` operations on ArmProviderAction
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation` | Check for resources that must have a delete operation. |
| `@azure-tools/typespec-azure-resource-manager/empty-updateable-properties` | Should have updateable properties. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator` | Each resource interface must have an @armResourceOperations decorator. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb` | Actions must be HTTP Post operations. |
| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb`](/libraries/azure-resource-manager/rules/arm-resource-invalid-action-verb.md) | Actions must be HTTP Post operations. |
| `@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. |
| [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](/libraries/azure-resource-manager/rules/lro-location-header.md) | A 202 response should include a Location response header. |
| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Array properties should describe their identifying properties with x-ms-identifiers. Decorate the property with @OpenAPI.extension("x-ms-identifiers", [id-prop]) where "id-prop" is a list of the names of identifying properties in the item type. |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
title: arm-resource-invalid-action-verb
---

```text title=- Full name-
@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb
```

For ARM http operations, the action verb must be `@post`. Any other action verb is flagged as incorrect.

#### ❌ Incorrect

```tsp
@get getAction is ArmProviderActionAsync<
AlitzelMendez marked this conversation as resolved.
Show resolved Hide resolved
{
name: string;
},
ArmCombinedLroHeaders,
SubscriptionActionScope
>;
```

#### ✅ Correct

```tsp
postAction is ArmProviderActionAsync<
AlitzelMendez marked this conversation as resolved.
Show resolved Hide resolved
{
name: string;
},
ArmCombinedLroHeaders,
SubscriptionActionScope
>;
```
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,31 @@ model LogAnalyticsCollection is Page<LogAnalyticsOperationResult>;
@armResourceOperations
interface ProviderOperations {
/** Operation to get virtual machines for subscription (/subscriptions/{subscriptionId}/providers/Microsoft.ContosoProviderHub/getVmSizes) */
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb" "Action can be used for get"
@get
getVmSizes is ArmProviderActionSync<Response = VmSizeCollection, Scope = SubscriptionActionScope>;
/** Operation to get virtual machines for tenant (/providers/Microsoft.ContosoProviderHub/getVmSizesTenant) */
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb" "Action can be used for get"
@get
getVmSizesTenant is ArmProviderActionSync<Response = VmSizeCollection, Scope = TenantActionScope>;
/** Operation to get virtual machines for subscription for specific location (/subscriptions/{subscriptionId}/providers/Microsoft.ContosoProviderHub/locations/{location}/getVmSizesLocation) */
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb" "Action can be used for get"
@get
getVmSizesLocation is ArmProviderActionSync<
Response = VmSizeCollection,
Scope = SubscriptionActionScope,
Parameters = LocationParameter
>;
/** Operation to get throttled requests sharing action (/subscriptions/{subscriptionId}/providers/Microsoft.ContosoProviderHub/logAnalytics/apiAccess/getThrottledRequests) */
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb" "Action can be used for get"
@get
@action("logAnalytics/apiAccess/getThrottledRequests")
getThrottledRequestsSubscription is ArmProviderActionSync<
Response = LogAnalyticsCollection,
Scope = SubscriptionActionScope
>;
/** Operation to get throttled requests sharing action for tenant (/providers/Microsoft.ContosoProviderHub/logAnalytics/apiAccess/getThrottledRequests) */
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb" "Action can be used for get"
@get
@action("logAnalytics/apiAccess/getThrottledRequests")
getThrottledRequestsTenant is ArmProviderActionSync<
Expand Down
2 changes: 1 addition & 1 deletion packages/typespec-azure-resource-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation` | Check for resources that must have a delete operation. |
| `@azure-tools/typespec-azure-resource-manager/empty-updateable-properties` | Should have updateable properties. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator` | Each resource interface must have an @armResourceOperations decorator. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb` | Actions must be HTTP Post operations. |
| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-invalid-action-verb) | Actions must be HTTP Post operations. |
| `@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. |
| [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/lro-location-header) | A 202 response should include a Location response header. |
| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/missing-x-ms-identifiers) | Array properties should describe their identifying properties with x-ms-identifiers. Decorate the property with @OpenAPI.extension("x-ms-identifiers", [id-prop]) where "id-prop" is a list of the names of identifying properties in the item type. |
Expand Down
4 changes: 2 additions & 2 deletions packages/typespec-azure-resource-manager/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ 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";
import { invalidActionVerbRule } from "./rules/arm-resource-invalid-action-verb.js";
import { armResourceInvalidActionVerbRule } from "./rules/arm-resource-invalid-action-verb.js";
import { armResourceEnvelopeProperties } from "./rules/arm-resource-invalid-envelope-property.js";
import { armResourceInvalidVersionFormatRule } from "./rules/arm-resource-invalid-version-format.js";
import { armResourceKeyInvalidCharsRule } from "./rules/arm-resource-key-invalid-chars.js";
Expand Down Expand Up @@ -51,7 +51,7 @@ const rules = [
deleteOperationMissingRule,
envelopePropertiesRules,
interfacesRule,
invalidActionVerbRule,
armResourceInvalidActionVerbRule,
listBySubscriptionRule,
lroLocationHeaderRule,
missingXmsIdentifiersRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,24 @@
import { Operation, createRule } from "@typespec/compiler";
import { getOperationVerb } from "@typespec/http";

import { getActionDetails } from "@typespec/rest";
import { isInternalTypeSpec, isSourceOperationResourceManagerInternal } from "./utils.js";

export const invalidActionVerbRule = createRule({
export const armResourceInvalidActionVerbRule = createRule({
name: "arm-resource-invalid-action-verb",
AlitzelMendez marked this conversation as resolved.
Show resolved Hide resolved
severity: "warning",
description: "Actions must be HTTP Post operations.",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-invalid-action-verb",
messages: {
default: "Actions must be HTTP Post operations.",
},
create(context) {
return {
operation: (operation: Operation) => {
if (
!isInternalTypeSpec(context.program, operation) &&
!isSourceOperationResourceManagerInternal(operation)
) {
const actionType = getActionDetails(context.program, operation);
const verb = getOperationVerb(context.program, operation);
if (actionType !== undefined && verb !== "post") {
context.reportDiagnostic({
target: operation,
});
}
const actionType = getActionDetails(context.program, operation);
const verb = getOperationVerb(context.program, operation);
if (actionType !== undefined && verb !== "post") {
context.reportDiagnostic({
target: operation,
});
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import { getHttpOperation } from "@typespec/http";
import { strictEqual } from "assert";
import { beforeEach, describe, it } from "vitest";
import { invalidActionVerbRule } from "../../src/rules/arm-resource-invalid-action-verb.js";
import { armResourceInvalidActionVerbRule } from "../../src/rules/arm-resource-invalid-action-verb.js";
import { listBySubscriptionRule } from "../../src/rules/list-operation.js";
import { createAzureResourceManagerTestRunner } from "../test-host.js";

Expand All @@ -19,7 +19,7 @@ describe("typespec-azure-resource-manager: detect non-post actions", () => {
runner = await createAzureResourceManagerTestRunner();
tester = createLinterRuleTester(
runner,
invalidActionVerbRule,
armResourceInvalidActionVerbRule,
"@azure-tools/typespec-azure-resource-manager",
);
});
Expand Down Expand Up @@ -87,8 +87,7 @@ describe("typespec-azure-resource-manager: detect non-post actions", () => {
message: "Actions must be HTTP Post operations.",
});
});

it("Allows post actions for authorized provider actions", async () => {
it("Detects non-post actions for internal operations", async () => {
await tester
.expect(
`
Expand All @@ -103,13 +102,13 @@ describe("typespec-azure-resource-manager: detect non-post actions", () => {
}

interface Operations extends Azure.ResourceManager.Operations {}

@doc("The VM Size")
model VmSize {
@doc("number of cpus ")
cpus: int32;
}

@armResourceOperations
interface ProviderOperations {
@get
Expand All @@ -118,7 +117,10 @@ describe("typespec-azure-resource-manager: detect non-post actions", () => {
}
`,
)
.toBeValid();
.toEmitDiagnostics({
code: "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb",
message: "Actions must be HTTP Post operations.",
});
});
});

Expand Down
Loading