From ed73d380e854ef9f3a4c2d5a2dd2787dbfecfc1c Mon Sep 17 00:00:00 2001 From: Maksim Date: Mon, 2 May 2022 03:42:33 +0200 Subject: [PATCH] refactor: improve commit message functionality (#13328) * refactor: improve commit message functionality * refactor: fix test coverage * refactor: fix by comments * refactor: fix build * refactor: fix linting * refactor: fix export type * refactor: js private fields * refactor: static private fields * fix: lint * refactor: fix tsconfig * refactor: implement method normalizeInput * refactor: fix by comments * Update lib/workers/repository/model/commit-message.ts * refactor: fix by comments * refactor: use private typescript fields again * refactor: fix by comments Co-authored-by: Michael Kriese --- lib/types/commit-message-json.ts | 5 ++ lib/types/index.ts | 1 + .../model/commit-message-factory.ts | 51 +++++++++++ .../repository/model/commit-message.spec.ts | 57 ------------- .../repository/model/commit-message.ts | 84 +++++++++++++------ .../model/custom-commit-message.spec.ts | 52 ++++++++++++ .../repository/model/custom-commit-message.ts | 27 ++++++ .../model/semantic-commit-message.spec.ts | 73 ++++++++++++++++ .../model/semantic-commit-message.ts | 72 ++++++++++++++++ .../onboarding/branch/commit-message.ts | 27 ++---- .../onboarding/branch/create.spec.ts | 21 +++-- tsconfig.strict.json | 1 - 12 files changed, 359 insertions(+), 112 deletions(-) create mode 100644 lib/types/commit-message-json.ts create mode 100644 lib/workers/repository/model/commit-message-factory.ts delete mode 100644 lib/workers/repository/model/commit-message.spec.ts create mode 100644 lib/workers/repository/model/custom-commit-message.spec.ts create mode 100644 lib/workers/repository/model/custom-commit-message.ts create mode 100644 lib/workers/repository/model/semantic-commit-message.spec.ts create mode 100644 lib/workers/repository/model/semantic-commit-message.ts diff --git a/lib/types/commit-message-json.ts b/lib/types/commit-message-json.ts new file mode 100644 index 00000000000000..e7fa54a37d6422 --- /dev/null +++ b/lib/types/commit-message-json.ts @@ -0,0 +1,5 @@ +export interface CommitMessageJSON { + body?: string; + footer?: string; + subject?: string; +} diff --git a/lib/types/index.ts b/lib/types/index.ts index a447eb060ecb01..bceb6c6344512e 100644 --- a/lib/types/index.ts +++ b/lib/types/index.ts @@ -1,3 +1,4 @@ +export type { CommitMessageJSON } from './commit-message-json'; export * from './host-rules'; export * from './skip-reason'; export * from './versioning'; diff --git a/lib/workers/repository/model/commit-message-factory.ts b/lib/workers/repository/model/commit-message-factory.ts new file mode 100644 index 00000000000000..3feb7c594eb519 --- /dev/null +++ b/lib/workers/repository/model/commit-message-factory.ts @@ -0,0 +1,51 @@ +import type { RenovateSharedConfig } from '../../../config/types'; +import type { CommitMessage } from './commit-message'; +import { CustomCommitMessage } from './custom-commit-message'; +import { SemanticCommitMessage } from './semantic-commit-message'; + +type CommitMessageConfig = Pick< + RenovateSharedConfig, + | 'commitMessagePrefix' + | 'semanticCommits' + | 'semanticCommitScope' + | 'semanticCommitType' +>; + +export class CommitMessageFactory { + private readonly _config: CommitMessageConfig; + + constructor(config: CommitMessageConfig) { + this._config = config; + } + + create(): CommitMessage { + const message = this.areSemanticCommitsEnabled + ? this.createSemanticCommitMessage() + : this.createCustomCommitMessage(); + + return message; + } + + private createSemanticCommitMessage(): SemanticCommitMessage { + const message = new SemanticCommitMessage(); + + message.type = this._config.semanticCommitType ?? ''; + message.scope = this._config.semanticCommitScope ?? ''; + + return message; + } + + private createCustomCommitMessage(): CustomCommitMessage { + const message = new CustomCommitMessage(); + message.prefix = this._config.commitMessagePrefix ?? ''; + + return message; + } + + private get areSemanticCommitsEnabled(): boolean { + return ( + !this._config.commitMessagePrefix && + this._config.semanticCommits === 'enabled' + ); + } +} diff --git a/lib/workers/repository/model/commit-message.spec.ts b/lib/workers/repository/model/commit-message.spec.ts deleted file mode 100644 index 6c554934381c29..00000000000000 --- a/lib/workers/repository/model/commit-message.spec.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { CommitMessage } from './commit-message'; - -describe('workers/repository/model/commit-message', () => { - describe('CommitMessage', () => { - const TEST_CASES: ReadonlyArray< - [message: string, prefix: string | undefined, result: string] - > = [ - ['test', undefined, 'Test'], - ['test', '', 'Test'], - [' test ', ' ', 'Test'], - ['test', 'fix', 'fix: test'], - ['test', 'fix:', 'fix: test'], - ]; - - it('has colon character separator', () => { - expect(CommitMessage.SEPARATOR).toBe(':'); - }); - - it.each(TEST_CASES)( - 'given %p and %p as arguments, returns %p', - (message, prefix, result) => { - const commitMessage = new CommitMessage(message); - commitMessage.setCustomPrefix(prefix); - - expect(commitMessage.toString()).toEqual(result); - } - ); - - it('should handle not defined semantic prefix', () => { - const message = new CommitMessage('test'); - message.setSemanticPrefix(); - - expect(message.toString()).toBe('Test'); - }); - - it('should handle empty semantic prefix', () => { - const message = new CommitMessage('test'); - message.setSemanticPrefix(' ', ' '); - - expect(message.toString()).toBe('Test'); - }); - - it('should format sematic prefix', () => { - const message = new CommitMessage('test'); - message.setSemanticPrefix(' fix '); - - expect(message.toString()).toBe('fix: test'); - }); - - it('should format sematic prefix with scope', () => { - const message = new CommitMessage('test'); - message.setSemanticPrefix(' fix ', ' scope '); - - expect(message.toString()).toBe('fix(scope): test'); - }); - }); -}); diff --git a/lib/workers/repository/model/commit-message.ts b/lib/workers/repository/model/commit-message.ts index bab8afd4f6f429..0344e5b6471fc0 100644 --- a/lib/workers/repository/model/commit-message.ts +++ b/lib/workers/repository/model/commit-message.ts @@ -1,15 +1,22 @@ -export class CommitMessage { - public static readonly SEPARATOR: string = ':'; +import is from '@sindresorhus/is'; +import type { CommitMessageJSON } from '../../../types'; - private message = ''; +/** + * @see https://git-scm.com/docs/git-commit#_discussion + * + * [optional prefix]: + * [optional body] + * [optional footer] + */ +export abstract class CommitMessage { + private static readonly SEPARATOR: string = ':'; + private static readonly EXTRA_WHITESPACES = /\s+/g; - private prefix = ''; + private _body = ''; + private _footer = ''; + private _subject = ''; - constructor(message = '') { - this.setMessage(message); - } - - public static formatPrefix(prefix: string): string { + static formatPrefix(prefix: string): string { if (!prefix) { return ''; } @@ -21,34 +28,61 @@ export class CommitMessage { return `${prefix}${CommitMessage.SEPARATOR}`; } - public setMessage(message: string): void { - this.message = (message || '').trim(); + toJSON(): CommitMessageJSON { + return { + body: this._body, + footer: this._footer, + subject: this._subject, + }; } - public setCustomPrefix(prefix?: string): void { - this.prefix = (prefix ?? '').trim(); + toString(): string { + const parts: ReadonlyArray = [ + this.title, + this._body, + this._footer, + ]; + + return parts.filter(is.nonEmptyStringAndNotWhitespace).join('\n\n'); } - public setSemanticPrefix(type?: string, scope?: string): void { - this.prefix = (type ?? '').trim(); + get title(): string { + return [CommitMessage.formatPrefix(this.prefix), this.formatSubject()] + .join(' ') + .trim(); + } - if (scope?.trim()) { - this.prefix += `(${scope.trim()})`; - } + set body(value: string) { + this._body = this.normalizeInput(value); } - public toString(): string { - const prefix = CommitMessage.formatPrefix(this.prefix); - const message = this.formatMessage(); + set footer(value: string) { + this._footer = this.normalizeInput(value); + } - return [prefix, message].join(' ').trim(); + set subject(value: string) { + this._subject = this.normalizeInput(value); + this._subject = this._subject?.replace( + CommitMessage.EXTRA_WHITESPACES, + ' ' + ); } - private formatMessage(): string { + formatSubject(): string { + if (!this._subject) { + return ''; + } + if (this.prefix) { - return this.message; + return this._subject.charAt(0).toLowerCase() + this._subject.slice(1); } - return this.message.charAt(0).toUpperCase() + this.message.slice(1); + return this._subject.charAt(0).toUpperCase() + this._subject.slice(1); + } + + protected abstract get prefix(): string; + + protected normalizeInput(value: string | null | undefined): string { + return value?.trim() ?? ''; } } diff --git a/lib/workers/repository/model/custom-commit-message.spec.ts b/lib/workers/repository/model/custom-commit-message.spec.ts new file mode 100644 index 00000000000000..18c9fb02740410 --- /dev/null +++ b/lib/workers/repository/model/custom-commit-message.spec.ts @@ -0,0 +1,52 @@ +import { CustomCommitMessage } from './custom-commit-message'; + +describe('workers/repository/model/custom-commit-message', () => { + describe('CustomCommitMessage', () => { + it.each` + subject | prefix | result + ${'test'} | ${''} | ${'Test'} + ${' test '} | ${' '} | ${'Test'} + ${'test'} | ${'fix'} | ${'fix: test'} + ${'test'} | ${'fix:'} | ${'fix: test'} + ${'Message With Extra Whitespaces '} | ${' refactor '} | ${'refactor: message With Extra Whitespaces'} + `( + 'given subject $subject and prefix $prefix as arguments, returns $result', + ({ + subject, + prefix, + result, + }: { + subject: string; + prefix: string; + result: string; + }) => { + const commitMessage = new CustomCommitMessage(); + commitMessage.subject = subject; + commitMessage.prefix = prefix; + + expect(commitMessage.toString()).toEqual(result); + } + ); + + it('should provide ability to set body and footer', () => { + const commitMessage = new CustomCommitMessage(); + commitMessage.subject = 'subject'; + commitMessage.body = 'body'; + commitMessage.footer = 'footer'; + + expect(commitMessage.toJSON()).toEqual({ + body: 'body', + footer: 'footer', + prefix: '', + subject: 'subject', + }); + expect(commitMessage.toString()).toBe('Subject\n\nbody\n\nfooter'); + }); + + it('should remove empty subject by default', () => { + const commitMessage = new CustomCommitMessage(); + + expect(commitMessage.formatSubject()).toBe(''); + }); + }); +}); diff --git a/lib/workers/repository/model/custom-commit-message.ts b/lib/workers/repository/model/custom-commit-message.ts new file mode 100644 index 00000000000000..1f57a48e557c1a --- /dev/null +++ b/lib/workers/repository/model/custom-commit-message.ts @@ -0,0 +1,27 @@ +import type { CommitMessageJSON } from '../../../types'; +import { CommitMessage } from './commit-message'; + +export interface CustomCommitMessageJSON extends CommitMessageJSON { + prefix?: string; +} + +export class CustomCommitMessage extends CommitMessage { + private _prefix = ''; + + get prefix(): string { + return this._prefix; + } + + set prefix(value: string) { + this._prefix = this.normalizeInput(value); + } + + override toJSON(): CustomCommitMessageJSON { + const json = super.toJSON(); + + return { + ...json, + prefix: this._prefix, + }; + } +} diff --git a/lib/workers/repository/model/semantic-commit-message.spec.ts b/lib/workers/repository/model/semantic-commit-message.spec.ts new file mode 100644 index 00000000000000..db76829ba5b957 --- /dev/null +++ b/lib/workers/repository/model/semantic-commit-message.spec.ts @@ -0,0 +1,73 @@ +import { SemanticCommitMessage } from './semantic-commit-message'; + +describe('workers/repository/model/semantic-commit-message', () => { + it('should format message without prefix', () => { + const message = new SemanticCommitMessage(); + message.subject = 'test'; + + expect(message.toString()).toBe('Test'); + }); + + it('should format sematic type', () => { + const message = new SemanticCommitMessage(); + message.subject = 'test'; + message.type = ' fix '; + + expect(message.toString()).toBe('fix: test'); + }); + + it('should format sematic prefix with scope', () => { + const message = new SemanticCommitMessage(); + message.subject = 'test'; + message.type = ' fix '; + message.scope = ' scope '; + + expect(message.toString()).toBe('fix(scope): test'); + }); + + it('should create instance from string without scope', () => { + const instance = SemanticCommitMessage.fromString('feat: ticket 123'); + + expect(SemanticCommitMessage.is(instance)).toBeTrue(); + expect(instance.toJSON()).toEqual({ + body: '', + footer: '', + scope: '', + subject: 'ticket 123', + type: 'feat', + }); + }); + + it('should create instance from string with scope', () => { + const instance = SemanticCommitMessage.fromString( + 'fix(dashboard): ticket 123' + ); + + expect(SemanticCommitMessage.is(instance)).toBeTrue(); + expect(instance.toJSON()).toEqual({ + body: '', + footer: '', + scope: 'dashboard', + subject: 'ticket 123', + type: 'fix', + }); + }); + + it('should create instance from string with empty description', () => { + const instance = SemanticCommitMessage.fromString('fix(deps): '); + + expect(SemanticCommitMessage.is(instance)).toBeTrue(); + expect(instance.toJSON()).toEqual({ + body: '', + footer: '', + scope: 'deps', + subject: '', + type: 'fix', + }); + }); + + it('should return undefined for invalid string', () => { + const instance = SemanticCommitMessage.fromString('test'); + expect(instance).toBeUndefined(); + }); +}); diff --git a/lib/workers/repository/model/semantic-commit-message.ts b/lib/workers/repository/model/semantic-commit-message.ts new file mode 100644 index 00000000000000..a3e5d15888d906 --- /dev/null +++ b/lib/workers/repository/model/semantic-commit-message.ts @@ -0,0 +1,72 @@ +import type { CommitMessageJSON } from '../../../types'; +import { CommitMessage } from './commit-message'; + +export interface SemanticCommitMessageJSON extends CommitMessageJSON { + scope?: string; + type?: string; +} + +/** + * @see https://www.conventionalcommits.org/en/v1.0.0/#summary + * + * [optional scope]: + * [optional body] + * [optional footer] + */ +export class SemanticCommitMessage extends CommitMessage { + private static readonly REGEXP = + /^(?[\w]+)(\((?[\w-]+)\))?(?!)?: ((?([A-Z]+-|#)[\d]+) )?(?.*)/; + + private _scope = ''; + private _type = ''; + + static is(value: unknown): value is SemanticCommitMessage { + return value instanceof SemanticCommitMessage; + } + + static fromString(value: string): SemanticCommitMessage | undefined { + const match = value.match(SemanticCommitMessage.REGEXP); + + if (!match) { + return undefined; + } + + const { groups = {} } = match; + const message = new SemanticCommitMessage(); + message.type = groups.type; + message.scope = groups.scope; + message.subject = groups.description; + + return message; + } + + override toJSON(): SemanticCommitMessageJSON { + const json = super.toJSON(); + + return { + ...json, + scope: this._scope, + type: this._type, + }; + } + + set scope(value: string) { + this._scope = this.normalizeInput(value); + } + + set type(value: string) { + this._type = this.normalizeInput(value); + } + + protected get prefix(): string { + if (this._type && !this._scope) { + return this._type; + } + + if (this._scope) { + return `${this._type}(${this._scope})`; + } + + return ''; + } +} diff --git a/lib/workers/repository/onboarding/branch/commit-message.ts b/lib/workers/repository/onboarding/branch/commit-message.ts index fb6612381b5c26..94f51489e3530b 100644 --- a/lib/workers/repository/onboarding/branch/commit-message.ts +++ b/lib/workers/repository/onboarding/branch/commit-message.ts @@ -1,5 +1,6 @@ import type { RenovateConfig } from '../../../../config/types'; -import { CommitMessage } from '../../model/commit-message'; +import type { CommitMessage } from '../../model/commit-message'; +import { CommitMessageFactory } from '../../model/commit-message-factory'; export class OnboardingCommitMessageFactory { private readonly config: RenovateConfig; @@ -12,30 +13,16 @@ export class OnboardingCommitMessageFactory { } create(): CommitMessage { - const { - commitMessagePrefix, - onboardingCommitMessage, - semanticCommitScope, - semanticCommitType, - } = this.config; - const commitMessage = new CommitMessage(); - - if (commitMessagePrefix) { - commitMessage.setCustomPrefix(commitMessagePrefix); - } else if (this.areSemanticCommitsEnabled()) { - commitMessage.setSemanticPrefix(semanticCommitType, semanticCommitScope); - } + const { onboardingCommitMessage } = this.config; + const commitMessageFactory = new CommitMessageFactory(this.config); + const commitMessage = commitMessageFactory.create(); if (onboardingCommitMessage) { - commitMessage.setMessage(onboardingCommitMessage); + commitMessage.subject = onboardingCommitMessage; } else { - commitMessage.setMessage(`add ${this.configFile}`); + commitMessage.subject = `add ${this.configFile}`; } return commitMessage; } - - private areSemanticCommitsEnabled(): boolean { - return this.config.semanticCommits === 'enabled'; - } } diff --git a/lib/workers/repository/onboarding/branch/create.spec.ts b/lib/workers/repository/onboarding/branch/create.spec.ts index 70b27386bf1d34..dd89a3172b2ef5 100644 --- a/lib/workers/repository/onboarding/branch/create.spec.ts +++ b/lib/workers/repository/onboarding/branch/create.spec.ts @@ -1,6 +1,5 @@ import { RenovateConfig, getConfig, platform } from '../../../../../test/util'; import { commitFiles } from '../../../../util/git'; -import { CommitMessage } from '../../model/commit-message'; import { createOnboardingBranch } from './create'; jest.mock('../../../../util/git'); @@ -82,7 +81,7 @@ describe('workers/repository/onboarding/branch/create', () => { describe('applies the commitMessagePrefix value', () => { it('to the default commit message', async () => { const prefix = 'RENOV-123'; - const message = `${prefix}${CommitMessage.SEPARATOR} add renovate.json`; + const message = `${prefix}: add renovate.json`; config.commitMessagePrefix = prefix; @@ -106,7 +105,9 @@ describe('workers/repository/onboarding/branch/create', () => { const prefix = 'RENOV-123'; const text = "Cause your deps need an update and if they dont update, well they're no deps of mine"; - const message = `${prefix}${CommitMessage.SEPARATOR} ${text}`; + const message = `${prefix}: ${text.charAt(0).toLowerCase()}${text.slice( + 1 + )}`; config.commitMessagePrefix = prefix; config.onboardingCommitMessage = text; @@ -131,7 +132,7 @@ describe('workers/repository/onboarding/branch/create', () => { describe('applies semanticCommit prefix', () => { it('to the default commit message', async () => { const prefix = 'chore(deps)'; - const message = `${prefix}${CommitMessage.SEPARATOR} add renovate.json`; + const message = `${prefix}: add renovate.json`; config.semanticCommits = 'enabled'; @@ -155,7 +156,9 @@ describe('workers/repository/onboarding/branch/create', () => { const prefix = 'chore(deps)'; const text = 'I say, we can update when we want to, a commit they will never mind'; - const message = `${prefix}${CommitMessage.SEPARATOR} ${text}`; + const message = `${prefix}: ${text.charAt(0).toLowerCase()}${text.slice( + 1 + )}`; config.semanticCommits = 'enabled'; config.onboardingCommitMessage = text; @@ -180,7 +183,7 @@ describe('workers/repository/onboarding/branch/create', () => { describe('setting the onboarding configuration file name', () => { it('falls back to the default option if not present', async () => { const prefix = 'chore(deps)'; - const message = `${prefix}${CommitMessage.SEPARATOR} add renovate.json`; + const message = `${prefix}: add renovate.json`; config.semanticCommits = 'enabled'; config.onboardingConfigFileName = undefined; @@ -203,7 +206,7 @@ describe('workers/repository/onboarding/branch/create', () => { it('falls back to the default option if in list of allowed names', async () => { const prefix = 'chore(deps)'; - const message = `${prefix}${CommitMessage.SEPARATOR} add renovate.json`; + const message = `${prefix}: add renovate.json`; config.semanticCommits = 'enabled'; config.onboardingConfigFileName = 'superConfigFile.yaml'; @@ -227,7 +230,7 @@ describe('workers/repository/onboarding/branch/create', () => { it('uses the given name if valid', async () => { const prefix = 'chore(deps)'; const path = '.gitlab/renovate.json'; - const message = `${prefix}${CommitMessage.SEPARATOR} add ${path}`; + const message = `${prefix}: add ${path}`; config.semanticCommits = 'enabled'; config.onboardingConfigFileName = path; @@ -251,7 +254,7 @@ describe('workers/repository/onboarding/branch/create', () => { it('applies to the default commit message', async () => { const prefix = 'chore(deps)'; const path = `.renovaterc`; - const message = `${prefix}${CommitMessage.SEPARATOR} add ${path}`; + const message = `${prefix}: add ${path}`; config.semanticCommits = 'enabled'; config.onboardingConfigFileName = path; diff --git a/tsconfig.strict.json b/tsconfig.strict.json index 6b8ffdfc2135d1..9bc11aee87cbc9 100644 --- a/tsconfig.strict.json +++ b/tsconfig.strict.json @@ -49,7 +49,6 @@ "lib/workers/repository/init/semantic.ts", "lib/workers/repository/init/vulnerability.ts", "lib/workers/repository/onboarding/branch/check.ts", - "lib/workers/repository/onboarding/branch/commit-message.ts", "lib/workers/repository/onboarding/branch/config.ts", "lib/workers/repository/onboarding/branch/create.ts", "lib/workers/repository/onboarding/branch/index.ts",