refactor(config-migration): invoke applyPrettierFormatting at the c…

…ommit stage (#18150)

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,7 +1,14 @@
+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 { MigratedDataFactory } from './migrated-data';
 import type { MigratedData } from './migrated-data';
 
 jest.mock('../../../../util/git');
@@ -11,6 +18,10 @@ 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(
+    MigratedDataFactory,
+    'applyPrettierFormatting'
+  );
 
   let config: RenovateConfig;
   let migratedConfigData: MigratedData;
@@ -19,7 +30,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

@@ -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,9 @@ export async function createConfigMigrationBranch(
   }
 
   await checkoutBranch(config.defaultBranch!);
+  const contents = await MigratedDataFactory.applyPrettierFormatting(
+    migratedConfigData
+  );
   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 () => {
@@ -86,16 +85,18 @@ describe('workers/repository/config-migration/branch/migrated-data', () => {
     });
 
     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.getAsync()).resolves.toEqual({
+        ...migratedData,
+        indent,
+      });
     });
 
     it('Migrate a JSON5 config file', async () => {
@@ -119,49 +120,76 @@ 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('does not format when no prettier config is present', async () => {
+      const { content: unformatted } = migratedData;
+      mockedFunction(readLocalFile).mockResolvedValueOnce(null);
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting(migratedData)
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('does not format when failing to fetch package.json file', async () => {
+      const { content: unformatted } = migratedData;
       mockedFunction(readLocalFile).mockRejectedValueOnce(null);
-      MigratedDataFactory.reset();
-      await expect(MigratedDataFactory.getAsync()).resolves.toEqual(
-        migratedData
-      );
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting(migratedData)
+      ).resolves.toEqual(unformatted);
+    });
+
+    it('does not format when there is an invalid package.json file', async () => {
+      const { content: unformatted } = migratedData;
+      mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json');
+      await MigratedDataFactory.getAsync();
+      await expect(
+        MigratedDataFactory.applyPrettierFormatting(migratedData)
+      ).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(migratedData)
+      ).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(migratedData)
+      ).resolves.toEqual(formatted);
     });
   });
 });

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

@@ -1,6 +1,7 @@
 import detectIndent from 'detect-indent';
 import JSON5 from 'json5';
-import prettier from 'prettier';
+import prettier, { BuiltInParserName } from 'prettier';
+import upath from 'upath';
 import { migrateConfig } from '../../../../config/migration';
 import { logger } from '../../../../logger';
 import { readLocalFile } from '../../../../util/fs';
@@ -10,7 +11,9 @@ import { detectRepoFileConfig } from '../../init/merge';
 export interface MigratedData {
   content: string;
   filename: string;
+  indent: Indent;
 }
+
 interface Indent {
   amount: number;
   indent: string;
@@ -30,44 +33,52 @@ const prettierConfigFilenames = new Set([
   '.prettierrc.toml',
 ]);
 
+export type PrettierParser = BuiltInParserName;
+
 export async function applyPrettierFormatting(
   content: string,
-  parser: string,
-  indent: Indent
+  parser: PrettierParser,
+  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'
-      );
+  try {
+    logger.trace('applyPrettierFormatting - START');
+    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;
+    if (!prettierExists) {
+      return content;
+    }
+    const options = {
+      parser,
+      tabWidth: indent?.amount === 0 ? 2 : indent?.amount,
+      useTabs: indent?.type === 'tab',
+    };
+
+    return prettier.format(content, options);
+  } finally {
+    logger.trace('applyPrettierFormatting - END');
   }
-  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;
 
-  public static async getAsync(): Promise<MigratedData | null> {
+  static async getAsync(): Promise<MigratedData | null> {
     if (this.data) {
       return this.data;
     }
@@ -81,10 +92,19 @@ export class MigratedDataFactory {
     return this.data;
   }
 
-  public static reset(): void {
+  static reset(): void {
     this.data = null;
   }
 
+  static applyPrettierFormatting({
+    content,
+    filename,
+    indent,
+  }: MigratedData): Promise<string> {
+    const parser = upath.extname(filename).replace('.', '') as PrettierParser;
+    return applyPrettierFormatting(content, parser, indent);
+  }
+
   private static async build(): Promise<MigratedData | null> {
     let res: MigratedData | null = null;
     try {
@@ -116,17 +136,11 @@ 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';
       }
 
-      res = { content, filename };
+      res = { content, filename, indent };
     } catch (err) {
       logger.debug(
         { err },