diff --git a/dev-infra/commit-message/parse.ts b/dev-infra/commit-message/parse.ts index bfaf566bcca..1892121a0e3 100644 --- a/dev-infra/commit-message/parse.ts +++ b/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. */ diff --git a/dev-infra/commit-message/validate.spec.ts b/dev-infra/commit-message/validate.spec.ts index 0d92bdccb20..0c0b534ea5d 100644 --- a/dev-infra/commit-message/validate.spec.ts +++ b/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, - [`The commit message body contains an invalid breaking change description.`]); + validateCommitMessage(msgWithWithDashedKeyword), INVALID, + [`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.']); }); }); }); diff --git a/dev-infra/commit-message/validate.ts b/dev-infra/commit-message/validate.ts index 4b630f7a7d4..4a9215d602f 100644 --- a/dev-infra/commit-message/validate.ts +++ b/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 - * - Followed by a colon - * - Followed by a single space or two consecutive new lines - * - * NB: Anything after `BREAKING CHANGE` is optional to facilitate the validation. + * - `BREAKING CHANGE ` | Here we assume the colon is missing by accident. + * - `BREAKING-CHANGE: ` | The wrong keyword is used here. + * - `BREAKING CHANGES: ` | The wrong keyword is used here. + * - `BREAKING-CHANGES: ` | The wrong keyword is used here. */ -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 ` | Here we assume the colon is missing by accident. + * - `DEPRECATIONS: ` | The wrong keyword is used here. + * - `DEPRECATE: ` | The wrong keyword is used here. + * - `DEPRECATES: ` | 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 (hasBreakingChange !== null) { - const [, breakingChangeDescription] = hasBreakingChange; - if (!breakingChangeDescription) { - // Not followed by :, space or two consecutive new lines, - errors.push(`The commit message body contains an invalid breaking change description.`); - return false; - } + if (INCORRECT_BREAKING_CHANGE_BODY_RE.test(commit.fullText)) { + errors.push(`The commit message body contains an invalid breaking change note.`); + return false; + } + + if (INCORRECT_DEPRECATION_BODY_RE.test(commit.fullText)) { + errors.push(`The commit message body contains an invalid deprecation note.`); + return false; } return true; diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index fdf9379078c..0d27dd2800f 100755 --- a/dev-infra/ng-dev.js +++ b/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 - * - Followed by a colon - * - Followed by a single space or two consecutive new lines - * - * NB: Anything after `BREAKING CHANGE` is optional to facilitate the validation. + * - `BREAKING CHANGE ` | Here we assume the colon is missing by accident. + * - `BREAKING-CHANGE: ` | The wrong keyword is used here. + * - `BREAKING CHANGES: ` | The wrong keyword is used here. + * - `BREAKING-CHANGES: ` | The wrong keyword is used here. */ -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 ` | Here we assume the colon is missing by accident. + * - `DEPRECATIONS: ` | The wrong keyword is used here. + * - `DEPRECATE: ` | The wrong keyword is used here. + * - `DEPRECATES: ` | 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 (hasBreakingChange !== null) { - const [, breakingChangeDescription] = hasBreakingChange; - if (!breakingChangeDescription) { - // Not followed by :, space or two consecutive new lines, - errors.push(`The commit message body contains an invalid breaking change description.`); - return false; - } + if (INCORRECT_BREAKING_CHANGE_BODY_RE.test(commit.fullText)) { + errors.push(`The commit message body contains an invalid breaking change note.`); + return false; + } + if (INCORRECT_DEPRECATION_BODY_RE.test(commit.fullText)) { + errors.push(`The commit message body contains an invalid deprecation note.`); + return false; } return true; }