diff --git a/.github/workflows/avocado-code.yml b/.github/workflows/avocado-code.yml index 30d4f42cb35e..03b467da55ba 100644 --- a/.github/workflows/avocado-code.yml +++ b/.github/workflows/avocado-code.yml @@ -29,6 +29,7 @@ jobs: "/package.json" \ "/package-lock.json" \ "/cadl/examples/" \ + '(?=/examples/)(?!(?:/stable/|/preview/))' \ --includePaths \ "data-plane" \ "resource-manager" diff --git a/eng/tools/spec-gen-sdk-runner/src/utils.ts b/eng/tools/spec-gen-sdk-runner/src/utils.ts index bd3e1248de52..27f4f96519ae 100644 --- a/eng/tools/spec-gen-sdk-runner/src/utils.ts +++ b/eng/tools/spec-gen-sdk-runner/src/utils.ts @@ -409,7 +409,7 @@ export function groupPathsByService( } 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); diff --git a/eng/tools/typespec-validation/src/rules/folder-structure.ts b/eng/tools/typespec-validation/src/rules/folder-structure.ts index b53277aaab09..5a777e6db198 100644 --- a/eng/tools/typespec-validation/src/rules/folder-structure.ts +++ b/eng/tools/typespec-validation/src/rules/folder-structure.ts @@ -21,6 +21,11 @@ export class FolderStructureRule implements Rule { 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 @@ export class FolderStructureRule implements Rule { } }); - // 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,114 @@ export class FolderStructureRule implements Rule { 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; - } 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'`; + } + + // 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'.`; + } + } + + 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; + } + + 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/FooAnalytics'."; + 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/Foo'."; + success = false; + } + + const rpNamespaceRegex = /^[A-Za-z0-9\.]+$/; + const rpNamespaceFolder = folderStruct[folderStruct.length - 2]; + + if (!rpNamespaceRegex.test(rpNamespaceFolder)) { + success = false; + errorOutput += `RPNamespace folder '${rpNamespaceFolder}' does not match regex ${rpNamespaceRegex}`; + } + } + + const serviceRegex = /^[A-Za-z0-9]+$/; + const serviceFolder = folderStruct[folderStruct.length - 1]; + + if (!serviceRegex.test(serviceFolder)) { success = false; + errorOutput += `Service folder '${serviceFolder}' does not match regex ${serviceRegex}`; } } // 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 +189,12 @@ export class FolderStructureRule implements Rule { 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; } } diff --git a/eng/tools/typespec-validation/test/folder-structure.test.ts b/eng/tools/typespec-validation/test/folder-structure.test.ts index e42b8bcd157a..bcda6f765fc9 100644 --- a/eng/tools/typespec-validation/test/folder-structure.test.ts +++ b/eng/tools/typespec-validation/test/folder-structure.test.ts @@ -24,6 +24,14 @@ describe("folder-structure", function () { 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"]; @@ -87,7 +95,7 @@ describe("folder-structure", function () { const result = await new FolderStructureRule().execute("/gitroot/specification/foo/data-plane"); assert(result.errorOutput); - assert(result.errorOutput.includes("must be capitalized")); + assert(result.errorOutput.includes("does not match regex")); }); it("should fail if second level folder is resource-manager", async function () { @@ -100,7 +108,7 @@ describe("folder-structure", function () { "/gitroot/specification/foo/resource-manager", ); assert(result.errorOutput); - assert(result.errorOutput.includes("must be capitalized")); + assert(result.errorOutput.includes("does not match regex")); }); it("should fail if Shared does not follow Management ", async function () { @@ -260,4 +268,78 @@ options: 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); + }); });