[tcgc] add linter rulesets framework

.chronus/changes/typespec_azure_rulsets-2024-6-19-19-0-31.md

@@ -0,0 +1,7 @@
+---
+changeKind: feature
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+add linter rulesets to TCGC, for both generic and language-specific linter rules

docs/libraries/typespec-client-generator-core/reference/linter.md

@@ -0,0 +1,31 @@
+---
+title: "Linter usage"
+toc_min_heading_level: 2
+toc_max_heading_level: 3
+---
+
+# Linter
+
+## Usage
+
+Add the following in `tspconfig.yaml`:
+
+```yaml
+linter:
+  extends:
+    - "@azure-tools/typespec-client-generator-core/all"
+```
+
+## RuleSets
+
+Available ruleSets:
+
+- `@azure-tools/typespec-client-generator-core/all`
+- `@azure-tools/typespec-client-generator-core/best-practices:csharp`
+
+## Rules
+
+| Name                                                                 | Description                                                             |
+| -------------------------------------------------------------------- | ----------------------------------------------------------------------- |
+| `@azure-tools/typespec-client-generator-core/require-client-suffix`  | Client names should end with 'Client'.                                  |
+| `@azure-tools/typespec-client-generator-core/property-name-conflict` | Avoid naming conflicts between a property and a model of the same name. |

docs/libraries/typespec-client-generator-core/rules/CONTRIBUTING.md

@@ -0,0 +1,36 @@
+# Contributing Linter Rules to the @azure-tools/typespec-client-generator-core Package
+
+For information about linter rules in typespec in general, view [here][generic-linter]
+
+In the `@azure-tools/typespec-client-generator-core` library, we have two main types of rules:
+
+1. Generic rule that applies to all emitted languages
+2. Specific rule that applies to a subset of all language: it can even apply to just one language
+
+The process for adding a rule starts off the same for both: for language-specific rules, there is an extra step
+
+1. Write the rule in `typespec-azure/packages/typespec-client-generator-core/src/rules/[rule-name].rule.ts
+2. Add reference to the rule in the `rules` array in [`typespec-azure/packages/typespec-client-generator-core/src/linter.ts`][tcgc-linter]
+   - This will automatically add it to a ruleset called `:all` for `@azure-tools/typespec-client-generator-core`
+3. Add the rule to the enable list for [`data-plane.ts`][data-plane-ruleset] and/or [`resource-manager.ts`][resource-manager-ruleset] in the [rulesets][rulesets] package. You can set `enable` to `false` here, if you want to delay enabling
+
+**If you are adding a language-specific rule**, you will also need this extra step 4. Add reference to the rule in the `[language]Rules` array in [`typespec-azure/packages/typespec-client-generator-core/src/linter.ts`][tcgc-linter]
+
+For Azure generations then, all rules, including all language-specific rules, will be run on the specs.
+For unbranded generations, since we've added the rules into specific `best-practices:[language]` rulesets, you can explicitly specify a subset of rules in your `tsp-config.yaml`, i.e. if I only want Python best-practices, I could add this in my `tsp-config.yaml`:
+
+```yaml
+linter:
+  extends:
+    - best-practices: python
+```
+
+Finally, we recommend that every warning or error you throw in your language emitter has a corresponding warning in TCGC. This is part of our shift-left policy, so tsp authors can catch these potential pitfalls earlier in the process.
+
+### Links
+
+[generic-linter]: https://typespec.io/docs/next/extending-typespec/linters "Generic Linter Docs"
+[tcgc-linter]: https://github.com/typespec-azure/packages/typespec-client-generator-core/src/linter.ts "Linter TS File"
+[rulesets]: https://github.com/typespec-azure/packages/typespec-azure-rulesets "Rulesets package"
+[data-plane-ruleset]: https://github.com/typespec-azure/packages/typespec-azure-rulesets/src/rulesets/data-plane.ts "Data Plane Ruleset"
+[resource-manager-ruleset]: https://github.com/typespec-azure/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts "Resource Manager Ruleset"

docs/libraries/typespec-client-generator-core/rules/property-name-conflict.md

@@ -0,0 +1,27 @@
+---
+title: "property-name-conflict"
+---
+
+```text title="Full name"
+@azure-tools/typespec-client-generator-core/property-name-conflict
+```
+
+Verify that there isn't a name conflict between a property in the model, and the name of the model itself
+
+#### ❌ Incorrect
+
+```ts
+model Widget {
+  widget: string;
+}
+```
+
+#### ✅ Ok
+
+Using items from a private namespace within the same library is allowed.
+
+```ts
+model Widget {
+  widgetName: string;
+}
+```

docs/libraries/typespec-client-generator-core/rules/require-client-suffix.md

@@ -0,0 +1,46 @@
+---
+title: "require-client-suffix"
+---
+
+```text title="Full name"
+@azure-tools/typespec-client-generator-core/require-client-suffix
+```
+
+Verify that the generated client's name will have the suffix `Client` in its name
+
+#### ❌ Incorrect
+
+```ts
+// main.tsp
+namespace MyService;
+```
+
+```ts
+// client.tsp
+namespace MyCustomizations;
+
+@@client(MyService);
+```
+
+#### ✅ Recommended
+
+If you would not like to make any changes to the generated client besides its name, you can rely on the [`@clientName`][client-name] decorator to rename the main namespace of the service.
+
+```ts
+@@clientName(MyService, "MyClient");
+```
+
+#### ✅ Ok
+
+If you are completely recreating a client namespace in your `client.tsp`, you can rely on that namespace to end in `Client`
+
+```ts
+// client.tsp
+@client({service: MyService})
+namespace MyClient {
+}
+```
+
+### Links
+
+[client-name]: https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/reference/decorators#@Azure.ClientGenerator.Core.clientName "@clientName Decorator"

packages/typespec-client-generator-core/README.md

@@ -8,6 +8,32 @@ TypeSpec Data Plane Generation library
 npm install @azure-tools/typespec-client-generator-core
 ```
 
+## Linter
+
+### Usage
+
+Add the following in `tspconfig.yaml`:
+
+```yaml
+linter:
+  extends:
+    - "@azure-tools/typespec-client-generator-core/all"
+```
+
+### RuleSets
+
+Available ruleSets:
+
+- `@azure-tools/typespec-client-generator-core/all`
+- `@azure-tools/typespec-client-generator-core/best-practices:csharp`
+
+### Rules
+
+| Name                                                                 | Description                                                             |
+| -------------------------------------------------------------------- | ----------------------------------------------------------------------- |
+| `@azure-tools/typespec-client-generator-core/require-client-suffix`  | Client names should end with 'Client'.                                  |
+| `@azure-tools/typespec-client-generator-core/property-name-conflict` | Avoid naming conflicts between a property and a model of the same name. |
+
 ## Decorators
 
 ### Azure.ClientGenerator.Core

packages/typespec-client-generator-core/src/decorators.ts

@@ -158,13 +158,6 @@ export const $client: ClientDecorator = (
     explicitService?.kind === "Namespace"
       ? explicitService
       : findClientService(context.program, target) ?? (target as any);
-  if (!name.endsWith("Client")) {
-    reportDiagnostic(context.program, {
-      code: "client-name",
-      format: { name },
-      target: context.decoratorTarget,
-    });
-  }
 
   if (!isService(context.program, service)) {
     reportDiagnostic(context.program, {
@@ -791,7 +784,7 @@ export const $clientFormat: ClientFormatDecorator = (
     setScopedDecoratorData(context, $clientFormat, clientFormatKey, target, format, scope); // eslint-disable-line deprecation/deprecation
   } else {
     reportDiagnostic(context.program, {
-      code: "incorrect-client-format",
+      code: "invalid-client-format",
       format: { format, expectedTargetTypes: expectedTargetTypes.join('", "') },
       target: context.decoratorTarget,
     });
@@ -939,7 +932,7 @@ export const $access: AccessDecorator = (
 ) => {
   if (typeof value.value !== "string" || (value.value !== "public" && value.value !== "internal")) {
     reportDiagnostic(context.program, {
-      code: "access",
+      code: "invalid-access",
       format: {},
       target: entity,
     });

packages/typespec-client-generator-core/src/index.ts

@@ -1,6 +1,7 @@
 export * from "./decorators.js";
 export * from "./interfaces.js";
 export * from "./lib.js";
+export { $linter } from "./linter.js";
 export * from "./public-utils.js";
 export * from "./types.js";
 export { $onValidate } from "./validate.js";

packages/typespec-client-generator-core/src/lib.ts

@@ -3,25 +3,13 @@ import { createTypeSpecLibrary, paramMessage } from "@typespec/compiler";
 export const $lib = createTypeSpecLibrary({
   name: "@azure-tools/typespec-client-generator-core",
   diagnostics: {
-    "client-name": {
-      severity: "warning",
-      messages: {
-        default: paramMessage`Client name "${"name"}" must end with Client. Use @client({name: "...Client"}`,
-      },
-    },
     "client-service": {
       severity: "warning",
       messages: {
         default: paramMessage`Client "${"name"}" is not inside a service namespace. Use @client({service: MyServiceNS}`,
       },
     },
-    "unknown-client-format": {
-      severity: "error",
-      messages: {
-        default: paramMessage`Client format "${"format"}" is unknown. Known values are "${"knownValues"}"`,
-      },
-    },
-    "incorrect-client-format": {
+    "invalid-client-format": {
       severity: "error",
       messages: {
         default: paramMessage`Format "${"format"}" can only apply to "${"expectedTargetTypes"}"`,
@@ -33,22 +21,7 @@ export const $lib = createTypeSpecLibrary({
         default: "Cannot have a union containing only null types.",
       },
     },
-    "union-unsupported": {
-      severity: "error",
-      messages: {
-        default:
-          "Unions cannot be emitted by our language generators unless all options are literals of the same type.",
-        null: "Unions containing multiple model types cannot be emitted unless the union is between one model type and 'null'.",
-      },
-    },
-    "use-enum-instead": {
-      severity: "warning",
-      messages: {
-        default:
-          "Use enum instead of union of string or number literals. Falling back to the literal type.",
-      },
-    },
-    access: {
+    "invalid-access": {
       severity: "error",
       messages: {
         default: `Access decorator value must be "public" or "internal".`,
@@ -60,13 +33,6 @@ export const $lib = createTypeSpecLibrary({
         default: `Usage decorator value must be 2 ("input") or 4 ("output").`,
       },
     },
-    "invalid-encode": {
-      severity: "error",
-      messages: {
-        default: "Invalid encoding",
-        wrongType: paramMessage`Encoding '${"encoding"}' cannot be used on type '${"type"}'`,
-      },
-    },
     "conflicting-multipart-model-usage": {
       severity: "error",
       messages: {

packages/typespec-client-generator-core/src/linter.ts

@@ -0,0 +1,23 @@
+import { defineLinter } from "@typespec/compiler";
+import { propertyNameConflictRule } from "./rules/property-name-conflict.rule.js";
+import { requireClientSuffixRule } from "./rules/require-client-suffix.rule.js";
+
+const rules = [requireClientSuffixRule, propertyNameConflictRule];
+
+const csharpRules = [propertyNameConflictRule];
+
+export const $linter = defineLinter({
+  rules,
+  ruleSets: {
+    "best-practices:csharp": {
+      enable: {
+        ...Object.fromEntries(
+          csharpRules.map((rule) => [
+            `@azure-tools/typespec-client-generator-core/${rule.name}`,
+            true,
+          ])
+        ),
+      },
+    },
+  },
+});

packages/typespec-client-generator-core/src/rules/property-name-conflict.rule.ts

@@ -0,0 +1,25 @@
+import { ModelProperty, createRule, paramMessage } from "@typespec/compiler";
+
+export const propertyNameConflictRule = createRule({
+  name: "property-name-conflict",
+  description: "Avoid naming conflicts between a property and a model of the same name.",
+  severity: "warning",
+  url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/property-name-conflict",
+  messages: {
+    default: paramMessage`Property '${"propertyName"}' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`,
+  },
+  create(context) {
+    return {
+      modelProperty: (property: ModelProperty) => {
+        const modelName = property.model?.name.toLocaleLowerCase();
+        const propertyName = property.name.toLocaleLowerCase();
+        if (propertyName === modelName) {
+          context.reportDiagnostic({
+            format: { propertyName: property.name },
+            target: property,
+          });
+        }
+      },
+    };
+  },
+});

packages/typespec-client-generator-core/src/rules/require-client-suffix.rule.ts

@@ -0,0 +1,42 @@
+import { createRule, Interface, Namespace, paramMessage } from "@typespec/compiler";
+import { createTCGCContext, getClient } from "../decorators.js";
+
+export const requireClientSuffixRule = createRule({
+  name: "require-client-suffix",
+  description: "Client names should end with 'Client'.",
+  severity: "warning",
+  url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/require-client-suffix",
+  messages: {
+    default: paramMessage`Client name "${"name"}" must end with Client. Use @client({name: "...Client"}`,
+  },
+  create(context) {
+    const tcgcContext = createTCGCContext(
+      context.program,
+      "@azure-tools/typespec-client-generator-core"
+    );
+    return {
+      namespace: (namespace: Namespace) => {
+        const sdkClient = getClient(tcgcContext, namespace);
+        if (sdkClient && !sdkClient.name.endsWith("Client")) {
+          context.reportDiagnostic({
+            target: namespace,
+            format: {
+              name: sdkClient.name,
+            },
+          });
+        }
+      },
+      interface: (interfaceType: Interface) => {
+        const sdkClient = getClient(tcgcContext, interfaceType);
+        if (sdkClient && !sdkClient.name.endsWith("Client")) {
+          context.reportDiagnostic({
+            target: interfaceType,
+            format: {
+              name: sdkClient.name,
+            },
+          });
+        }
+      },
+    };
+  },
+});