feat(dev-infra): create commit-message validation script/tooling (#36117)

PR Close #36117

.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"
+    ]
+  }
+}

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",
     ],
 )

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');
 }

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",
+    ],
+)

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[];
+}

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 "<type>(<scope>): <subject>" 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: -`);
+        });
+      });
+    });
+  });
+});

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` +
+        `<type>(<scope>): <subject>\n\n<body>`);
+  }
+
+  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;
+}

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",

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<K, T>(): DevInfraConfig<K, T> {
   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 {}
+ * 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, T> { [K: string]: T; }