feat(dev-infra): validate deprecation notes in commit messages (#42436)

Currently the commit message validation tool from `ng-dev` validates
the `BREAKING CHANGE:` commit message notes. This commit adds a similar
check for `DEPRECATED:` commit message notes.

Additionally, the check for breaking changes is reworked slightly to
be more tolerant (i.e. if there is only a single line break after the
summary; this is acceptable as per the parser and commonly done in the
COMP repo). The checks have been updated to capture wrong keywords that
are commonly used instead of the correct one. e.g. if a commit message
uses `DEPRECATIONS:` instead of `DEPRECATED:`, the validation will fail.

This prevents changelog generation issues where breaking change notes,
or deprecations are missing. This happened in the COMP repo where
the `DEPRECATED:` keyword was used incorrectly. See:

99391e7939

PR Close #42436

dev-infra/commit-message/parse.ts

@@ -111,7 +111,7 @@ const parseOptions: Options&{notesPattern: (keywords: string) => RegExp} = {
   headerPattern,
   headerCorrespondence,
   noteKeywords: [NoteSections.BREAKING_CHANGE, NoteSections.DEPRECATED],
-  notesPattern: (keywords: string) => new RegExp(`(${keywords})(?:: ?)(.*)`),
+  notesPattern: (keywords: string) => new RegExp(`(${keywords}): ?(.*)`),
 };
 
 /** Parse a commit message into its composite parts. */

dev-infra/commit-message/validate.spec.ts

@@ -8,6 +8,7 @@
 
 // Imports
 import * as validateConfig from './config';
+import {parseCommitMessage} from './parse';
 import {validateCommitMessage, ValidateCommitMessageResult} from './validate';
 
 type CommitMessageConfig = validateConfig.CommitMessageConfig;
@@ -278,6 +279,78 @@ describe('validate-commit-message.js', () => {
          });
     });
 
+    describe('deprecations', () => {
+      it('should allow valid deprecation notes in commit messages', () => {
+        const msgWithListOfDeprecations =
+            'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'DEPRECATED:\n' +
+            ' * A to be removed\n' +
+            ' * B to be removed';
+        expectValidationResult(validateCommitMessage(msgWithListOfDeprecations), VALID);
+        expect(parseCommitMessage(msgWithListOfDeprecations).deprecations.length).toBe(1);
+
+        const msgWithSummary = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'DEPRECATED: All methods in X to be removed in v12.';
+
+        expectValidationResult(validateCommitMessage(msgWithSummary), VALID);
+        expect(parseCommitMessage(msgWithSummary).deprecations.length).toBe(1);
+
+        const msgWithSummaryAndDescription =
+            'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'DEPRECATED: All methods in X to be removed in v12.\n' +
+            '' +
+            'This is the more detailed description about the deprecation of X.';
+
+        expectValidationResult(validateCommitMessage(msgWithSummaryAndDescription), VALID);
+        expect(parseCommitMessage(msgWithSummaryAndDescription).deprecations.length).toBe(1);
+
+        const msgWithNoDeprecation =
+            'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is not a\n' +
+            'deprecation commit.';
+        expectValidationResult(validateCommitMessage(msgWithNoDeprecation), VALID);
+        expect(parseCommitMessage(msgWithNoDeprecation).deprecations.length).toBe(0);
+      });
+
+      it('should fail for non-valid deprecation notes in commit messages', () => {
+        const incorrectKeyword1 = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'DEPRECATE:\n' +
+            ' * A to be removed\n' +
+            ' * B to be removed';
+        expectValidationResult(
+            validateCommitMessage(incorrectKeyword1), INVALID,
+            ['The commit message body contains an invalid deprecation note.']);
+
+        const incorrectKeyword2 = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'DEPRECATES:\n' +
+            ' * A to be removed\n' +
+            ' * B to be removed';
+        expectValidationResult(
+            validateCommitMessage(incorrectKeyword2), INVALID,
+            ['The commit message body contains an invalid deprecation note.']);
+
+        const incorrectKeyword3 = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'DEPRECATIONS:\n' +
+            ' * A to be removed\n' +
+            ' * B to be removed';
+        expectValidationResult(
+            validateCommitMessage(incorrectKeyword3), INVALID,
+            ['The commit message body contains an invalid deprecation note.']);
+      });
+    });
+
     describe('breaking change', () => {
       it('should allow valid breaking change commit descriptions', () => {
         const msgWithSummary = 'feat(compiler): this is just a usual commit message title\n\n' +
@@ -285,13 +358,31 @@ describe('validate-commit-message.js', () => {
             'limit. For more details see the following super long URL:\n\n' +
             'BREAKING CHANGE: This is a summary of a breaking change.';
         expectValidationResult(validateCommitMessage(msgWithSummary), VALID);
+        expect(parseCommitMessage(msgWithSummary).breakingChanges.length).toBe(1);
 
-        const msgWithDescription = 'feat(compiler): this is just a usual commit message title\n\n' +
+        const msgWithDescriptionDoubleLineBreakSeparator =
+            'feat(compiler): this is just a usual commit message title\n\n' +
             'This is a normal commit message body which does not exceed the max length\n' +
             'limit. For more details see the following super long URL:\n\n' +
             'BREAKING CHANGE:\n\n' +
             'This is a full description of the breaking change.';
-        expectValidationResult(validateCommitMessage(msgWithDescription), VALID);
+        expectValidationResult(
+            validateCommitMessage(msgWithDescriptionDoubleLineBreakSeparator), VALID);
+        expect(
+            parseCommitMessage(msgWithDescriptionDoubleLineBreakSeparator).breakingChanges.length)
+            .toBe(1);
+
+        const msgWithDescriptionSingleLineBreakSeparator =
+            'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'BREAKING CHANGE:\n' +
+            'This is a full description of the breaking change.';
+        expectValidationResult(
+            validateCommitMessage(msgWithDescriptionSingleLineBreakSeparator), VALID);
+        expect(
+            parseCommitMessage(msgWithDescriptionSingleLineBreakSeparator).breakingChanges.length)
+            .toBe(1);
 
         const msgWithSummaryAndDescription =
             'feat(compiler): this is just a usual commit message title\n\n' +
@@ -300,11 +391,13 @@ describe('validate-commit-message.js', () => {
             'BREAKING CHANGE: This is a summary of a breaking change.\n\n' +
             'This is a full description of the breaking change.';
         expectValidationResult(validateCommitMessage(msgWithSummaryAndDescription), VALID);
+        expect(parseCommitMessage(msgWithSummaryAndDescription).breakingChanges.length).toBe(1);
 
         const msgWithNonBreaking = 'feat(compiler): this is just a usual commit message title\n\n' +
             'This is not a\n' +
             'breaking change commit.';
         expectValidationResult(validateCommitMessage(msgWithNonBreaking), VALID);
+        expect(parseCommitMessage(msgWithNonBreaking).breakingChanges.length).toBe(0);
       });
 
       it('should fail for non-valid breaking change commit descriptions', () => {
@@ -314,7 +407,7 @@ describe('validate-commit-message.js', () => {
             'BREAKING CHANGE This is a summary of a breaking change.';
         expectValidationResult(
             validateCommitMessage(msgWithSummary), INVALID,
-            [`The commit message body contains an invalid breaking change description.`]);
+            [`The commit message body contains an invalid breaking change note.`]);
 
         const msgWithPlural = 'feat(compiler): this is just a usual commit message title\n\n' +
             'This is a normal commit message body which does not exceed the max length\n' +
@@ -322,16 +415,17 @@ describe('validate-commit-message.js', () => {
             'BREAKING CHANGES: This is a summary of a breaking change.';
         expectValidationResult(
             validateCommitMessage(msgWithPlural), INVALID,
-            [`The commit message body contains an invalid breaking change description.`]);
+            [`The commit message body contains an invalid breaking change note.`]);
 
-        const msgWithDescription = 'feat(compiler): this is just a usual commit message title\n\n' +
+        const msgWithWithDashedKeyword =
+            'feat(compiler): this is just a usual commit message title\n\n' +
             'This is a normal commit message body which does not exceed the max length\n' +
             'limit. For more details see the following super long URL:\n\n' +
-            'BREAKING CHANGE:\n' +
+            'BREAKING-CHANGE:' +
             'This is a full description of the breaking change.';
         expectValidationResult(
-            validateCommitMessage(msgWithDescription), INVALID,
+            validateCommitMessage(msgWithWithDashedKeyword), INVALID,
-            [`The commit message body contains an invalid breaking change description.`]);
+            [`The commit message body contains an invalid breaking change note.`]);
 
         const msgWithSummaryAndDescription =
             'feat(compiler): this is just a usual commit message title\n\n' +
@@ -341,7 +435,34 @@ describe('validate-commit-message.js', () => {
             'This is a full description of the breaking change.';
         expectValidationResult(
             validateCommitMessage(msgWithSummaryAndDescription), INVALID,
-            [`The commit message body contains an invalid breaking change description.`]);
+            [`The commit message body contains an invalid breaking change note.`]);
+
+        const incorrectKeyword1 = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'BREAKING CHANGES:\n' +
+            ' * A has been removed\n';
+        expectValidationResult(
+            validateCommitMessage(incorrectKeyword1), INVALID,
+            ['The commit message body contains an invalid breaking change note.']);
+
+        const incorrectKeyword2 = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'BREAKING-CHANGE:\n' +
+            ' * A has been removed\n';
+        expectValidationResult(
+            validateCommitMessage(incorrectKeyword2), INVALID,
+            ['The commit message body contains an invalid breaking change note.']);
+
+        const incorrectKeyword3 = 'feat(compiler): this is just a usual commit message title\n\n' +
+            'This is a normal commit message body which does not exceed the max length\n' +
+            'limit. For more details see the following super long URL:\n\n' +
+            'BREAKING-CHANGES:\n' +
+            ' * A has been removed\n';
+        expectValidationResult(
+            validateCommitMessage(incorrectKeyword3), INVALID,
+            ['The commit message body contains an invalid breaking change note.']);
       });
     });
   });

dev-infra/commit-message/validate.ts

@@ -26,16 +26,29 @@ export interface ValidateCommitMessageResult {
 
 /** Regex matching a URL for an entire commit body line. */
 const COMMIT_BODY_URL_LINE_RE = /^https?:\/\/.*$/;
+
 /**
- * Regex matching a breaking change.
+ * Regular expression matching potential misuse of the `BREAKING CHANGE:` marker in a
+ * commit message. Commit messages containing one of the following snippets will fail:
  *
- * - Starts with BREAKING CHANGE
+ *   - `BREAKING CHANGE <some-content>` | Here we assume the colon is missing by accident.
- * - Followed by a colon
+ *   - `BREAKING-CHANGE: <some-content>` | The wrong keyword is used here.
- * - Followed by a single space or two consecutive new lines
+ *   - `BREAKING CHANGES: <some-content>` | The wrong keyword is used here.
- *
+ *   - `BREAKING-CHANGES: <some-content>` | The wrong keyword is used here.
- * NB: Anything after `BREAKING CHANGE` is optional to facilitate the validation.
  */
-const COMMIT_BODY_BREAKING_CHANGE_RE = /^BREAKING CHANGE(:( |\n{2}))?/m;
+const INCORRECT_BREAKING_CHANGE_BODY_RE =
+    /^(BREAKING CHANGE[^:]|BREAKING-CHANGE|BREAKING[ -]CHANGES)/m;
+
+/**
+ * Regular expression matching potential misuse of the `DEPRECATED:` marker in a commit
+ * message. Commit messages containing one of the following snippets will fail:
+ *
+ *   - `DEPRECATED <some-content>` | Here we assume the colon is missing by accident.
+ *   - `DEPRECATIONS: <some-content>` | The wrong keyword is used here.
+ *   - `DEPRECATE: <some-content>` | The wrong keyword is used here.
+ *   - `DEPRECATES: <some-content>` | The wrong keyword is used here.
+ */
+const INCORRECT_DEPRECATION_BODY_RE = /^(DEPRECATED[^:]|DEPRECATIONS|DEPRECATE:|DEPRECATES)/m;
 
 /** Validate a commit message against using the local repo's config. */
 export function validateCommitMessage(
@@ -161,14 +174,14 @@ export function validateCommitMessage(
     // Breaking change
     // Check if the commit message contains a valid break change description.
     // https://github.com/angular/angular/blob/88fbc066775ab1a2f6a8c75f933375b46d8fa9a4/CONTRIBUTING.md#commit-message-footer
-    const hasBreakingChange = COMMIT_BODY_BREAKING_CHANGE_RE.exec(commit.fullText);
+    if (INCORRECT_BREAKING_CHANGE_BODY_RE.test(commit.fullText)) {
-    if (hasBreakingChange !== null) {
+      errors.push(`The commit message body contains an invalid breaking change note.`);
-      const [, breakingChangeDescription] = hasBreakingChange;
+      return false;
-      if (!breakingChangeDescription) {
+    }
-        // Not followed by :, space or two consecutive new lines,
+
-        errors.push(`The commit message body contains an invalid breaking change description.`);
+    if (INCORRECT_DEPRECATION_BODY_RE.test(commit.fullText)) {
-        return false;
+      errors.push(`The commit message body contains an invalid deprecation note.`);
-      }
+      return false;
     }
 
     return true;

dev-infra/ng-dev.js

@@ -1843,7 +1843,7 @@ const parseOptions = {
     headerPattern,
     headerCorrespondence,
     noteKeywords: [NoteSections.BREAKING_CHANGE, NoteSections.DEPRECATED],
-    notesPattern: (keywords) => new RegExp(`(${keywords})(?:: ?)(.*)`),
+    notesPattern: (keywords) => new RegExp(`(${keywords}): ?(.*)`),
 };
 /** Parse a commit message into its composite parts. */
 const parseCommitMessage = parseInternal;
@@ -1902,15 +1902,25 @@ function parseInternal(fullText) {
 /** Regex matching a URL for an entire commit body line. */
 const COMMIT_BODY_URL_LINE_RE = /^https?:\/\/.*$/;
 /**
- * Regex matching a breaking change.
+ * Regular expression matching potential misuse of the `BREAKING CHANGE:` marker in a
+ * commit message. Commit messages containing one of the following snippets will fail:
  *
- * - Starts with BREAKING CHANGE
+ *   - `BREAKING CHANGE <some-content>` | Here we assume the colon is missing by accident.
- * - Followed by a colon
+ *   - `BREAKING-CHANGE: <some-content>` | The wrong keyword is used here.
- * - Followed by a single space or two consecutive new lines
+ *   - `BREAKING CHANGES: <some-content>` | The wrong keyword is used here.
- *
+ *   - `BREAKING-CHANGES: <some-content>` | The wrong keyword is used here.
- * NB: Anything after `BREAKING CHANGE` is optional to facilitate the validation.
  */
-const COMMIT_BODY_BREAKING_CHANGE_RE = /^BREAKING CHANGE(:( |\n{2}))?/m;
+const INCORRECT_BREAKING_CHANGE_BODY_RE = /^(BREAKING CHANGE[^:]|BREAKING-CHANGE|BREAKING[ -]CHANGES)/m;
+/**
+ * Regular expression matching potential misuse of the `DEPRECATED:` marker in a commit
+ * message. Commit messages containing one of the following snippets will fail:
+ *
+ *   - `DEPRECATED <some-content>` | Here we assume the colon is missing by accident.
+ *   - `DEPRECATIONS: <some-content>` | The wrong keyword is used here.
+ *   - `DEPRECATE: <some-content>` | The wrong keyword is used here.
+ *   - `DEPRECATES: <some-content>` | The wrong keyword is used here.
+ */
+const INCORRECT_DEPRECATION_BODY_RE = /^(DEPRECATED[^:]|DEPRECATIONS|DEPRECATE:|DEPRECATES)/m;
 /** Validate a commit message against using the local repo's config. */
 function validateCommitMessage(commitMsg, options = {}) {
     const config = getCommitMessageConfig().commitMessage;
@@ -2008,14 +2018,13 @@ function validateCommitMessage(commitMsg, options = {}) {
         // Breaking change
         // Check if the commit message contains a valid break change description.
         // https://github.com/angular/angular/blob/88fbc066775ab1a2f6a8c75f933375b46d8fa9a4/CONTRIBUTING.md#commit-message-footer
-        const hasBreakingChange = COMMIT_BODY_BREAKING_CHANGE_RE.exec(commit.fullText);
+        if (INCORRECT_BREAKING_CHANGE_BODY_RE.test(commit.fullText)) {
-        if (hasBreakingChange !== null) {
+            errors.push(`The commit message body contains an invalid breaking change note.`);
-            const [, breakingChangeDescription] = hasBreakingChange;
+            return false;
-            if (!breakingChangeDescription) {
+        }
-                // Not followed by :, space or two consecutive new lines,
+        if (INCORRECT_DEPRECATION_BODY_RE.test(commit.fullText)) {
-                errors.push(`The commit message body contains an invalid breaking change description.`);
+            errors.push(`The commit message body contains an invalid deprecation note.`);
-                return false;
+            return false;
-            }
         }
         return true;
     }