Support Sub Rule Suppression in TypeSpec Validation Tool

eng/tools/typespec-validation/src/index.ts

@@ -1,6 +1,5 @@
 import { ParseArgsConfig, parseArgs } from "node:util";
-import { Suppression, getSuppressions } from "suppressions";
-
+import { Suppression } from "suppressions";
 import { CompileRule } from "./rules/compile.js";
 import { EmitAutorestRule } from "./rules/emit-autorest.js";
 import { FlavorAzureRule } from "./rules/flavor-azure.js";
@@ -34,8 +33,12 @@
   }
   console.log("Running TypeSpecValidation on folder: ", absolutePath);
 
-  const suppressions: Suppression[] = await getSuppressions("TypeSpecValidation", absolutePath);
-  if (suppressions && suppressions[0]) {
+  const suppressions: Suppression[] = await host.getSuppressions(absolutePath);
+
+  // Suppressions for the whole tool must have no rules or sub-rules
+  const toolSuppressions = suppressions.filter((s) => !s.rules?.length && !s.subRules?.length);
+
+  if (toolSuppressions && toolSuppressions[0]) {
     // Use reason from first matching suppression and ignore rest
     console.log(`  Suppressed: ${suppressions[0].reason}`);
     return;

eng/tools/typespec-validation/src/rule.ts

@@ -1,4 +1,4 @@
 import { RuleResult } from "./rule-result.js";
 import { TsvHost } from "./tsv-host.js";
 
 export interface Rule {
@@ -8,5 +8,5 @@
   readonly action?: string;
   // TODO: required when all rules apply it
   readonly link?: string;
-  execute(host?: TsvHost, folder?: string): Promise<RuleResult>;
+  execute(host: TsvHost, folder: string): Promise<RuleResult>;
 }

eng/tools/typespec-validation/src/rules/sdk-tspconfig-validation.ts

@@ -1,8 +1,9 @@
-import { RuleResult } from "../rule-result.js";
-import { Rule } from "../rule.js";
-import { TsvHost } from "../tsv-host.js";
 import { join } from "path";
 import { parse as yamlParse } from "yaml";
+import { Rule } from "../rule.js";
+import { RuleResult } from "../rule-result.js";
+import { TsvHost } from "../tsv-host.js";
+import { Suppression } from "suppressions";
 
 type ExpectedValueType = string | boolean | RegExp;
 type SkipResult = { shouldSkip: boolean; reason?: string };
@@ -48,8 +49,6 @@
     return { shouldSkip: false };
   }
 
-  protected abstract validate(config: any): RuleResult;
-
   protected validateValue(
     actual: string | boolean | undefined,
     expected: ExpectedValueType,
@@ -72,6 +71,9 @@
       errorOutput: `- ${error}. ${action}.`,
     };
   }
+
+  public abstract getPathOfKeyToValidate(): string;
+  protected abstract validate(config: any): RuleResult;
 }
 
 class TspconfigParameterSubRuleBase extends TspconfigSubRuleBase {
@@ -95,6 +97,10 @@
 
     return { success: true };
   }
+
+  public getPathOfKeyToValidate() {
+    return `parameters.${this.keyToValidate}.default`;
+  }
 }
 
 class TspconfigEmitterOptionsSubRuleBase extends TspconfigSubRuleBase {
@@ -132,6 +138,10 @@
 
     return { success: true };
   }
+
+  public getPathOfKeyToValidate() {
+    return `options.${this.emitterName}.${this.keyToValidate}`;
+  }
 }
 
 function isManagementSdk(folder: string): boolean {
@@ -444,18 +454,26 @@
 ];
 
 export class SdkTspConfigValidationRule implements Rule {
-  private rules: TspconfigSubRuleBase[] = [];
+  private subRules: TspconfigSubRuleBase[] = [];
+  private suppressedKeyPaths: Set<string> = new Set();
   name = "SdkTspConfigValidation";
   description = "Validate the SDK tspconfig.yaml file";
 
-  constructor(rules?: TspconfigSubRuleBase[]) {
-    this.rules = rules || defaultRules;
+  constructor(subRules: TspconfigSubRuleBase[] = defaultRules) {
+    this.subRules = subRules;
   }
-  async execute(host?: TsvHost, folder?: string): Promise<RuleResult> {
+
+  async execute(host: TsvHost, folder: string): Promise<RuleResult> {
+    const tspConfigPath = join(folder, "tspconfig.yaml");
+    const suppressions = await host.getSuppressions(tspConfigPath);
+    this.setSuppressedKeyPaths(suppressions);
+
     const failedResults = [];
     let success = true;
-    for (const rule of this.rules) {
-      const result = await rule.execute(host!, folder!);
+    for (const subRule of this.subRules) {
+      // TODO: support wildcard
+      if (this.suppressedKeyPaths.has(subRule.getPathOfKeyToValidate())) continue;
+      const result = await subRule.execute(host, folder!);
       if (!result.success) failedResults.push(result);
       success &&= result.success;
     }
@@ -466,4 +484,12 @@
       stdOutput: `[${this.name}]: validation ${success ? "passed" : "failed"}.\n${failedResults.map((r) => r.errorOutput).join("\n")}`,
     };
   }
+
+  private setSuppressedKeyPaths(suppressions: Suppression[]) {
+    this.suppressedKeyPaths = new Set<string>();
+    for (const suppression of suppressions) {
+      if (!suppression.rules?.includes(this.name)) continue;
+      for (const ignoredKey of suppression.subRules ?? []) this.suppressedKeyPaths.add(ignoredKey);
+    }
+  }
 }

eng/tools/typespec-validation/src/tsv-host.ts

@@ -1,3 +1,4 @@
+import { Suppression } from "suppressions";
 import { RuleResult } from "./rule-result.js";
 
 export interface TsvHost {
@@ -9,6 +10,7 @@
   normalizePath(folder: string): string;
   gitDiffTopSpecFolder(host: TsvHost, folder: string): Promise<RuleResult>;
   globby(patterns: string[]): Promise<string[]>;
+  getSuppressions(path: string): Promise<Suppression[]>;
 }
 
 export interface IGitOperation {

eng/tools/typespec-validation/src/tsv-runner-host.ts

@@ -1,16 +1,17 @@
-import { join } from "path";
 import { readFile } from "fs/promises";
-import { IGitOperation, TsvHost } from "./tsv-host.js";
 import { globby } from "globby";
+import { join } from "path";
 import { simpleGit } from "simple-git";
+import { getSuppressions as getSuppressionsImpl, Suppression } from "suppressions";
+import { RuleResult } from "./rule-result.js";
+import { IGitOperation, TsvHost } from "./tsv-host.js";
 import {
   checkFileExists,
+  gitDiffTopSpecFolder,
   isDirectory,
   normalizePath,
   runCmd,
-  gitDiffTopSpecFolder,
 } from "./utils.js";
-import { RuleResult } from "./rule-result.js";
 
 export class TsvRunnerHost implements TsvHost {
   checkFileExists(file: string): Promise<boolean> {
@@ -44,4 +45,8 @@
   globby(patterns: string[]): Promise<string[]> {
     return globby(patterns);
   }
+
+  getSuppressions(path: string): Promise<Suppression[]> {
+    return getSuppressionsImpl("TypeSpecValidation", path);
+  }
 }

eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts

@@ -1,4 +1,4 @@
 import { describe, it } from "vitest";
 
 import {
   SdkTspConfigValidationRule,
@@ -162,6 +162,7 @@
   subRules: TspconfigSubRuleBase[];
   tspconfigContent: string;
   success: boolean;
+  ignoredKeyPaths?: string[];
 }
 
 const managementTspconfigFolder = "contosowidgetmanager/Contoso.Management/";
@@ -463,6 +464,32 @@
   [new TspConfigCsharpMgmtPackageDirectorySubRule()],
 );
 
+const suppressSubRuleTestCases: Case[] = [
+  {
+    description: "Suppress parameter",
+    folder: "",
+    subRules: [new TspConfigCommonAzServiceDirMatchPatternSubRule()],
+    tspconfigContent: `
+parameters:
+  service-dir-x: ""
+`,
+    success: true,
+    ignoredKeyPaths: ["parameters.service-dir.default"],
+  },
+  {
+    description: "Suppress option",
+    folder: managementTspconfigFolder,
+    subRules: [new TspConfigTsMgmtModularPackageDirectorySubRule()],
+    tspconfigContent: `
+options:
+  "@azure-tools/typespec-ts":
+    package-dir: "*@#$%(@)*$#@!()#*&"
+`,
+    success: true,
+    ignoredKeyPaths: ["options.@azure-tools/typespec-ts.package-dir"],
+  },
+];
+
 describe("tspconfig", function () {
   it.each([
     // common
@@ -504,12 +531,23 @@
     ...csharpAzNamespaceTestCases,
     ...csharpAzClearOutputFolderTestCases,
     ...csharpMgmtPackageDirTestCases,
+    // suppression
+    ...suppressSubRuleTestCases,
   ])(`$description`, async (c: Case) => {
     let host = new TsvTestHost();
     host.checkFileExists = async (file: string) => {
       return file === join(c.folder, "tspconfig.yaml");
     };
     host.readTspConfig = async (_folder: string) => c.tspconfigContent;
+    host.getSuppressions = async (_path: string) => [
+      {
+        tool: "TypeSpecValidation",
+        paths: ["tspconfig.yaml"],
+        reason: "Test reason",
+        rules: ["SdkTspConfigValidation"],
+        subRules: c.ignoredKeyPaths,
+      },
+    ];
     const rule = new SdkTspConfigValidationRule(c.subRules);
     const result = await rule.execute(host, c.folder);
     strictEqual(result.success, true);

eng/tools/typespec-validation/test/tsv-test-host.ts

@@ -1,7 +1,8 @@
+import defaultPath, { PlatformPath } from "path";
+import { Suppression } from "suppressions";
 import { RuleResult } from "../src/rule-result.js";
 import { IGitOperation, TsvHost } from "../src/tsv-host.js";
 import { normalizePath } from "../src/utils.js";
-import defaultPath, { PlatformPath } from "path";
 
 export { IGitOperation } from "../src/tsv-host.js";
 
@@ -86,4 +87,8 @@
   async globby(patterns: string[]): Promise<string[]> {
     return Promise.resolve(patterns);
   }
+
+  async getSuppressions(_path: string): Promise<Suppression[]> {
+    return Promise.resolve([]);
+  }
 }