diff --git a/eng/tools/suppressions/src/suppressions.ts b/eng/tools/suppressions/src/suppressions.ts index a6f3d6433c29..1ff33d987997 100644 --- a/eng/tools/suppressions/src/suppressions.ts +++ b/eng/tools/suppressions/src/suppressions.ts @@ -11,6 +11,8 @@ export interface Suppression { tool: string; // Output only exposes "paths". For input, if "path" is defined, it is inserted at the start of "paths". paths: string[]; + rule: string; + subRules: string[]; reason: string; } @@ -21,6 +23,8 @@ const suppressionSchema = z.array( // For now, input allows "path" alongside "paths". Lather, may deprecate "path". path: z.string().optional(), paths: z.array(z.string()).optional(), + rule: z.string().optional(), + "sub-rules": z.array(z.string()).optional(), reason: z.string(), }) .refine((data) => data.path || data.paths?.[0], { @@ -36,6 +40,8 @@ const suppressionSchema = z.array( return { tool: s.tool, paths: paths, + rule: s.rule, + subRules: s["sub-rules"], reason: s.reason, } as Suppression; }), diff --git a/eng/tools/suppressions/test/e2e/subRuleSuppressionsYaml/suppressions.yaml b/eng/tools/suppressions/test/e2e/subRuleSuppressionsYaml/suppressions.yaml new file mode 100644 index 000000000000..729d74b1e2c4 --- /dev/null +++ b/eng/tools/suppressions/test/e2e/subRuleSuppressionsYaml/suppressions.yaml @@ -0,0 +1,6 @@ +- tool: TypeSpecValidation + rule: SdkTspConfigValidation + path: . + sub-rules: + - parameters.service-dir.default + reason: test diff --git a/eng/tools/suppressions/test/suppressions.e2e.test.ts b/eng/tools/suppressions/test/suppressions.e2e.test.ts index 848dfdd842a8..cfc57a2042cd 100644 --- a/eng/tools/suppressions/test/suppressions.e2e.test.ts +++ b/eng/tools/suppressions/test/suppressions.e2e.test.ts @@ -152,3 +152,19 @@ test.concurrent("merge, get foo.json", async () => { }, ]); }); + +test.concurrent("suppress with sub rules", async () => { + const suppressions: Suppression[] = await getTestSuppressions( + "subRuleSuppressionsYaml", + "TypeSpecValidation", + ); + expect(suppressions).toEqual([ + { + tool: "TypeSpecValidation", + paths: ["."], + reason: "test", + rule: "SdkTspConfigValidation", + subRules: ["parameters.service-dir.default"], + }, + ]); +}); diff --git a/eng/tools/suppressions/test/suppressions.test.ts b/eng/tools/suppressions/test/suppressions.test.ts index 68d630f5f6d9..c9a8d0becef7 100644 --- a/eng/tools/suppressions/test/suppressions.test.ts +++ b/eng/tools/suppressions/test/suppressions.test.ts @@ -249,3 +249,29 @@ test("yaml array not suppression", () => { `[ZodValidationError: Validation error: Required at "[0].tool"; Required at "[0].reason"]`, ); }); + +test("suppression with sub rules", () => { + let suppressions: Suppression[] = getSuppressionsFromYaml( + "TestTool", + "foo", + "suppressions.yaml", + ` +- tool: TestTool + path: foo + rule: my-rule + sub-rules: + - my.option.a + - my.option.b + reason: test + `, + ); + expect(suppressions).toStrictEqual([ + { + tool: "TestTool", + paths: ["foo"], + rule: "my-rule", + subRules: ["my.option.a", "my.option.b"], + reason: "test", + }, + ]); +}); diff --git a/eng/tools/typespec-validation/src/index.ts b/eng/tools/typespec-validation/src/index.ts index d3aad77fda91..f297017befa5 100755 --- a/eng/tools/typespec-validation/src/index.ts +++ b/eng/tools/typespec-validation/src/index.ts @@ -1,15 +1,10 @@ +import { readdir } from "node:fs/promises"; +import { join, dirname } from "node:path"; import { ParseArgsConfig, parseArgs } from "node:util"; import { Suppression, getSuppressions } from "suppressions"; - -import { CompileRule } from "./rules/compile.js"; -import { EmitAutorestRule } from "./rules/emit-autorest.js"; -import { FlavorAzureRule } from "./rules/flavor-azure.js"; -import { FolderStructureRule } from "./rules/folder-structure.js"; -import { FormatRule } from "./rules/format.js"; -import { LinterRulesetRule } from "./rules/linter-ruleset.js"; -import { NpmPrefixRule } from "./rules/npm-prefix.js"; -import { SdkTspConfigValidationRule } from "./rules/sdk-tspconfig-validation.js"; import { TsvRunnerHost } from "./tsv-runner-host.js"; +import { fileURLToPath } from "url"; +import { Rule } from "./rule.js"; export async function main() { const host = new TsvRunnerHost(); @@ -38,24 +33,17 @@ export async function main() { if (suppressions && suppressions[0]) { // Use reason from first matching suppression and ignore rest console.log(` Suppressed: ${suppressions[0].reason}`); - return; } - const rules = [ - new FolderStructureRule(), - new NpmPrefixRule(), - new EmitAutorestRule(), - new FlavorAzureRule(), - new LinterRulesetRule(), - new CompileRule(), - new FormatRule(), - new SdkTspConfigValidationRule(), - ]; + const ruleMap = await loadRules(); + const subRuleMap = getSubRules(ruleMap, suppressions); + const rules = [...ruleMap.values()]; + let success = true; for (let i = 0; i < rules.length; i++) { const rule = rules[i]; console.log("\nExecuting rule: " + rule.name); - const result = await rule.execute(host, absolutePath); + const result = await rule.execute(host, absolutePath, subRuleMap.get(rule.name)); if (result.stdOutput) console.log(result.stdOutput); if (!result.success) { success = false; @@ -72,3 +60,32 @@ export async function main() { process.exitCode = 1; } } + +async function loadRules(): Promise> { + const currentPath = fileURLToPath(import.meta.url); + const currentDir = dirname(currentPath); + const ruleFileNames = (await readdir(join(currentDir, "rules"))).filter((p) => p.endsWith(".js")); + const ruleMap = new Map(); + for (const ruleFileName of ruleFileNames) { + const module = await import(join("file://", currentDir, "rules", ruleFileName)); + const rule = new module.default() as Rule; + ruleMap.set(rule.name, rule); + } + return ruleMap; +} + +function getSubRules(ruleMap: Map, suppressions: Suppression[]) { + const subRuleMap = new Map>(); + for (const suppression of suppressions) { + if ( + ruleMap.has(suppression.rule) && + (!suppression["subRules"] || suppression["subRules"].length == 0) + ) { + ruleMap.delete(suppression.rule); + } + const subRules = suppression["subRules"]; + if (subRules && subRules.length > 0) + subRuleMap.set(suppression.rule, new Set([...subRules])); + } + return subRuleMap; +} diff --git a/eng/tools/typespec-validation/src/rule.ts b/eng/tools/typespec-validation/src/rule.ts index b0f2494dd2f6..f1cf56b96369 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, subRules?: Set): Promise; } diff --git a/eng/tools/typespec-validation/src/rules/compile.ts b/eng/tools/typespec-validation/src/rules/compile.ts index 3e472075778e..d919ccfb79e6 100644 --- a/eng/tools/typespec-validation/src/rules/compile.ts +++ b/eng/tools/typespec-validation/src/rules/compile.ts @@ -60,3 +60,5 @@ export class CompileRule implements Rule { }; } } + +export default CompileRule; diff --git a/eng/tools/typespec-validation/src/rules/emit-autorest.ts b/eng/tools/typespec-validation/src/rules/emit-autorest.ts index 6e3e0882cb2d..5128c3b4a377 100644 --- a/eng/tools/typespec-validation/src/rules/emit-autorest.ts +++ b/eng/tools/typespec-validation/src/rules/emit-autorest.ts @@ -41,3 +41,5 @@ export class EmitAutorestRule implements Rule { }; } } + +export default EmitAutorestRule; diff --git a/eng/tools/typespec-validation/src/rules/flavor-azure.ts b/eng/tools/typespec-validation/src/rules/flavor-azure.ts index 1c01955448b3..4443cbfd1460 100644 --- a/eng/tools/typespec-validation/src/rules/flavor-azure.ts +++ b/eng/tools/typespec-validation/src/rules/flavor-azure.ts @@ -51,3 +51,5 @@ export class FlavorAzureRule implements Rule { return regex.test(name); } } + +export default FlavorAzureRule; diff --git a/eng/tools/typespec-validation/src/rules/folder-structure.ts b/eng/tools/typespec-validation/src/rules/folder-structure.ts index 9e010e12d664..4ba8a2baaf1e 100644 --- a/eng/tools/typespec-validation/src/rules/folder-structure.ts +++ b/eng/tools/typespec-validation/src/rules/folder-structure.ts @@ -110,3 +110,5 @@ export class FolderStructureRule implements Rule { }; } } + +export default FolderStructureRule; diff --git a/eng/tools/typespec-validation/src/rules/format.ts b/eng/tools/typespec-validation/src/rules/format.ts index 1ce13f5c870c..e4d9ccaf81e4 100644 --- a/eng/tools/typespec-validation/src/rules/format.ts +++ b/eng/tools/typespec-validation/src/rules/format.ts @@ -50,3 +50,5 @@ export class FormatRule implements Rule { }; } } + +export default FormatRule; diff --git a/eng/tools/typespec-validation/src/rules/linter-ruleset.ts b/eng/tools/typespec-validation/src/rules/linter-ruleset.ts index 5bc11f75bbc6..aca5990c433c 100644 --- a/eng/tools/typespec-validation/src/rules/linter-ruleset.ts +++ b/eng/tools/typespec-validation/src/rules/linter-ruleset.ts @@ -105,3 +105,5 @@ export class LinterRulesetRule implements Rule { }; } } + +export default LinterRulesetRule; diff --git a/eng/tools/typespec-validation/src/rules/npm-prefix.ts b/eng/tools/typespec-validation/src/rules/npm-prefix.ts index 6f73792211cc..cc81e01fe499 100644 --- a/eng/tools/typespec-validation/src/rules/npm-prefix.ts +++ b/eng/tools/typespec-validation/src/rules/npm-prefix.ts @@ -47,3 +47,5 @@ export class NpmPrefixRule implements Rule { }; } } + +export default NpmPrefixRule; 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 dda07055a2b7..d70cd1a11d28 100644 --- a/eng/tools/typespec-validation/src/rules/sdk-tspconfig-validation.ts +++ b/eng/tools/typespec-validation/src/rules/sdk-tspconfig-validation.ts @@ -48,8 +48,6 @@ export abstract class TspconfigSubRuleBase { return { shouldSkip: false }; } - protected abstract validate(config: any): RuleResult; - protected validateValue( actual: string | boolean | undefined, expected: ExpectedValueType, @@ -72,6 +70,9 @@ export abstract class TspconfigSubRuleBase { errorOutput: `- ${error}. ${action}.`, }; } + + public abstract getPathOfKeyToValidate(): string; + protected abstract validate(config: any): RuleResult; } class TspconfigParameterSubRuleBase extends TspconfigSubRuleBase { @@ -95,6 +96,10 @@ class TspconfigParameterSubRuleBase extends TspconfigSubRuleBase { return { success: true }; } + + public getPathOfKeyToValidate() { + return `parameters.${this.keyToValidate}.default`; + } } class TspconfigEmitterOptionsSubRuleBase extends TspconfigSubRuleBase { @@ -132,6 +137,10 @@ class TspconfigEmitterOptionsSubRuleBase extends TspconfigSubRuleBase { return { success: true }; } + + public getPathOfKeyToValidate() { + return `options.${this.emitterName}.${this.keyToValidate}`; + } } function isManagementSdk(folder: string): boolean { @@ -400,18 +409,24 @@ export const defaultRules = [ ]; export class SdkTspConfigValidationRule implements Rule { - private rules: TspconfigSubRuleBase[] = []; + private subRules: TspconfigSubRuleBase[] = []; name = "SdkTspConfigValidation"; description = "Validate the SDK tspconfig.yaml file"; - constructor(rules?: TspconfigSubRuleBase[]) { - this.rules = rules || defaultRules; + constructor(subRules?: TspconfigSubRuleBase[]) { + this.subRules = subRules || defaultRules; } - async execute(host?: TsvHost, folder?: string): Promise { + async execute( + host?: TsvHost, + folder?: string, + ignoredOptions?: Set, + ): Promise { const failedResults = []; let success = true; - for (const rule of this.rules) { - const result = await rule.execute(host!, folder!); + for (const subRule of this.subRules) { + if (ignoredOptions && ignoredOptions.has(subRule.getPathOfKeyToValidate())) continue; + console.log(subRule.getPathOfKeyToValidate()); + const result = await subRule.execute(host!, folder!); if (!result.success) failedResults.push(result); success &&= result.success; } @@ -423,3 +438,5 @@ export class SdkTspConfigValidationRule implements Rule { }; } } + +export default SdkTspConfigValidationRule; 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 9555a1a74a93..6c3240674f3c 100644 --- a/eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts +++ b/eng/tools/typespec-validation/test/sdk-tspconfig-validation.test.ts @@ -150,6 +150,7 @@ interface Case { subRules: TspconfigSubRuleBase[]; tspconfigContent: string; success: boolean; + ignoredOptions?: Set; } const managementTspconfigFolder = "contosowidgetmanager/Contoso.Management/"; @@ -369,6 +370,32 @@ const csharpMgmtPackageDirTestCases = createEmitterOptionTestCases( [new TspConfigCsharpMgmtPackageDirectorySubRule()], ); +const suppressSubRuleTestCases: Case[] = [ + { + description: "Suppress parameter", + folder: "", + subRules: [new TspConfigCommonAzServiceDirMatchPatternSubRule()], + tspconfigContent: ` +parameters: + service-dir-x: "" +`, + success: true, + ignoredOptions: new Set(["parameters.service-dir.default"]), + }, + { + description: "Suppress option", + folder: managementTspconfigFolder, + subRules: [new TspConfigTsMgmtModularPackageDirectorySubRule()], + tspconfigContent: ` +options: + "@azure-tools/typespec-ts": + package-dir: "*@#$%(@)*$#@!()#*&" +`, + success: true, + ignoredOptions: new Set(["options.@azure-tools/typespec-ts.package-dir"]), + }, +]; + describe("tspconfig", function () { it.each([ // common @@ -401,6 +428,8 @@ describe("tspconfig", function () { ...csharpAzNamespaceTestCases, ...csharpAzClearOutputFolderTestCases, ...csharpMgmtPackageDirTestCases, + // suppression + ...suppressSubRuleTestCases, ])(`$description`, async (c: Case) => { let host = new TsvTestHost(); host.checkFileExists = async (file: string) => { @@ -408,7 +437,7 @@ describe("tspconfig", function () { }; host.readTspConfig = async (_folder: string) => c.tspconfigContent; const rule = new SdkTspConfigValidationRule(c.subRules); - const result = await rule.execute(host, c.folder); + const result = await rule.execute(host, c.folder, c.ignoredOptions); strictEqual(result.success, true); if (c.success) strictEqual(result.stdOutput?.includes("[SdkTspConfigValidation]: validation passed."), true);