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

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

@@ -3,9 +3,14 @@ import { RenovateConfig, getConfig, platform } from '../../../../../test/util';
 import { checkoutBranch, commitFiles } from '../../../../util/git';
 import { createConfigMigrationBranch } from './create';
 import type { MigratedData } from './migrated-data';
+import { MigratedDataFactory } from './migrated-data';
 
 jest.mock('../../../../util/git');
 
+const formattedMigratedData = Fixtures.getJson(
+  './migrated-data-formatted.json'
+);
+
 describe('workers/repository/config-migration/branch/create', () => {
   const raw = Fixtures.getJson('./renovate.json');
   const indent = '  ';
@@ -24,6 +29,11 @@ describe('workers/repository/config-migration/branch/create', () => {
   });
 
   describe('createConfigMigrationBranch', () => {
+    const prettierSpy = jest.spyOn(
+      MigratedDataFactory,
+      'applyPrettierFormatting'
+    );
+
     it('applies the default commit message', async () => {
       await createConfigMigrationBranch(config, migratedConfigData);
       expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
@@ -178,6 +188,25 @@ describe('workers/repository/config-migration/branch/create', () => {
           platformCommit: false,
         });
       });
+
+      it('applies prettier formatting to the committed content', async () => {
+        const formatted = formattedMigratedData.content;
+        prettierSpy.mockResolvedValueOnce(formattedMigratedData.content);
+        await createConfigMigrationBranch(config, migratedConfigData);
+        expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
+        expect(commitFiles).toHaveBeenCalledWith({
+          branchName: 'renovate/migrate-config',
+          files: [
+            {
+              type: 'addition',
+              path: 'renovate.json',
+              contents: formatted,
+            },
+          ],
+          message: 'Migrate config renovate.json',
+          platformCommit: false,
+        });
+      });
     });
   });
 });

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

@@ -5,14 +5,14 @@ import { commitAndPush } from '../../../../modules/platform/commit';
 import { checkoutBranch } from '../../../../util/git';
 import { getMigrationBranchName } from '../common';
 import { ConfigMigrationCommitMessageFactory } from './commit-message';
+import { MigratedDataFactory } 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 +30,11 @@ export async function createConfigMigrationBranch(
   }
 
   await checkoutBranch(config.defaultBranch!);
+  let contents = migratedConfigData.content;
+  const prettified = await MigratedDataFactory.applyPrettierFormatting();
+  if (prettified) {
+    contents = prettified;
+  }
   return commitAndPush({
     branchName: getMigrationBranchName(config),
     files: [

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

@@ -7,7 +7,7 @@ import { logger } from '../../../../logger';
 import { readLocalFile } from '../../../../util/fs';
 import { getFileList } from '../../../../util/git';
 import { detectRepoFileConfig } from '../../init/merge';
-import { MigratedDataFactory, applyPrettierFormatting } from './migrated-data';
+import { MigratedDataFactory } from './migrated-data';
 
 jest.mock('../../../../config/migration');
 jest.mock('../../../../util/git');
@@ -41,7 +41,6 @@ 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 () => {
@@ -119,49 +118,82 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {
         '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('returns null if invoked before factory is initialized', async () => {
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting()
+      ).resolves.toBeNull();
+    });
+
+    it('does not format when no prettier config is present', async () => {
+      const unformatted = migratedData.content;
+      mockedFunction(readLocalFile).mockResolvedValueOnce(null);
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting()
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('does not format when failing to fetch package.json file', async () => {
+      const unformatted = migratedData.content;
       mockedFunction(readLocalFile).mockRejectedValueOnce(null);
-      MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting()
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('does not format when there is an invalid package.json file', async () => {
+      const unformatted = migratedData.content;
+      mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json');
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting()
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('formats when prettier config file is found', async () => {
+      const formatted = formattedMigratedData.content;
+      mockedFunction(getFileList).mockResolvedValue(['.prettierrc']);
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting()
+      ).resolves.toEqual(formatted);
     });
 
-    it('return original content if its invalid', async () => {
+    it('formats when finds prettier config inside the package.json file', async () => {
+      const formatted = formattedMigratedData.content;
+      mockedFunction(detectRepoFileConfig).mockResolvedValueOnce({
+        configFileName: 'renovate.json',
+      });
+      mockedFunction(readLocalFile).mockResolvedValueOnce('{"prettier":{}}');
+      await MigratedDataFactory.getAsync();
       await expect(
-        applyPrettierFormatting(`{"name":"Rahul"`, 'json', {
-          indent: '  ',
-          amount: 2,
-        })
-      ).resolves.toBe(`{"name":"Rahul"`);
+        MigratedDataFactory.applyPrettierFormatting()
+      ).resolves.toEqual(formatted);
     });
   });
 });

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

@@ -11,6 +11,7 @@ export interface MigratedData {
   content: string;
   filename: string;
 }
+
 interface Indent {
   amount: number;
   indent: string;
@@ -30,42 +31,10 @@ const prettierConfigFilenames = new Set([
   '.prettierrc.toml',
 ]);
 
-export async function applyPrettierFormatting(
-  content: string,
-  parser: string,
-  indent: Indent
-): Promise<string> {
-  const fileList = await getFileList();
-  let prettierExists = fileList.some((file) =>
-    prettierConfigFilenames.has(file)
-  );
-  if (!prettierExists) {
-    try {
-      const packageJsonContent = await readLocalFile('package.json', 'utf8');
-      prettierExists =
-        packageJsonContent && JSON.parse(packageJsonContent).prettier;
-    } catch {
-      logger.warn(
-        'applyPrettierFormatting() - Error processing package.json file'
-      );
-    }
-  }
-
-  if (!prettierExists) {
-    return content;
-  }
-  const options = {
-    parser,
-    tabWidth: indent.amount === 0 ? 2 : indent.amount,
-    useTabs: indent.type === 'tab',
-  };
-
-  return prettier.format(content, options);
-}
-
 export class MigratedDataFactory {
   // singleton
   private static data: MigratedData | null;
+  private static indent: Indent;
 
   public static async getAsync(): Promise<MigratedData | null> {
     if (this.data) {
@@ -81,6 +50,50 @@ export class MigratedDataFactory {
     return this.data;
   }
 
+  public static async applyPrettierFormatting(): Promise<string | null> {
+    try {
+      logger.trace('applyPrettierFormatting() - START');
+      if (!this.data) {
+        return null;
+      }
+
+      const { content, filename } = this.data;
+      const indent = this.indent;
+      const fileList = await getFileList();
+      let prettierExists = fileList.some((file) =>
+        prettierConfigFilenames.has(file)
+      );
+
+      if (!prettierExists) {
+        try {
+          const packageJsonContent = await readLocalFile(
+            'package.json',
+            'utf8'
+          );
+          prettierExists =
+            packageJsonContent && JSON.parse(packageJsonContent).prettier;
+        } catch {
+          logger.warn(
+            'applyPrettierFormatting() - Error processing package.json file'
+          );
+        }
+      }
+
+      if (!prettierExists) {
+        return content;
+      }
+      const options = {
+        parser: filename.endsWith('.json5') ? 'json5' : 'json',
+        tabWidth: indent.amount === 0 ? 2 : indent.amount,
+        useTabs: indent.type === 'tab',
+      };
+
+      return prettier.format(content, options);
+    } finally {
+      logger.trace('applyPrettierFormatting() - END');
+    }
+  }
+
   public static reset(): void {
     this.data = null;
   }
@@ -105,8 +118,8 @@ export class MigratedDataFactory {
 
       // indent defaults to 2 spaces
       // TODO #7154
-      const indent = detectIndent(raw!);
-      const indentSpace = indent.indent ?? '  ';
+      this.indent = detectIndent(raw!);
+      const indentSpace = this.indent.indent ?? '  ';
       const filename = configFileName!;
       let content: string;
 
@@ -116,12 +129,6 @@ export class MigratedDataFactory {
         content = JSON.stringify(migratedConfig, undefined, indentSpace);
       }
 
-      // format if prettier is found in the user's repo
-      content = await applyPrettierFormatting(
-        content,
-        filename.endsWith('.json5') ? 'json5' : 'json',
-        indent
-      );
       if (!content.endsWith('\n')) {
         content += '\n';
       }

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

@@ -6,13 +6,23 @@ import {
   platform,
 } from '../../../../../test/util';
 import { GlobalConfig } from '../../../../config/global';
-import { checkoutBranch } from '../../../../util/git';
+import { checkoutBranch, commitFiles } from '../../../../util/git';
+import { MigratedDataFactory } from './migrated-data';
 import type { MigratedData } from './migrated-data';
 import { rebaseMigrationBranch } from './rebase';
 
 jest.mock('../../../../util/git');
 
+const formattedMigratedData = Fixtures.getJson(
+  './migrated-data-formatted.json'
+);
+
 describe('workers/repository/config-migration/branch/rebase', () => {
+  const prettierSpy = jest.spyOn(
+    MigratedDataFactory,
+    'applyPrettierFormatting'
+  );
+
   beforeAll(() => {
     GlobalConfig.set({
       localDir: '',
@@ -63,6 +73,27 @@ describe('workers/repository/config-migration/branch/rebase', () => {
       expect(git.commitFiles).toHaveBeenCalledTimes(1);
     });
 
+    it('applies prettier formatting when rebasing the migration branch ', async () => {
+      const formatted = formattedMigratedData.content;
+      prettierSpy.mockResolvedValueOnce(formattedMigratedData.content);
+      git.isBranchBehindBase.mockResolvedValueOnce(true);
+      await rebaseMigrationBranch(config, migratedConfigData);
+      expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch);
+      expect(git.commitFiles).toHaveBeenCalledTimes(1);
+      expect(commitFiles).toHaveBeenCalledWith({
+        branchName: 'renovate/migrate-config',
+        files: [
+          {
+            type: 'addition',
+            path: 'renovate.json',
+            contents: formatted,
+          },
+        ],
+        message: 'Migrate config renovate.json',
+        platformCommit: false,
+      });
+    });
+
     it('does not rebases migration branch when in dryRun is on', async () => {
       GlobalConfig.set({
         dryRun: 'full',