[EngSys] Add "if" property to suppressions (Azure#35605)

- Fixes Azure#35153

Author: mikeharder

eng/scripts/TypeSpec-Validation.ps1

@@ -18,7 +18,7 @@ if ($TotalShards -gt 0 -and $Shard -ge $TotalShards) {
 . $PSScriptRoot/Suppressions-Functions.ps1
 . $PSScriptRoot/Array-Functions.ps1
 
-$typespecFolders, $checkedAll = &"$PSScriptRoot/Get-TypeSpec-Folders.ps1" `
+$typespecFolders, $checkingAllSpecs = &"$PSScriptRoot/Get-TypeSpec-Folders.ps1" `
   -BaseCommitish:$BaseCommitish `
   -HeadCommitish:$HeadCommitish `
   -CheckAll:$CheckAll `
@@ -35,11 +35,11 @@ foreach ($typespecFolder in $typespecFolders) {
 
 $typespecFoldersWithFailures = @()
 if ($typespecFolders) {
-  $typespecFolders = $typespecFolders.Split('',[System.StringSplitOptions]::RemoveEmptyEntries)
+  $typespecFolders = $typespecFolders.Split('', [System.StringSplitOptions]::RemoveEmptyEntries)
   foreach ($typespecFolder in $typespecFolders) {
     LogGroupStart "Validating $typespecFolder"
 
-    if ($checkedAll) {
+    if ($checkingAllSpecs) {
       $suppression = Get-Suppression "TypeSpecValidationAll" $typespecFolder
       if ($suppression) {
         $reason = $suppression["reason"] ?? "<no reason specified>"
@@ -49,14 +49,17 @@ if ($typespecFolders) {
       }
     }
 
-    LogInfo "npm exec --no -- tsv $typespecFolder"
+    # Example: '{"checkingAllSpecs"=true}'
+    $context = @{ checkingAllSpecs = $checkingAllSpecs } | ConvertTo-Json -Compress
+
+    LogInfo "npm exec --no -- tsv $typespecFolder ""$context"""
 
     if ($DryRun) {
       LogGroupEnd
       continue
     }
 
-    npm exec --no -- tsv $typespecFolder 2>&1 | Write-Host
+    npm exec --no -- tsv $typespecFolder "$context" 2>&1 | Write-Host
     if ($LASTEXITCODE) {
       $typespecFoldersWithFailures += $typespecFolder
       $errorString = "TypeSpec Validation failed for project $typespecFolder run the following command locally to validate."
@@ -71,7 +74,8 @@ if ($typespecFolders) {
     }
     LogGroupEnd
   }
-} else {
+}
+else {
   if ($CheckAll) {
     LogError "TypeSpec Validation - All did not validate any specs"
     LogJobFailure

eng/tools/suppressions/src/suppressions.ts

@@ -1,14 +1,18 @@
 import { Stats } from "fs";
 import { access, constants, lstat, readFile } from "fs/promises";
 import { minimatch } from "minimatch";
+import { createRequire } from "module";
 import { dirname, join, resolve, sep } from "path";
 import { sep as posixSep } from "path/posix";
+import vm from "vm";
 import { parse as yamlParse } from "yaml";
 import { z } from "zod";
 import { fromError } from "zod-validation-error";
 
 export interface Suppression {
   tool: string;
+  // String of JavaScript CJS code, executed in a prepared context, that determines if a suppression should be included
+  if?: string;
   // Output only exposes "paths".  For input, if "path" is defined, it is inserted at the start of "paths".
   paths: string[];
   rules?: string[];
@@ -20,6 +24,7 @@ const suppressionSchema = z.array(
   z
     .object({
       tool: z.string(),
+      if: z.string().optional(),
       // For now, input allows "path" alongside "paths".  Lather, may deprecate "path".
       path: z.string().optional(),
       paths: z.array(z.string()).optional(),
@@ -39,6 +44,7 @@ const suppressionSchema = z.array(
       }
       return {
         tool: s.tool,
+        if: s.if,
         paths: paths,
         rules: s.rules,
         subRules: s["sub-rules"],
@@ -70,7 +76,11 @@ const suppressionSchema = z.array(
  * );
  * ```
  */
-export async function getSuppressions(tool: string, path: string): Promise<Suppression[]> {
+export async function getSuppressions(
+  tool: string,
+  path: string,
+  context: Record<string, any> = {},
+): Promise<Suppression[]> {
   path = resolve(path);
 
   // If path doesn't exist, throw instead of returning "[]" to prevent confusion
@@ -86,6 +96,7 @@ export async function getSuppressions(tool: string, path: string): Promise<Suppr
         path,
         suppressionsFile,
         await readFile(suppressionsFile, { encoding: "utf8" }),
+        context,
       ),
     );
   }
@@ -125,6 +136,7 @@ export function getSuppressionsFromYaml(
   path: string,
   suppressionsFile: string,
   suppressionsYaml: string,
+  context: Record<string, any> = {},
 ): Suppression[] {
   path = resolve(path);
   suppressionsFile = resolve(suppressionsFile);
@@ -140,19 +152,28 @@ export function getSuppressionsFromYaml(
     throw fromError(err);
   }
 
-  return suppressions
-    .filter((s) => s.tool === tool)
-    .filter((s) => {
-      // Minimatch only allows forward-slashes in patterns and input
-      const pathPosix: string = path.split(sep).join(posixSep);
-
-      return s.paths.some((suppressionPath) => {
-        const pattern: string = join(dirname(suppressionsFile), suppressionPath)
-          .split(sep)
-          .join(posixSep);
-        return minimatch(pathPosix, pattern);
-      });
-    });
+  // Make "require" available inside sandbox for CJS imports
+  const sandbox = { ...context, require: createRequire(import.meta.url) };
+
+  return (
+    suppressions
+      // Tool name
+      .filter((s) => s.tool === tool)
+      // Path
+      .filter((s) => {
+        // Minimatch only allows forward-slashes in patterns and input
+        const pathPosix: string = path.split(sep).join(posixSep);
+
+        return s.paths.some((suppressionPath) => {
+          const pattern: string = join(dirname(suppressionsFile), suppressionPath)
+            .split(sep)
+            .join(posixSep);
+          return minimatch(pathPosix, pattern);
+        });
+      })
+      // If
+      .filter((s) => s.if === undefined || vm.runInNewContext(s.if, sandbox))
+  );
 }
 
 /**

eng/tools/suppressions/test/suppressions.test.ts

@@ -268,10 +268,77 @@ test("suppression with rules", () => {
   expect(suppressions).toStrictEqual([
     {
       tool: "TestTool",
+      if: undefined,
       paths: ["foo"],
       rules: ["my-rule"],
       subRules: ["my.option.a", "my.option.b"],
       reason: "test",
     },
   ]);
 });
+
+test.each([
+  { context: { foo: false, bar: false }, expected: ["no-if", "process-version"] },
+  {
+    context: { foo: true, bar: false },
+    expected: ["no-if", "if-foo", "if-foo-or-bar", "process-version"],
+  },
+  {
+    context: { foo: false, bar: true },
+    expected: ["no-if", "if-bar", "if-foo-or-bar", "process-version"],
+  },
+  {
+    context: { foo: true, bar: true },
+    expected: ["no-if", "if-foo", "if-bar", "if-foo-or-bar", "if-foo-and-bar", "process-version"],
+  },
+])("if($context)", ({ context, expected }) => {
+  const suppressionYaml = `
+- tool: TestTool
+  path: "**"
+  reason: no-if
+- tool: TestTool
+  path: "**"
+  if: foo
+  reason: if-foo
+- tool: TestTool
+  path: "**"
+  if: bar
+  reason: if-bar
+- tool: TestTool
+  path: "**"
+  if: foo || bar
+  reason: if-foo-or-bar
+- tool: TestTool
+  path: "**"
+  if: foo && bar
+  reason: if-foo-and-bar
+- tool: TestTool
+  path: "**"
+  if: require("process").version.startsWith("v")
+  reason: process-version
+`;
+
+  let suppressions: Suppression[] = getSuppressionsFromYaml(
+    "TestTool",
+    "test-path",
+    "suppressions.yaml",
+    suppressionYaml,
+    context,
+  );
+
+  expect(suppressions.map((s) => s.reason).sort()).toEqual(expected.sort());
+});
+
+test.each([
+  ["invalid javascript", "Unexpected identifier 'javascript'"],
+  ["1(1)", "1 is not a function"],
+])("if: %s", (ifExpression, expectedException) => {
+  expect(() =>
+    getSuppressionsFromYaml(
+      "TestTool",
+      "test-path",
+      "suppressions.yaml",
+      `- tool: TestTool\n  if: "${ifExpression}"\n  path: "**"\n  reason: test`,
+    ),
+  ).throws(expectedException);
+});

eng/tools/suppressions/vitest.config.ts

@@ -0,0 +1,9 @@
+import { configDefaults, defineConfig } from "vitest/config";
+
+export default defineConfig({
+  test: {
+    coverage: {
+      exclude: [...configDefaults.coverage.exclude!, "cmd/**", "src/index.ts"],
+    },
+  },
+});

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

@@ -1,5 +1,5 @@
-import { ParseArgsConfig, parseArgs } from "node:util";
 import { stat } from "node:fs/promises";
+import { ParseArgsConfig, parseArgs } from "node:util";
 import { Suppression } from "suppressions";
 import { CompileRule } from "./rules/compile.js";
 import { EmitAutorestRule } from "./rules/emit-autorest.js";
@@ -11,16 +11,28 @@ import { NpmPrefixRule } from "./rules/npm-prefix.js";
 import { SdkTspConfigValidationRule } from "./rules/sdk-tspconfig-validation.js";
 import { fileExists, getSuppressions, normalizePath } from "./utils.js";
 
+// Context argument may add new properties or override checkingAllSpecs
+export var context: Record<string, any> = { checkingAllSpecs: false };
+
 export async function main() {
   const args = process.argv.slice(2);
   const options = {
     folder: {
       type: "string",
       short: "f",
     },
+    context: {
+      type: "string",
+      short: "c",
+    },
   };
   const parsedArgs = parseArgs({ args, options, allowPositionals: true } as ParseArgsConfig);
   const folder = parsedArgs.positionals[0];
+
+  if (parsedArgs.positionals[1]) {
+    context = { ...context, ...JSON.parse(parsedArgs.positionals[1]) };
+  }
+
   const absolutePath = normalizePath(folder);
 
   if (!(await fileExists(absolutePath))) {

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

@@ -5,6 +5,7 @@ import { access, readFile } from "fs/promises";
 import defaultPath, { join, PlatformPath } from "path";
 import { simpleGit } from "simple-git";
 import { getSuppressions as getSuppressionsImpl, Suppression } from "suppressions";
+import { context } from "./index.js";
 
 // Enable simple-git debug logging to improve console output
 debug.enable("simple-git");
@@ -41,7 +42,7 @@ export async function readTspConfig(folder: string) {
 }
 
 export async function getSuppressions(path: string): Promise<Suppression[]> {
-  return getSuppressionsImpl("TypeSpecValidation", path);
+  return getSuppressionsImpl("TypeSpecValidation", path, context);
 }
 
 export function normalizePath(folder: string, path: PlatformPath = defaultPath) {