refactor(config-migration): invoke applyPrettierFormatting at the commit stage

lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json

@@ -1,4 +1,9 @@
 {
+  "content": "{\n  \"extends\": [\n    \":separateMajorReleases\",\n    \":prImmediately\",\n    \":renovatePrefix\",\n    \":semanticPrefixFixDepsChoreOthers\",\n    \":updateNotScheduled\",\n    \":automergeDisabled\",\n    \":maintainLockFilesDisabled\",\n    \":autodetectPinVersions\",\n    \"group:monorepos\"\n  ],\n  \"onboarding\": false,\n  \"rangeStrategy\": \"replace\",\n  \"semanticCommits\": \"enabled\",\n  \"timezone\": \"US/Central\",\n  \"baseBranches\": [\n    \"main\"\n  ]\n}\n",
   "filename": "renovate.json",
-  "content": "{\n  \"extends\": [\n    \":separateMajorReleases\",\n    \":prImmediately\",\n    \":renovatePrefix\",\n    \":semanticPrefixFixDepsChoreOthers\",\n    \":updateNotScheduled\",\n    \":automergeDisabled\",\n    \":maintainLockFilesDisabled\",\n    \":autodetectPinVersions\",\n    \"group:monorepos\"\n  ],\n  \"onboarding\": false,\n  \"rangeStrategy\": \"replace\",\n  \"semanticCommits\": \"enabled\",\n  \"timezone\": \"US/Central\",\n  \"baseBranches\": [\n    \"main\"\n  ]\n}\n"
+  "indent": {
+    "amount": 2,
+    "indent": "  ",
+    "type": "space"
+  }
 }

lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5

@@ -1,4 +1,9 @@
 {
+  "content": "{\n  extends: [\n    ':separateMajorReleases',\n    ':prImmediately',\n    ':renovatePrefix',\n    ':semanticPrefixFixDepsChoreOthers',\n    ':updateNotScheduled',\n    ':automergeDisabled',\n    ':maintainLockFilesDisabled',\n    ':autodetectPinVersions',\n    'group:monorepos',\n  ],\n  onboarding: false,\n  rangeStrategy: 'replace',\n  semanticCommits: 'enabled',\n  timezone: 'US/Central',\n  baseBranches: [\n    'main',\n  ],\n}\n",
   "filename": "renovate.json5",
-  "content": "{\n  extends: [\n    ':separateMajorReleases',\n    ':prImmediately',\n    ':renovatePrefix',\n    ':semanticPrefixFixDepsChoreOthers',\n    ':updateNotScheduled',\n    ':automergeDisabled',\n    ':maintainLockFilesDisabled',\n    ':autodetectPinVersions',\n    'group:monorepos',\n  ],\n  onboarding: false,\n  rangeStrategy: 'replace',\n  semanticCommits: 'enabled',\n  timezone: 'US/Central',\n  baseBranches: [\n    'main',\n  ],\n}\n"
+  "indent": {
+    "amount": 2,
+    "indent": "  ",
+    "type": "space"
+  }
 }

lib/workers/repository/config-migration/branch/create.spec.ts

@@ -1,8 +1,15 @@
+import type { Indent } from 'detect-indent';
 import { Fixtures } from '../../../../../test/fixtures';
-import { RenovateConfig, getConfig, platform } from '../../../../../test/util';
+import {
+  RenovateConfig,
+  getConfig,
+  partial,
+  platform,
+} from '../../../../../test/util';
 import { checkoutBranch, commitFiles } from '../../../../util/git';
 import { createConfigMigrationBranch } from './create';
 import type { MigratedData } from './migrated-data';
+import * as MigratedDataModule from './migrated-data';
 
 jest.mock('../../../../util/git');
 
@@ -11,6 +18,7 @@ describe('workers/repository/config-migration/branch/create', () => {
   const indent = '  ';
   const renovateConfig = JSON.stringify(raw, undefined, indent) + '\n';
   const filename = 'renovate.json';
+  const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting');
 
   let config: RenovateConfig;
   let migratedConfigData: MigratedData;
@@ -20,7 +28,12 @@ describe('workers/repository/config-migration/branch/create', () => {
     config = getConfig();
     config.baseBranch = 'dev';
     config.defaultBranch = 'master';
-    migratedConfigData = { content: renovateConfig, filename };
+    migratedConfigData = {
+      content: renovateConfig,
+      filename,
+      indent: partial<Indent>({}),
+    };
+    prettierSpy.mockResolvedValueOnce(migratedConfigData.content);
   });
 
   describe('createConfigMigrationBranch', () => {

lib/workers/repository/config-migration/branch/create.ts

@@ -1,18 +1,19 @@
+import upath from 'upath';
 import { GlobalConfig } from '../../../../config/global';
 import type { RenovateConfig } from '../../../../config/types';
 import { logger } from '../../../../logger';
 import { commitAndPush } from '../../../../modules/platform/commit';
 import { checkoutBranch } from '../../../../util/git';
 import { getMigrationBranchName } from '../common';
 import { ConfigMigrationCommitMessageFactory } from './commit-message';
+import { PrettierParser, applyPrettierFormatting } from './migrated-data';
 import type { MigratedData } from './migrated-data';
 
 export async function createConfigMigrationBranch(
   config: Partial<RenovateConfig>,
   migratedConfigData: MigratedData
 ): Promise<string | null> {
   logger.debug('createConfigMigrationBranch()');
-  const contents = migratedConfigData.content;
   const configFileName = migratedConfigData.filename;
   logger.debug('Creating config migration branch');
 
@@ -30,6 +31,9 @@ export async function createConfigMigrationBranch(
   }
 
   await checkoutBranch(config.defaultBranch!);
+  const { content, filename, indent } = migratedConfigData;
+  const parser = upath.extname(filename).replace('.', '') as PrettierParser;
+  const contents = await applyPrettierFormatting(content, parser, indent);
   return commitAndPush({
     branchName: getMigrationBranchName(config),
     files: [

lib/workers/repository/config-migration/branch/migrated-data.spec.ts

@@ -41,61 +41,56 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {
         isMigrated: true,
         migratedConfig: migratedConfigObj,
       });
-      mockedFunction(getFileList).mockResolvedValue([]);
     });
 
     it('Calls getAsync a first when migration not needed', async () => {
       mockedFunction(migrateConfig).mockReturnValueOnce({
         isMigrated: false,
         migratedConfig: {},
       });
-      await expect(MigratedDataFactory.getAsync()).resolves.toBeNull();
+      await expect(MigratedDataFactory.get()).resolves.toBeNull();
     });
 
     it('Calls getAsync a first time to initialize the factory', async () => {
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData);
       expect(detectRepoFileConfig).toHaveBeenCalledTimes(1);
     });
 
     it('Calls getAsync a second time to get the saved data from before', async () => {
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData);
       expect(detectRepoFileConfig).toHaveBeenCalledTimes(0);
     });
 
     describe('MigratedData class', () => {
       it('gets the filename from the class instance', async () => {
-        const data = await MigratedDataFactory.getAsync();
+        const data = await MigratedDataFactory.get();
         expect(data?.filename).toBe('renovate.json');
       });
 
       it('gets the content from the class instance', async () => {
-        const data = await MigratedDataFactory.getAsync();
+        const data = await MigratedDataFactory.get();
         expect(data?.content).toBe(migratedData.content);
       });
     });
 
     it('Resets the factory and gets a new value', async () => {
       MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await expect(MigratedDataFactory.get()).resolves.toEqual(migratedData);
     });
 
     it('Resets the factory and gets a new value with default indentation', async () => {
-      mockedFunction(detectIndent).mockReturnValueOnce({
+      const indent = {
         type: undefined,
         amount: 0,
         // TODO: incompatible types (#7154)
         indent: null as never,
-      });
+      };
+      mockedFunction(detectIndent).mockReturnValueOnce(indent);
       MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await expect(MigratedDataFactory.get()).resolves.toEqual({
+        ...migratedData,
+        indent,
+      });
     });
 
     it('Migrate a JSON5 config file', async () => {
@@ -104,7 +99,7 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {
         configFileRaw: rawNonMigratedJson5,
       });
       MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
+      await expect(MigratedDataFactory.get()).resolves.toEqual(
         migratedDataJson5
       );
     });
@@ -113,55 +108,90 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {
       const err = new Error('error-message');
       mockedFunction(detectRepoFileConfig).mockRejectedValueOnce(err);
       MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toBeNull();
+      await expect(MigratedDataFactory.get()).resolves.toBeNull();
       expect(logger.debug).toHaveBeenCalledWith(
         { err },
         'MigratedDataFactory.getAsync() Error initializing renovate MigratedData'
       );
     });
+  });
 
-    it('format and migrate a JSON config file', async () => {
+  describe(' MigratedDataFactory.applyPrettierFormatting', () => {
+    beforeAll(() => {
+      mockedFunction(detectIndent).mockReturnValueOnce({
+        type: 'space',
+        amount: 2,
+        indent: '  ',
+      });
       mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({
         configFileName: 'renovate.json',
+        configFileRaw: rawNonMigrated,
+      });
+      mockedFunction(migrateConfig).mockReturnValueOnce({
+        isMigrated: true,
+        migratedConfig: migratedConfigObj,
       });
-      mockedFunction(getFileList).mockResolvedValue(['.prettierrc']);
       MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        formattedMigratedData
-      );
     });
 
-    it('should not stop run for invalid package.json', async () => {
-      mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({
-        configFileName: 'renovate.json',
-        configFileRaw: 'abci',
-      });
-      mockedFunction(readLocalFile).mockResolvedValueOnce(rawNonMigrated);
-      MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+    beforeEach(() => {
+      mockedFunction(getFileList).mockResolvedValue([]);
     });
 
-    it('should not stop run for readLocalFile error', async () => {
-      mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({
-        configFileName: 'renovate.json',
-        configFileRaw: 'abci',
-      });
+    it('does not format when no prettier config is present', async () => {
+      const { content: unformatted, filename } = migratedData;
+      mockedFunction(readLocalFile).mockResolvedValueOnce(null);
+      await MigratedDataFactory.get();
+      await expect(
+        applyPrettierFormatting(unformatted, filename)
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('does not format when failing to fetch package.json file', async () => {
+      const { content: unformatted, filename } = migratedData;
       mockedFunction(readLocalFile).mockRejectedValueOnce(null);
-      MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await MigratedDataFactory.get();
+      await expect(
+        applyPrettierFormatting(unformatted, filename)
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('does not format when there is an invalid package.json file', async () => {
+      const { content: unformatted, filename } = migratedData;
+      mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json');
+      await MigratedDataFactory.get();
+      await expect(
+        applyPrettierFormatting(unformatted, filename)
+      ).resolves.toEqual(unformatted);
     });
 
-    it('return original content if its invalid', async () => {
+    it('formats when prettier config file is found', async () => {
+      const { content, filename } = migratedData;
+      const formatted = formattedMigratedData.content;
+      mockedFunction(getFileList).mockResolvedValue(['.prettierrc']);
+      await MigratedDataFactory.get();
+      await expect(
+        applyPrettierFormatting(
+          content,
+          filename.endsWith('json') ? 'json' : 'json5'
+        )
+      ).resolves.toEqual(formatted);
+    });
+
+    it('formats when finds prettier config inside the package.json file', async () => {
+      const { content, filename } = migratedData;
+      const formatted = formattedMigratedData.content;
+      mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({
+        configFileName: 'renovate.json',
+      });
+      mockedFunction(readLocalFile).mockResolvedValueOnce('{"prettier":{}}');
+      await MigratedDataFactory.get();
       await expect(
-        applyPrettierFormatting(`{"name":"Rahul"`, 'json', {
-          indent: '  ',
-          amount: 2,
-        })
-      ).resolves.toBe(`{"name":"Rahul"`);
+        applyPrettierFormatting(
+          content,
+          filename.endsWith('json5') ? 'json5' : 'json'
+        )
+      ).resolves.toEqual(formatted);
     });
   });
 });