ARM: Add rule to allow disabling LintDiff AvoidAdditionalProperties

.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-no-record` rule.

docs/libraries/azure-resource-manager/reference/linter.md

@@ -26,6 +26,7 @@ Available ruleSets:
 
 | 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.                                      |

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 {
+  address: Record<string>;
+  city: string;
+  state: string;
+}
+```
+
+#### ✅ Correct
+
+```tsp
+model Address {
+  street: string;
+  city: string;
+  state: string;
+  country: string;
+  postalCode: string;
+}
+```
+
+#### ❌ Incorrect
+
+```tsp
+model Address is Record<string>;
+```
+
+#### ✅ Correct
+
+```tsp
+model Address {
+  street: string;
+  city: string;
+  state: string;
+  country: string;
+  postalCode: string;
+}
+```
+
+#### ❌ Incorrect
+
+```tsp
+model Address extends Record<string> {}
+```
+
+#### ✅ Correct
+
+```tsp
+model Address {
+  street: string;
+  city: string;
+  state: string;
+  country: string;
+  postalCode: string;
+}
+```

packages/typespec-azure-resource-manager/README.md

@@ -30,6 +30,7 @@ Available ruleSets:
 
 | 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.                                      |

packages/typespec-azure-resource-manager/src/linter.ts

@@ -1,5 +1,6 @@
 import { defineLinter } from "@typespec/compiler";
 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";
@@ -25,6 +26,7 @@ import { retryAfterRule } from "./rules/retry-after.js";
 import { unsupportedTypeRule } from "./rules/unsupported-type.js";
 
 const rules = [
+  armNoRecordRule,
   armCommonTypesVersionRule,
   armResourceActionNoSegmentRule,
   armResourceDuplicatePropertiesRule,
@@ -64,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"],
     },
   },

packages/typespec-azure-resource-manager/src/rules/arm-no-record.ts

@@ -0,0 +1,48 @@
+import { DiagnosticTarget, Model, SemanticNodeListener, createRule } from "@typespec/compiler";
+import { getArmResources } from "../resource.js";
+
+export const armNoRecordRule = createRule({
+  name: "arm-no-record",
+  severity: "warning",
+  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:
+      "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.",
+  },
+  create(context): SemanticNodeListener {
+    return {
+      root: (_) => {
+        function checkModel(model: Model, target: DiagnosticTarget, kind?: "extends" | "is") {
+          if (model.name === "Record") {
+            context.reportDiagnostic({
+              code: "arm-no-record",
+              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");
+          }
+          if (model?.properties !== undefined) {
+            for (const prop of model.properties.values()) {
+              if (prop.type.kind === "Model") {
+                checkModel(prop.type, prop);
+              }
+            }
+          }
+        }
+
+        // ensure only ARM resources and models they touch are checked
+        const resources = getArmResources(context.program);
+        for (const resource of resources) {
+          checkModel(resource.typespecType, resource.typespecType);
+        }
+      },
+    };
+  },
+});

packages/typespec-azure-resource-manager/test/rules/arm-no-record.test.ts

@@ -0,0 +1,206 @@
+import {
+  BasicTestRunner,
+  LinterRuleTester,
+  createLinterRuleTester,
+} from "@typespec/compiler/testing";
+import { beforeEach, it } from "vitest";
+import { armNoRecordRule } from "../../src/rules/arm-no-record.js";
+import { createAzureResourceManagerTestRunner } from "../test-host.js";
+
+let runner: BasicTestRunner;
+let tester: LinterRuleTester;
+
+beforeEach(async () => {
+  runner = await createAzureResourceManagerTestRunner();
+  tester = createLinterRuleTester(
+    runner,
+    armNoRecordRule,
+    "@azure-tools/typespec-azure-resource-manager"
+  );
+});
+
+const nsDef = `
+@armProviderNamespace
+@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1)
+namespace Microsoft.Contoso;
+`;
+
+const resource = `
+@Azure.ResourceManager.tenantResource
+model Widget is ProxyResource<WidgetProperties> {
+  @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<string>;
+    }
+    `
+    )
+    .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.",
+    });
+});
+
+it("emits diagnostic when a model extends Record type", async () => {
+  await tester
+    .expect(
+      `
+    ${nsDef}
+    ${resource}  
+    model WidgetProperties extends Record<string> {}
+    `
+    )
+    .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.",
+    });
+});
+
+it("emits diagnostic when a model is Record type", async () => {
+  await tester
+    .expect(
+      `
+    ${nsDef}
+    ${resource}
+    model WidgetProperties is Record<string>;
+    `
+    )
+    .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<string>;
+    `
+    )
+    .toBeValid();
+});
+
+it("does not emit diagnostic when Record is used outside an ARM namespace", async () => {
+  await tester
+    .expect(
+      `
+    namespace Test {
+      model Props is Record<unknown>;
+
+      @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<string>;
+
+      @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<string>;
+      }
+    }
+    `
+    )
+    .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 if ArmTagsProperty is used", async () => {
+  await tester
+    .expect(
+      `
+    ${nsDef}
+    ${resource}
+    model WidgetProperties {
+      ...Foundations.ArmTagsProperty;
+    }
+    `
+    )
+    .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<unknown>;
+
+    ${resource}
+    model WidgetProperties {
+      props: Foo;
+    }
+    `
+    )
+    .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.",
+    });
+});