feat: add subject-exclamation-mark rule to improve error messages

When using the conventional commit feature of an `!` in the commit
subject the angular config get really confused and gives some error
messages that do not relate to the issue due to the message failing at
the parse stage. This overrides the angular parser preset to add in
the exclamation mark then uses the new rule to give a better error
message.

The result is with the message "fix!: the fix" previously the error
message would be "subject may not be empty" now the error message is
"subject must not have an exclamation mark in the subject to identify
a breaking change". This message it more descriptive and will give the
user info they need to resolve the issue.

This also updates the docs to highlight the difference in angular and
conventional configs, and point them in the direction of the
conventional config if they want to use the `!` in the commit messages

Author: AdeAttwood

@commitlint/config-angular/README.md

@@ -2,7 +2,7 @@
 
 # @commitlint/config-angular
 
-Shareable `commitlint` config enforcing the [Angular commit convention](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-guidelines).
+Shareable `commitlint` config enforcing the [Angular commit convention](https://docs.google.com/document/d/1QrDFcIiPjSLDn3EL15IJygNPiHORgU1_OOAqWjiDU5Y).
 Use with [@commitlint/cli](../cli) and [@commitlint/prompt-cli](../prompt-cli).
 
 ## Getting started
@@ -122,6 +122,17 @@ echo "fix: some message." # fails
 echo "fix: some message" # passes
 ```
 
+#### subject-exclamation-mark
+
+- **condition**: `subject` must not have a `!` before the `:` marker
+- **rule**: `never`
+
+The [angular commit
+convention](https://docs.google.com/document/d/1QrDFcIiPjSLDn3EL15IJygNPiHORgU1_OOAqWjiDU5Y)
+dose not use a `!` to define a breaking change in the commit subject. If you
+want to use this feature please consider using the [conventional commit
+config](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#commitlintconfig-conventional).
+
 #### header-max-length
 
 - **condition**: `header` has `value` or less characters

@commitlint/config-angular/index.js

@@ -1,7 +1,9 @@
 const typeEnum = require('@commitlint/config-angular-type-enum');
 
 module.exports = {
+	parserPreset: {parserOpts: {headerPattern: /^(\w*)(?:\((.*)\))?!?: (.*)$/}},
 	rules: {
+		'subject-exclamation-mark': [2, 'never'],
 		'body-leading-blank': [1, 'always'],
 		'footer-leading-blank': [1, 'always'],
 		'header-max-length': [2, 'always', 72],

@commitlint/config-angular/index.test.js

@@ -0,0 +1,84 @@
+import lint from '@commitlint/lint';
+import {rules, parserPreset} from '.';
+
+export const testCommit = async ({message, valid, error, warning}) => {
+	const m = message.replace(/^\s+/, '').trim();
+	test(m.split('\n')[0], async () => {
+		const parserOpts = parserPreset.parserOpts;
+		const result = await lint(m, rules, {parserOpts});
+
+		expect(result.valid).toBe(valid);
+
+		if (result.errors.length > 1) {
+			console.log(errors);
+			fail(
+				'Commit test should only have one error message to validate against'
+			);
+		} else if (result.errors.length) {
+			expect(result.errors[0].message).toBe(error);
+		}
+
+		if (result.warnings.length > 1) {
+			fail(
+				'Commit test should only have one warning message to validate against'
+			);
+		} else if (result.warnings.length) {
+			expect(result.warnings[0].message).toBe(warning);
+		}
+	});
+};
+
+testCommit({
+	message: `test: a valid angular commit`,
+	valid: true,
+});
+
+testCommit({
+	message: `test(scope): a valid angular commit with a scope`,
+	valid: true,
+});
+
+testCommit({
+	message: `
+		test(scope): a valid angular commit with a scope
+
+		Some content in the body
+	`,
+	valid: true,
+});
+
+testCommit({
+	message: `
+		test(scope): a valid angular commit with a scope
+		Some content in the body
+	`,
+	valid: true,
+	warning: 'body must have leading blank line',
+});
+
+testCommit({
+	message: `no: no is not not an invalid commit type`,
+	valid: false,
+	error:
+		'type must be one of [build, ci, docs, feat, fix, perf, refactor, revert, style, test]',
+});
+
+testCommit({
+	message: `test: that its an error when there is ia realllllllllllllllllllllly long header`,
+	valid: false,
+	error: 'header must not be longer than 72 characters, current length is 79',
+});
+
+testCommit({
+	message: `test!: with a breaking change in the type`,
+	valid: false,
+	error:
+		'subject must not have an exclamation mark in the subject to identify a breaking change',
+});
+
+testCommit({
+	message: `test(scope)!: with a breaking change in the type with scope`,
+	valid: false,
+	error:
+		'subject must not have an exclamation mark in the subject to identify a breaking change',
+});

@commitlint/config-conventional/index.test.js

@@ -1,5 +1,10 @@
 import lint from '@commitlint/lint';
-import {rules} from '.';
+import {rules, parserPreset} from '.';
+
+const commitLint = async (message) => {
+	const preset = await require(parserPreset)();
+	return lint(message, rules, {...preset});
+};
 
 const messages = {
 	invalidTypeEnum: 'foo: some message',
@@ -28,6 +33,7 @@ const messages = {
 		'fix(scope): some Message',
 		'fix(scope): some message\n\nBREAKING CHANGE: it will be significant!',
 		'fix(scope): some message\n\nbody',
+		'fix(scope)!: some message\n\nbody',
 	],
 };
 
@@ -107,31 +113,29 @@ const warnings = {
 };
 
 test('type-enum', async () => {
-	const result = await lint(messages.invalidTypeEnum, rules);
+	const result = await commitLint(messages.invalidTypeEnum);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.typeEnum]);
 });
 
 test('type-case', async () => {
-	const result = await lint(messages.invalidTypeCase, rules);
+	const result = await commitLint(messages.invalidTypeCase);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.typeCase, errors.typeEnum]);
 });
 
 test('type-empty', async () => {
-	const result = await lint(messages.invalidTypeEmpty, rules);
+	const result = await commitLint(messages.invalidTypeEmpty);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.typeEmpty]);
 });
 
 test('subject-case', async () => {
 	const invalidInputs = await Promise.all(
-		messages.invalidSubjectCases.map((invalidInput) =>
-			lint(invalidInput, rules)
-		)
+		messages.invalidSubjectCases.map((invalidInput) => commitLint(invalidInput))
 	);
 
 	invalidInputs.forEach((result) => {
@@ -141,57 +145,57 @@ test('subject-case', async () => {
 });
 
 test('subject-empty', async () => {
-	const result = await lint(messages.invalidSubjectEmpty, rules);
+	const result = await commitLint(messages.invalidSubjectEmpty);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.subjectEmpty, errors.typeEmpty]);
 });
 
 test('subject-full-stop', async () => {
-	const result = await lint(messages.invalidSubjectFullStop, rules);
+	const result = await commitLint(messages.invalidSubjectFullStop);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.subjectFullStop]);
 });
 
 test('header-max-length', async () => {
-	const result = await lint(messages.invalidHeaderMaxLength, rules);
+	const result = await commitLint(messages.invalidHeaderMaxLength);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.headerMaxLength]);
 });
 
 test('footer-leading-blank', async () => {
-	const result = await lint(messages.warningFooterLeadingBlank, rules);
+	const result = await commitLint(messages.warningFooterLeadingBlank, rules);
 
 	expect(result.valid).toBe(true);
 	expect(result.warnings).toEqual([warnings.footerLeadingBlank]);
 });
 
 test('footer-max-line-length', async () => {
-	const result = await lint(messages.invalidFooterMaxLineLength, rules);
+	const result = await commitLint(messages.invalidFooterMaxLineLength);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.footerMaxLineLength]);
 });
 
 test('body-leading-blank', async () => {
-	const result = await lint(messages.warningBodyLeadingBlank, rules);
+	const result = await commitLint(messages.warningBodyLeadingBlank);
 
 	expect(result.valid).toBe(true);
 	expect(result.warnings).toEqual([warnings.bodyLeadingBlank]);
 });
 
 test('body-max-line-length', async () => {
-	const result = await lint(messages.invalidBodyMaxLineLength, rules);
+	const result = await commitLint(messages.invalidBodyMaxLineLength);
 
 	expect(result.valid).toBe(false);
 	expect(result.errors).toEqual([errors.bodyMaxLineLength]);
 });
 
 test('valid messages', async () => {
 	const validInputs = await Promise.all(
-		messages.validMessages.map((input) => lint(input, rules))
+		messages.validMessages.map((input) => commitLint(input))
 	);
 
 	validInputs.forEach((result) => {

@commitlint/rules/src/index.ts

@@ -26,6 +26,7 @@ import {subjectEmpty} from './subject-empty';
 import {subjectFullStop} from './subject-full-stop';
 import {subjectMaxLength} from './subject-max-length';
 import {subjectMinLength} from './subject-min-length';
+import {subjectExclamationMark} from './subject-exclamation-mark';
 import {typeCase} from './type-case';
 import {typeEmpty} from './type-empty';
 import {typeEnum} from './type-enum';
@@ -61,6 +62,7 @@ export default {
 	'subject-full-stop': subjectFullStop,
 	'subject-max-length': subjectMaxLength,
 	'subject-min-length': subjectMinLength,
+	'subject-exclamation-mark': subjectExclamationMark,
 	'type-case': typeCase,
 	'type-empty': typeEmpty,
 	'type-enum': typeEnum,

@commitlint/rules/src/subject-exclamation-mark.test.ts

@@ -0,0 +1,52 @@
+import parse from '@commitlint/parse';
+import {subjectExclamationMark} from './subject-exclamation-mark';
+
+const {parserOpts} = require('@commitlint/config-angular');
+
+const messages = {
+	empty: 'test:\n',
+	with: `test!: subject\n`,
+	without: `test: subject\n`,
+};
+
+const parsed = {
+	empty: parse(messages.empty, undefined, parserOpts),
+	with: parse(messages.with, undefined, parserOpts),
+	without: parse(messages.without, undefined, parserOpts),
+};
+
+test('empty against "always" should fail', async () => {
+	const [actual] = subjectExclamationMark(await parsed.empty, 'always');
+	const expected = false;
+	expect(actual).toEqual(expected);
+});
+
+test('empty against "never" should succeed', async () => {
+	const [actual] = subjectExclamationMark(await parsed.empty, 'never');
+	const expected = true;
+	expect(actual).toEqual(expected);
+});
+
+test('with against "always" should succeed', async () => {
+	const [actual] = subjectExclamationMark(await parsed.with, 'always');
+	const expected = true;
+	expect(actual).toEqual(expected);
+});
+
+test('with against "never" should fail', async () => {
+	const [actual] = subjectExclamationMark(await parsed.with, 'never');
+	const expected = false;
+	expect(actual).toEqual(expected);
+});
+
+test('without against "always" should fail', async () => {
+	const [actual] = subjectExclamationMark(await parsed.without, 'always');
+	const expected = false;
+	expect(actual).toEqual(expected);
+});
+
+test('without against "never" should succeed', async () => {
+	const [actual] = subjectExclamationMark(await parsed.without, 'never');
+	const expected = true;
+	expect(actual).toEqual(expected);
+});

@commitlint/rules/src/subject-exclamation-mark.ts

@@ -0,0 +1,21 @@
+import message from '@commitlint/message';
+import {SyncRule} from '@commitlint/types';
+
+export const subjectExclamationMark: SyncRule = (parsed, when = 'always') => {
+	const input = parsed.header;
+	if (!input) {
+		return [true, ''];
+	}
+
+	const negated = when === 'never';
+	const hasExclamationMark = /!:/.test(input);
+
+	return [
+		negated ? !hasExclamationMark : hasExclamationMark,
+		message([
+			'subject',
+			negated ? 'must not' : 'must',
+			'have an exclamation mark in the subject to identify a breaking change',
+		]),
+	];
+};

docs/reference-rules.md

@@ -336,6 +336,11 @@ Infinity
 0
 ```
 
+#### subject-exclamation-mark
+
+- **condition**: `subject` has exclamation before the `:` marker
+- **rule**: `never`
+
 #### type-enum
 
 - **condition**: `type` is found in value