[suppressions] Merge all suppressions.yaml files found up the directo…

…ry tree (#29860)

eng/tools/suppressions/src/suppressions.ts

@@ -1,3 +1,4 @@
+import { Stats } from "fs";
 import { access, constants, lstat, readFile } from "fs/promises";
 import { minimatch } from "minimatch";
 import { dirname, join, resolve, sep } from "path";
@@ -37,12 +38,13 @@ const suppressionSchema = z.array(
         paths: paths,
         reason: s.reason,
       } as Suppression;
-    })
+    }),
 );
 
 /**
- * Returns the suppressions for a tool applicable to a path.  Walks up the directory tree to the first file named
+ * Returns the suppressions for a tool applicable to a path.  Walks up the directory tree to find all files named
  * "suppressions.yaml", parses and validates the contents, and returns the suppressions matching the tool and path.
+ * Suppressions are ordered by file (closest to path is first), then within the file (closest to top is first).
  *
  * @param tool Name of tool. Matched against property "tool" in suppressions.yaml.
  * @param path Path to file or directory under analysis.
@@ -68,17 +70,21 @@ export async function getSuppressions(tool: string, path: string): Promise<Suppr
   // If path doesn't exist, throw instead of returning "[]" to prevent confusion
   await access(path, constants.R_OK);
 
-  let suppressionsFile: string | undefined = await findSuppressionsYaml(path);
-  if (suppressionsFile) {
-    return getSuppressionsFromYaml(
-      tool,
-      path,
-      suppressionsFile,
-      await readFile(suppressionsFile, { encoding: "utf8" })
+  let suppressionsFiles: string[] = await findSuppressionsFiles(path);
+  let suppressions: Suppression[] = [];
+
+  for (let suppressionsFile of suppressionsFiles) {
+    suppressions = suppressions.concat(
+      getSuppressionsFromYaml(
+        tool,
+        path,
+        suppressionsFile,
+        await readFile(suppressionsFile, { encoding: "utf8" }),
+      ),
     );
-  } else {
-    return [];
   }
+
+  return suppressions;
 }
 
 /**
@@ -112,7 +118,7 @@ export function getSuppressionsFromYaml(
   tool: string,
   path: string,
   suppressionsFile: string,
-  suppressionsYaml: string
+  suppressionsYaml: string,
 ): Suppression[] {
   path = resolve(path);
   suppressionsFile = resolve(suppressionsFile);
@@ -144,39 +150,47 @@ export function getSuppressionsFromYaml(
 }
 
 /**
- * Returns absolute path to suppressions.yaml applying to input (or "undefined" if none found).
- * Walks up directory tree until first file matching "suppressions.yaml".
+ * Returns absolute paths to all suppressions.yaml files applying to input (or "undefined" if none found).
+ * Walks up directory tree, returning all files matching "suppressions.yaml", in order (may be empty);
  *
  * @param path Path to file under analysis.
  *
  * @example
  * ```
- * // Prints '/home/user/specs/specification/foo/suppressions.yaml':
+ * // Prints
+ * //   '/home/user/specs/specification/foo/suppressions.yaml'
+ * //   '/home/user/specs/specification/suppressions.yaml
  * console.log(findSuppressionsYaml(
  *   "specification/foo/data-plane/Foo/stable/2024-01-01/foo.json"
  * ));
  * ```
  */
-async function findSuppressionsYaml(path: string): Promise<string | undefined> {
+async function findSuppressionsFiles(path: string): Promise<string[]> {
+  const suppressionsFiles: string[] = [];
+
   path = resolve(path);
 
-  const stats = await lstat(path);
+  const stats: Stats = await lstat(path);
   let currentDirectory: string = stats.isDirectory() ? path : dirname(path);
 
   while (true) {
     const suppressionsFile: string = join(currentDirectory, "suppressions.yaml");
     try {
       // Throws if file cannot be read
       await access(suppressionsFile, constants.R_OK);
-      return suppressionsFile;
+      suppressionsFiles.push(suppressionsFile);
     } catch {
-      const parentDirectory: string = dirname(currentDirectory);
-      if (parentDirectory !== currentDirectory) {
-        currentDirectory = parentDirectory;
-      } else {
-        // Reached fs root but no "suppressions.yaml" found
-        return;
-      }
+      // File does not exist (or cannot be read), so skip this directory and check the parent
+    }
+
+    const parentDirectory: string = dirname(currentDirectory);
+    if (parentDirectory !== currentDirectory) {
+      currentDirectory = parentDirectory;
+    } else {
+      // Reached fs root
+      break;
     }
   }
+
+  return suppressionsFiles;
 }

eng/tools/suppressions/test/e2e/merge/foo/bar/suppressions.yaml

@@ -0,0 +1,11 @@
+- tool: TestTool
+  path: '**'
+  reason: bar-globstar
+
+- tool: TestTool
+  path: '*'
+  reason: bar-star
+
+- tool: TestTool
+  path: bar.json
+  reason: bar-exact

eng/tools/suppressions/test/e2e/merge/foo/suppressions.yaml

@@ -0,0 +1,23 @@
+- tool: TestTool
+  path: '**'
+  reason: foo-globstar
+
+- tool: TestTool
+  path: '*'
+  reason: foo-star
+
+- tool: TestTool
+  path: foo.json
+  reason: foo-exact
+
+- tool: TestTool
+  path: bar/**
+  reason: foo-bar-globstar
+
+- tool: TestTool
+  path: bar/*
+  reason: foo-bar-star
+
+- tool: TestTool
+  path: bar/bar.json
+  reason: foo-bar-exact

eng/tools/suppressions/test/e2e/merge/suppressions.yaml

@@ -0,0 +1,23 @@
+- tool: TestTool
+  path: foo/**
+  reason: root-foo-globstar
+
+- tool: TestTool
+  path: foo/*
+  reason: root-foo-star
+
+- tool: TestTool
+  path: foo/foo.json
+  reason: root-foo-exact
+
+- tool: TestTool
+  path: foo/bar/**
+  reason: root-foo-bar-globstar
+
+- tool: TestTool
+  path: foo/bar/*
+  reason: root-foo-bar-star
+
+- tool: TestTool
+  path: foo/bar/bar.json
+  reason: root-foo-bar-exact

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

@@ -2,6 +2,13 @@ import { expect, test } from "vitest";
 import { Suppression, getSuppressions } from "../src/suppressions.js";
 import { join } from "path";
 
+/**
+ * Returns the suppressions for a tool (default "TestTool") applicable to a path under folder "e2e".
+ *
+ * @param path Relative path to to file or directory to analyze, under folder "e2e".
+ * @param tool Name of tool. Matched against property "tool" in suppressions.yaml.  Defaults to "TestTool".
+ * @returns Array of suppressions matching tool and path (may be empty).
+ */
 async function getTestSuppressions(
   path: string,
   tool: string = "TestTool",
@@ -46,3 +53,102 @@ test.concurrent("suppress foo.json, get foo.json w/ different tool", async () =>
   );
   expect(suppressions).toEqual([]);
 });
+
+test.concurrent("merge, get bar.json", async () => {
+  const suppressions: Suppression[] = await getTestSuppressions(
+    join("merge", "foo", "bar", "bar.json"),
+  );
+  expect(suppressions).toEqual([
+    {
+      tool: "TestTool",
+      paths: ["**"],
+      reason: "bar-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["*"],
+      reason: "bar-star",
+    },
+    {
+      tool: "TestTool",
+      paths: ["bar.json"],
+      reason: "bar-exact",
+    },
+    {
+      tool: "TestTool",
+      paths: ["**"],
+      reason: "foo-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["bar/**"],
+      reason: "foo-bar-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["bar/*"],
+      reason: "foo-bar-star",
+    },
+    {
+      tool: "TestTool",
+      paths: ["bar/bar.json"],
+      reason: "foo-bar-exact",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/**"],
+      reason: "root-foo-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/bar/**"],
+      reason: "root-foo-bar-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/bar/*"],
+      reason: "root-foo-bar-star",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/bar/bar.json"],
+      reason: "root-foo-bar-exact",
+    },
+  ]);
+});
+
+test.concurrent("merge, get foo.json", async () => {
+  const suppressions: Suppression[] = await getTestSuppressions(join("merge", "foo", "foo.json"));
+  expect(suppressions).toEqual([
+    {
+      tool: "TestTool",
+      paths: ["**"],
+      reason: "foo-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["*"],
+      reason: "foo-star",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo.json"],
+      reason: "foo-exact",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/**"],
+      reason: "root-foo-globstar",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/*"],
+      reason: "root-foo-star",
+    },
+    {
+      tool: "TestTool",
+      paths: ["foo/foo.json"],
+      reason: "root-foo-exact",
+    },
+  ]);
+});