Code review feedback.

Azure · Apr 10, 2024 · c00615e · c00615e
1 parent e4e0b24
commit c00615e
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 12 deletions.
diff --git a/.chronus/changes/core-IntegerTypesRule-2024-3-5-18-54-1.md b/.chronus/changes/core-IntegerTypesRule-2024-3-5-18-54-1.md
diff --git a/docs/libraries/azure-core/rules/no-generic-numeric.md b/docs/libraries/azure-core/rules/no-generic-numeric.md
@@ -6,20 +6,42 @@ title: "no-generic-numeric"
 @azure-tools/typespec-azure-core/no-generic-numeric
 ```
 
-Azure services should use types which specify the bit-width instead of generic types.
+Azure services should use numeric types that specify the bit-width instead of generic types.
 
 #### ❌ Incorrect
 
 ```tsp
 model Widget {
   id: integer;
+  cost: float;
 }
 ```
 
 #### ✅ Correct
 
 ```tsp
 model Widget {
-  id: int32;
+  id: safeint;
+  cost: float32;
+}
+```
+
+This includes extending generic numeric types.
+
+#### ❌ Incorrect
+
+```tsp
+model GenericInteger extends integer;
+
+model Widget {
+  id: GenericInteger;
+}
+```
+
+#### ✅ Correct
+
+```tsp
+model Widget {
+  id: safeint;
 }
 ```
diff --git a/packages/typespec-azure-core/src/rules/no-generic-numeric.ts b/packages/typespec-azure-core/src/rules/no-generic-numeric.ts
@@ -1,4 +1,4 @@
-import { Model, createRule, paramMessage } from "@typespec/compiler";
+import { Model, Scalar, createRule, paramMessage } from "@typespec/compiler";
 
 const disallowList = new Set(["integer", "numeric", "float", "decimal"]);
 const alternatives = new Map([
@@ -15,6 +15,7 @@ export const noGenericNumericRule = createRule({
   url: "https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-generic-numeric",
   messages: {
     default: paramMessage`Don't use generic type '${"name"}'. Use a more specific type that specifies the bit size, such as '${"alternative"}' instead.`,
+    extend: paramMessage`Don't extend generic type '${"name"}'. Use a more specific type that specifies the bit size, such as '${"alternative"}' instead.`,
   },
   create(context) {
     return {
@@ -33,6 +34,26 @@ export const noGenericNumericRule = createRule({
           }
         }
       },
+      scalar: (scalar: Scalar) => {
+        let baseScalar: Scalar | undefined = undefined;
+        while (scalar.baseScalar !== undefined) {
+          baseScalar = scalar.baseScalar;
+          break;
+        }
+        if (baseScalar === undefined) {
+          return;
+        }
+        if (disallowList.has(baseScalar.name)) {
+          context.reportDiagnostic({
+            target: scalar,
+            messageId: "extend",
+            format: {
+              name: baseScalar.name,
+              alternative: alternatives.get(baseScalar.name)!,
+            },
+          });
+        }
+      },
     };
   },
 });
diff --git a/packages/typespec-azure-core/test/rules/no-generic-numeric.test.ts b/packages/typespec-azure-core/test/rules/no-generic-numeric.test.ts
@@ -15,7 +15,7 @@ beforeEach(async () => {
   tester = createLinterRuleTester(runner, noGenericNumericRule, "@azure-tools/typespec-azure-core");
 });
 
-it("emits a warning diagnostic for generic types", async () => {
+it("emits a warning for generic numeric types", async () => {
   await tester
     .expect(
       `
@@ -44,3 +44,38 @@ it("emits a warning diagnostic for generic types", async () => {
       },
     ]);
 });
+
+it("emits a warning when extending generic numeric types", async () => {
+  await tester
+    .expect(
+      `
+      namespace Azure.Widget;
+
+      scalar GenericInteger extends integer;
+      scalar GenericNumeric extends numeric;
+      scalar GenericFloat extends float;
+      scalar GenericDecimal extends decimal;
+      
+      model Widget {
+        prop1: GenericInteger;
+        prop2: GenericNumeric;
+        prop3: GenericFloat;
+        prop4: GenericDecimal;
+      }
+      `
+    )
+    .toEmitDiagnostics([
+      {
+        code: "@azure-tools/typespec-azure-core/no-generic-numeric",
+      },
+      {
+        code: "@azure-tools/typespec-azure-core/no-generic-numeric",
+      },
+      {
+        code: "@azure-tools/typespec-azure-core/no-generic-numeric",
+      },
+      {
+        code: "@azure-tools/typespec-azure-core/no-generic-numeric",
+      },
+    ]);
+});