RFC: Do not modify config files if version is outdated

.changeset/smart-drinks-design.md

@@ -0,0 +1,17 @@
+---
+'skuba': patch
+---
+
+format, lint: Do not modify config files if version is outdated
+
+Starting from this version, `skuba format` will not modify config files if it detects that it is older than the version that last wrote to the project `package.json`:
+
+```json
+{
+  "skuba": {
+    "version": "1.0.0"
+  }
+}
+```
+
+This is intended to address the scenario where you have an older version of skuba installed locally, run `skuba format`, and inadvertently revert config files to a prior state.

src/cli/lint/internalLints/refreshConfigFiles.test.ts

@@ -5,6 +5,7 @@ import * as fsExtra from 'fs-extra';
 import { Git } from '../../..';
 import { log } from '../../../utils/logging';
 import { detectPackageManager } from '../../../utils/packageManager';
+import * as packageAnalysis from '../../configure/analysis/package';
 import * as project from '../../configure/analysis/project';
 
 import {
@@ -48,15 +49,29 @@ const givenMockPackageManager = (command: 'pnpm' | 'yarn') => {
 
 jest.mock('../../../utils/packageManager');
 
+let getDestinationManifest = jest.spyOn(
+  packageAnalysis,
+  'getDestinationManifest',
+);
+
 beforeEach(() => {
+  getDestinationManifest = jest.spyOn(
+    packageAnalysis,
+    'getDestinationManifest',
+  );
+
   jest
     .spyOn(console, 'log')
     .mockImplementation((...args) => stdoutMock(`${args.join(' ')}\n`));
 
   givenMockPackageManager('pnpm');
 });
 
-afterEach(jest.resetAllMocks);
+afterEach(() => {
+  jest.resetAllMocks();
+
+  getDestinationManifest.mockRestore();
+});
 
 describe('refreshConfigFiles', () => {
   const writeFile = jest.mocked(fsExtra.writeFile);
@@ -98,6 +113,27 @@ describe('refreshConfigFiles', () => {
       expect(writeFile).not.toHaveBeenCalled();
     });
 
+    it('should report ok if local skuba version is out of date, and output nothing', async () => {
+      getDestinationManifest.mockResolvedValue({
+        packageJson: {
+          skuba: {
+            version: '2147483647.0.0',
+          },
+        },
+      } as any);
+
+      setupDestinationFiles({});
+
+      await expect(refreshConfigFiles('lint', log)).resolves.toEqual({
+        ok: true,
+        fixable: false,
+      });
+
+      expect(stdout()).toBe('');
+
+      expect(writeFile).not.toHaveBeenCalled();
+    });
+
     it('should report not ok + fixable if files are out of date, and output a message', async () => {
       setupDestinationFiles({
         '.eslintignore':
@@ -252,6 +288,27 @@ The .dockerignore file is out of date. Run \`pnpm exec skuba format\` to update
       expect(writeFile).not.toHaveBeenCalled();
     });
 
+    it('should report ok if local skuba version is out of date, and output nothing', async () => {
+      getDestinationManifest.mockResolvedValue({
+        packageJson: {
+          skuba: {
+            version: '2147483647.0.0',
+          },
+        },
+      } as any);
+
+      setupDestinationFiles({});
+
+      await expect(refreshConfigFiles('format', log)).resolves.toEqual({
+        ok: true,
+        fixable: false,
+      });
+
+      expect(stdout()).toBe('');
+
+      expect(writeFile).not.toHaveBeenCalled();
+    });
+
     it('should report ok if files are out of date, and update them and output filenames', async () => {
       setupDestinationFiles({
         '.eslintignore':

src/cli/lint/internalLints/refreshConfigFiles.ts

@@ -2,6 +2,7 @@ import path from 'path';
 import { inspect } from 'util';
 
 import { writeFile } from 'fs-extra';
+import { gt } from 'semver';
 import stripAnsi from 'strip-ansi';
 
 import { Git } from '../../..';
@@ -12,6 +13,7 @@ import {
   detectPackageManager,
 } from '../../../utils/packageManager';
 import { readBaseTemplateFile } from '../../../utils/template';
+import { getSkubaVersion, manifestSkubaVersion } from '../../../utils/version';
 import { getDestinationManifest } from '../../configure/analysis/package';
 import { createDestinationFileReader } from '../../configure/analysis/project';
 import { mergeWithConfigFile } from '../../configure/processing/configFile';
@@ -74,12 +76,24 @@ export const REFRESHABLE_CONFIG_FILES: RefreshableConfigFile[] = [
 export const refreshConfigFiles = async (
   mode: 'format' | 'lint',
   logger: Logger,
-) => {
-  const [manifest, gitRoot] = await Promise.all([
+): Promise<InternalLintResult> => {
+  const [currentVersion, manifest, gitRoot] = await Promise.all([
+    getSkubaVersion(),
     getDestinationManifest(),
     Git.findRoot({ dir: process.cwd() }),
   ]);
 
+  const manifestVersion = manifestSkubaVersion(manifest);
+
+  // The local version that we're running is older than the version that last
+  // wrote to the manifest; avoid reverting config files back to a prior state.
+  if (gt(manifestVersion, currentVersion)) {
+    return {
+      ok: true,
+      fixable: false,
+    };
+  }
+
   const destinationRoot = path.dirname(manifest.path);
 
   const readDestinationFile = createDestinationFileReader(destinationRoot);

src/cli/lint/internalLints/upgrade/index.ts

@@ -10,7 +10,10 @@ import {
   type PackageManagerConfig,
   detectPackageManager,
 } from '../../../../utils/packageManager';
-import { getSkubaVersion } from '../../../../utils/version';
+import {
+  getSkubaVersion,
+  manifestSkubaVersion,
+} from '../../../../utils/version';
 import { formatPackage } from '../../../configure/processing/package';
 import type { SkubaPackageJson } from '../../../init/writePackageJson';
 import type { InternalLintResult } from '../../internal';
@@ -84,10 +87,7 @@ export const upgradeSkuba = async (
     throw new Error('Could not find a package json for this project');
   }
 
-  manifest.packageJson.skuba ??= { version: '1.0.0' };
-
-  const manifestVersion = (manifest.packageJson.skuba as SkubaPackageJson)
-    .version;
+  const manifestVersion = manifestSkubaVersion(manifest);
 
   // We are up to date, skip patches
   if (gte(manifestVersion, currentVersion)) {
@@ -160,7 +160,8 @@ export const upgradeSkuba = async (
     }
   }
 
-  (manifest.packageJson.skuba as SkubaPackageJson).version = currentVersion;
+  ((manifest.packageJson.skuba ??= {}) as SkubaPackageJson).version =
+    currentVersion;
 
   const updatedPackageJson = await formatPackage(manifest.packageJson);
 

src/utils/version.ts

@@ -1,6 +1,9 @@
 import searchNpm from 'libnpmsearch';
+import type readPkgUp from 'read-pkg-up';
 import validatePackageName from 'validate-npm-package-name';
 
+import type { SkubaPackageJson } from '../cli/init/writePackageJson';
+
 import { getSkubaManifest } from './manifest';
 import { withTimeout } from './wait';
 
@@ -34,6 +37,14 @@ const latestSkubaVersion = async (): Promise<string | null> => {
   }
 };
 
+export const manifestSkubaVersion = (
+  manifest: readPkgUp.NormalizedReadResult,
+) => {
+  const skuba = manifest.packageJson.skuba as SkubaPackageJson | undefined;
+
+  return skuba?.version ?? '1.0.0';
+};
+
 export const getSkubaVersion = async (): Promise<string> => {
   const { version } = await getSkubaManifest();