From 2b289250d8af319d4ad3d9866c716183f8ae9a4d Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 6 Aug 2019 20:17:17 +0300 Subject: [PATCH] refactor: clean up `validate-commit-message` script (#32023) This sets the ground for adding stricter rules for fixup commits in a follow-up PR. PR Close #32023 --- .../validate-commit-message.js | 65 ++++++++++--------- .../validate-commit-message.spec.js | 12 ++-- 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/tools/validate-commit-message/validate-commit-message.js b/tools/validate-commit-message/validate-commit-message.js index d83b00d076..855c989c2a 100644 --- a/tools/validate-commit-message/validate-commit-message.js +++ b/tools/validate-commit-message/validate-commit-message.js @@ -16,55 +16,62 @@ 'use strict'; -const fs = require('fs'); -const path = require('path'); -const configPath = path.resolve(__dirname, './commit-message.json'); -const config = JSON.parse(fs.readFileSync(configPath, 'utf8')); -const PATTERN = /^(\w+)(?:\(([^)]+)\))?\: (.+)$/; -const FIXUP_SQUASH = /^(fixup|squash)\! /i; -const REVERT = /^revert:? /i; +const config = require('./commit-message.json'); +const FIXUP_SQUASH_PREFIX_RE = /^(?:fixup|squash)! /i; +const REVERT_PREFIX_RE = /^revert:? /i; -module.exports = function(commitSubject) { - const subject = commitSubject.replace(FIXUP_SQUASH, ''); - - if (subject.match(REVERT)) { +module.exports = commitHeader => { + if (REVERT_PREFIX_RE.test(commitHeader)) { return true; } - if (subject.length > config['maxLength']) { - error(`The commit message is longer than ${config['maxLength']} characters`, commitSubject); + const {header, type, scope} = parseCommitHeader(commitHeader); + + if (header.length > config.maxLength) { + error(`The commit message header is longer than ${config.maxLength} characters`, commitHeader); return false; } - const match = PATTERN.exec(subject); - if (!match) { + if (!type) { + const format = '(): '; error( - `The commit message does not match the format of '(): ' OR 'Revert: "type(): "'`, - commitSubject); + `The commit message header does not match the format of '${format}' or 'Revert: "${format}"'`, + commitHeader); return false; } - const type = match[1]; - if (config['types'].indexOf(type) === -1) { - error( - `${type} is not an allowed type.\n => TYPES: ${config['types'].join(', ')}`, commitSubject); + if (!config.types.includes(type)) { + error(`'${type}' is not an allowed type.\n => TYPES: ${config.types.join(', ')}`, commitHeader); return false; } - const scope = match[2]; - - if (scope && !config['scopes'].includes(scope)) { + if (scope && !config.scopes.includes(scope)) { error( - `"${scope}" is not an allowed scope.\n => SCOPES: ${config['scopes'].join(', ')}`, - commitSubject); + `'${scope}' is not an allowed scope.\n => SCOPES: ${config.scopes.join(', ')}`, + commitHeader); return false; } return true; }; -function error(errorMessage, commitMessage) { - console.error(`INVALID COMMIT MSG: "${commitMessage}"\n => ERROR: ${errorMessage}`); +module.exports.config = config; + +// Helpers +function error(errorMessage, commitHeader) { + console.error(`INVALID COMMIT MSG: ${commitHeader}\n => ERROR: ${errorMessage}`); } -module.exports.config = config; +function parseCommitHeader(header) { + const isFixupOrSquash = FIXUP_SQUASH_PREFIX_RE.test(header); + header = header.replace(FIXUP_SQUASH_PREFIX_RE, ''); + + const match = /^(\w+)(?:\(([^)]+)\))?\: (.+)$/.exec(header) || []; + + return { + header, + type: match[1], + scope: match[2], + subject: match[3], isFixupOrSquash, + }; +} diff --git a/tools/validate-commit-message/validate-commit-message.spec.js b/tools/validate-commit-message/validate-commit-message.spec.js index 2cc56002f3..47a14dd1aa 100644 --- a/tools/validate-commit-message/validate-commit-message.spec.js +++ b/tools/validate-commit-message/validate-commit-message.spec.js @@ -58,7 +58,7 @@ describe('validate-commit-message.js', () => { expect(validateMessage(msg)).toBe(INVALID); expect(errors).toEqual([ - `INVALID COMMIT MSG: "${msg}"\n => ERROR: The commit message is longer than 120 characters` + `INVALID COMMIT MSG: ${msg}\n => ERROR: The commit message header is longer than 120 characters` ]); }); @@ -67,7 +67,7 @@ describe('validate-commit-message.js', () => { expect(validateMessage(msg)).toBe(INVALID); expect(errors).toEqual([ - `INVALID COMMIT MSG: "${msg}"\n => ERROR: The commit message does not match the format of '(): ' OR 'Revert: "type(): "'`, + `INVALID COMMIT MSG: ${msg}\n => ERROR: The commit message header does not match the format of '(): ' or 'Revert: "(): "'`, ]); }); @@ -76,13 +76,13 @@ describe('validate-commit-message.js', () => { expect(validateMessage(msg)).toBe(INVALID); expect(errors).toEqual([ - `INVALID COMMIT MSG: "${msg}"\n => ERROR: weird is not an allowed type.\n => TYPES: ${TYPES}`, + `INVALID COMMIT MSG: ${msg}\n => ERROR: 'weird' is not an allowed type.\n => TYPES: ${TYPES}`, ]); }); it('should fail when scope is invalid', () => { const errorMessageFor = (scope, header) => - `INVALID COMMIT MSG: "${header}"\n => ERROR: "${scope}" is not an allowed scope.\n => SCOPES: ${SCOPES}`; + `INVALID COMMIT MSG: ${header}\n => ERROR: '${scope}' is not an allowed scope.\n => SCOPES: ${SCOPES}`; expect(validateMessage('fix(Compiler): something')).toBe(INVALID); expect(validateMessage('feat(bah): something')).toBe(INVALID); @@ -112,7 +112,7 @@ describe('validate-commit-message.js', () => { expect(validateMessage(msg)).toBe(INVALID); expect(errors).toEqual([ - `INVALID COMMIT MSG: "${msg}"\n => ERROR: WIP is not an allowed type.\n => TYPES: ${TYPES}`, + `INVALID COMMIT MSG: ${msg}\n => ERROR: 'WIP' is not an allowed type.\n => TYPES: ${TYPES}`, ]); }); @@ -131,7 +131,7 @@ describe('validate-commit-message.js', () => { expect(validateMessage(msg)).toBe(INVALID); expect(errors).toEqual([ - `INVALID COMMIT MSG: "${msg}"\n => ERROR: revert is not an allowed type.\n => TYPES: ${TYPES}`, + `INVALID COMMIT MSG: ${msg}\n => ERROR: 'revert' is not an allowed type.\n => TYPES: ${TYPES}`, ]); });