From 9e1ad5133b421ebeee9fcbeefc2efe14d8af369e Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Thu, 7 Dec 2023 18:41:14 -0300 Subject: [PATCH] Add support for ignored patterns - Improve the existing merge commit detection. - Allow users to disable merge commit detection. - Allow users to provide custom patterns to ignore Signed-off-by: Luiz Ferraz --- action.yml | 10 ++++++++ src/__tests__/input.test.ts | 10 ++++++++ src/__tests__/inspection.test.ts | 40 +++++++++++++++++++++++++++++--- src/input.ts | 33 ++++++++++++++++++++++++++ src/inspection.ts | 26 ++++++++++++++++----- src/mainImpl.ts | 10 ++++++++ 6 files changed, 120 insertions(+), 9 deletions(-) diff --git a/action.yml b/action.yml index 803fa21..a0fb897 100644 --- a/action.yml +++ b/action.yml @@ -40,6 +40,16 @@ inputs: description: 'If set to "true", validate commits in pull requests instead of the pull request body' required: false default: '' + ignore-merge-commits: + description: 'If set to "false", merge commits will not be ignored.' + required: false + default: 'true' + ignore-patterns: + description: >- + Pattern to ignore in commit messages. One pattern per line. + Patterns are matched against the whole commit message as regular expressions without any flags. + required: false + default: '' github-token: description: 'GitHub token' required: false diff --git a/src/__tests__/input.test.ts b/src/__tests__/input.test.ts index b7bb953..ca78464 100644 --- a/src/__tests__/input.test.ts +++ b/src/__tests__/input.test.ts @@ -39,6 +39,11 @@ it('parses the inputs.', () => { enforceSignOffInput: 'true', validatePullRequestCommitsInput: 'true', skipBodyCheckInput: 'true', + ignoreMergeCommitsInput: 'false', + ignorePatternsInput: ` + ^Some pattern$ + Another pattern + ` }); expect(maybeInputs.error).toBeNull(); @@ -55,4 +60,9 @@ it('parses the inputs.', () => { expect(inputs.enforceSignOff).toBeTruthy(); expect(inputs.validatePullRequestCommits).toBeTruthy(); expect(inputs.skipBodyCheck).toBeTruthy(); + expect(inputs.ignoreMergeCommits).toBeFalsy(); + expect(inputs.ignorePatterns).toEqual([ + /^Some pattern$/, + /Another pattern/ + ]); }); diff --git a/src/__tests__/inspection.test.ts b/src/__tests__/inspection.test.ts index a671d95..f9059d8 100644 --- a/src/__tests__/inspection.test.ts +++ b/src/__tests__/inspection.test.ts @@ -1,3 +1,4 @@ +import {Inputs} from '../input'; import * as input from '../input'; import * as inspection from '../inspection'; @@ -221,6 +222,8 @@ it( enforceSignOff: false, validatePullRequestCommits: false, skipBodyCheck: false, + ignoreMergeCommits: false, + ignorePatterns: [], }); const message = @@ -427,10 +430,41 @@ it('reports duplicate starting word in subject and body.', () => { ]); }); -it('ignores merge messages.', () => { - const message = "Merge branch 'V20DataModel' into miho/Conform-to-spec"; +it.each([ + // Local merge to default branch + "Merge branch 'fantastic-feature'", + // Local merge to alternate branch + "Merge branch 'V20DataModel' into miho/Conform-to-spec", + // Local merge from remote + "Merge remote-tracking branch 'origin/remote-branch' into local-branch", + // Web UI merge pull request + "Merge pull request #11 from acme-corp/the-project" +])('ignores merge messages.', (message) => { + const inputs = new Inputs({ + ...defaultInputs, + ignoreMergeCommits: true, + // Ensure all messages would fail if not for ignoring merge commits. + maxSubjectLength: 1, + }) - const errors = inspection.check(message, defaultInputs); + const errors = inspection.check(message, inputs); + expect(errors).toEqual([]); +}); + +it('ignores messages with given pattern.', () => { + const inputs = new Inputs({ + ...defaultInputs, + ignorePatterns: [/\[ALWAYS VALID]/], + // Ensure all messages would fail if not for the ignore pattern. + maxSubjectLength: 1, + }) + +const message = 'Change SomeClass to OtherClass\n' + + '\n' + + 'This replaces the SomeClass with OtherClass in all of the module.\n' + + '[ALWAYS VALID] ' + + const errors = inspection.check(message, inputs); expect(errors).toEqual([]); }); diff --git a/src/input.ts b/src/input.ts index 80aa85e..572ffb3 100644 --- a/src/input.ts +++ b/src/input.ts @@ -10,6 +10,8 @@ interface InputValues { enforceSignOff: boolean; validatePullRequestCommits: boolean; skipBodyCheck: boolean; + ignoreMergeCommits: boolean; + ignorePatterns: RegExp[]; } export class Inputs implements InputValues { @@ -20,6 +22,8 @@ export class Inputs implements InputValues { public maxBodyLineLength: number; public skipBodyCheck: boolean; public validatePullRequestCommits: boolean; + public ignoreMergeCommits: boolean; + public ignorePatterns: RegExp[]; // This is a complete appendix to the whitelist parsed both from // the GitHub action input "additional-verbs" and from the file @@ -38,6 +42,8 @@ export class Inputs implements InputValues { this.enforceSignOff = values.enforceSignOff; this.validatePullRequestCommits = values.validatePullRequestCommits; this.skipBodyCheck = values.skipBodyCheck; + this.ignoreMergeCommits = values.ignoreMergeCommits; + this.ignorePatterns = values.ignorePatterns; } } @@ -80,6 +86,8 @@ interface RawInputs { enforceSignOffInput?: string; validatePullRequestCommitsInput?: string; skipBodyCheckInput?: string; + ignoreMergeCommitsInput?: string; + ignorePatternsInput?: string; } export function parseInputs(rawInputs: RawInputs): MaybeInputs { @@ -92,6 +100,8 @@ export function parseInputs(rawInputs: RawInputs): MaybeInputs { enforceSignOffInput = '', validatePullRequestCommitsInput = '', skipBodyCheckInput = '', + ignoreMergeCommitsInput = '', + ignorePatternsInput = '', } = rawInputs; const additionalVerbs = new Set(); @@ -193,6 +203,27 @@ export function parseInputs(rawInputs: RawInputs): MaybeInputs { ); } + const ignoreMergeCommits: boolean | null = !ignoreMergeCommitsInput + ? true + : parseBooleanFromString(ignoreMergeCommitsInput); + + if (ignoreMergeCommits === null) { + return new MaybeInputs( + null, + 'Unexpected value for ignore-merge-commits. ' + + `Expected either 'true' or 'false', got: ${ignoreMergeCommitsInput}`, + ); + } + + const ignorePatterns: RegExp[] = + ignorePatternsInput == null + ? [] + : ignorePatternsInput + .split('\n') + .map(s => s.trim()) + .filter(s => s.length > 0) + .map(s => new RegExp(s)); + return new MaybeInputs( new Inputs({ hasAdditionalVerbsInput, @@ -204,6 +235,8 @@ export function parseInputs(rawInputs: RawInputs): MaybeInputs { enforceSignOff, validatePullRequestCommits, skipBodyCheck, + ignoreMergeCommits, + ignorePatterns, }), null, ); diff --git a/src/inspection.ts b/src/inspection.ts index 09e56c2..06bc63c 100644 --- a/src/inspection.ts +++ b/src/inspection.ts @@ -293,16 +293,30 @@ function checkSignedOff(bodyLines: string[]): string[] { return errors; } -const mergeMessageRe = new RegExp( - "^Merge branch '[^\\000-\\037\\177 ~^:?*[]+' " + - 'into [^\\000-\\037\\177 ~^:?*[]+$', -); +const mergeMessagePatterns: RegExp[] = [ + // Local merges to default branch + /^Merge branch (?:'\S+'|"\S+"|\S+)/, + // Local merges to alternate branch + /^Merge (:?remote-tracking )?branch (?:'\S+'|"\S+"|\S+) into \S+/, + // GitHub Web UI merge pull request + /^Merge pull request #\d+ from \S+/, +]; export function check(message: string, inputs: input.Inputs): string[] { const errors: string[] = []; - if (mergeMessageRe.test(message)) { - return errors; + if (inputs.ignoreMergeCommits) { + for (const pattern of mergeMessagePatterns) { + if (pattern.test(message)) { + return errors; + } + } + } + + for (const ignorePattern of inputs.ignorePatterns) { + if (ignorePattern.test(message)) { + return errors; + } } const lines = splitLines(message); diff --git a/src/mainImpl.ts b/src/mainImpl.ts index 212e2ff..2c3cc01 100644 --- a/src/mainImpl.ts +++ b/src/mainImpl.ts @@ -44,6 +44,14 @@ async function runWithExceptions(): Promise { required: false, }); + const ignoreMergeCommitsInput = core.getInput('ignore-merge-commits', { + required: false, + }); + + const ignorePatternsInput = core.getInput('ignore-patterns', { + required: false, + }); + const maybeInputs = input.parseInputs({ additionalVerbsInput, pathToAdditionalVerbsInput, @@ -53,6 +61,8 @@ async function runWithExceptions(): Promise { enforceSignOffInput, validatePullRequestCommitsInput, skipBodyCheckInput, + ignoreMergeCommitsInput, + ignorePatternsInput, }); if (maybeInputs.error !== null) {