Display which dependencies were fixed when running with --fix (#320)

bmish · web-flow · commit f92f5a65019d · 2022-01-17T14:12:32.000-05:00
diff --git a/bin/check-dependency-version-consistency.ts b/bin/check-dependency-version-consistency.ts
@@ -8,9 +8,13 @@ import {
   calculateMismatchingVersions,
   filterOutIgnoredDependencies,
   fixMismatchingVersions,
+  MismatchingDependencyVersions,
 } from '../lib/dependency-versions.js';
 import { getPackages } from '../lib/workspace.js';
-import { mismatchingVersionsToOutput } from '../lib/output.js';
+import {
+  mismatchingVersionsToOutput,
+  mismatchingVersionsFixedToOutput,
+} from '../lib/output.js';
 import { join, dirname } from 'node:path';
 import type { PackageJson } from 'type-fest';
 import { fileURLToPath } from 'node:url';
@@ -100,21 +104,31 @@ function run() {
         options.ignorePath,
         options.ignorePathPattern.map((s) => new RegExp(s))
       );
+
       const dependencyVersions = calculateVersionsForEachDependency(packages);
+
       let mismatchingVersions = filterOutIgnoredDependencies(
         calculateMismatchingVersions(dependencyVersions),
         options.ignoreDep,
         options.ignoreDepPattern.map((s) => new RegExp(s))
       );
+      let mismatchingVersionsFixed: MismatchingDependencyVersions = [];
 
       if (options.fix) {
-        mismatchingVersions = fixMismatchingVersions(
+        const resultsAfterFix = fixMismatchingVersions(
           packages,
           mismatchingVersions
         );
+        mismatchingVersions = resultsAfterFix.notFixed;
+        mismatchingVersionsFixed = resultsAfterFix.fixed;
+      }
+
+      // Show output for dependencies we fixed.
+      if (mismatchingVersionsFixed.length > 0) {
+        console.log(mismatchingVersionsFixedToOutput(mismatchingVersionsFixed));
       }
 
-      // Show output.
+      // Show output for dependencies that still have mismatches.
       if (mismatchingVersions.length > 0) {
         console.log(mismatchingVersionsToOutput(mismatchingVersions));
         process.exitCode = 1;
diff --git a/lib/dependency-versions.ts b/lib/dependency-versions.ts
@@ -202,59 +202,71 @@ function writeDependencyVersion(
 export function fixMismatchingVersions(
   packages: Package[],
   mismatchingVersions: MismatchingDependencyVersions
-): MismatchingDependencyVersions {
-  // Return any mismatching versions that are still present after attempting fixes.
-  return mismatchingVersions
-    .map((mismatchingVersion) => {
-      const versions = mismatchingVersion.versions.map(
-        (object) => object.version
-      );
-      let sortedVersions;
-      try {
-        sortedVersions = versions.sort(compareRanges);
-      } catch {
-        // Unable to sort so skip this dependency (return it to indicate it was not fixed).
-        return mismatchingVersion;
-      }
-      const fixedVersion = sortedVersions[sortedVersions.length - 1]; // Highest version will be sorted to end of list.
+): {
+  fixed: MismatchingDependencyVersions;
+  notFixed: MismatchingDependencyVersions;
+} {
+  const fixed = [];
+  const notFixed = [];
+  for (const mismatchingVersion of mismatchingVersions) {
+    const versions = mismatchingVersion.versions.map(
+      (object) => object.version
+    );
+    let sortedVersions;
+    try {
+      sortedVersions = versions.sort(compareRanges);
+    } catch {
+      // Unable to sort so skip this dependency.
+      notFixed.push(mismatchingVersion);
+      continue;
+    }
 
-      for (const package_ of packages) {
-        if (
-          package_.packageJson.devDependencies &&
-          package_.packageJson.devDependencies[mismatchingVersion.dependency] &&
-          package_.packageJson.devDependencies[
-            mismatchingVersion.dependency
-          ] !== fixedVersion
-        ) {
-          writeDependencyVersion(
-            package_.pathPackageJson,
-            package_.packageJsonEndsInNewline,
-            false,
-            mismatchingVersion.dependency,
-            fixedVersion
-          );
-        }
+    const fixedVersion = sortedVersions[sortedVersions.length - 1]; // Highest version will be sorted to end of list.
 
-        if (
-          package_.packageJson.dependencies &&
-          package_.packageJson.dependencies[mismatchingVersion.dependency] &&
-          package_.packageJson.dependencies[mismatchingVersion.dependency] !==
-            fixedVersion
-        ) {
-          writeDependencyVersion(
-            package_.pathPackageJson,
-            package_.packageJsonEndsInNewline,
-            true,
-            mismatchingVersion.dependency,
-            fixedVersion
-          );
-        }
+    let isFixed = false;
+    for (const package_ of packages) {
+      if (
+        package_.packageJson.devDependencies &&
+        package_.packageJson.devDependencies[mismatchingVersion.dependency] &&
+        package_.packageJson.devDependencies[mismatchingVersion.dependency] !==
+          fixedVersion
+      ) {
+        writeDependencyVersion(
+          package_.pathPackageJson,
+          package_.packageJsonEndsInNewline,
+          false,
+          mismatchingVersion.dependency,
+          fixedVersion
+        );
+        isFixed = true;
       }
 
-      // Fixed successfully.
-      return undefined;
-    })
-    .filter((item) => item !== undefined) as MismatchingDependencyVersions;
+      if (
+        package_.packageJson.dependencies &&
+        package_.packageJson.dependencies[mismatchingVersion.dependency] &&
+        package_.packageJson.dependencies[mismatchingVersion.dependency] !==
+          fixedVersion
+      ) {
+        writeDependencyVersion(
+          package_.pathPackageJson,
+          package_.packageJsonEndsInNewline,
+          true,
+          mismatchingVersion.dependency,
+          fixedVersion
+        );
+        isFixed = true;
+      }
+    }
+
+    if (isFixed) {
+      fixed.push(mismatchingVersion);
+    }
+  }
+
+  return {
+    fixed,
+    notFixed,
+  };
 }
 
 export function compareRanges(a: string, b: string): 0 | -1 | 1 {
diff --git a/lib/output.ts b/lib/output.ts
@@ -61,3 +61,18 @@ export function mismatchingVersionsToOutput(
     tables,
   ].join('\n');
 }
+
+export function mismatchingVersionsFixedToOutput(
+  mismatchingDependencyVersions: MismatchingDependencyVersions
+): string {
+  if (mismatchingDependencyVersions.length === 0) {
+    throw new Error('No fixes to output.');
+  }
+
+  const dependencies = mismatchingDependencyVersions
+    .map((mismatchingVersion) => mismatchingVersion.dependency)
+    .sort();
+  return `Fixed versions for ${dependencies.length} ${
+    dependencies.length === 1 ? 'dependency' : 'dependencies'
+  }: ${dependencies.join(', ')}`;
+}
diff --git a/test/lib/dependency-versions-test.ts b/test/lib/dependency-versions-test.ts
@@ -310,7 +310,7 @@ describe('Utils | dependency-versions', function () {
       const mismatchingVersions = calculateMismatchingVersions(
         calculateVersionsForEachDependency(packages)
       );
-      const fixedMismatchingVersions = fixMismatchingVersions(
+      const { fixed, notFixed } = fixMismatchingVersions(
         packages,
         mismatchingVersions
       );
@@ -388,8 +388,8 @@ describe('Utils | dependency-versions', function () {
       ).toStrictEqual('1.0.1');
 
       // Check return value.
-      // Should return only the dependency that could not be fixed due to the abnormal version present.
-      expect(fixedMismatchingVersions).toStrictEqual([
+      // Only one dependency should not be fixed due to the abnormal version present.
+      expect(notFixed).toStrictEqual([
         {
           dependency: 'bar',
           versions: [
@@ -412,6 +412,95 @@ describe('Utils | dependency-versions', function () {
           ],
         },
       ]);
+      expect(fixed).toStrictEqual([
+        {
+          dependency: '@types/one',
+          versions: [
+            {
+              version: '1.0.0',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package2'),
+                }),
+              ],
+            },
+            {
+              version: '1.0.1',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package1'),
+                }),
+              ],
+            },
+          ],
+        },
+        {
+          dependency: 'a.b.c',
+          versions: [
+            {
+              version: '5.0.0',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package1'),
+                }),
+              ],
+            },
+            {
+              version: '~5.5.0',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package2'),
+                }),
+              ],
+            },
+          ],
+        },
+        {
+          dependency: 'foo',
+          versions: [
+            {
+              version: '^1.0.0',
+              packages: [
+                expect.objectContaining({
+                  path: '.',
+                }),
+                expect.objectContaining({
+                  path: join('@scope1', 'package1'),
+                }),
+              ],
+            },
+            {
+              version: '^2.0.0',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package2'),
+                }),
+              ],
+            },
+          ],
+        },
+        {
+          dependency: 'one.two.three',
+          versions: [
+            {
+              version: '^4.0.0',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package2'),
+                }),
+              ],
+            },
+            {
+              version: '^4.1.0',
+              packages: [
+                expect.objectContaining({
+                  path: join('@scope1', 'package1'),
+                }),
+              ],
+            },
+          ],
+        },
+      ]);
 
       // Existing newline at end of file should be maintained.
       expect(packageJson1Contents).not.toMatch(/\n$/); // package1 should not end in newline
diff --git a/test/lib/output-test.ts b/test/lib/output-test.ts
@@ -2,7 +2,10 @@ import {
   calculateVersionsForEachDependency,
   calculateMismatchingVersions,
 } from '../../lib/dependency-versions.js';
-import { mismatchingVersionsToOutput } from '../../lib/output.js';
+import {
+  mismatchingVersionsToOutput,
+  mismatchingVersionsFixedToOutput,
+} from '../../lib/output.js';
 import { getPackages } from '../../lib/workspace.js';
 import {
   FIXTURE_PATH_TESTING_OUTPUT,
@@ -92,4 +95,38 @@ describe('Utils | output', function () {
       );
     });
   });
+
+  describe('#mismatchingVersionsFixedToOutputLines', function () {
+    it('behaves correctly', function () {
+      expect(
+        mismatchingVersionsFixedToOutput(
+          calculateMismatchingVersions(
+            calculateVersionsForEachDependency(
+              getPackages(FIXTURE_PATH_TESTING_OUTPUT, [], [], [], [])
+            )
+          )
+        )
+      ).toMatchInlineSnapshot(
+        '"Fixed versions for 4 dependencies: bar, baz, biz, foo"'
+      );
+    });
+
+    it('behaves correctly with a single fix', function () {
+      expect(
+        mismatchingVersionsFixedToOutput(
+          calculateMismatchingVersions(
+            calculateVersionsForEachDependency(
+              getPackages(FIXTURE_PATH_TESTING_OUTPUT, [], [], [], [])
+            )
+          ).slice(0, 1)
+        )
+      ).toMatchInlineSnapshot('"Fixed versions for 1 dependency: bar"');
+    });
+
+    it('behaves correctly with empty input', function () {
+      expect(() =>
+        mismatchingVersionsFixedToOutput([])
+      ).toThrowErrorMatchingInlineSnapshot('"No fixes to output."');
+    });
+  });
 });
