diff --git a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json b/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json index 9febe12a31cb7f..440ee5392db4da 100644 --- a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json +++ b/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" + } } diff --git a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 b/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 index 0540d1f8ccb642..259972dafa88f5 100644 --- a/lib/workers/repository/config-migration/branch/__fixtures__/migrated-data.json5 +++ b/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" + } } diff --git a/lib/workers/repository/config-migration/branch/create.spec.ts b/lib/workers/repository/config-migration/branch/create.spec.ts index abfa45bd1ac180..adeac4742f9279 100644 --- a/lib/workers/repository/config-migration/branch/create.spec.ts +++ b/lib/workers/repository/config-migration/branch/create.spec.ts @@ -1,21 +1,24 @@ +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 { MigratedDataFactory } from './migrated-data'; +import * as MigratedDataModule 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 = ' '; const renovateConfig = JSON.stringify(raw, undefined, indent) + '\n'; const filename = 'renovate.json'; + const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting'); let config: RenovateConfig; let migratedConfigData: MigratedData; @@ -25,15 +28,15 @@ 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({}), + }; + prettierSpy.mockResolvedValueOnce(migratedConfigData.content); }); describe('createConfigMigrationBranch', () => { - const prettierSpy = jest.spyOn( - MigratedDataFactory, - 'applyPrettierFormatting' - ); - it('applies the default commit message', async () => { await createConfigMigrationBranch(config, migratedConfigData); expect(checkoutBranch).toHaveBeenCalledWith(config.defaultBranch); @@ -188,25 +191,6 @@ 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, - }); - }); }); }); }); diff --git a/lib/workers/repository/config-migration/branch/create.ts b/lib/workers/repository/config-migration/branch/create.ts index 5e5bc5b3223387..285823426d56e9 100644 --- a/lib/workers/repository/config-migration/branch/create.ts +++ b/lib/workers/repository/config-migration/branch/create.ts @@ -1,3 +1,4 @@ +import upath from 'upath'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -5,7 +6,7 @@ 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 { PrettierParser, applyPrettierFormatting } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function createConfigMigrationBranch( @@ -30,11 +31,9 @@ export async function createConfigMigrationBranch( } await checkoutBranch(config.defaultBranch!); - let contents = migratedConfigData.content; - const prettified = await MigratedDataFactory.applyPrettierFormatting(); - if (prettified) { - contents = prettified; - } + 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: [ diff --git a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts b/lib/workers/repository/config-migration/branch/migrated-data.spec.ts index ecc445e53739a9..55fd85b4234faf 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.spec.ts +++ b/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 } from './migrated-data'; +import { MigratedDataFactory, applyPrettierFormatting } from './migrated-data'; jest.mock('../../../../config/migration'); jest.mock('../../../../util/git'); @@ -48,53 +48,49 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { 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 () => { @@ -103,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 ); }); @@ -112,7 +108,7 @@ 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' @@ -142,57 +138,59 @@ describe('workers/repository/config-migration/branch/migrated-data', () => { mockedFunction(getFileList).mockResolvedValue([]); }); - 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; + const { content: unformatted, filename } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce(null); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting(unformatted, filename) ).resolves.toEqual(unformatted); }); it('does not format when failing to fetch package.json file', async () => { - const unformatted = migratedData.content; + const { content: unformatted, filename } = migratedData; mockedFunction(readLocalFile).mockRejectedValueOnce(null); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting(unformatted, filename) ).resolves.toEqual(unformatted); }); it('does not format when there is an invalid package.json file', async () => { - const unformatted = migratedData.content; + const { content: unformatted, filename } = migratedData; mockedFunction(readLocalFile).mockResolvedValueOnce('invalid json'); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting(unformatted, filename) ).resolves.toEqual(unformatted); }); it('formats when prettier config file is found', async () => { + const { content, filename } = migratedData; const formatted = formattedMigratedData.content; mockedFunction(getFileList).mockResolvedValue(['.prettierrc']); - await MigratedDataFactory.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + 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.getAsync(); + await MigratedDataFactory.get(); await expect( - MigratedDataFactory.applyPrettierFormatting() + applyPrettierFormatting( + content, + filename.endsWith('json5') ? 'json5' : 'json' + ) ).resolves.toEqual(formatted); }); }); diff --git a/lib/workers/repository/config-migration/branch/migrated-data.ts b/lib/workers/repository/config-migration/branch/migrated-data.ts index 5a67b31cea4662..66b6e6080bbf93 100644 --- a/lib/workers/repository/config-migration/branch/migrated-data.ts +++ b/lib/workers/repository/config-migration/branch/migrated-data.ts @@ -1,6 +1,6 @@ import detectIndent from 'detect-indent'; import JSON5 from 'json5'; -import prettier from 'prettier'; +import prettier, { BuiltInParserName } from 'prettier'; import { migrateConfig } from '../../../../config/migration'; import { logger } from '../../../../logger'; import { readLocalFile } from '../../../../util/fs'; @@ -10,6 +10,7 @@ import { detectRepoFileConfig } from '../../init/merge'; export interface MigratedData { content: string; filename: string; + indent: Indent; } interface Indent { @@ -31,12 +32,51 @@ const prettierConfigFilenames = new Set([ '.prettierrc.toml', ]); +export type PrettierParser = BuiltInParserName; +export async function applyPrettierFormatting( + content: string, + parser: PrettierParser, + indent?: Indent +): Promise { + 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; + } + const options = { + parser, + tabWidth: indent?.amount === 0 ? 2 : indent?.amount, + useTabs: indent?.type === 'tab', + }; + + return prettier.format(content, options); + } finally { + logger.trace('applyPrettierFormatting - END'); + } +} + export class MigratedDataFactory { // singleton private static data: MigratedData | null; - private static indent: Indent; - public static async getAsync(): Promise { + static async get(): Promise { if (this.data) { return this.data; } @@ -50,51 +90,7 @@ export class MigratedDataFactory { return this.data; } - public static async applyPrettierFormatting(): Promise { - 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 { + static reset(): void { this.data = null; } @@ -118,8 +114,8 @@ export class MigratedDataFactory { // indent defaults to 2 spaces // TODO #7154 - this.indent = detectIndent(raw!); - const indentSpace = this.indent.indent ?? ' '; + const indent = detectIndent(raw!); + const indentSpace = indent.indent ?? ' '; const filename = configFileName!; let content: string; @@ -133,7 +129,7 @@ export class MigratedDataFactory { content += '\n'; } - res = { content, filename }; + res = { content, filename, indent }; } catch (err) { logger.debug( { err }, diff --git a/lib/workers/repository/config-migration/branch/rebase.spec.ts b/lib/workers/repository/config-migration/branch/rebase.spec.ts index 718fe80d9754a0..70993b0d348b7e 100644 --- a/lib/workers/repository/config-migration/branch/rebase.spec.ts +++ b/lib/workers/repository/config-migration/branch/rebase.spec.ts @@ -1,13 +1,15 @@ +import type { Indent } from 'detect-indent'; import { Fixtures } from '../../../../../test/fixtures'; import { RenovateConfig, getConfig, git, + partial, platform, } from '../../../../../test/util'; import { GlobalConfig } from '../../../../config/global'; import { checkoutBranch, commitFiles } from '../../../../util/git'; -import { MigratedDataFactory } from './migrated-data'; +import * as MigratedDataModule from './migrated-data'; import type { MigratedData } from './migrated-data'; import { rebaseMigrationBranch } from './rebase'; @@ -18,10 +20,7 @@ const formattedMigratedData = Fixtures.getJson( ); describe('workers/repository/config-migration/branch/rebase', () => { - const prettierSpy = jest.spyOn( - MigratedDataFactory, - 'applyPrettierFormatting' - ); + const prettierSpy = jest.spyOn(MigratedDataModule, 'applyPrettierFormatting'); beforeAll(() => { GlobalConfig.set({ @@ -41,7 +40,11 @@ describe('workers/repository/config-migration/branch/rebase', () => { beforeEach(() => { jest.resetAllMocks(); GlobalConfig.reset(); - migratedConfigData = { content: renovateConfig, filename }; + migratedConfigData = { + content: renovateConfig, + filename, + indent: partial({}), + }; config = { ...getConfig(), repository: 'some/repo', diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 47bb7fbbcaf693..e638eb387bb4c2 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -1,4 +1,5 @@ import hasha from 'hasha'; +import upath from 'upath'; import { GlobalConfig } from '../../../../config/global'; import type { RenovateConfig } from '../../../../config/types'; import { logger } from '../../../../logger'; @@ -11,7 +12,7 @@ import { import { quickStringify } from '../../../../util/stringify'; import { getMigrationBranchName } from '../common'; import { ConfigMigrationCommitMessageFactory } from './commit-message'; -import { MigratedDataFactory } from './migrated-data'; +import { PrettierParser, applyPrettierFormatting } from './migrated-data'; import type { MigratedData } from './migrated-data'; export async function rebaseMigrationBranch( @@ -45,10 +46,10 @@ export async function rebaseMigrationBranch( const commitMessage = commitMessageFactory.getCommitMessage(); await checkoutBranch(config.defaultBranch!); - const prettified = await MigratedDataFactory.applyPrettierFormatting(); - if (prettified) { - contents = prettified; - } + + const { content, filename, indent } = migratedConfigData; + const parser = upath.extname(filename).replace('.', '') as PrettierParser; + contents = await applyPrettierFormatting(content, parser, indent); return commitAndPush({ branchName, files: [ diff --git a/lib/workers/repository/config-migration/index.spec.ts b/lib/workers/repository/config-migration/index.spec.ts index ad29c89f68e57a..6b1512647da54f 100644 --- a/lib/workers/repository/config-migration/index.spec.ts +++ b/lib/workers/repository/config-migration/index.spec.ts @@ -1,5 +1,6 @@ +import type { Indent } from 'detect-indent'; import { Fixtures } from '../../../../test/fixtures'; -import { getConfig, mockedFunction } from '../../../../test/util'; +import { getConfig, mockedFunction, partial } from '../../../../test/util'; import { checkConfigMigrationBranch } from './branch'; import { MigratedDataFactory } from './branch/migrated-data'; import { ensureConfigMigrationPr } from './pr'; @@ -20,15 +21,16 @@ const config = { describe('workers/repository/config-migration/index', () => { beforeEach(() => { jest.resetAllMocks(); - mockedFunction(MigratedDataFactory.getAsync).mockResolvedValue({ + mockedFunction(MigratedDataFactory.get).mockResolvedValue({ filename, content, + indent: partial({}), }); }); it('does nothing when config migration is disabled', async () => { await configMigration({ ...config, configMigration: false }, []); - expect(MigratedDataFactory.getAsync).toHaveBeenCalledTimes(0); + expect(MigratedDataFactory.get).toHaveBeenCalledTimes(0); expect(checkConfigMigrationBranch).toHaveBeenCalledTimes(0); expect(ensureConfigMigrationPr).toHaveBeenCalledTimes(0); }); diff --git a/lib/workers/repository/config-migration/index.ts b/lib/workers/repository/config-migration/index.ts index 33a401d0635750..eddcd14fdb14cc 100644 --- a/lib/workers/repository/config-migration/index.ts +++ b/lib/workers/repository/config-migration/index.ts @@ -8,7 +8,7 @@ export async function configMigration( branchList: string[] ): Promise { if (config.configMigration) { - const migratedConfigData = await MigratedDataFactory.getAsync(); + const migratedConfigData = await MigratedDataFactory.get(); const migrationBranch = await checkConfigMigrationBranch( config, migratedConfigData diff --git a/lib/workers/repository/config-migration/pr/index.spec.ts b/lib/workers/repository/config-migration/pr/index.spec.ts index 6b2053ae61ccb9..48eaacbcc2a9e5 100644 --- a/lib/workers/repository/config-migration/pr/index.spec.ts +++ b/lib/workers/repository/config-migration/pr/index.spec.ts @@ -1,3 +1,4 @@ +import type { Indent } from 'detect-indent'; import type { RequestError, Response } from 'got'; import { mock } from 'jest-mock-extended'; import { Fixtures } from '../../../../../test/fixtures'; @@ -30,6 +31,7 @@ describe('workers/repository/config-migration/pr/index', () => { const migratedData: MigratedData = { content: migratedContent, filename: configFileName, + indent: partial({}), }; let config: RenovateConfig; @@ -166,6 +168,7 @@ describe('workers/repository/config-migration/pr/index', () => { await ensureConfigMigrationPr(config, { content: migratedContent, filename: 'renovate.json5', + indent: partial({}), }); expect(platform.createPr).toHaveBeenCalledTimes(1); expect(platform.createPr.mock.calls[0][0].prBody).toMatchSnapshot();