[openapi-diff-runner] Support renamed files in breaking change detection

eng/tools/openapi-diff-runner/src/command-helpers.ts

@@ -131,6 +131,7 @@ export async function getSwaggerDiffs(
   additions: string[];
   modifications: string[];
   deletions: string[];
+  renames: { from: string; to: string }[];
   total: number;
 }> {
   try {
@@ -143,22 +144,60 @@ export async function getSwaggerDiffs(
     });
 
     // Filter each array to only include Swagger files using the swagger filter from changed-files.js
-    const filteredAdditions = result.additions.filter(swagger);
+    let filteredAdditions = result.additions.filter(swagger);
     const filteredModifications = result.modifications.filter(swagger);
-    const filteredDeletions = result.deletions.filter(swagger);
-    const filteredRenames = result.renames.filter(
+    let filteredDeletions = result.deletions.filter(swagger);
+    let filteredRenames = result.renames.filter(
       (rename) => swagger(rename.from) && swagger(rename.to),
     );
 
-    // Add renamed files to the additions array and deletions array
-    filteredAdditions.push(...filteredRenames.map((rename) => rename.to));
-    filteredDeletions.push(...filteredRenames.map((rename) => rename.from));
+    // Try to manually match deletions with additions to form renames
+
+    // Store matches in temp array, to avoid modifying "filteredDeletions" while iterating
+    const matchedRenames: { from: string; to: string }[] = [];
+
+    for (const deletion of filteredDeletions) {
+      const deletionDir = path.dirname(deletion).toLowerCase();
+
+      const deletionMatches = filteredDeletions.filter(
+        (d) => path.dirname(d).toLowerCase() === deletionDir,
+      );
+      if (deletionMatches.length > 1) {
+        // If more than one deletion from same folder, we can't match.
+        // Example would be migrating from "a.json, b.json" to "c.json"
+        continue;
+      }
+
+      const additionMatches = filteredAdditions.filter(
+        (a) => path.dirname(a).toLowerCase() === deletionDir,
+      );
+      if (additionMatches.length !== 1) {
+        // If more than one addition in same folder, we can't match.
+        // Example would be migrating from "a.json" to "b.json, c.json"
+        continue;
+      }
+
+      // Exactly one matching deletion and addition, so treat it like a "git rename"
+      matchedRenames.push({ from: deletion, to: additionMatches[0] });
+    }
+
+    // Update arrays with matchedRenames (if any)
+    const renamesFrom = matchedRenames.map((r) => r.from);
+    const renamesTo = matchedRenames.map((r) => r.to);
+    filteredAdditions = filteredAdditions.filter((a) => !renamesTo.includes(a));
+    filteredDeletions = filteredDeletions.filter((d) => !renamesFrom.includes(d));
+    filteredRenames = filteredRenames.concat(matchedRenames);
 
     return {
       additions: filteredAdditions,
       modifications: filteredModifications,
       deletions: filteredDeletions,
-      total: filteredAdditions.length + filteredModifications.length + filteredDeletions.length,
+      renames: filteredRenames,
+      total:
+        filteredAdditions.length +
+        filteredModifications.length +
+        filteredDeletions.length +
+        filteredRenames.length,
     };
   } catch (error) {
     logError(`Error getting categorized changed files: ${error}`);
@@ -167,6 +206,7 @@ export async function getSwaggerDiffs(
       additions: [],
       modifications: [],
       deletions: [],
+      renames: [],
       total: 0,
     };
   }

eng/tools/openapi-diff-runner/src/commands.ts

@@ -75,6 +75,8 @@ export async function validateBreakingChange(context: Context): Promise<number>
 
   const deletedSwaggers = diffs.deletions || [];
 
+  const renamedSwaggers = diffs.renames || [];
+
   const newExistingVersionDirs: string[] = [];
 
   const addedVersionDirs = [...newSwaggers.map((f: string) => path.dirname(f))];
@@ -156,6 +158,7 @@ export async function validateBreakingChange(context: Context): Promise<number>
       needCompareOldSwaggers,
       newVersionSwaggers,
       newVersionChangedSwaggers,
+      renamedSwaggers,
       oadTracer,
     );
 

eng/tools/openapi-diff-runner/src/detect-breaking-change.ts

@@ -61,6 +61,7 @@ export interface BreakingChangeDetectionContext {
   existingVersionSwaggers: string[]; // Files in existing API version directories
   newVersionSwaggers: string[]; // Files in completely new API version directories
   newVersionChangedSwaggers: string[]; // Files in existing API version directories that have changed
+  renamedSwaggers: { from: string; to: string }[];
   oadTracer: OadTraceData;
   msgs: ResultMessageRecord[];
   runtimeErrors: RawMessageRecord[];
@@ -75,13 +76,15 @@ export function createBreakingChangeDetectionContext(
   existingVersionSwaggers: string[],
   newVersionSwaggers: string[],
   newVersionChangedSwaggers: string[],
+  renamedSwaggers: { from: string; to: string }[],
   oadTracer: OadTraceData,
 ): BreakingChangeDetectionContext {
   return {
     context,
     existingVersionSwaggers,
     newVersionSwaggers,
     newVersionChangedSwaggers,
+    renamedSwaggers,
     oadTracer,
     msgs: [],
     runtimeErrors: [],
@@ -132,6 +135,20 @@ export async function checkBreakingChangeOnSameVersion(
     logMessage("Processing completed", LogLevel.EndGroup);
   }
 
+  for (const { from, to } of detectionContext.renamedSwaggers) {
+    logMessage(`Processing rename: ${from} -> ${to}`, LogLevel.Group);
+    const { oadViolationsCnt, errorCnt } = await doBreakingChangeDetection(
+      detectionContext,
+      path.resolve(detectionContext.context.prInfo!.tempRepoFolder, from),
+      to,
+      BREAKING_CHANGES_CHECK_TYPES.SAME_VERSION,
+      specIsPreview(to) ? ApiVersionLifecycleStage.PREVIEW : ApiVersionLifecycleStage.STABLE,
+    );
+    aggregateOadViolationsCnt += oadViolationsCnt;
+    aggregateErrorCnt += errorCnt;
+    logMessage("Processing completed", LogLevel.EndGroup);
+  }
+
   logMessage(
     `RETURN definition checkBreakingChangeOnSameVersion. ` +
       `msgs.length: ${detectionContext.msgs.length}, ` +

eng/tools/openapi-diff-runner/test/command-helpers.test.ts

@@ -349,14 +349,15 @@ describe("command-helpers", () => {
 
       // Expected result should have renames added to additions and deletions, and no renames property
       const expectedResult = {
-        additions: [...mockResult.additions, ...mockResult.renames.map((rename) => rename.to)],
+        additions: mockResult.additions,
         modifications: mockResult.modifications,
-        deletions: [...mockResult.deletions, ...mockResult.renames.map((rename) => rename.from)],
+        deletions: mockResult.deletions,
+        renames: mockResult.renames,
         total:
           mockResult.additions.length +
           mockResult.modifications.length +
           mockResult.deletions.length +
-          mockResult.renames.length * 2,
+          mockResult.renames.length,
       };
 
       expect(result).toEqual(expectedResult);
@@ -375,6 +376,7 @@ describe("command-helpers", () => {
         additions: [],
         modifications: [],
         deletions: [],
+        renames: [],
         total: 0,
       });
     });
@@ -415,17 +417,16 @@ describe("command-helpers", () => {
 
       // Only Swagger files should be returned, with renames added to additions and deletions
       expect(result).toEqual({
-        additions: [
-          TEST_CONSTANTS.SWAGGER_PATHS.FOO,
-          TEST_CONSTANTS.SWAGGER_PATHS.BAZ,
-          TEST_CONSTANTS.SWAGGER_PATHS.NEW_MGMT, // from valid rename
-        ],
+        additions: [TEST_CONSTANTS.SWAGGER_PATHS.FOO, TEST_CONSTANTS.SWAGGER_PATHS.BAZ],
         modifications: [TEST_CONSTANTS.SWAGGER_PATHS.QUX_MGMT],
-        deletions: [
-          TEST_CONSTANTS.SWAGGER_PATHS.OLD_DATA,
-          TEST_CONSTANTS.SWAGGER_PATHS.OLD_MGMT, // from valid rename
+        deletions: [TEST_CONSTANTS.SWAGGER_PATHS.OLD_DATA],
+        renames: [
+          {
+            from: TEST_CONSTANTS.SWAGGER_PATHS.OLD_MGMT,
+            to: TEST_CONSTANTS.SWAGGER_PATHS.NEW_MGMT,
+          },
         ],
-        total: 6, // 3 additions + 1 modification + 2 deletions (including rename files)
+        total: 5, // 2 additions + 1 modification + 1 deletion + 1 rename
       });
     });
 
@@ -474,18 +475,20 @@ describe("command-helpers", () => {
 
       // Renames should be added to additions and deletions, not returned as renames
       expect(result).toEqual({
-        additions: [
-          TEST_CONSTANTS.SWAGGER_PATHS.FOO,
-          TEST_CONSTANTS.SWAGGER_PATHS.NEW_MGMT, // from rename.to
-          "specification/newapi/data-plane/stable/2023-01-01/newapi.json", // from rename.to
-        ],
+        additions: [TEST_CONSTANTS.SWAGGER_PATHS.FOO],
         modifications: [TEST_CONSTANTS.SWAGGER_PATHS.BAZ],
-        deletions: [
-          TEST_CONSTANTS.SWAGGER_PATHS.OLD_DATA,
-          TEST_CONSTANTS.SWAGGER_PATHS.OLD_MGMT, // from rename.from
-          "specification/oldapi/data-plane/stable/2023-01-01/oldapi.json", // from rename.from
+        deletions: [TEST_CONSTANTS.SWAGGER_PATHS.OLD_DATA],
+        renames: [
+          {
+            from: TEST_CONSTANTS.SWAGGER_PATHS.OLD_MGMT,
+            to: TEST_CONSTANTS.SWAGGER_PATHS.NEW_MGMT,
+          },
+          {
+            from: "specification/oldapi/data-plane/stable/2023-01-01/oldapi.json",
+            to: "specification/newapi/data-plane/stable/2023-01-01/newapi.json",
+          },
         ],
-        total: 7, // 3 additions + 1 modification + 3 deletions (including from renames)
+        total: 5, // 1 addition + 1 modification + 1 deletion + 2 renames
       });
     });
   });