diff --git a/.dev-infra.json b/.dev-infra.json new file mode 100644 index 0000000000..0a0997a8e9 --- /dev/null +++ b/.dev-infra.json @@ -0,0 +1,47 @@ +{ + "commitMessage": { + "maxLength": 120, + "minBodyLength": 0, + "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" + ] + } +} \ No newline at end of file diff --git a/dev-infra/BUILD.bazel b/dev-infra/BUILD.bazel index 2d70417db3..348009afd6 100644 --- a/dev-infra/BUILD.bazel +++ b/dev-infra/BUILD.bazel @@ -8,7 +8,9 @@ ts_library( ], module_name = "@angular/dev-infra-private", deps = [ + "//dev-infra/commit-message", "//dev-infra/pullapprove", + "//dev-infra/utils:config", "@npm//@types/node", ], ) @@ -33,6 +35,7 @@ pkg_npm( deps = [ ":cli", ":package-json", + "//dev-infra/commit-message", "//dev-infra/ts-circular-dependencies", ], ) diff --git a/dev-infra/cli.ts b/dev-infra/cli.ts index b6285c768d..3aa6d2e733 100644 --- a/dev-infra/cli.ts +++ b/dev-infra/cli.ts @@ -6,7 +6,11 @@ * 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 {readFileSync} from 'fs'; +import {join} from 'path'; import {verify} from './pullapprove/verify'; +import {validateCommitMessage} from './commit-message/validate'; +import {getRepoBaseDir} from './utils/config'; const args = process.argv.slice(2); @@ -16,6 +20,12 @@ switch (args[0]) { case 'pullapprove:verify': verify(); break; + case 'commit-message:pre-commit-validate': + const commitMessage = readFileSync(join(getRepoBaseDir(), '.git/COMMIT_EDITMSG'), 'utf8'); + if (validateCommitMessage(commitMessage)) { + console.info('√ Valid commit message'); + } + break; default: console.info('No commands were matched'); } diff --git a/dev-infra/commit-message/BUILD.bazel b/dev-infra/commit-message/BUILD.bazel new file mode 100644 index 0000000000..e6029084c3 --- /dev/null +++ b/dev-infra/commit-message/BUILD.bazel @@ -0,0 +1,39 @@ +load("//tools:defaults.bzl", "jasmine_node_test") +load("@npm_bazel_typescript//:index.bzl", "ts_library") + +ts_library( + name = "commit-message", + srcs = [ + "config.ts", + "validate.ts", + ], + module_name = "@angular/dev-infra-private/commit-message", + visibility = ["//dev-infra:__subpackages__"], + deps = [ + "//dev-infra/utils:config", + "@npm//@types/node", + "@npm//tslib", + ], +) + +ts_library( + name = "validate-test", + testonly = True, + srcs = ["validate.spec.ts"], + deps = [ + ":commit-message", + "//dev-infra/utils:config", + "@npm//@types/events", + "@npm//@types/jasmine", + "@npm//@types/node", + ], +) + +jasmine_node_test( + name = "test", + bootstrap = ["//tools/testing:node_no_angular_es5"], + deps = [ + ":commit-message", + ":validate-test", + ], +) diff --git a/dev-infra/commit-message/config.ts b/dev-infra/commit-message/config.ts new file mode 100644 index 0000000000..8b01bf3053 --- /dev/null +++ b/dev-infra/commit-message/config.ts @@ -0,0 +1,13 @@ +/** + * @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 + */ +export interface CommitMessageConfig { + maxLineLength: number; + minBodyLength: number; + types: string[]; + scopes: string[]; +} \ No newline at end of file diff --git a/dev-infra/commit-message/validate.spec.ts b/dev-infra/commit-message/validate.spec.ts new file mode 100644 index 0000000000..f9f3166f06 --- /dev/null +++ b/dev-infra/commit-message/validate.spec.ts @@ -0,0 +1,248 @@ +/** + * @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 +import * as utilConfig from '../utils/config'; + +import {validateCommitMessage} from './validate'; + + +// Constants +const config = { + 'commitMessage': { + 'maxLineLength': 120, + 'minBodyLength': 0, + 'types': [ + 'feat', + 'fix', + 'refactor', + 'release', + 'style', + ], + 'scopes': [ + 'common', + 'compiler', + 'core', + 'packaging', + ] + } +}; +const TYPES = config.commitMessage.types.join(', '); +const SCOPES = config.commitMessage.scopes.join(', '); +const INVALID = false; +const VALID = true; + +// TODO(josephperrott): Clean up tests to test script rather than for +// specific commit messages we want to use. +describe('validate-commit-message.js', () => { + let lastError: string = ''; + + beforeEach(() => { + lastError = ''; + + spyOn(console, 'error').and.callFake((msg: string) => lastError = msg); + spyOn(utilConfig, 'getAngularDevConfig').and.returnValue(config); + }); + + describe('validateMessage()', () => { + + it('should be valid', () => { + expect(validateCommitMessage('feat(packaging): something')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('release(packaging): something')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('fixup! release(packaging): something')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('squash! release(packaging): something')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('Revert: "release(packaging): something"')).toBe(VALID); + expect(lastError).toBe(''); + + }); + + it('should validate max length', () => { + const msg = + '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`); + }); + + it('should validate "(): " format', () => { + const msg = 'not correct format'; + + expect(validateCommitMessage(msg)).toBe(INVALID); + expect(lastError).toContain(`The commit message header does not match the expected format.`); + }); + + it('should fail when type is invalid', () => { + const msg = 'weird(core): something'; + + expect(validateCommitMessage(msg)).toBe(INVALID); + expect(lastError).toContain(`'weird' is not an allowed type.\n => TYPES: ${TYPES}`); + }); + + it('should fail when scope is invalid', () => { + const errorMessageFor = (scope: string, header: string) => + `'${scope}' is not an allowed scope.\n => SCOPES: ${SCOPES}`; + + expect(validateCommitMessage('fix(Compiler): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('Compiler', 'fix(Compiler): something')); + + expect(validateCommitMessage('feat(bah): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('bah', 'feat(bah): something')); + + expect(validateCommitMessage('style(webworker): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('webworker', 'style(webworker): something')); + + expect(validateCommitMessage('refactor(security): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('security', 'refactor(security): something')); + + expect(validateCommitMessage('refactor(docs): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('docs', 'refactor(docs): something')); + + expect(validateCommitMessage('release(angular): something')).toBe(INVALID); + expect(lastError).toContain(errorMessageFor('angular', 'release(angular): something')); + }); + + it('should allow empty scope', () => { + expect(validateCommitMessage('fix: blablabla')).toBe(VALID); + expect(lastError).toBe(''); + }); + + // 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(validateCommitMessage(msg)).toBe(INVALID); + expect(lastError).toContain(`'WIP' is not an allowed type.\n => TYPES: ${TYPES}`); + }); + + describe('(revert)', () => { + + it('should allow valid "revert: ..." syntaxes', () => { + expect(validateCommitMessage('revert: anything')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('Revert: "anything"')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('revert anything')).toBe(VALID); + expect(lastError).toBe(''); + + expect(validateCommitMessage('rEvErT anything')).toBe(VALID); + expect(lastError).toBe(''); + + }); + + it('should not allow "revert(scope): ..." syntax', () => { + const msg = 'revert(compiler): reduce generated code payload size by 65%'; + + expect(validateCommitMessage(msg)).toBe(INVALID); + expect(lastError).toContain(`'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(validateCommitMessage(msg)).toBe(VALID); + expect(lastError).toBe(''); + }); + }); + + 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); + + // Invalid messages. + expect(validateCommitMessage('squash! fix a typo', 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(lastError).toContain( + '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 errorMessage = `The commit message header does not match the expected format.`; + + // Valid messages. + expect(validateCommitMessage('fixup! feat(core): add feature')).toBe(VALID); + expect(validateCommitMessage('fixup! fix: a bug')).toBe(VALID); + + // Invalid messages. + expect(validateCommitMessage('fixup! fix a typo')).toBe(INVALID); + expect(lastError).toContain('fixup! fix a typo'); + expect(lastError).toContain(errorMessage); + + 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, false, ['qux', 'quux', 'quuux'])).toBe(INVALID); + expect(lastError).toContain( + 'Unable to find match for fixup commit among prior commits: \n' + + ' qux\n' + + ' quux\n' + + ' quuux'); + }); + + it('should fail if `nonFixupCommitHeaders` is empty', () => { + expect(validateCommitMessage('refactor(core): make reactive', false, [])).toBe(VALID); + expect(validateCommitMessage('fixup! foo', false, [])).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 new file mode 100644 index 0000000000..1213cd416c --- /dev/null +++ b/dev-infra/commit-message/validate.ts @@ -0,0 +1,134 @@ +/** + * @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 {getAngularDevConfig} from '../utils/config'; +import {CommitMessageConfig} from './config'; + +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; +const REVERT_PREFIX_RE = /^revert:? /i; +const TYPE_SCOPE_RE = /^(\w+)(?:\(([^)]+)\))?\:\s(.+)$/; +const COMMIT_HEADER_RE = /^(.*)/i; +const COMMIT_BODY_RE = /^.*\n\n(.*)/i; + +/** Parse a full commit message into its composite parts. */ +export function parseCommitMessage(commitMsg: string) { + let header = ''; + let body = ''; + let bodyWithoutLinking = ''; + let type = ''; + let scope = ''; + let subject = ''; + + if (COMMIT_HEADER_RE.test(commitMsg)) { + 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]; + bodyWithoutLinking = body.replace(GITHUB_LINKING_RE, ''); + } + + if (TYPE_SCOPE_RE.test(header)) { + const parsedCommitHeader = TYPE_SCOPE_RE.exec(header) !; + type = parsedCommitHeader[1]; + scope = parsedCommitHeader[2]; + subject = parsedCommitHeader[3]; + } + return { + header, + body, + bodyWithoutLinking, + type, + scope, + subject, + isFixup: FIXUP_PREFIX_RE.test(commitMsg), + isSquash: SQUASH_PREFIX_RE.test(commitMsg), + isRevert: REVERT_PREFIX_RE.test(commitMsg), + }; +} + + +/** Validate a commit message against using the local repo's config. */ +export function validateCommitMessage( + commitMsg: string, disallowSquash: boolean = false, nonFixupCommitHeaders?: string[]) { + function error(errorMessage: string) { + console.error( + `INVALID COMMIT MSG: \n` + + `${'─'.repeat(40)}\n` + + `${commitMsg}\n` + + `${'─'.repeat(40)}\n` + + `ERROR: \n` + + ` ${errorMessage}` + + `\n\n` + + `The expected format for a commit is: \n` + + `(): \n\n`); + } + + const config = getAngularDevConfig<'commitMessage', CommitMessageConfig>().commitMessage; + const commit = parseCommitMessage(commitMsg); + + if (commit.isRevert) { + return true; + } + + if (commit.isSquash && disallowSquash) { + error('The commit must be manually squashed into the target commit'); + 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 (commit.isFixup && nonFixupCommitHeaders) { + if (!nonFixupCommitHeaders.includes(commit.header)) { + error( + 'Unable to find match for fixup commit among prior commits: ' + + (nonFixupCommitHeaders.map(x => `\n ${x}`).join('') || '-')); + return false; + } + + return true; + } + + if (commit.header.length > config.maxLineLength) { + error(`The commit message header is longer than ${config.maxLineLength} characters`); + return false; + } + + if (!commit.type) { + error(`The commit message header does not match the expected format.`); + return false; + } + + if (!config.types.includes(commit.type)) { + error(`'${commit.type}' is not an allowed type.\n => TYPES: ${config.types.join(', ')}`); + return false; + } + + if (commit.scope && !config.scopes.includes(commit.scope)) { + error(`'${commit.scope}' is not an allowed scope.\n => SCOPES: ${config.scopes.join(', ')}`); + return false; + } + + if (commit.bodyWithoutLinking.trim().length < config.minBodyLength) { + error( + `The commit message body does not meet the minimum length of ${config.minBodyLength} characters`); + return false; + } + + const bodyByLine = commit.body.split('\n'); + if (bodyByLine.some(line => line.length > config.maxLineLength)) { + error( + `The commit messsage body contains lines greater than ${config.maxLineLength} characters`); + return false; + } + + return true; +} diff --git a/dev-infra/utils/BUILD.bazel b/dev-infra/utils/BUILD.bazel index 44785fa1b8..35637ade55 100644 --- a/dev-infra/utils/BUILD.bazel +++ b/dev-infra/utils/BUILD.bazel @@ -5,6 +5,7 @@ ts_library( srcs = [ "config.ts", ], + module_name = "@angular/dev-infra-private/utils", visibility = ["//dev-infra:__subpackages__"], deps = [ "@npm//@types/json5", diff --git a/dev-infra/utils/config.ts b/dev-infra/utils/config.ts index ad92b8946f..78b165d223 100644 --- a/dev-infra/utils/config.ts +++ b/dev-infra/utils/config.ts @@ -5,16 +5,15 @@ * 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 {parse} from 'json5'; import {readFileSync} from 'fs'; +import {parse} from 'json5'; import {join} from 'path'; import {exec} from 'shelljs'; - /** * Gets the path of the directory for the repository base. */ -function getRepoBaseDir() { +export function getRepoBaseDir() { const baseRepoDir = exec(`git rev-parse --show-toplevel`, {silent: true}); if (baseRepoDir.code) { throw Error( @@ -28,7 +27,7 @@ function getRepoBaseDir() { /** * Retrieve the configuration from the .dev-infra.json file. */ -export function getAngularDevConfig(): DevInfraConfig { +export function getAngularDevConfig(): DevInfraConfig { const configPath = join(getRepoBaseDir(), '.dev-infra.json'); let rawConfig = ''; try { @@ -41,5 +40,8 @@ export function getAngularDevConfig(): DevInfraConfig { return parse(rawConfig); } -// Interface exressing the expected structure of the DevInfraConfig. -export interface DevInfraConfig {} \ No newline at end of file +/** + * Interface exressing the expected structure of the DevInfraConfig. + * Allows for providing a typing for a part of the config to read. + */ +export interface DevInfraConfig { [K: string]: T; }