From e6b4cdb15a3e4eb4ad591a3b85b6d338a8ebd939 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Fri, 23 Feb 2024 09:13:20 -0800 Subject: [PATCH 01/10] Add rule for `AvoidAdditionalProperties`. --- .../src/linter.ts | 2 + .../rules/arm-avoid-additional-properties.ts | 40 ++++++++ .../arm-avoid-additional-properties.test.ts | 99 +++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts create mode 100644 packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 03b76ca9f8..007c692a33 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -1,4 +1,5 @@ import { defineLinter } from "@typespec/compiler"; +import { armAvoidAdditionalPropertiesRule } from "./rules/arm-avoid-additional-properties.js"; import { armCommonTypesVersionRule } from "./rules/arm-common-types-version.js"; import { armResourceActionNoSegmentRule } from "./rules/arm-resource-action-no-segment.js"; import { armResourceDuplicatePropertiesRule } from "./rules/arm-resource-duplicate-property.js"; @@ -25,6 +26,7 @@ import { retryAfterRule } from "./rules/retry-after.js"; import { unsupportedTypeRule } from "./rules/unsupported-type.js"; const rules = [ + armAvoidAdditionalPropertiesRule, armCommonTypesVersionRule, armResourceActionNoSegmentRule, armResourceDuplicatePropertiesRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts b/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts new file mode 100644 index 0000000000..878ff46c9f --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts @@ -0,0 +1,40 @@ +import { Model, SemanticNodeListener, createRule } from "@typespec/compiler"; + +function isRecordType(model: Model): boolean { + return model.name === "Record"; +} + +export const armAvoidAdditionalPropertiesRule = createRule({ + name: "arm-avoid-additional-properties", + severity: "warning", + description: "Avoid use of additional properties in ARM resources.", + messages: { + default: "...", + }, + create(context): SemanticNodeListener { + return { + model: (model: Model) => { + for (const prop of model.properties.values()) { + if (prop.type.kind === "Model" && isRecordType(prop.type)) { + context.reportDiagnostic({ + code: "arm-avoid-additional-properties", + target: prop, + }); + } + } + if (model.baseModel !== undefined && isRecordType(model.baseModel)) { + context.reportDiagnostic({ + code: "arm-avoid-additional-properties", + target: model, + }); + } + if (model.sourceModel !== undefined && isRecordType(model.sourceModel)) { + context.reportDiagnostic({ + code: "arm-avoid-additional-properties", + target: model, + }); + } + }, + }; + }, +}); diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts new file mode 100644 index 0000000000..c836b6267e --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts @@ -0,0 +1,99 @@ +import { + BasicTestRunner, + LinterRuleTester, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, describe, it } from "vitest"; +import { armAvoidAdditionalPropertiesRule } from "../../src/rules/arm-avoid-additional-properties.js"; +import { createAzureResourceManagerTestRunner } from "../test-host.js"; + +describe("typespec-azure-resource-manager: ARM avoid-additional-properties rule", () => { + let runner: BasicTestRunner; + let tester: LinterRuleTester; + + beforeEach(async () => { + runner = await createAzureResourceManagerTestRunner(); + tester = createLinterRuleTester( + runner, + armAvoidAdditionalPropertiesRule, + "@azure-tools/typespec-azure-resource-manager" + ); + }); + + it("emits diagnostic when a model property uses Record type", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + @Azure.ResourceManager.tenantResource + model Widget is ProxyResource { + @key("widgetName") + @segment("widgets") + @path + @visibility("read") + name: string; + + props: Record; + } + + model WidgetProperties {} + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-avoid-additional-properties", + }); + }); + + it("emits diagnostic when a model extends Record type", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + @Azure.ResourceManager.tenantResource + model Widget is ProxyResource { + @key("widgetName") + @segment("widgets") + @path + @visibility("read") + name: string; + } + + model WidgetProperties extends Record {} + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-avoid-additional-properties", + }); + }); + + it("emits diagnostic when a model is Record type", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + @Azure.ResourceManager.tenantResource + model Widget is ProxyResource { + @key("widgetName") + @segment("widgets") + @path + @visibility("read") + name: string; + } + + model WidgetProperties is Record; + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-avoid-additional-properties", + }); + }); +}); From 753c864b85d7cea1aa7eeb1576f8128980c6b1d0 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Fri, 23 Feb 2024 10:23:29 -0800 Subject: [PATCH 02/10] Add docs. --- ...AdditionalProperties-2024-1-23-10-13-55.md | 7 +++++ .../rules/avoid-additional-properties.md | 27 +++++++++++++++++++ .../rules/arm-avoid-additional-properties.ts | 9 ++++++- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 .chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md create mode 100644 docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md diff --git a/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md b/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md new file mode 100644 index 0000000000..5d46bc36f9 --- /dev/null +++ b/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" +--- + +Add `arm-avoid-additional-properties` rule. \ No newline at end of file diff --git a/docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md b/docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md new file mode 100644 index 0000000000..e5f2336925 --- /dev/null +++ b/docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md @@ -0,0 +1,27 @@ +--- +title: avoid-additional-properties +--- + +```text title=- Full name- +@azure-tools/typespec-azure-resource-manager/avoid-additional-properties +``` + +ARM requires Resource provider teams to define types explicitly. This is to ensure good customer experience in terms of the discoverability of concrete type definitions. + +#### ❌ Incorrect + +```tsp +model Address extends Record {} +``` + +#### ✅ Correct + +```tsp +model Address { + street: string; + city: string; + state: string; + country: string; + postalCode: string; +} +``` diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts b/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts index 878ff46c9f..d0b0336182 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts @@ -8,8 +8,13 @@ export const armAvoidAdditionalPropertiesRule = createRule({ name: "arm-avoid-additional-properties", severity: "warning", description: "Avoid use of additional properties in ARM resources.", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/avoid-additional-properties", messages: { - default: "...", + default: + "Definitions must not be of type Record. ARM requires Resource provider teams to define types explicitly.", + extends: + "Definitions must not extend type Record. ARM requires Resource provider teams to define types explicitly.", + is: "Definitions must not equate to type Record. ARM requires Resource provider teams to define types explicitly.", }, create(context): SemanticNodeListener { return { @@ -26,12 +31,14 @@ export const armAvoidAdditionalPropertiesRule = createRule({ context.reportDiagnostic({ code: "arm-avoid-additional-properties", target: model, + messageId: "extends", }); } if (model.sourceModel !== undefined && isRecordType(model.sourceModel)) { context.reportDiagnostic({ code: "arm-avoid-additional-properties", target: model, + messageId: "is", }); } }, From d504958728784c0ab35b033616e1cfeb9d9e2700 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Mon, 26 Feb 2024 09:11:10 -0800 Subject: [PATCH 03/10] Rename rule to "arm-no-record" --- ...AdditionalProperties-2024-1-23-10-13-55.md | 2 +- .../reference/linter.md | 53 ++++++++------- .../rules/avoid-additional-properties.md | 27 -------- .../azure-resource-manager/rules/no-record.md | 67 +++++++++++++++++++ .../typespec-azure-resource-manager/README.md | 53 ++++++++------- .../src/linter.ts | 4 +- ...itional-properties.ts => arm-no-record.ts} | 14 ++-- ...operties.test.ts => arm-no-record.test.ts} | 12 ++-- 8 files changed, 137 insertions(+), 95 deletions(-) delete mode 100644 docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md create mode 100644 docs/libraries/azure-resource-manager/rules/no-record.md rename packages/typespec-azure-resource-manager/src/rules/{arm-avoid-additional-properties.ts => arm-no-record.ts} (77%) rename packages/typespec-azure-resource-manager/test/rules/{arm-avoid-additional-properties.test.ts => arm-no-record.test.ts} (88%) diff --git a/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md b/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md index 5d46bc36f9..3a41df0cf2 100644 --- a/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md +++ b/.chronus/changes/AvoidAdditionalProperties-2024-1-23-10-13-55.md @@ -4,4 +4,4 @@ packages: - "@azure-tools/typespec-azure-resource-manager" --- -Add `arm-avoid-additional-properties` rule. \ No newline at end of file +Add `arm-no-record` rule. diff --git a/docs/libraries/azure-resource-manager/reference/linter.md b/docs/libraries/azure-resource-manager/reference/linter.md index 89d4322c77..ebb39d2951 100644 --- a/docs/libraries/azure-resource-manager/reference/linter.md +++ b/docs/libraries/azure-resource-manager/reference/linter.md @@ -24,29 +24,30 @@ Available ruleSets: ## Rules -| Name | Description | -| ---------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | -| `@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-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. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | -| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | -| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | -| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Azure services should not use enums. | -| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | -| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | -| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | -| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | -| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | -| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](/libraries/azure-resource-manager/rules/unsupported-type.md) | Check for unsupported ARM types. | +| Name | Description | +| ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | +| `@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-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. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | +| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | +| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | +| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Azure services should not use enums. | +| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | +| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | +| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | +| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | +| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | +| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](/libraries/azure-resource-manager/rules/unsupported-type.md) | Check for unsupported ARM types. | diff --git a/docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md b/docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md deleted file mode 100644 index e5f2336925..0000000000 --- a/docs/libraries/azure-resource-manager/rules/avoid-additional-properties.md +++ /dev/null @@ -1,27 +0,0 @@ ---- -title: avoid-additional-properties ---- - -```text title=- Full name- -@azure-tools/typespec-azure-resource-manager/avoid-additional-properties -``` - -ARM requires Resource provider teams to define types explicitly. This is to ensure good customer experience in terms of the discoverability of concrete type definitions. - -#### ❌ Incorrect - -```tsp -model Address extends Record {} -``` - -#### ✅ Correct - -```tsp -model Address { - street: string; - city: string; - state: string; - country: string; - postalCode: string; -} -``` diff --git a/docs/libraries/azure-resource-manager/rules/no-record.md b/docs/libraries/azure-resource-manager/rules/no-record.md new file mode 100644 index 0000000000..823b3b1524 --- /dev/null +++ b/docs/libraries/azure-resource-manager/rules/no-record.md @@ -0,0 +1,67 @@ +--- +title: no-record +--- + +```text title=- Full name- +@azure-tools/typespec-azure-resource-manager/no-record +``` + +ARM requires Resource provider teams to define types explicitly. This is to ensure good customer experience in terms of the discoverability of concrete type definitions. + +#### ❌ Incorrect + +```tsp +model Address extends Record {} +``` + +#### ✅ Correct + +```tsp +model Address { + street: string; + city: string; + state: string; + country: string; + postalCode: string; +} +``` + +#### ❌ Incorrect + +```tsp +model Address { + address: Record; + city: string; + state: string; +} +``` + +#### ✅ Correct + +```tsp +model Address { + street: string; + city: string; + state: string; + country: string; + postalCode: string; +} +``` + +#### ❌ Incorrect + +```tsp +model Address is Record; +``` + +#### ✅ Correct + +```tsp +model Address { + street: string; + city: string; + state: string; + country: string; + postalCode: string; +} +``` diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 4f1bf21ae5..b36c354fe4 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -28,32 +28,33 @@ Available ruleSets: ### Rules -| Name | Description | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | -| `@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-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. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | -| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | -| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | -| [`@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) | Azure services should not use enums. | -| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | -| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | -| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | -| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | -| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | -| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. | +| Name | Description | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | +| `@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-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. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | +| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | +| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | +| [`@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) | Azure services should not use enums. | +| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | +| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | +| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | +| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | +| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | +| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. | ## Decorators diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 007c692a33..13750e3052 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -1,6 +1,6 @@ import { defineLinter } from "@typespec/compiler"; -import { armAvoidAdditionalPropertiesRule } from "./rules/arm-avoid-additional-properties.js"; import { armCommonTypesVersionRule } from "./rules/arm-common-types-version.js"; +import { armNoRecordRule } from "./rules/arm-no-record.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"; @@ -26,7 +26,7 @@ import { retryAfterRule } from "./rules/retry-after.js"; import { unsupportedTypeRule } from "./rules/unsupported-type.js"; const rules = [ - armAvoidAdditionalPropertiesRule, + armNoRecordRule, armCommonTypesVersionRule, armResourceActionNoSegmentRule, armResourceDuplicatePropertiesRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts similarity index 77% rename from packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts rename to packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts index d0b0336182..678ed5fcce 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-avoid-additional-properties.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts @@ -4,11 +4,11 @@ function isRecordType(model: Model): boolean { return model.name === "Record"; } -export const armAvoidAdditionalPropertiesRule = createRule({ - name: "arm-avoid-additional-properties", +export const armNoRecordRule = createRule({ + name: "arm-no-record", severity: "warning", - description: "Avoid use of additional properties in ARM resources.", - url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/avoid-additional-properties", + description: "Don't use Record types for ARM resources.", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-record", messages: { default: "Definitions must not be of type Record. ARM requires Resource provider teams to define types explicitly.", @@ -22,21 +22,21 @@ export const armAvoidAdditionalPropertiesRule = createRule({ for (const prop of model.properties.values()) { if (prop.type.kind === "Model" && isRecordType(prop.type)) { context.reportDiagnostic({ - code: "arm-avoid-additional-properties", + code: "arm-no-record", target: prop, }); } } if (model.baseModel !== undefined && isRecordType(model.baseModel)) { context.reportDiagnostic({ - code: "arm-avoid-additional-properties", + code: "arm-no-record", target: model, messageId: "extends", }); } if (model.sourceModel !== undefined && isRecordType(model.sourceModel)) { context.reportDiagnostic({ - code: "arm-avoid-additional-properties", + code: "arm-no-record", target: model, messageId: "is", }); diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts similarity index 88% rename from packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts rename to packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index c836b6267e..5896238bc9 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-avoid-additional-properties.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -4,10 +4,10 @@ import { createLinterRuleTester, } from "@typespec/compiler/testing"; import { beforeEach, describe, it } from "vitest"; -import { armAvoidAdditionalPropertiesRule } from "../../src/rules/arm-avoid-additional-properties.js"; +import { armNoRecordRule } from "../../src/rules/arm-no-record.js"; import { createAzureResourceManagerTestRunner } from "../test-host.js"; -describe("typespec-azure-resource-manager: ARM avoid-additional-properties rule", () => { +describe("typespec-azure-resource-manager: ARM no-record rule", () => { let runner: BasicTestRunner; let tester: LinterRuleTester; @@ -15,7 +15,7 @@ describe("typespec-azure-resource-manager: ARM avoid-additional-properties rule" runner = await createAzureResourceManagerTestRunner(); tester = createLinterRuleTester( runner, - armAvoidAdditionalPropertiesRule, + armNoRecordRule, "@azure-tools/typespec-azure-resource-manager" ); }); @@ -43,7 +43,7 @@ describe("typespec-azure-resource-manager: ARM avoid-additional-properties rule" ` ) .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-avoid-additional-properties", + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", }); }); @@ -68,7 +68,7 @@ describe("typespec-azure-resource-manager: ARM avoid-additional-properties rule" ` ) .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-avoid-additional-properties", + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", }); }); @@ -93,7 +93,7 @@ describe("typespec-azure-resource-manager: ARM avoid-additional-properties rule" ` ) .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-avoid-additional-properties", + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", }); }); }); From bcef7ea1db19c234610a139138d0516bd068ccd0 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Mon, 26 Feb 2024 09:27:50 -0800 Subject: [PATCH 04/10] Disable Azure.Core `bad-record-type` rule for ARM. --- .../reference/linter.md | 54 +++++++++---------- .../typespec-azure-resource-manager/README.md | 54 +++++++++---------- .../src/linter.ts | 4 ++ 3 files changed, 58 insertions(+), 54 deletions(-) diff --git a/docs/libraries/azure-resource-manager/reference/linter.md b/docs/libraries/azure-resource-manager/reference/linter.md index ebb39d2951..0f29c5088d 100644 --- a/docs/libraries/azure-resource-manager/reference/linter.md +++ b/docs/libraries/azure-resource-manager/reference/linter.md @@ -24,30 +24,30 @@ Available ruleSets: ## Rules -| Name | Description | -| ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | -| `@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-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. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | -| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | -| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | -| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Azure services should not use enums. | -| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | -| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | -| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | -| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | -| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | -| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](/libraries/azure-resource-manager/rules/unsupported-type.md) | Check for unsupported ARM types. | +| Name | Description | +| ---------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | +| [`@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-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. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | +| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | +| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | +| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Azure services should not use enums. | +| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | +| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | +| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | +| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | +| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | +| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](/libraries/azure-resource-manager/rules/unsupported-type.md) | Check for unsupported ARM types. | diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index b36c354fe4..49d3ee7f13 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -28,33 +28,33 @@ Available ruleSets: ### Rules -| Name | Description | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | -| `@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-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. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | -| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | -| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | -| [`@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) | Azure services should not use enums. | -| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | -| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | -| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | -| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | -| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | -| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | -| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. | +| Name | Description | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------- | +| [`@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-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. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | +| `@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels` | Tracked Resources must use 3 or fewer levels of nesting. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-operation` | Validate ARM Resource operations. | +| `@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/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. | +| [`@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) | Azure services should not use enums. | +| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. | +| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. | +| `@azure-tools/typespec-azure-resource-manager/patch-envelope` | Patch envelope properties should match the resource properties. | +| `@azure-tools/typespec-azure-resource-manager/arm-resource-patch` | Validate ARM PATCH operations. | +| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. | +| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. | +| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. | ## Decorators diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 13750e3052..6361bdcc6b 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -66,6 +66,10 @@ export const $linter = defineLinter({ // TODO: Enable this rule once azure-rest-api-specs repo is ready (issue #3839) [`@azure-tools/typespec-azure-resource-manager/${armCommonTypesVersionRule.name}`]: false, }, + disable: { + [`@azure-tools/typespec-azure-core/bad-record-type`]: + "This clashes with the ARM `no-record` rule.", + }, extends: ["@azure-tools/typespec-azure-core/all"], }, }, From e3fb115b96506a0da47ac70107dfd94215a03f5e Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Tue, 27 Feb 2024 12:34:33 -0800 Subject: [PATCH 05/10] Simplify tests. --- .../src/rules/arm-no-record.ts | 6 +- .../test/rules/arm-no-record.test.ts | 131 +++++++----------- 2 files changed, 51 insertions(+), 86 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts index 678ed5fcce..8c568c9913 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts @@ -11,10 +11,10 @@ export const armNoRecordRule = createRule({ url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-record", messages: { default: - "Definitions must not be of type Record. ARM requires Resource provider teams to define types explicitly.", + "Properties should not be of type Record. ARM requires Resource provider teams to define types explicitly.", extends: - "Definitions must not extend type Record. ARM requires Resource provider teams to define types explicitly.", - is: "Definitions must not equate to type Record. ARM requires Resource provider teams to define types explicitly.", + "Models should not extend type Record. ARM requires Resource provider teams to define types explicitly.", + is: "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", }, create(context): SemanticNodeListener { return { diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index 5896238bc9..a5d89c0e23 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -3,97 +3,62 @@ import { LinterRuleTester, createLinterRuleTester, } from "@typespec/compiler/testing"; -import { beforeEach, describe, it } from "vitest"; +import { beforeEach, it } from "vitest"; import { armNoRecordRule } from "../../src/rules/arm-no-record.js"; import { createAzureResourceManagerTestRunner } from "../test-host.js"; -describe("typespec-azure-resource-manager: ARM no-record rule", () => { - let runner: BasicTestRunner; - let tester: LinterRuleTester; +let runner: BasicTestRunner; +let tester: LinterRuleTester; - beforeEach(async () => { - runner = await createAzureResourceManagerTestRunner(); - tester = createLinterRuleTester( - runner, - armNoRecordRule, - "@azure-tools/typespec-azure-resource-manager" - ); - }); - - it("emits diagnostic when a model property uses Record type", async () => { - await tester - .expect( - ` - @armProviderNamespace - @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) - namespace Microsoft.Contoso; - - @Azure.ResourceManager.tenantResource - model Widget is ProxyResource { - @key("widgetName") - @segment("widgets") - @path - @visibility("read") - name: string; - - props: Record; - } +beforeEach(async () => { + runner = await createAzureResourceManagerTestRunner(); + tester = createLinterRuleTester( + runner, + armNoRecordRule, + "@azure-tools/typespec-azure-resource-manager" + ); +}); - model WidgetProperties {} +it("emits diagnostic when a model property uses Record type", async () => { + await tester + .expect( ` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", - }); - }); - - it("emits diagnostic when a model extends Record type", async () => { - await tester - .expect( - ` - @armProviderNamespace - @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) - namespace Microsoft.Contoso; - - @Azure.ResourceManager.tenantResource - model Widget is ProxyResource { - @key("widgetName") - @segment("widgets") - @path - @visibility("read") - name: string; - } + model WidgetProperties { + props: Record; + } + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", + message: + "Properties should not be of type Record. ARM requires Resource provider teams to define types explicitly.", + }); +}); - model WidgetProperties extends Record {} +it("emits diagnostic when a model extends Record type", async () => { + await tester + .expect( ` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", - }); - }); - - it("emits diagnostic when a model is Record type", async () => { - await tester - .expect( - ` - @armProviderNamespace - @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) - namespace Microsoft.Contoso; - - @Azure.ResourceManager.tenantResource - model Widget is ProxyResource { - @key("widgetName") - @segment("widgets") - @path - @visibility("read") - name: string; - } + model WidgetProperties extends Record {} + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", + message: + "Models should not extend type Record. ARM requires Resource provider teams to define types explicitly.", + }); +}); - model WidgetProperties is Record; +it("emits diagnostic when a model is Record type", async () => { + await tester + .expect( ` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", - }); - }); + model WidgetProperties is Record; + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", + message: + "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", + }); }); From df7d723ae274637bd105cdb46d5b9a0189f4e5c9 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Tue, 27 Feb 2024 13:21:15 -0800 Subject: [PATCH 06/10] Update rule logic and tests to ensure this only applies to ARM resources. --- .../src/rules/arm-no-record.ts | 56 +++++---- .../test/rules/arm-no-record.test.ts | 106 ++++++++++++++++++ 2 files changed, 141 insertions(+), 21 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts index 8c568c9913..e65e326500 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts @@ -1,8 +1,5 @@ import { Model, SemanticNodeListener, createRule } from "@typespec/compiler"; - -function isRecordType(model: Model): boolean { - return model.name === "Record"; -} +import { getArmResources } from "../resource.js"; export const armNoRecordRule = createRule({ name: "arm-no-record", @@ -18,28 +15,45 @@ export const armNoRecordRule = createRule({ }, create(context): SemanticNodeListener { return { - model: (model: Model) => { - for (const prop of model.properties.values()) { - if (prop.type.kind === "Model" && isRecordType(prop.type)) { + root: (program) => { + function isRecordType(model: Model): boolean { + return model.name === "Record"; + } + + function checkModel(model: Model) { + if (model.baseModel !== undefined && isRecordType(model.baseModel)) { context.reportDiagnostic({ code: "arm-no-record", - target: prop, + target: model, + messageId: "extends", }); } + if (model.sourceModel !== undefined && isRecordType(model.sourceModel)) { + context.reportDiagnostic({ + code: "arm-no-record", + target: model, + messageId: "is", + }); + } + for (const prop of model.properties.values()) { + if (prop.type.kind === "Model") { + if (isRecordType(prop.type)) { + context.reportDiagnostic({ + code: "arm-no-record", + target: prop, + messageId: "default", + }); + } else { + checkModel(prop.type); + } + } + } } - if (model.baseModel !== undefined && isRecordType(model.baseModel)) { - context.reportDiagnostic({ - code: "arm-no-record", - target: model, - messageId: "extends", - }); - } - if (model.sourceModel !== undefined && isRecordType(model.sourceModel)) { - context.reportDiagnostic({ - code: "arm-no-record", - target: model, - messageId: "is", - }); + + // ensure only ARM resources and models they touch are checked + const resources = getArmResources(context.program); + for (const resource of resources) { + checkModel(resource.typespecType); } }, }; diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index a5d89c0e23..a9b7a4bea8 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -19,10 +19,29 @@ beforeEach(async () => { ); }); +const nsDef = ` +@armProviderNamespace +@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) +namespace Microsoft.Contoso; +`; + +const resource = ` +@Azure.ResourceManager.tenantResource +model Widget is ProxyResource { + @key("widgetName") + @segment("widgets") + @path + @visibility("read") + name: string; +} +`; + it("emits diagnostic when a model property uses Record type", async () => { await tester .expect( ` + ${nsDef} + ${resource} model WidgetProperties { props: Record; } @@ -39,6 +58,8 @@ it("emits diagnostic when a model extends Record type", async () => { await tester .expect( ` + ${nsDef} + ${resource} model WidgetProperties extends Record {} ` ) @@ -53,9 +74,94 @@ it("emits diagnostic when a model is Record type", async () => { await tester .expect( ` + ${nsDef} + ${resource} + model WidgetProperties is Record; + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", + message: + "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", + }); +}); + +it("does not emit diagnostic when Record is used but not referenced by an ARM resource", async () => { + await tester + .expect( + ` + ${nsDef} + // should not throw because WidgetProperties is not an ARM resources and is not + // referenced by an ARM resource. model WidgetProperties is Record; ` ) + .toBeValid(); +}); + +it("does not emit diagnostic when Record is used outside an ARM namespace", async () => { + await tester + .expect( + ` + namespace Test { + model Props is Record; + + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Arm { + model WidgetProperties {}; + } + } + ` + ) + .toBeValid(); +}); + +it("emits diagnostic if an ARM Resource references a model that uses Record type", async () => { + await tester + .expect( + ` + namespace NonArm { + model Properties is Record; + + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Arm { + ${resource} + + model WidgetProperties { + props: Properties; + } + } + } + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", + message: + "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", + }); +}); + +it("emits diagnostic if an ARM Resource references a subnamespace model that uses Record type", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Arm { + ${resource} + + model WidgetProperties { + props: Sub.Properties; + } + + namespace Sub { + model Properties is Record; + } + } + ` + ) .toEmitDiagnostics({ code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", message: From 702efb63cd11c05fa6124d56b5970e95cb77194f Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Tue, 27 Feb 2024 13:25:33 -0800 Subject: [PATCH 07/10] Code review feedback. --- .../azure-resource-manager/rules/no-record.md | 14 +++++++------- .../src/rules/arm-no-record.ts | 2 +- .../test/rules/arm-no-record.test.ts | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/libraries/azure-resource-manager/rules/no-record.md b/docs/libraries/azure-resource-manager/rules/no-record.md index 823b3b1524..b917f154e8 100644 --- a/docs/libraries/azure-resource-manager/rules/no-record.md +++ b/docs/libraries/azure-resource-manager/rules/no-record.md @@ -11,7 +11,11 @@ ARM requires Resource provider teams to define types explicitly. This is to ensu #### ❌ Incorrect ```tsp -model Address extends Record {} +model Address { + address: Record; + city: string; + state: string; +} ``` #### ✅ Correct @@ -29,11 +33,7 @@ model Address { #### ❌ Incorrect ```tsp -model Address { - address: Record; - city: string; - state: string; -} +model Address is Record; ``` #### ✅ Correct @@ -51,7 +51,7 @@ model Address { #### ❌ Incorrect ```tsp -model Address is Record; +model Address extends Record {} ``` #### ✅ Correct diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts index e65e326500..9ec08835d8 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts @@ -8,7 +8,7 @@ export const armNoRecordRule = createRule({ url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-record", messages: { default: - "Properties should not be of type Record. ARM requires Resource provider teams to define types explicitly.", + "Model properties or operation parameters should not be of type Record. ARM requires Resource provider teams to define types explicitly.", extends: "Models should not extend type Record. ARM requires Resource provider teams to define types explicitly.", is: "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index a9b7a4bea8..1f10c757e4 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -50,7 +50,7 @@ it("emits diagnostic when a model property uses Record type", async () => { .toEmitDiagnostics({ code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", message: - "Properties should not be of type Record. ARM requires Resource provider teams to define types explicitly.", + "Model properties or operation parameters should not be of type Record. ARM requires Resource provider teams to define types explicitly.", }); }); From 5a0ca969499894aa5f21ce60376490d8b0a239bc Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Tue, 27 Feb 2024 13:37:43 -0800 Subject: [PATCH 08/10] Add test for ArmTagsProperty --- .../test/rules/arm-no-record.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index 1f10c757e4..a609c23eed 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -168,3 +168,17 @@ it("emits diagnostic if an ARM Resource references a subnamespace model that use "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", }); }); + +it("does not emit diagnostic if ArmTagsProperty is used", async () => { + await tester + .expect( + ` + ${nsDef} + ${resource} + model WidgetProperties { + ...Foundations.ArmTagsProperty; + } + ` + ) + .toBeValid(); +}); From 2b2f0c83dc447f26962c3b7ee281af40149cc107 Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Tue, 27 Feb 2024 14:33:31 -0800 Subject: [PATCH 09/10] Add test case. --- .../test/rules/arm-no-record.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index a609c23eed..dc9723731d 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -182,3 +182,25 @@ it("does not emit diagnostic if ArmTagsProperty is used", async () => { ) .toBeValid(); }); + +it("emits a diagnostic if a deeply aliased model use Record type", async () => { + await tester + .expect( + ` + ${nsDef} + + model Foo is Bar; + model Bar is Record; + + ${resource} + model WidgetProperties { + props: Foo; + } + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", + message: + "Model properties or operation parameters should not be of type Record. ARM requires Resource provider teams to define types explicitly.", + }); +}); From 043809128232bc9aaac84bbae3eb35386dc2e66c Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Tue, 27 Feb 2024 15:02:38 -0800 Subject: [PATCH 10/10] Refactor logic. --- .../src/rules/arm-no-record.ts | 43 +++++++------------ .../test/rules/arm-no-record.test.ts | 2 +- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts index 9ec08835d8..d4cbf1ed1e 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts @@ -1,4 +1,4 @@ -import { Model, SemanticNodeListener, createRule } from "@typespec/compiler"; +import { DiagnosticTarget, Model, SemanticNodeListener, createRule } from "@typespec/compiler"; import { getArmResources } from "../resource.js"; export const armNoRecordRule = createRule({ @@ -15,36 +15,23 @@ export const armNoRecordRule = createRule({ }, create(context): SemanticNodeListener { return { - root: (program) => { - function isRecordType(model: Model): boolean { - return model.name === "Record"; - } - - function checkModel(model: Model) { - if (model.baseModel !== undefined && isRecordType(model.baseModel)) { - context.reportDiagnostic({ - code: "arm-no-record", - target: model, - messageId: "extends", - }); - } - if (model.sourceModel !== undefined && isRecordType(model.sourceModel)) { + root: (_) => { + function checkModel(model: Model, target: DiagnosticTarget, kind?: "extends" | "is") { + if (model.name === "Record") { context.reportDiagnostic({ code: "arm-no-record", - target: model, - messageId: "is", + target: target, + messageId: kind || "default", }); + } else if (model.baseModel !== undefined) { + checkModel(model.baseModel, model, "extends"); + } else if (model.sourceModel !== undefined) { + checkModel(model.sourceModel, model, "is"); } - for (const prop of model.properties.values()) { - if (prop.type.kind === "Model") { - if (isRecordType(prop.type)) { - context.reportDiagnostic({ - code: "arm-no-record", - target: prop, - messageId: "default", - }); - } else { - checkModel(prop.type); + if (model?.properties !== undefined) { + for (const prop of model.properties.values()) { + if (prop.type.kind === "Model") { + checkModel(prop.type, prop); } } } @@ -53,7 +40,7 @@ export const armNoRecordRule = createRule({ // ensure only ARM resources and models they touch are checked const resources = getArmResources(context.program); for (const resource of resources) { - checkModel(resource.typespecType); + checkModel(resource.typespecType, resource.typespecType); } }, }; diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts index dc9723731d..c543413846 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts @@ -201,6 +201,6 @@ it("emits a diagnostic if a deeply aliased model use Record type", async () => { .toEmitDiagnostics({ code: "@azure-tools/typespec-azure-resource-manager/arm-no-record", message: - "Model properties or operation parameters should not be of type Record. ARM requires Resource provider teams to define types explicitly.", + "Models should not equate to type Record. ARM requires Resource provider teams to define types explicitly.", }); });