diff --git a/eng/tools/typespec-validation/src/index.ts b/eng/tools/typespec-validation/src/index.ts index d3aad77fda91..af72393153ef 100755 --- a/eng/tools/typespec-validation/src/index.ts +++ b/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 @@ export async function main() { } 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; diff --git a/eng/tools/typespec-validation/src/rule.ts b/eng/tools/typespec-validation/src/rule.ts index b0f2494dd2f6..64b26927c53a 100644 --- a/eng/tools/typespec-validation/src/rule.ts +++ b/eng/tools/typespec-validation/src/rule.ts @@ -8,5 +8,5 @@ export interface Rule { readonly action?: string; // TODO: required when all rules apply it readonly link?: string; - execute(host?: TsvHost, folder?: string): Promise; + execute(host: TsvHost, folder: string): Promise; } diff --git a/eng/tools/typespec-validation/src/rules/sdk-tspconfig-validation.ts b/eng/tools/typespec-validation/src/rules/sdk-tspconfig-validation.ts index 4800a8a14698..bf5c4b4b1604 100644 --- a/eng/tools/typespec-validation/src/rules/sdk-tspconfig-validation.ts +++ b/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 @@ export abstract class TspconfigSubRuleBase { return { shouldSkip: false }; } - protected abstract validate(config: any): RuleResult; - protected validateValue( actual: string | boolean | undefined, expected: ExpectedValueType, @@ -72,6 +71,9 @@ export abstract class TspconfigSubRuleBase { errorOutput: `- ${error}. ${action}.`, }; } + + public abstract getPathOfKeyToValidate(): string; + protected abstract validate(config: any): RuleResult; } class TspconfigParameterSubRuleBase extends TspconfigSubRuleBase { @@ -95,6 +97,10 @@ class TspconfigParameterSubRuleBase extends TspconfigSubRuleBase { return { success: true }; } + + public getPathOfKeyToValidate() { + return `parameters.${this.keyToValidate}.default`; + } } class TspconfigEmitterOptionsSubRuleBase extends TspconfigSubRuleBase { @@ -132,6 +138,10 @@ class TspconfigEmitterOptionsSubRuleBase extends TspconfigSubRuleBase { return { success: true }; } + + public getPathOfKeyToValidate() { + return `options.${this.emitterName}.${this.keyToValidate}`; + } } function isManagementSdk(folder: string): boolean { @@ -444,18 +454,26 @@ export const defaultRules = [ ]; export class SdkTspConfigValidationRule implements Rule { - private rules: TspconfigSubRuleBase[] = []; + private subRules: TspconfigSubRuleBase[] = []; + private suppressedKeyPaths: Set = 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 { + + async execute(host: TsvHost, folder: string): Promise { + 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 @@ export class SdkTspConfigValidationRule implements Rule { stdOutput: `[${this.name}]: validation ${success ? "passed" : "failed"}.\n${failedResults.map((r) => r.errorOutput).join("\n")}`, }; } + + private setSuppressedKeyPaths(suppressions: Suppression[]) { + this.suppressedKeyPaths = new Set(); + for (const suppression of suppressions) { + if (!suppression.rules?.includes(this.name)) continue; + for (const ignoredKey of suppression.subRules ?? []) this.suppressedKeyPaths.add(ignoredKey); + } + } } diff --git a/eng/tools/typespec-validation/src/tsv-host.ts b/eng/tools/typespec-validation/src/tsv-host.ts index 0965a498eaab..13aaff86df43 100644 --- a/eng/tools/typespec-validation/src/tsv-host.ts +++ b/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 @@ export interface TsvHost { normalizePath(folder: string): string; gitDiffTopSpecFolder(host: TsvHost, folder: string): Promise; globby(patterns: string[]): Promise; + getSuppressions(path: string): Promise; } export interface IGitOperation { diff --git a/eng/tools/typespec-validation/src/tsv-runner-host.ts b/eng/tools/typespec-validation/src/tsv-runner-host.ts index 80179e3f5132..e1d39a8187c1 100644 --- a/eng/tools/typespec-validation/src/tsv-runner-host.ts +++ b/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 { @@ -44,4 +45,8 @@ export class TsvRunnerHost implements TsvHost { globby(patterns: string[]): Promise { return globby(patterns); } + + getSuppressions(path: string): Promise { + return getSuppressionsImpl("TypeSpecValidation", path); + } } diff --git a/eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts b/eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts index 848dc4a41add..14666e0bd1f6 100644 --- a/eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts +++ b/eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts @@ -162,6 +162,7 @@ interface Case { subRules: TspconfigSubRuleBase[]; tspconfigContent: string; success: boolean; + ignoredKeyPaths?: string[]; } const managementTspconfigFolder = "contosowidgetmanager/Contoso.Management/"; @@ -463,6 +464,32 @@ const csharpMgmtPackageDirTestCases = createEmitterOptionTestCases( [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 @@ describe("tspconfig", function () { ...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); diff --git a/eng/tools/typespec-validation/test/tsv-test-host.ts b/eng/tools/typespec-validation/test/tsv-test-host.ts index 86a54425d23d..29c04c2b3c31 100644 --- a/eng/tools/typespec-validation/test/tsv-test-host.ts +++ b/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 @@ options: async globby(patterns: string[]): Promise { return Promise.resolve(patterns); } + + async getSuppressions(_path: string): Promise { + return Promise.resolve([]); + } }