diff --git a/.circleci/config.yml b/.circleci/config.yml index d628e5dce4..3c45a72654 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -278,9 +278,10 @@ jobs: - run: 'yarn bazel:lint || (echo -e "\n.bzl files have lint errors. Please run ''yarn bazel:lint-fix''"; exit 1)' - - run: yarn lint --branch $CI_GIT_BASE_REVISION - - run: yarn ts-circular-deps:check + - run: yarn -s lint --branch $CI_GIT_BASE_REVISION + - run: yarn -s ts-circular-deps:check - run: yarn -s ng-dev pullapprove verify + - run: yarn -s ng-dev commit-message validate-range --range $CI_COMMIT_RANGE test: executor: diff --git a/.pullapprove.yml b/.pullapprove.yml index 2d181f59c6..6d79b06082 100644 --- a/.pullapprove.yml +++ b/.pullapprove.yml @@ -981,7 +981,6 @@ groups: 'tools/ts-api-guardian/**', 'tools/tslint/**', 'tools/utils/**', - 'tools/validate-commit-message/**', 'tools/yarn/**', 'tools/*', '**/*.bzl', diff --git a/dev-infra/commit-message/BUILD.bazel b/dev-infra/commit-message/BUILD.bazel index fe11b49270..1197df1dcc 100644 --- a/dev-infra/commit-message/BUILD.bazel +++ b/dev-infra/commit-message/BUILD.bazel @@ -8,13 +8,16 @@ ts_library( "config.ts", "validate.ts", "validate-file.ts", + "validate-range.ts", ], module_name = "@angular/dev-infra-private/commit-message", visibility = ["//dev-infra:__subpackages__"], deps = [ "//dev-infra/utils:config", "@npm//@types/node", + "@npm//@types/shelljs", "@npm//@types/yargs", + "@npm//shelljs", "@npm//tslib", "@npm//yargs", ], diff --git a/dev-infra/commit-message/cli.ts b/dev-infra/commit-message/cli.ts index 83bd1b8a66..aada6a3178 100644 --- a/dev-infra/commit-message/cli.ts +++ b/dev-infra/commit-message/cli.ts @@ -7,13 +7,38 @@ */ import * as yargs from 'yargs'; import {validateFile} from './validate-file'; +import {validateCommitRange} from './validate-range'; /** Build the parser for the commit-message commands. */ export function buildCommitMessageParser(localYargs: yargs.Argv) { - return localYargs.help().strict().command( - 'pre-commit-validate', 'Validate the most recent commit message', {}, () => { - validateFile('.git/COMMIT_EDITMSG'); - }); + return localYargs.help() + .strict() + .command( + 'pre-commit-validate', 'Validate the most recent commit message', {}, + () => { + validateFile('.git/COMMIT_EDITMSG'); + }) + .command( + 'validate-range', 'Validate a range of commit messages', { + 'range': { + description: 'The range of commits to check, e.g. --range abc123..xyz456', + demandOption: ' A range must be provided, e.g. --range abc123..xyz456', + type: 'string', + requiresArg: true, + }, + }, + argv => { + // If on CI, and not pull request number is provided, assume the branch + // being run on is an upstream branch. + if (process.env['CI'] && process.env['CI_PULL_REQUEST'] === 'false') { + console.info( + `Since valid commit messages are enforced by PR linting on CI, we do not\n` + + `need to validate commit messages on CI runs on upstream branches.\n\n` + + `Skipping check of provided commit range`); + return; + } + validateCommitRange(argv.range); + }); } if (require.main == module) { diff --git a/dev-infra/commit-message/config.ts b/dev-infra/commit-message/config.ts index 8b01bf3053..4085e809b1 100644 --- a/dev-infra/commit-message/config.ts +++ b/dev-infra/commit-message/config.ts @@ -10,4 +10,4 @@ export interface CommitMessageConfig { minBodyLength: number; types: string[]; scopes: string[]; -} \ No newline at end of file +} diff --git a/dev-infra/commit-message/validate-file.ts b/dev-infra/commit-message/validate-file.ts index 1bac796df1..b5c02555a9 100644 --- a/dev-infra/commit-message/validate-file.ts +++ b/dev-infra/commit-message/validate-file.ts @@ -17,5 +17,8 @@ export function validateFile(filePath: string) { const commitMessage = readFileSync(join(getRepoBaseDir(), filePath), 'utf8'); if (validateCommitMessage(commitMessage)) { console.info('√ Valid commit message'); + return; } + // If the validation did not return true, exit as a failure. + process.exit(1); } diff --git a/dev-infra/commit-message/validate-range.ts b/dev-infra/commit-message/validate-range.ts new file mode 100644 index 0000000000..4da1f681e2 --- /dev/null +++ b/dev-infra/commit-message/validate-range.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {exec} from 'shelljs'; +import {parseCommitMessage, validateCommitMessage, ValidateCommitMessageOptions} from './validate'; + +// Whether the provided commit is a fixup commit. +const isNonFixup = (m: string) => !parseCommitMessage(m).isFixup; + +/** Validate all commits in a provided git commit range. */ +export function validateCommitRange(range: string) { + // A random value is used as a string to allow for a definite split point in the git log result. + const randomValueSeparator = `${Math.random()}`; + // Custom git log format that provides the commit header and body, separated as expected with + // the custom separator as the trailing value. + const gitLogFormat = `%s%n%n%b${randomValueSeparator}`; + + // Retrieve the commits in the provided range. + const result = exec(`git log --reverse --format=${gitLogFormat} ${range}`, {silent: true}); + if (result.code) { + throw new Error(`Failed to get all commits in the range: \n ${result.stderr}`); + } + + // Separate the commits from a single string into individual commits + const commits = result.split(randomValueSeparator).map(l => l.trim()).filter(line => !!line); + + console.info(`Examining ${commits.length} commit(s) in the provided range: ${range}`); + + // Check each commit in the commit range. Commits are allowed to be fixup commits for other + // commits in the provided commit range. + const allCommitsInRangeValid = commits.every((m, i) => { + const options: ValidateCommitMessageOptions = { + disallowSquash: true, + nonFixupCommitHeaders: isNonFixup(m) ? undefined : commits.slice(0, i).filter(isNonFixup) + }; + return validateCommitMessage(m, options); + }); + if (allCommitsInRangeValid) { + console.info('√ All commit messages in range valid.'); + } +} diff --git a/dev-infra/commit-message/validate.spec.ts b/dev-infra/commit-message/validate.spec.ts index f9f3166f06..3029613e11 100644 --- a/dev-infra/commit-message/validate.spec.ts +++ b/dev-infra/commit-message/validate.spec.ts @@ -50,7 +50,6 @@ describe('validate-commit-message.js', () => { }); describe('validateMessage()', () => { - it('should be valid', () => { expect(validateCommitMessage('feat(packaging): something')).toBe(VALID); expect(lastError).toBe(''); @@ -66,7 +65,6 @@ describe('validate-commit-message.js', () => { expect(validateCommitMessage('Revert: "release(packaging): something"')).toBe(VALID); expect(lastError).toBe(''); - }); it('should validate max length', () => { @@ -74,8 +72,8 @@ describe('validate-commit-message.js', () => { 'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer...'; expect(validateCommitMessage(msg)).toBe(INVALID); - expect(lastError).toContain( - `The commit message header is longer than ${config.commitMessage.maxLineLength} characters`); + expect(lastError).toContain(`The commit message header is longer than ${ + config.commitMessage.maxLineLength} characters`); }); it('should validate "(): " format', () => { @@ -130,7 +128,6 @@ describe('validate-commit-message.js', () => { }); describe('(revert)', () => { - it('should allow valid "revert: ..." syntaxes', () => { expect(validateCommitMessage('revert: anything')).toBe(VALID); expect(lastError).toBe(''); @@ -143,7 +140,6 @@ describe('validate-commit-message.js', () => { expect(validateCommitMessage('rEvErT anything')).toBe(VALID); expect(lastError).toBe(''); - }); it('should not allow "revert(scope): ..." syntax', () => { @@ -164,31 +160,29 @@ describe('validate-commit-message.js', () => { }); describe('(squash)', () => { - it('should strip the `squash! ` prefix and validate the rest', () => { const errorMessage = `The commit message header does not match the expected format.`; // Valid messages. expect(validateCommitMessage('squash! feat(core): add feature')).toBe(VALID); - expect(validateCommitMessage('squash! fix: a bug', false)).toBe(VALID); + expect(validateCommitMessage('squash! fix: a bug', {disallowSquash: false})).toBe(VALID); // Invalid messages. - expect(validateCommitMessage('squash! fix a typo', false)).toBe(INVALID); + expect(validateCommitMessage('squash! fix a typo', {disallowSquash: false})).toBe(INVALID); expect(lastError).toContain('squash! fix a typo'); expect(lastError).toContain(errorMessage); expect(validateCommitMessage('squash! squash! fix: a bug')).toBe(INVALID); expect(lastError).toContain('squash! squash! fix: a bug'); expect(lastError).toContain(errorMessage); - - }); describe('with `disallowSquash`', () => { - it('should fail', () => { - expect(validateCommitMessage('fix(core): something', true)).toBe(VALID); - expect(validateCommitMessage('squash! fix(core): something', true)).toBe(INVALID); + expect(validateCommitMessage('fix(core): something', {disallowSquash: true})).toBe(VALID); + expect(validateCommitMessage('squash! fix(core): something', { + disallowSquash: true + })).toBe(INVALID); expect(lastError).toContain( 'The commit must be manually squashed into the target commit'); }); @@ -196,9 +190,7 @@ describe('validate-commit-message.js', () => { }); describe('(fixup)', () => { - describe('without `nonFixupCommitHeaders`', () => { - it('should strip the `fixup! ` prefix and validate the rest', () => { const errorMessage = `The commit message header does not match the expected format.`; @@ -214,21 +206,26 @@ describe('validate-commit-message.js', () => { expect(validateCommitMessage('fixup! fixup! fix: a bug')).toBe(INVALID); expect(lastError).toContain('fixup! fixup! fix: a bug'); expect(lastError).toContain(errorMessage); - - }); }); describe('with `nonFixupCommitHeaders`', () => { - it('should check that the fixup commit matches a non-fixup one', () => { const msg = 'fixup! foo'; - expect(validateCommitMessage(msg, false, ['foo', 'bar', 'baz'])).toBe(VALID); - expect(validateCommitMessage(msg, false, ['bar', 'baz', 'foo'])).toBe(VALID); - expect(validateCommitMessage(msg, false, ['baz', 'foo', 'bar'])).toBe(VALID); + expect(validateCommitMessage( + msg, {disallowSquash: false, nonFixupCommitHeaders: ['foo', 'bar', 'baz']})) + .toBe(VALID); + expect(validateCommitMessage( + msg, {disallowSquash: false, nonFixupCommitHeaders: ['bar', 'baz', 'foo']})) + .toBe(VALID); + expect(validateCommitMessage( + msg, {disallowSquash: false, nonFixupCommitHeaders: ['baz', 'foo', 'bar']})) + .toBe(VALID); - expect(validateCommitMessage(msg, false, ['qux', 'quux', 'quuux'])).toBe(INVALID); + expect(validateCommitMessage( + msg, {disallowSquash: false, nonFixupCommitHeaders: ['qux', 'quux', 'quuux']})) + .toBe(INVALID); expect(lastError).toContain( 'Unable to find match for fixup commit among prior commits: \n' + ' qux\n' + @@ -237,8 +234,13 @@ describe('validate-commit-message.js', () => { }); it('should fail if `nonFixupCommitHeaders` is empty', () => { - expect(validateCommitMessage('refactor(core): make reactive', false, [])).toBe(VALID); - expect(validateCommitMessage('fixup! foo', false, [])).toBe(INVALID); + expect(validateCommitMessage('refactor(core): make reactive', { + disallowSquash: false, + nonFixupCommitHeaders: [] + })).toBe(VALID); + expect(validateCommitMessage( + 'fixup! foo', {disallowSquash: false, nonFixupCommitHeaders: []})) + .toBe(INVALID); expect(lastError).toContain( `Unable to find match for fixup commit among prior commits: -`); }); diff --git a/dev-infra/commit-message/validate.ts b/dev-infra/commit-message/validate.ts index 1213cd416c..7088b9081c 100644 --- a/dev-infra/commit-message/validate.ts +++ b/dev-infra/commit-message/validate.ts @@ -8,6 +8,12 @@ import {getAngularDevConfig} from '../utils/config'; import {CommitMessageConfig} from './config'; +/** Options for commit message validation. */ +export interface ValidateCommitMessageOptions { + disallowSquash?: boolean; + nonFixupCommitHeaders?: string[]; +} + const FIXUP_PREFIX_RE = /^fixup! /i; const GITHUB_LINKING_RE = /((closed?s?)|(fix(es)?(ed)?)|(resolved?s?))\s\#(\d+)/ig; const SQUASH_PREFIX_RE = /^squash! /i; @@ -26,17 +32,17 @@ export function parseCommitMessage(commitMsg: string) { let subject = ''; if (COMMIT_HEADER_RE.test(commitMsg)) { - header = COMMIT_HEADER_RE.exec(commitMsg) ![1] + header = COMMIT_HEADER_RE.exec(commitMsg)![1] .replace(FIXUP_PREFIX_RE, '') .replace(SQUASH_PREFIX_RE, ''); } if (COMMIT_BODY_RE.test(commitMsg)) { - body = COMMIT_BODY_RE.exec(commitMsg) ![1]; + body = COMMIT_BODY_RE.exec(commitMsg)![1]; bodyWithoutLinking = body.replace(GITHUB_LINKING_RE, ''); } if (TYPE_SCOPE_RE.test(header)) { - const parsedCommitHeader = TYPE_SCOPE_RE.exec(header) !; + const parsedCommitHeader = TYPE_SCOPE_RE.exec(header)!; type = parsedCommitHeader[1]; scope = parsedCommitHeader[2]; subject = parsedCommitHeader[3]; @@ -54,10 +60,9 @@ export function parseCommitMessage(commitMsg: string) { }; } - /** Validate a commit message against using the local repo's config. */ export function validateCommitMessage( - commitMsg: string, disallowSquash: boolean = false, nonFixupCommitHeaders?: string[]) { + commitMsg: string, options: ValidateCommitMessageOptions = {}) { function error(errorMessage: string) { console.error( `INVALID COMMIT MSG: \n` + @@ -78,7 +83,7 @@ export function validateCommitMessage( return true; } - if (commit.isSquash && disallowSquash) { + if (commit.isSquash && options.disallowSquash) { error('The commit must be manually squashed into the target commit'); return false; } @@ -86,11 +91,11 @@ export function validateCommitMessage( // If it is a fixup commit and `nonFixupCommitHeaders` is not empty, we only care to check whether // there is a corresponding non-fixup commit (i.e. a commit whose header is identical to this // commit's header after stripping the `fixup! ` prefix). - if (commit.isFixup && nonFixupCommitHeaders) { - if (!nonFixupCommitHeaders.includes(commit.header)) { + if (commit.isFixup && options.nonFixupCommitHeaders) { + if (!options.nonFixupCommitHeaders.includes(commit.header)) { error( 'Unable to find match for fixup commit among prior commits: ' + - (nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-')); + (options.nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-')); return false; } @@ -118,8 +123,8 @@ export function validateCommitMessage( } if (commit.bodyWithoutLinking.trim().length < config.minBodyLength) { - error( - `The commit message body does not meet the minimum length of ${config.minBodyLength} characters`); + error(`The commit message body does not meet the minimum length of ${ + config.minBodyLength} characters`); return false; } diff --git a/gulpfile.js b/gulpfile.js index 35852b3103..eed70c10b7 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -49,8 +49,7 @@ gulp.task('format:changed:enforce', ['format:untracked:enforce', 'format:diff:en // Alias for `format:changed` that formerly formatted all files. gulp.task('format', ['format:changed']); -gulp.task('lint', ['format:changed:enforce', 'validate-commit-messages']); -gulp.task('validate-commit-messages', loadTask('validate-commit-message')); +gulp.task('lint', ['format:changed:enforce']); gulp.task('source-map-test', loadTask('source-map-test')); gulp.task('changelog', loadTask('changelog')); gulp.task('changelog:zonejs', loadTask('changelog-zonejs')); diff --git a/package.json b/package.json index 0d77e7f75f..5b6197158c 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "preinstall": "node tools/yarn/check-yarn.js", "postinstall": "node scripts/webdriver-manager-update.js && node --preserve-symlinks --preserve-symlinks-main ./tools/postinstall-patches.js", "check-env": "gulp check-env", - "commitmsg": "node ./scripts/git/commit-msg.js", + "commitmsg": "yarn -s ng-dev commit-message pre-commit-validate", "test-ivy-aot": "bazelisk test --config=ivy --build_tag_filters=-no-ivy-aot,-fixme-ivy-aot --test_tag_filters=-no-ivy-aot,-fixme-ivy-aot", "test-non-ivy": "bazelisk test --build_tag_filters=-ivy-only --test_tag_filters=-ivy-only", "test-fixme-ivy-aot": "bazelisk test --config=ivy --build_tag_filters=-no-ivy-aot --test_tag_filters=-no-ivy-aot", diff --git a/scripts/git/commit-msg.js b/scripts/git/commit-msg.js deleted file mode 100755 index 7b80d058f9..0000000000 --- a/scripts/git/commit-msg.js +++ /dev/null @@ -1,34 +0,0 @@ -#! /usr/bin/env node -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -// git commit-msg hook to check the commit message against Angular conventions -// see `/CONTRIBUTING.md` for mode details. - -'use strict'; - -const fs = require('fs'); -const checkMsg = require('../../tools/validate-commit-message'); -const msgFile = process.env['GIT_PARAMS']; - -let isValid = true; - -if (msgFile) { - const commitMsg = fs.readFileSync(msgFile, {encoding: 'utf-8'}); - const firstLine = commitMsg.split('\n')[0]; - isValid = checkMsg(firstLine); - - if (!isValid) { - console.error( - '\nCheck CONTRIBUTING.md at the root of the repo for more information.' + - '\n' + - '\n(In case you need the invalid commit message, it should be stored in \'.git/COMMIT_EDITMSG\'.)'); - } -} - -process.exit(isValid ? 0 : 1); diff --git a/tools/gulp-tasks/validate-commit-message.js b/tools/gulp-tasks/validate-commit-message.js deleted file mode 100644 index 5761856c0c..0000000000 --- a/tools/gulp-tasks/validate-commit-message.js +++ /dev/null @@ -1,92 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - - -// tslint:disable:no-console -module.exports = (gulp) => async() => { - try { - const validateCommitMessage = require('../validate-commit-message'); - const shelljs = require('shelljs'); - const getRefsAndShasForTarget = require('../utils/get-refs-and-shas-for-target'); - - // Break on error. - shelljs.set('-e'); - - - let target = {}; - if (process.env['CI'] === 'true') { - // Validation of commit messages on CI - if (process.env['CI_PULL_REQUEST'] === 'false') { - // Skip commit message validation on CI for non-PR branches as we are not testing new - // unreviewed commits. By enforcing correctness on the incoming changes in the PR - // branches, we are able to render this check unnecessary on non-PR branches. - console.info( - `Since valid commit messages are enforced by PR linting on CI,\n` + - `we do not need to validate commit messages on CI runs on upstream branches.\n\n` + - `Skipping validate-commit-message check`); - process.exit(); - } - target = await getRefsAndShasForTarget(process.env['CI_PULL_REQUEST']); - } else { - // Validation of commit messages locally - const baseRef = 'master'; - const headRef = shelljs.exec('git symbolic-ref --short HEAD', {silent: true}).trim(); - const baseSha = shelljs.exec(`git rev-parse origin/master`, {silent: true}).trim(); - const headSha = shelljs.exec(`git rev-parse HEAD`, {silent: true}).trim(); - const commonAncestorSha = - shelljs.exec(`git merge-base origin/master ${headSha}`, {silent: true}).trim(); - target = { - base: { - ref: baseRef, - sha: baseSha, - }, - head: { - ref: headRef, - sha: headSha, - }, - commonAncestorSha: commonAncestorSha, - latestShaOfTargetBranch: baseSha, - latestShaOfPrBranch: headSha, - }; - } - - const result = shelljs.exec( - `git log --reverse --format=%s ${target.commonAncestorSha}..${target.latestShaOfPrBranch}`, - {silent: true}); - - if (result.code) { - throw new Error(`Failed to fetch commits: ${result.stderr}`); - } - - const commitsByLine = result.trim().split(/\n/).filter(line => line != ''); - - console.log(`Examining ${commitsByLine.length} commit(s) between ${target.base.ref} and HEAD`); - - if (commitsByLine.length == 0) { - console.log(`There are zero new commits between ${target.base.ref} and HEAD`); - } - - const disallowSquashCommits = true; - const isNonFixup = m => !validateCommitMessage.FIXUP_PREFIX_RE.test(m); - const someCommitsInvalid = !commitsByLine.every((m, i) => { - // `priorNonFixupCommits` is only needed if the current commit is a fixup commit. - const priorNonFixupCommits = - isNonFixup(m) ? undefined : commitsByLine.slice(0, i).filter(isNonFixup); - return validateCommitMessage(m, disallowSquashCommits, priorNonFixupCommits); - }); - - if (someCommitsInvalid) { - throw new Error( - 'Please fix the failing commit messages before continuing...\n' + - 'Commit message guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines'); - } - } catch (err) { - console.error(err); - process.exit(1); - } -}; diff --git a/tools/validate-commit-message/BUILD.bazel b/tools/validate-commit-message/BUILD.bazel deleted file mode 100644 index 29c0c64cc0..0000000000 --- a/tools/validate-commit-message/BUILD.bazel +++ /dev/null @@ -1,9 +0,0 @@ -load("//tools:defaults.bzl", "jasmine_node_test") - -jasmine_node_test( - name = "validate-commit-message", - srcs = glob(["*.js"]), - data = [ - "commit-message.json", - ], -) diff --git a/tools/validate-commit-message/commit-message.json b/tools/validate-commit-message/commit-message.json deleted file mode 100644 index 09d8e70e0e..0000000000 --- a/tools/validate-commit-message/commit-message.json +++ /dev/null @@ -1,44 +0,0 @@ -{ - "maxLength": 120, - "types": [ - "build", - "ci", - "docs", - "feat", - "fix", - "perf", - "refactor", - "release", - "style", - "test" - ], - "scopes": [ - "animations", - "bazel", - "benchpress", - "changelog", - "common", - "compiler", - "compiler-cli", - "core", - "dev-infra", - "docs-infra", - "elements", - "forms", - "http", - "language-service", - "localize", - "ngcc", - "packaging", - "platform-browser", - "platform-browser-dynamic", - "platform-server", - "platform-webworker", - "platform-webworker-dynamic", - "router", - "service-worker", - "upgrade", - "ve", - "zone.js" - ] -} diff --git a/tools/validate-commit-message/index.js b/tools/validate-commit-message/index.js deleted file mode 100644 index 6e45729380..0000000000 --- a/tools/validate-commit-message/index.js +++ /dev/null @@ -1,9 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -module.exports = require('./validate-commit-message'); \ No newline at end of file diff --git a/tools/validate-commit-message/validate-commit-message.js b/tools/validate-commit-message/validate-commit-message.js deleted file mode 100644 index e264f0f513..0000000000 --- a/tools/validate-commit-message/validate-commit-message.js +++ /dev/null @@ -1,100 +0,0 @@ -#!/usr/bin/env node - -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -/** - * GIT commit message format enforcement - * - * Note: this script was originally written by Vojta for AngularJS :-) - */ - -'use strict'; - -const config = require('./commit-message.json'); -const FIXUP_PREFIX_RE = /^fixup! /i; -const SQUASH_PREFIX_RE = /^squash! /i; -const REVERT_PREFIX_RE = /^revert:? /i; - -module.exports = (commitHeader, disallowSquash, nonFixupCommitHeaders) => { - if (REVERT_PREFIX_RE.test(commitHeader)) { - return true; - } - - const {header, type, scope, isFixup, isSquash} = parseCommitHeader(commitHeader); - - if (isSquash && disallowSquash) { - error('The commit must be manually squashed into the target commit', commitHeader); - return false; - } - - // If it is a fixup commit and `nonFixupCommitHeaders` is not empty, we only care to check whether - // there is a corresponding non-fixup commit (i.e. a commit whose header is identical to this - // commit's header after stripping the `fixup! ` prefix). - if (isFixup && nonFixupCommitHeaders) { - if (!nonFixupCommitHeaders.includes(header)) { - error( - 'Unable to find match for fixup commit among prior commits: ' + - (nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-'), - commitHeader); - return false; - } - - return true; - } - - if (header.length > config.maxLength) { - error(`The commit message header is longer than ${config.maxLength} characters`, commitHeader); - return false; - } - - if (!type) { - const format = '(): '; - error( - `The commit message header does not match the format of '${format}' or 'Revert: "${format}"'`, - commitHeader); - return false; - } - - if (!config.types.includes(type)) { - error(`'${type}' is not an allowed type.\n => TYPES: ${config.types.join(', ')}`, commitHeader); - return false; - } - - if (scope && !config.scopes.includes(scope)) { - error( - `'${scope}' is not an allowed scope.\n => SCOPES: ${config.scopes.join(', ')}`, - commitHeader); - return false; - } - - return true; -}; - -module.exports.FIXUP_PREFIX_RE = FIXUP_PREFIX_RE; -module.exports.config = config; - -// Helpers -function error(errorMessage, commitHeader) { - console.error(`INVALID COMMIT MSG: ${commitHeader}\n => ERROR: ${errorMessage}`); -} - -function parseCommitHeader(header) { - const isFixup = FIXUP_PREFIX_RE.test(header); - const isSquash = SQUASH_PREFIX_RE.test(header); - header = header.replace(FIXUP_PREFIX_RE, '').replace(SQUASH_PREFIX_RE, ''); - - const match = /^(\w+)(?:\(([^)]+)\))?\: (.+)$/.exec(header) || []; - - return { - header, - type: match[1], - scope: match[2], - subject: match[3], isFixup, isSquash, - }; -} diff --git a/tools/validate-commit-message/validate-commit-message.spec.js b/tools/validate-commit-message/validate-commit-message.spec.js deleted file mode 100644 index 181eaa7782..0000000000 --- a/tools/validate-commit-message/validate-commit-message.spec.js +++ /dev/null @@ -1,234 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -// Imports -const validateMessage = require('./validate-commit-message'); - -// Constants -const TYPES = validateMessage.config.types.join(', '); -const SCOPES = validateMessage.config.scopes.join(', '); - -const INVALID = false; -const VALID = true; - - -describe('validate-commit-message.js', () => { - let errors = []; - let logs = []; - - // Helpers - const stripColor = msg => msg.replace(/\x1B\[\d+m/g, ''); - - beforeEach(() => { - errors = []; - logs = []; - - spyOn(console, 'error').and.callFake(msg => errors.push(stripColor(msg))); - spyOn(console, 'log').and.callFake(msg => logs.push(stripColor(msg))); - }); - - describe('validateMessage()', () => { - - it('should be valid', () => { - expect(validateMessage('fix(core): something')).toBe(VALID); - expect(validateMessage('feat(common): something')).toBe(VALID); - expect(validateMessage('docs(compiler): something')).toBe(VALID); - expect(validateMessage('style(http): something')).toBe(VALID); - expect(validateMessage('refactor(platform-webworker): something')).toBe(VALID); - expect(validateMessage('test(language-service): something')).toBe(VALID); - expect(validateMessage('test(packaging): something')).toBe(VALID); - expect(validateMessage('release: something')).toBe(VALID); - expect(validateMessage('release(packaging): something')).toBe(VALID); - expect(validateMessage('release(packaging): something')).toBe(VALID); - expect(validateMessage('fixup! release(packaging): something')).toBe(VALID); - expect(validateMessage('squash! release(packaging): something')).toBe(VALID); - expect(validateMessage('Revert: "release(packaging): something"')).toBe(VALID); - expect(validateMessage('Revert "release(packaging): something"')).toBe(VALID); - expect(errors).toEqual([]); - }); - - it('should validate max length', () => { - var msg = - 'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer...'; - - expect(validateMessage(msg)).toBe(INVALID); - expect(errors).toEqual([ - `INVALID COMMIT MSG: ${msg}\n => ERROR: The commit message header is longer than 120 characters` - ]); - }); - - it('should validate "(): " format', () => { - const msg = 'not correct format'; - - expect(validateMessage(msg)).toBe(INVALID); - expect(errors).toEqual([ - `INVALID COMMIT MSG: ${msg}\n => ERROR: The commit message header does not match the format of '(): ' or 'Revert: "(): "'`, - ]); - }); - - it('should fail when type is invalid', () => { - const msg = 'weird(common): something'; - - expect(validateMessage(msg)).toBe(INVALID); - expect(errors).toEqual([ - `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}`; - - expect(validateMessage('fix(Compiler): something')).toBe(INVALID); - expect(validateMessage('feat(bah): something')).toBe(INVALID); - expect(validateMessage('style(webworker): something')).toBe(INVALID); - expect(validateMessage('refactor(security): something')).toBe(INVALID); - expect(validateMessage('refactor(docs): something')).toBe(INVALID); - expect(validateMessage('release(angular): something')).toBe(INVALID); - expect(errors).toEqual([ - errorMessageFor('Compiler', 'fix(Compiler): something'), - errorMessageFor('bah', 'feat(bah): something'), - errorMessageFor('webworker', 'style(webworker): something'), - errorMessageFor('security', 'refactor(security): something'), - errorMessageFor('docs', 'refactor(docs): something'), - errorMessageFor('angular', 'release(angular): something'), - ]); - }); - - it('should allow empty scope', () => { - expect(validateMessage('fix: blablabla')).toBe(VALID); - expect(errors).toEqual([]); - }); - - // We do not want to allow WIP. It is OK to fail the PR build in this case to show that there is - // work still to be done (i.e. fixing the commit message). - it('should not allow "WIP: ..." syntax', () => { - const msg = 'WIP: fix: something'; - - expect(validateMessage(msg)).toBe(INVALID); - expect(errors).toEqual([ - `INVALID COMMIT MSG: ${msg}\n => ERROR: 'WIP' is not an allowed type.\n => TYPES: ${TYPES}`, - ]); - }); - - describe('(revert)', () => { - - it('should allow valid "revert: ..." syntaxes', () => { - expect(validateMessage('revert: anything')).toBe(VALID); - expect(validateMessage('Revert: "anything"')).toBe(VALID); - expect(validateMessage('revert anything')).toBe(VALID); - expect(validateMessage('rEvErT anything')).toBe(VALID); - expect(errors).toEqual([]); - }); - - it('should not allow "revert(scope): ..." syntax', () => { - const msg = 'revert(compiler): reduce generated code payload size by 65%'; - - expect(validateMessage(msg)).toBe(INVALID); - expect(errors).toEqual([ - `INVALID COMMIT MSG: ${msg}\n => ERROR: 'revert' is not an allowed type.\n => TYPES: ${TYPES}`, - ]); - }); - - // https://github.com/angular/angular/issues/23479 - it('should allow typical Angular messages generated by git', () => { - const msg = - 'Revert "fix(compiler): Pretty print object instead of [Object object] (#22689)" (#23442)'; - - expect(validateMessage(msg)).toBe(VALID); - expect(errors).toEqual([]); - }); - }); - - describe('(squash)', () => { - - it('should strip the `squash! ` prefix and validate the rest', () => { - const errorMessageFor = header => - `INVALID COMMIT MSG: ${header}\n => ERROR: The commit message header does not match the format of ` + - '\'(): \' or \'Revert: "(): "\''; - - // Valid messages. - expect(validateMessage('squash! feat(core): add feature')).toBe(VALID); - expect(validateMessage('squash! fix: a bug', false)).toBe(VALID); - - // Invalid messages. - expect(validateMessage('squash! fix a typo', false)).toBe(INVALID); - expect(validateMessage('squash! squash! fix: a bug')).toBe(INVALID); - expect(errors).toEqual([ - errorMessageFor('squash! fix a typo'), - errorMessageFor('squash! squash! fix: a bug'), - ]); - }); - - describe('with `disallowSquash`', () => { - - it('should fail', () => { - expect(validateMessage('fix: something', true)).toBe(VALID); - expect(validateMessage('squash! fix: something', true)).toBe(INVALID); - expect(errors).toEqual([ - 'INVALID COMMIT MSG: squash! fix: something\n' + - ' => ERROR: The commit must be manually squashed into the target commit', - ]); - }); - }); - }); - - describe('(fixup)', () => { - - describe('without `nonFixupCommitHeaders`', () => { - - it('should strip the `fixup! ` prefix and validate the rest', () => { - const errorMessageFor = header => - `INVALID COMMIT MSG: ${header}\n => ERROR: The commit message header does not match the format of ` + - '\'(): \' or \'Revert: "(): "\''; - - // Valid messages. - expect(validateMessage('fixup! feat(core): add feature')).toBe(VALID); - expect(validateMessage('fixup! fix: a bug')).toBe(VALID); - - // Invalid messages. - expect(validateMessage('fixup! fix a typo')).toBe(INVALID); - expect(validateMessage('fixup! fixup! fix: a bug')).toBe(INVALID); - expect(errors).toEqual([ - errorMessageFor('fixup! fix a typo'), - errorMessageFor('fixup! fixup! fix: a bug'), - ]); - }); - }); - - describe('with `nonFixupCommitHeaders`', () => { - - it('should check that the fixup commit matches a non-fixup one', () => { - const msg = 'fixup! foo'; - - expect(validateMessage(msg, false, ['foo', 'bar', 'baz'])).toBe(VALID); - expect(validateMessage(msg, false, ['bar', 'baz', 'foo'])).toBe(VALID); - expect(validateMessage(msg, false, ['baz', 'foo', 'bar'])).toBe(VALID); - - expect(validateMessage(msg, false, ['qux', 'quux', 'quuux'])).toBe(INVALID); - expect(errors).toEqual([ - `INVALID COMMIT MSG: ${msg}\n` + - ' => ERROR: Unable to find match for fixup commit among prior commits: \n' + - ' qux\n' + - ' quux\n' + - ' quuux', - ]); - }); - - it('should fail if `nonFixupCommitHeaders` is empty', () => { - expect(validateMessage('refactor(router): make reactive', false, [])).toBe(VALID); - expect(validateMessage('fixup! foo', false, [])).toBe(INVALID); - expect(errors).toEqual([ - 'INVALID COMMIT MSG: fixup! foo\n' + - ' => ERROR: Unable to find match for fixup commit among prior commits: -', - ]); - }); - }); - }); - }); -});