Linter all rulesets automatically created (#3462)

Provide a built in way for Azure/typespec-azure#330
microsoft · Jun 3, 2024 · 5da8fe7 · 5da8fe7
1 parent 90b8d6e
commit 5da8fe7
Show file tree

Hide file tree

Showing 10 changed files with 113 additions and 33 deletions.
diff --git a/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md b/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-46.md
@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: feature
+packages:
+  - "@typespec/compiler"
+---
+
+Linter `all` rulesets is automatically created if not explicitly provided
diff --git a/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md b/.chronus/changes/linter-auto-all-ruleset-2024-4-28-16-18-47.md
@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: feature
+packages:
+  - "@typespec/http"
+---
+
+Use new compiler automatic `all` ruleset instead of explicitly provided one
diff --git a/packages/compiler/src/core/index.ts b/packages/compiler/src/core/index.ts
@@ -40,6 +40,7 @@ export {
   setCadlNamespace,
   setTypeSpecNamespace,
 } from "./library.js";
+export { resolveLinterDefinition } from "./linter.js";
 export * from "./module-resolver.js";
 export { NodeHost } from "./node-host.js";
 export { Numeric, isNumeric } from "./numeric.js";

diff --git a/packages/compiler/src/core/linter.ts b/packages/compiler/src/core/linter.ts
@@ -8,6 +8,7 @@ import {
   DiagnosticMessages,
   LibraryInstance,
   LinterDefinition,
+  LinterResolvedDefinition,
   LinterRule,
   LinterRuleContext,
   LinterRuleDiagnosticReport,
@@ -22,6 +23,34 @@ export interface Linter {
   lint(): readonly Diagnostic[];
 }
 
+/**
+ * Resolve the linter definition
+ */
+export function resolveLinterDefinition(
+  libName: string,
+  linter: LinterDefinition
+): LinterResolvedDefinition {
+  const rules: LinterRule<string, any>[] = linter.rules.map((rule) => {
+    return { ...rule, id: `${libName}/${rule.name}` };
+  });
+  if (linter.ruleSets && "all" in linter.ruleSets) {
+    return {
+      rules,
+      ruleSets: linter.ruleSets as any,
+    };
+  } else {
+    return {
+      rules,
+      ruleSets: {
+        all: {
+          enable: Object.fromEntries(rules.map((x) => [x.id, true])) as any,
+        },
+        ...linter.ruleSets,
+      },
+    };
+  }
+}
+
 export function createLinter(
   program: Program,
   loadLibrary: (name: string) => Promise<LibraryInstance | undefined>
@@ -37,11 +66,6 @@ export function createLinter(
     lint,
   };
 
-  function getLinterDefinition(library: LibraryInstance): LinterDefinition | undefined {
-    // eslint-disable-next-line deprecation/deprecation
-    return library?.linter ?? library?.definition?.linter;
-  }
-
   async function extendRuleSet(ruleSet: LinterRuleSet): Promise<readonly Diagnostic[]> {
     tracer.trace("extend-rule-set.start", JSON.stringify(ruleSet, null, 2));
     const diagnostics = createDiagnosticCollector();
@@ -50,7 +74,7 @@ export function createLinter(
         const ref = diagnostics.pipe(parseRuleReference(extendingRuleSetName));
         if (ref) {
           const library = await resolveLibrary(ref.libraryName);
-          const libLinterDefinition = library && getLinterDefinition(library);
+          const libLinterDefinition = library?.linter;
           const extendingRuleSet = libLinterDefinition?.ruleSets?.[ref.name];
           if (extendingRuleSet) {
             await extendRuleSet(extendingRuleSet);
@@ -146,19 +170,17 @@ export function createLinter(
     tracer.trace("register-library", name);
 
     const library = await loadLibrary(name);
-    const linter = library && getLinterDefinition(library);
+    const linter = library?.linter;
     if (linter?.rules) {
-      for (const ruleDef of linter.rules) {
-        const ruleId = `${name}/${ruleDef.name}`;
+      for (const rule of linter.rules) {
         tracer.trace(
           "register-library.rule",
-          `Registering rule "${ruleId}" for library "${name}".`
+          `Registering rule "${rule.id}" for library "${name}".`
         );
-        const rule: LinterRule<string, any> = { ...ruleDef, id: ruleId };
-        if (ruleMap.has(ruleId)) {
-          compilerAssert(false, `Unexpected duplicate linter rule: "${ruleId}"`);
+        if (ruleMap.has(rule.id)) {
+          compilerAssert(false, `Unexpected duplicate linter rule: "${rule.id}"`);
         } else {
-          ruleMap.set(ruleId, rule);
+          ruleMap.set(rule.id, rule);
         }
       }
     }

diff --git a/packages/compiler/src/core/program.ts b/packages/compiler/src/core/program.ts
@@ -21,7 +21,7 @@ import {
 } from "./entrypoint-resolution.js";
 import { ExternalError } from "./external-error.js";
 import { getLibraryUrlsLoaded } from "./library.js";
-import { createLinter } from "./linter.js";
+import { createLinter, resolveLinterDefinition } from "./linter.js";
 import { createLogger } from "./logger/index.js";
 import { createTracer } from "./logger/tracer.js";
 import { createDiagnostic } from "./messages.js";
@@ -580,12 +580,13 @@ export async function compile(
 
     const libDefinition: TypeSpecLibrary<any> | undefined = entrypoint?.esmExports.$lib;
     const metadata = computeLibraryMetadata(module, libDefinition);
-
+    // eslint-disable-next-line deprecation/deprecation
+    const linterDef = entrypoint?.esmExports.$linter ?? libDefinition?.linter;
     return {
       ...resolution,
       metadata,
       definition: libDefinition,
-      linter: entrypoint?.esmExports.$linter,
+      linter: linterDef && resolveLinterDefinition(libraryNameOrPath, linterDef),
     };
   }
 

diff --git a/packages/compiler/src/core/types.ts b/packages/compiler/src/core/types.ts
@@ -2047,7 +2047,7 @@ export interface LibraryInstance {
   entrypoint: JsSourceFileNode | undefined;
   metadata: LibraryMetadata;
   definition?: TypeSpecLibrary<any>;
-  linter: LinterDefinition;
+  linter: LinterResolvedDefinition;
 }
 
 export type LibraryMetadata = FileLibraryMetadata | ModuleLibraryMetadata;
@@ -2437,6 +2437,14 @@ export interface LinterDefinition {
   ruleSets?: Record<string, LinterRuleSet>;
 }
 
+export interface LinterResolvedDefinition {
+  readonly rules: LinterRule<string, DiagnosticMessages>[];
+  readonly ruleSets: {
+    all: LinterRuleSet;
+    [name: string]: LinterRuleSet;
+  };
+}
+
 export interface LinterRuleDefinition<N extends string, DM extends DiagnosticMessages> {
   /** Rule name (without the library name) */
   name: N;

diff --git a/packages/compiler/test/core/linter.test.ts b/packages/compiler/test/core/linter.test.ts
@@ -1,7 +1,7 @@
 import { describe, it } from "vitest";
 
 import { createLinterRule, createTypeSpecLibrary } from "../../src/core/library.js";
-import { Linter, createLinter } from "../../src/core/linter.js";
+import { Linter, createLinter, resolveLinterDefinition } from "../../src/core/linter.js";
 import type { LibraryInstance, LinterDefinition } from "../../src/index.js";
 import {
   createTestHost,
@@ -45,13 +45,13 @@ describe("compiler: linter", () => {
 
     const library: LibraryInstance = {
       entrypoint: undefined,
-      metadata: { type: "module", name: "@typespec/test" },
+      metadata: { type: "module", name: "@typespec/test-linter" },
       module: { type: "module", path: "", mainFile: "", manifest: { name: "", version: "" } },
       definition: createTypeSpecLibrary({
-        name: "@typespec/test",
+        name: "@typespec/test-linter",
         diagnostics: {},
       }),
-      linter: linterDef,
+      linter: resolveLinterDefinition("@typespec/test-linter", linterDef),
     };
 
     await host.compile("main.tsp");
@@ -212,6 +212,44 @@ describe("compiler: linter", () => {
     });
   });
 
+  describe("when enabling a ruleset", () => {
+    it("/all ruleset is automatically provided and include all rules", async () => {
+      const linter = await createTestLinter(`model Foo {}`, {
+        rules: [noModelFoo],
+      });
+      expectDiagnosticEmpty(
+        await linter.extendRuleSet({
+          extends: ["@typespec/test-linter/all"],
+        })
+      );
+      expectDiagnostics(linter.lint(), {
+        severity: "warning",
+        code: "@typespec/test-linter/no-model-foo",
+        message: `Cannot call model 'Foo'`,
+      });
+    });
+    it("extending specific ruleset enable the rules inside", async () => {
+      const linter = await createTestLinter(`model Foo {}`, {
+        rules: [noModelFoo],
+        ruleSets: {
+          custom: {
+            enable: { "@typespec/test-linter/no-model-foo": true },
+          },
+        },
+      });
+      expectDiagnosticEmpty(
+        await linter.extendRuleSet({
+          extends: ["@typespec/test-linter/custom"],
+        })
+      );
+      expectDiagnostics(linter.lint(), {
+        severity: "warning",
+        code: "@typespec/test-linter/no-model-foo",
+        message: `Cannot call model 'Foo'`,
+      });
+    });
+  });
+
   describe("(integration) loading in program", () => {
     async function diagnoseReal(code: string) {
       const host = await createTestHost();

diff --git a/packages/http/src/linter.ts b/packages/http/src/linter.ts
@@ -3,11 +3,4 @@ import { opReferenceContainerRouteRule } from "./rules/op-reference-container-ro
 
 export const $linter = defineLinter({
   rules: [opReferenceContainerRouteRule],
-  ruleSets: {
-    all: {
-      enable: {
-        [`@typespec/http/${opReferenceContainerRouteRule.name}`]: true,
-      },
-    },
-  },
 });
diff --git a/packages/tspd/src/ref-doc/extractor.ts b/packages/tspd/src/ref-doc/extractor.ts
@@ -16,7 +16,7 @@ import {
   isTemplateDeclaration,
   joinPaths,
   JSONSchemaType,
-  LinterDefinition,
+  LinterResolvedDefinition,
   LinterRuleDefinition,
   LinterRuleSet,
   Model,
@@ -29,6 +29,7 @@ import {
   NoTarget,
   Operation,
   Program,
+  resolveLinterDefinition,
   resolvePath,
   Scalar,
   SyntaxKind,
@@ -109,7 +110,7 @@ export async function extractLibraryRefDocs(
     // eslint-disable-next-line deprecation/deprecation
     const linter = entrypoint.$linter ?? lib?.linter;
     if (lib && linter) {
-      refDoc.linter = extractLinterRefDoc(lib.name, linter);
+      refDoc.linter = extractLinterRefDoc(lib.name, resolveLinterDefinition(lib.name, linter));
     }
   }
 
@@ -622,7 +623,7 @@ function extractEmitterOptionsRefDoc(
   });
 }
 
-function extractLinterRefDoc(libName: string, linter: LinterDefinition): LinterRefDoc {
+function extractLinterRefDoc(libName: string, linter: LinterResolvedDefinition): LinterRefDoc {
   return {
     ruleSets: linter.ruleSets && extractLinterRuleSetsRefDoc(libName, linter.ruleSets),
     rules: linter.rules.map((rule) => extractLinterRuleRefDoc(libName, rule)),

diff --git a/packages/tspd/src/ref-doc/types.ts b/packages/tspd/src/ref-doc/types.ts
@@ -51,7 +51,7 @@ export type EmitterRefDoc = {
 
 export type LinterRefDoc = {
   /** List of rulesets provided. */
-  readonly ruleSets?: LinterRuleSetRefDoc[];
+  readonly ruleSets: LinterRuleSetRefDoc[];
   readonly rules: LinterRuleRefDoc[];
 };