[EngSys] Support folder structure v2

.github/workflows/avocado-code.yml

@@ -1,4 +1,4 @@
 name: "[TEST-IGNORE] Swagger Avocado - Analyze Code"
 
 on: pull_request
 
@@ -29,6 +29,7 @@
             "/package.json" \
             "/package-lock.json" \
             "/cadl/examples/" \
+            '(?=/examples/)(?!(?:/stable/|/preview/))' \
           --includePaths \
             "data-plane" \
             "resource-manager"

eng/tools/spec-gen-sdk-runner/src/utils.ts

@@ -1,4 +1,4 @@
 import { spawn, spawnSync, exec } from "node:child_process";
 import path from "node:path";
 import fs from "node:fs";
 import { LogLevel, logMessage } from "./log.js";
@@ -409,7 +409,7 @@
     }
 
     const info = serviceMap.get(serviceName)!;
-    if (folderPath.endsWith(".Management")) {
+    if (folderPath.endsWith(".Management") || folderPath.includes("resource-manager")) {
       info.managementPaths.push(folderPath);
     } else {
       info.otherTypeSpecPaths.push(folderPath);

eng/tools/typespec-validation/src/rules/folder-structure.ts

@@ -1,4 +1,4 @@
 import debug from "debug";
 import { readFile } from "fs/promises";
 import { globby } from "globby";
 import path from "path";
@@ -21,6 +21,11 @@
     const gitRoot = normalizePath(await simpleGit(folder).revparse("--show-toplevel"));
     const relativePath = path.relative(gitRoot, folder).split(path.sep).join("/");
 
+    // If the folder containing TypeSpec sources is under "data-plane" or "resource-manager", the spec
+    // must be using "folder structure v2".  Otherwise, it must be using v1.
+    const structureVersion =
+      relativePath.includes("data-plane") || relativePath.includes("resource-manager") ? 2 : 1;
+
     stdOutput += `folder: ${folder}\n`;
     if (!(await fileExists(folder))) {
       return {
@@ -39,35 +44,6 @@
       }
     });
 
-    // Verify top level folder is lower case and remove empty entries when splitting by slash
-    const folderStruct = relativePath.split("/").filter(Boolean);
-    if (folderStruct[1].match(/[A-Z]/g)) {
-      success = false;
-      errorOutput += `Invalid folder name. Folders under specification/ must be lower case.\n`;
-    }
-
-    const packageFolder = folderStruct[folderStruct.length - 1];
-
-    // Verify package folder is at most 3 levels deep
-    if (folderStruct.length > 4) {
-      success = false;
-      errorOutput += `Please limit TypeSpec folder depth to 3 levels or less`;
-    }
-
-    // Verify second level folder is capitalized after each '.'
-    if (/(^|\. *)([a-z])/g.test(packageFolder)) {
-      success = false;
-      errorOutput += `Invalid folder name. Folders under specification/${folderStruct[1]} must be capitalized after each '.'\n`;
-    }
-
-    // Verify 'Shared' follows 'Management'
-    if (packageFolder.includes("Management") && packageFolder.includes("Shared")) {
-      if (!packageFolder.includes("Management.Shared")) {
-        success = false;
-        errorOutput += `Invalid folder name. For management libraries with a shared component, 'Shared' should follow 'Management'.`;
-      }
-    }
-
     // Verify tspconfig, main.tsp, examples/
     const mainExists = await fileExists(path.join(folder, "main.tsp"));
     const clientExists = await fileExists(path.join(folder, "client.tsp"));
@@ -83,45 +59,117 @@
       success = false;
     }
 
-    if (!packageFolder.includes("Shared") && !tspConfigExists) {
-      errorOutput += `Invalid folder structure: Spec folder must contain tspconfig.yaml.`;
+    const folderStruct = relativePath.split("/").filter(Boolean);
+
+    // Verify top level folder is lower case and remove empty entries when splitting by slash
+    if (folderStruct[1].match(/[A-Z]/g)) {
       success = false;
+      errorOutput += `Invalid folder name. Folders under specification/ must be lower case.\n`;
     }
 
-    if (tspConfigExists) {
-      const configText = await readTspConfig(folder);
-      const config = yamlParse(configText);
-      const rpFolder =
-        config?.options?.["@azure-tools/typespec-autorest"]?.["azure-resource-provider-folder"];
-      stdOutput += `azure-resource-provider-folder: ${JSON.stringify(rpFolder)}\n`;
-
-      if (
-        rpFolder?.trim()?.endsWith("resource-manager") &&
-        !packageFolder.endsWith(".Management")
-      ) {
-        errorOutput += `Invalid folder structure: TypeSpec for resource-manager specs must be in a folder ending with '.Management'`;
+    if (structureVersion === 1) {
+      const packageFolder = folderStruct[folderStruct.length - 1];
+
+      if (!packageFolder.includes("Shared") && !tspConfigExists) {
+        errorOutput += `Invalid folder structure: Spec folder must contain tspconfig.yaml.`;
+        success = false;
+      }
+
+      // Verify package folder is at most 3 levels deep
+      if (folderStruct.length > 4) {
+        success = false;
+        errorOutput += `Please limit TypeSpec folder depth to 3 levels or less`;
+      }
+
+      // Verify second level folder is capitalized after each '.'
+      if (/(^|\. *)([a-z])/g.test(packageFolder)) {
         success = false;
-      } else if (
-        !rpFolder?.trim()?.endsWith("resource-manager") &&
-        packageFolder.endsWith(".Management")
-      ) {
-        errorOutput += `Invalid folder structure: TypeSpec for data-plane specs or shared code must be in a folder NOT ending with '.Management'`;
+        errorOutput += `Invalid folder name. Folders under specification/${folderStruct[1]} must be capitalized after each '.'\n`;
+      }
+
+      // Verify 'Shared' follows 'Management'
+      if (packageFolder.includes("Management") && packageFolder.includes("Shared")) {
+        if (!packageFolder.includes("Management.Shared")) {
+          success = false;
+          errorOutput += `Invalid folder name. For management libraries with a shared component, 'Shared' should follow 'Management'.`;
+        }
+      }
+
+      if (tspConfigExists) {
+        const configText = await readTspConfig(folder);
+        const config = yamlParse(configText);
+        const rpFolder =
+          config?.options?.["@azure-tools/typespec-autorest"]?.["azure-resource-provider-folder"];
+        stdOutput += `azure-resource-provider-folder: ${JSON.stringify(rpFolder)}\n`;
+
+        if (
+          rpFolder?.trim()?.endsWith("resource-manager") &&
+          !packageFolder.endsWith(".Management")
+        ) {
+          errorOutput += `Invalid folder structure: TypeSpec for resource-manager specs must be in a folder ending with '.Management'`;
+          success = false;
+        } else if (
+          !rpFolder?.trim()?.endsWith("resource-manager") &&
+          packageFolder.endsWith(".Management")
+        ) {
+          errorOutput += `Invalid folder structure: TypeSpec for data-plane specs or shared code must be in a folder NOT ending with '.Management'`;
+          success = false;
+        }
+      }
+    } else if (structureVersion === 2) {
+      if (!tspConfigExists) {
+        errorOutput += `Invalid folder structure: Spec folder must contain tspconfig.yaml.`;
+        success = false;
+      }
+
+      // TODO: Decide if v2 should validate folder names and structure to ensure specs align, or allow a more loose structure to ease
+      // migration from v1 to v2.
+
+      const specType = folder.includes("data-plane") ? "data-plane" : "resource-manager";
+      if (specType === "data-plane") {
+        if (folderStruct.length !== 4) {
+          errorOutput +=
+            "Invalid folder structure: TypeSpec for data-plane specs must be in a folder exactly one level under 'data-plane', like 'specification/foo/data-plane/Foo'.";
+          success = false;
+        }
+      } else if (specType === "resource-manager") {
+        if (folderStruct.length !== 5) {
+          errorOutput +=
+            "Invalid folder structure: TypeSpec for resource-manager specs must be in a folder exactly two levels under 'resource-manager', like 'specification/foo/resource-management/Microsoft.Foo/FooManagement'.";
+          success = false;
+        }
+
+        const rpNamespaceFolder = folderStruct[folderStruct.length - 2];
+
+        // Verify service folder is capitalized after each '.'
+        if (/(^|\. *)([a-z])/g.test(rpNamespaceFolder)) {
+          success = false;
+          errorOutput += `Invalid RP namespace folder '${rpNamespaceFolder}'. RP namespace folders must be capitalized after each '.'`;
+        }
+      }
+
+      const serviceFolder = folderStruct[folderStruct.length - 1];
+
+      // Verify service folder is capitalized after each '.'
+      if (/(^|\. *)([a-z])/g.test(serviceFolder)) {
         success = false;
+        errorOutput += `Invalid service folder '${serviceFolder}'. Service folders must be capitalized after each '.'`;
       }
     }
 
     // Ensure specs only import files from same folder under "specification"
     stdOutput += "imports:\n";
 
-    const teamFolder = path.join(...folderStruct.slice(0, 2));
-    stdOutput += `  ${teamFolder}\n`;
+    const allowedImportRoot =
+      structureVersion === 1 ? path.join(...folderStruct.slice(0, 2)) : folder;
+    stdOutput += `  ${allowedImportRoot}\n`;
 
-    const teamFolderResolved = path.resolve(gitRoot, teamFolder);
+    const allowedImportRootResolved = path.resolve(gitRoot, allowedImportRoot);
 
-    const tsps = await globby("**/*.tsp", { cwd: teamFolderResolved });
+    const tsps = await globby("**/*.tsp", { cwd: allowedImportRootResolved });
 
     for (const tsp of tsps) {
-      const tspResolved = path.resolve(teamFolderResolved, tsp);
+      const tspResolved = path.resolve(allowedImportRootResolved, tsp);
 
       const pattern = /^\s*import\s+['"]([^'"]+)['"]\s*;\s*$/gm;
       const text = await readFile(tspResolved, { encoding: "utf8" });
@@ -144,12 +192,12 @@
       for (const fileImport of fileImports) {
         const fileImportResolved = path.resolve(path.dirname(tspResolved), fileImport);
 
-        const relative = path.relative(teamFolderResolved, fileImportResolved);
+        const relative = path.relative(allowedImportRootResolved, fileImportResolved);
 
         if (relative.startsWith("..")) {
           errorOutput +=
             `Invalid folder structure: '${tsp}' imports '${fileImport}', ` +
-            `which is outside '${path.relative(gitRoot, teamFolder)}'`;
+            `which is outside '${path.relative(gitRoot, allowedImportRoot)}'`;
           success = false;
         }
       }

eng/tools/typespec-validation/test/folder-structure.test.ts

@@ -1,4 +1,4 @@
 import { mockAll, mockFolder } from "./mocks.js";
 mockAll();
 
 import { contosoTspConfig } from "@azure-tools/specs-shared/test/examples";
@@ -24,6 +24,14 @@
     vi.clearAllMocks();
   });
 
+  it("should fail if folder doesn't exist", async function () {
+    fileExistsSpy.mockResolvedValue(false);
+
+    const result = await new FolderStructureRule().execute(mockFolder);
+    assert(result.errorOutput);
+    assert(result.errorOutput.includes("does not exist"));
+  });
+
   it("should fail if tspconfig has incorrect extension", async function () {
     vi.mocked(globby.globby).mockImplementation(async () => {
       return ["/foo/bar/tspconfig.yml"];
@@ -260,4 +268,78 @@
     assert(result.errorOutput);
     assert(result.errorOutput.includes(".Management"));
   });
+
+  it("v2: should fail if no tspconfig.yaml", async function () {
+    vi.mocked(globby.globby).mockImplementation(async () => {
+      return ["main.tsp"];
+    });
+    normalizePathSpy.mockReturnValue("/gitroot");
+
+    fileExistsSpy.mockImplementation(async (file: string) => {
+      if (file.includes("tspconfig.yaml")) {
+        return false;
+      }
+      return true;
+    });
+
+    const result = await new FolderStructureRule().execute(
+      "/gitroot/specification/foo/data-plane/Foo",
+    );
+
+    assert(result.errorOutput?.includes("must contain"));
+  });
+
+  it("v2: should fail if incorrect folder depth", async function () {
+    vi.mocked(globby.globby).mockImplementation(async () => {
+      return ["tspconfig.yaml"];
+    });
+    normalizePathSpy.mockReturnValue("/gitroot");
+
+    let result = await new FolderStructureRule().execute("/gitroot/specification/foo/data-plane");
+    assert(result.errorOutput?.includes("level under"));
+
+    result = await new FolderStructureRule().execute(
+      "/gitroot/specification/foo/data-plane/Foo/too-deep",
+    );
+    assert(result.errorOutput?.includes("level under"));
+
+    result = await new FolderStructureRule().execute("/gitroot/specification/foo/resource-manager");
+    assert(result.errorOutput?.includes("levels under"));
+
+    result = await new FolderStructureRule().execute(
+      "/gitroot/specification/foo/resource-manager/RP.Namespace",
+    );
+    assert(result.errorOutput?.includes("levels under"));
+
+    result = await new FolderStructureRule().execute(
+      "/gitroot/specification/foo/resource-manager/RP.Namespace/FooManagement/too-deep",
+    );
+    assert(result.errorOutput?.includes("levels under"));
+  });
+
+  it("v2: should succeed with data-plane", async function () {
+    vi.mocked(globby.globby).mockImplementation(async (patterns) => {
+      return patterns[0].includes("tspconfig") ? ["tspconfig.yaml"] : ["main.tsp"];
+    });
+    normalizePathSpy.mockReturnValue("/gitroot");
+
+    const result = await new FolderStructureRule().execute(
+      "/gitroot/specification/foo/data-plane/Foo",
+    );
+
+    assert(result.success);
+  });
+
+  it("v2: should succeed with resource-manager", async function () {
+    vi.mocked(globby.globby).mockImplementation(async (patterns) => {
+      return patterns[0].includes("tspconfig") ? ["tspconfig.yaml"] : ["main.tsp"];
+    });
+    normalizePathSpy.mockReturnValue("/gitroot");
+
+    const result = await new FolderStructureRule().execute(
+      "/gitroot/specification/foo/resource-manager/Microsoft.Foo/FooManagement",
+    );
+
+    assert(result.success);
+  });
 });