Update arm no record to warn about all use of record (#1359)

fix #1356
Azure · Aug 13, 2024 · 5edf99d · 5edf99d
1 parent 4a1e325
commit 5edf99d
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 39 deletions.
diff --git a/.chronus/changes/fix-arm-no-record-2024-7-12-22-45-52.md b/.chronus/changes/fix-arm-no-record-2024-7-12-22-45-52.md
@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: fix
+packages:
+  - "@azure-tools/typespec-azure-resource-manager"
+---
+
+`arm-no-record` rule should warn about any use of `Record<X>` not just when inside resource properties
diff --git a/docs/libraries/azure-resource-manager/reference/linter.md b/docs/libraries/azure-resource-manager/reference/linter.md
@@ -26,7 +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-no-record`](/libraries/azure-resource-manager/rules/arm-no-record.md)                                         | Don't use Record types for ARM resources.                                                                                                                                                                                                            |
 | `@azure-tools/typespec-azure-resource-manager/arm-common-types-version`                                                                                          | Specify the ARM common-types version using @armCommonTypesVersion.                                                                                                                                                                                   |
 | [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](/libraries/azure-resource-manager/rules/delete-operation-response-codes.md) | Ensure delete operations have the appropriate status codes.                                                                                                                                                                                          |
 | [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](/libraries/azure-resource-manager/rules/put-operation-response-codes.md)       | Ensure put operations have the appropriate status codes.                                                                                                                                                                                             |

diff --git a/...azure-resource-manager/rules/no-record.md → ...e-resource-manager/rules/arm-no-record.md b/...azure-resource-manager/rules/no-record.md → ...e-resource-manager/rules/arm-no-record.md
@@ -1,9 +1,9 @@
 ---
-title: no-record
+title: arm-no-record
 ---
 
 ```text title=- Full name-
-@azure-tools/typespec-azure-resource-manager/no-record
+@azure-tools/typespec-azure-resource-manager/arm-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.

diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md
@@ -30,7 +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-no-record`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-record)                                         | Don't use Record types for ARM resources.                                                                                                                                                                                                            |
 | `@azure-tools/typespec-azure-resource-manager/arm-common-types-version`                                                                                                                                  | Specify the ARM common-types version using @armCommonTypesVersion.                                                                                                                                                                                   |
 | [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/delete-operation-response-codes) | Ensure delete operations have the appropriate status codes.                                                                                                                                                                                          |
 | [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/put-operation-response-codes)       | Ensure put operations have the appropriate status codes.                                                                                                                                                                                             |

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
@@ -1,11 +1,16 @@
-import { DiagnosticTarget, Model, SemanticNodeListener, createRule } from "@typespec/compiler";
-import { getArmResources } from "../resource.js";
+import {
+  DiagnosticTarget,
+  Model,
+  SemanticNodeListener,
+  Type,
+  createRule,
+} from "@typespec/compiler";
 
 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",
+  url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-record",
   messages: {
     default:
       "Model properties or operation parameters should not be of type Record. ARM requires Resource provider teams to define types explicitly.",
@@ -14,35 +19,29 @@ export const armNoRecordRule = createRule({
     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);
-              }
-            }
-          }
-        }
+    function checkModel(model: Model) {
+      if (model.baseModel !== undefined) {
+        checkNoRecord(model.baseModel, model, "extends");
+      } else if (model.sourceModel !== undefined) {
+        checkNoRecord(model.sourceModel, model, "is");
+      }
+    }
+
+    function checkNoRecord(type: Type, target: DiagnosticTarget, kind?: "extends" | "is") {
+      if (type.kind === "Model" && type.name === "Record") {
+        context.reportDiagnostic({
+          code: "arm-no-record",
+          target: target,
+          messageId: kind || "default",
+        });
+      }
+    }
 
-        // 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);
-        }
-      },
+    return {
+      operation: (op) => checkNoRecord(op.returnType, op),
+      model: (type) => checkModel(type),
+      modelProperty: (prop) => checkNoRecord(prop.type, prop),
+      unionVariant: (variant) => checkNoRecord(variant.type, variant),
     };
   },
 });
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
@@ -86,7 +86,7 @@ it("emits diagnostic when a model is Record type", async () => {
     });
 });
 
-it("does not emit diagnostic when Record is used but not referenced by an ARM resource", async () => {
+it("emits diagnostic when Record is used but not referenced by an ARM resource", async () => {
   await tester
     .expect(
       `
@@ -96,10 +96,14 @@ it("does not emit diagnostic when Record is used but not referenced by an ARM re
     model WidgetProperties is Record<string>;
     `
     )
-    .toBeValid();
+    .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 outside an ARM namespace", async () => {
+it("emits diagnostic when Record is used outside an ARM namespace", async () => {
   await tester
     .expect(
       `
@@ -114,7 +118,11 @@ it("does not emit diagnostic when Record is used outside an ARM namespace", asyn
     }
     `
     )
-    .toBeValid();
+    .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 model that uses Record type", async () => {