Add support for ignored patterns (#124)

There was already logic to ignore merge commits, but it was using a very restricted regular expression. This PR: - Improve the existing merge commit detection. - Allow users to disable merge commit detection. - Allow users to provide custom patterns to ignore Related issues: - Closes #110 Signed-off-by: Luiz Ferraz <[email protected]>
mristin · Dec 26, 2023 · 252b102 · 252b102
1 parent e4d81e5
commit 252b102
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 14 deletions.
diff --git 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/dist/index.js b/dist/index.js
@@ -32686,6 +32686,8 @@ class Inputs {
         this.enforceSignOff = values.enforceSignOff;
         this.validatePullRequestCommits = values.validatePullRequestCommits;
         this.skipBodyCheck = values.skipBodyCheck;
+        this.ignoreMergeCommits = values.ignoreMergeCommits;
+        this.ignorePatterns = values.ignorePatterns;
     }
 }
 exports.Inputs = Inputs;
@@ -32710,7 +32712,7 @@ class MaybeInputs {
 }
 exports.MaybeInputs = MaybeInputs;
 function parseInputs(rawInputs) {
-    const { additionalVerbsInput = '', pathToAdditionalVerbsInput = '', allowOneLinersInput = '', maxSubjectLengthInput = '', maxBodyLineLengthInput = '', enforceSignOffInput = '', validatePullRequestCommitsInput = '', skipBodyCheckInput = '', } = rawInputs;
+    const { additionalVerbsInput = '', pathToAdditionalVerbsInput = '', allowOneLinersInput = '', maxSubjectLengthInput = '', maxBodyLineLengthInput = '', enforceSignOffInput = '', validatePullRequestCommitsInput = '', skipBodyCheckInput = '', ignoreMergeCommitsInput = '', ignorePatternsInput = '', } = rawInputs;
     const additionalVerbs = new Set();
     const hasAdditionalVerbsInput = additionalVerbsInput.length > 0;
     if (additionalVerbsInput) {
@@ -32770,6 +32772,20 @@ function parseInputs(rawInputs) {
         return new MaybeInputs(null, 'Unexpected value for skip-body-check. ' +
             `Expected either 'true' or 'false', got: ${skipBodyCheckInput}`);
     }
+    const ignoreMergeCommits = !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 = 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,
         pathToAdditionalVerbs: pathToAdditionalVerbsInput,
@@ -32780,6 +32796,8 @@ function parseInputs(rawInputs) {
         enforceSignOff,
         validatePullRequestCommits,
         skipBodyCheck,
+        ignoreMergeCommits,
+        ignorePatterns,
     }), null);
 }
 exports.parseInputs = parseInputs;
@@ -33042,12 +33060,27 @@ function checkSignedOff(bodyLines) {
     }
     return errors;
 }
-const mergeMessageRe = new RegExp("^Merge branch '[^\\000-\\037\\177 ~^:?*[]+' " +
-    'into [^\\000-\\037\\177 ~^:?*[]+$');
+const mergeMessagePatterns = [
+    // 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+/,
+];
 function check(message, inputs) {
     const errors = [];
-    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);
     if (lines.length === 0) {
@@ -33197,6 +33230,12 @@ function runWithExceptions() {
         const skipBodyCheckInput = core.getInput('skip-body-check', {
             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,
@@ -33206,6 +33245,8 @@ function runWithExceptions() {
             enforceSignOffInput,
             validatePullRequestCommitsInput,
             skipBodyCheckInput,
+            ignoreMergeCommitsInput,
+            ignorePatternsInput,
         });
         if (maybeInputs.error !== null) {
             core.error(maybeInputs.error);
@@ -33980,6 +34021,7 @@ exports.SET = new Set([
     'override',
     'pack',
     'package',
+    'parse',
     'patch',
     'pin',
     'privatize',

diff --git 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
@@ -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
@@ -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<string>();
@@ -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
@@ -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
@@ -44,6 +44,14 @@ async function runWithExceptions(): Promise<void> {
     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<void> {
     enforceSignOffInput,
     validatePullRequestCommitsInput,
     skipBodyCheckInput,
+    ignoreMergeCommitsInput,
+    ignorePatternsInput,
   });
 
   if (maybeInputs.error !== null) {