From f40d51733ab7f598aa419006322051106534f407 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Fri, 20 Mar 2020 12:24:12 -0700 Subject: [PATCH] fix(dev-infra): use commit message validation from @angular/dev-infra-private (#36172) Prior to this change we manage a local version of commit message validation in addition to the commit message validation tool contained in the ng-dev tooling. By adding the ability to validate a range of commit messages together, the remaining piece of commit message validation that is in the local version is replicated. We use both commands provided by the `ng-dev commit-message` tooling: - pre-commit-validate: Set to automatically run on an git hook to validate commits as they are created locally. - validate-range: Run by CI for every PR, testing that all of the commits added by the PR are valid when considered together. Ensuring that all fixups are matched to another commit in the change. PR Close #36172 --- .circleci/config.yml | 5 +- .pullapprove.yml | 1 - dev-infra/commit-message/BUILD.bazel | 3 + dev-infra/commit-message/cli.ts | 33 ++- dev-infra/commit-message/config.ts | 2 +- dev-infra/commit-message/validate-file.ts | 3 + dev-infra/commit-message/validate-range.ts | 45 ++++ dev-infra/commit-message/validate.spec.ts | 52 ++-- dev-infra/commit-message/validate.ts | 27 +- gulpfile.js | 3 +- package.json | 2 +- scripts/git/commit-msg.js | 34 --- tools/gulp-tasks/validate-commit-message.js | 92 ------- tools/validate-commit-message/BUILD.bazel | 9 - .../commit-message.json | 44 ---- tools/validate-commit-message/index.js | 9 - .../validate-commit-message.js | 100 -------- .../validate-commit-message.spec.js | 234 ------------------ 18 files changed, 129 insertions(+), 569 deletions(-) create mode 100644 dev-infra/commit-message/validate-range.ts delete mode 100755 scripts/git/commit-msg.js delete mode 100644 tools/gulp-tasks/validate-commit-message.js delete mode 100644 tools/validate-commit-message/BUILD.bazel delete mode 100644 tools/validate-commit-message/commit-message.json delete mode 100644 tools/validate-commit-message/index.js delete mode 100644 tools/validate-commit-message/validate-commit-message.js delete mode 100644 tools/validate-commit-message/validate-commit-message.spec.js 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: -', - ]); - }); - }); - }); - }); -});