Skip to content

Commit c4d419b

Browse files
adamchristiansenadamchristiansen
authored
feat(rules): add breaking-change-exclamation-mark (#4548)
* feat(rules): add breaking-change-exclamation-mark Implements and closes #4547. * feat(rules): breaking-change-exclamation-mark pull request changes Addresses review feedback from @JounQin about pull request #4548. - Use `breakingHeaderPattern` to search for the exclamation mark in the header. - Correct the regular expression to require that BREAKING CHANGE in the footer be anchored at the beginning of a line. - Updated the `RulesConfig` type. * feat(rules): breaking-change-exclamation-mark pull request feedback Address pull request feedback from @JounQin. - Fixed regex for `subject-exclamation-mark`. - Fixed sorting for `RulesConfig`. - Fixed sorting for `rules.md`.
1 parent 63e7ad4 commit c4d419b

File tree

6 files changed

+268
-130
lines changed

6 files changed

+268
-130
lines changed

‎@commitlint/rules/src/breaking-change-exclamation-mark.test.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { test, expect } from "vitest";
2+
import parse from "@commitlint/parse";
3+
import { breakingChangeExclamationMark } from "./breaking-change-exclamation-mark.js";
4+
5+
const noHeader = "commit message";
6+
const plainHeader = "type: subject";
7+
const breakingHeader = "type!: subject";
8+
const noFooter = "";
9+
const plainFooter = "Some-Other-Trailer: content";
10+
const breakingFooter = "BREAKING CHANGE: reason";
11+
12+
// These are equivalence partitions.
13+
const messages = {
14+
noHeaderNoFooter: `${noHeader}\n\n${noFooter}`,
15+
noHeaderPlainFooter: `${noHeader}\n\n${plainFooter}`,
16+
noHeaderBreakingFooter: `${noHeader}\n\n${breakingFooter}`,
17+
plainHeaderPlainFooter: `${plainHeader}\n\n${plainFooter}`,
18+
plainHeaderBreakingFooter: `${plainHeader}\n\n${breakingFooter}`,
19+
breakingHeaderPlainFooter: `${breakingHeader}\n\n${plainFooter}`,
20+
breakingHeaderBreakingFooter: `${breakingHeader}\n\n${breakingFooter}`,
21+
};
22+
23+
const parsed = {
24+
noHeaderNoFooter: parse(messages.noHeaderNoFooter),
25+
noHeaderPlainFooter: parse(messages.noHeaderPlainFooter),
26+
noHeaderBreakingFooter: parse(messages.noHeaderBreakingFooter),
27+
plainHeaderPlainFooter: parse(messages.plainHeaderPlainFooter),
28+
plainHeaderBreakingFooter: parse(messages.plainHeaderBreakingFooter),
29+
breakingHeaderPlainFooter: parse(messages.breakingHeaderPlainFooter),
30+
breakingHeaderBreakingFooter: parse(messages.breakingHeaderBreakingFooter),
31+
};
32+
33+
test("with noHeaderNoFooter should succeed", async () => {
34+
const [actual] = breakingChangeExclamationMark(await parsed.noHeaderNoFooter);
35+
const expected = true;
36+
expect(actual).toEqual(expected);
37+
});
38+
39+
test("with noHeaderPlainFooter should succeed", async () => {
40+
const [actual] = breakingChangeExclamationMark(
41+
await parsed.noHeaderPlainFooter,
42+
);
43+
const expected = true;
44+
expect(actual).toEqual(expected);
45+
});
46+
47+
test("with noHeaderBreakingFooter should fail", async () => {
48+
const [actual] = breakingChangeExclamationMark(
49+
await parsed.noHeaderBreakingFooter,
50+
);
51+
const expected = false;
52+
expect(actual).toEqual(expected);
53+
});
54+
55+
test("with plainHeaderPlainFooter should succeed", async () => {
56+
const [actual] = breakingChangeExclamationMark(
57+
await parsed.plainHeaderPlainFooter,
58+
);
59+
const expected = true;
60+
expect(actual).toEqual(expected);
61+
});
62+
63+
test("with plainHeaderBreakingFooter should fail", async () => {
64+
const [actual] = breakingChangeExclamationMark(
65+
await parsed.plainHeaderBreakingFooter,
66+
);
67+
const expected = false;
68+
expect(actual).toEqual(expected);
69+
});
70+
71+
test("with breakingHeaderPlainFooter should fail", async () => {
72+
const [actual] = breakingChangeExclamationMark(
73+
await parsed.breakingHeaderPlainFooter,
74+
);
75+
const expected = false;
76+
expect(actual).toEqual(expected);
77+
});
78+
79+
test("with breakingHeaderBreakingFooter should succeed", async () => {
80+
const [actual] = breakingChangeExclamationMark(
81+
await parsed.breakingHeaderBreakingFooter,
82+
);
83+
const expected = true;
84+
expect(actual).toEqual(expected);
85+
});

‎@commitlint/rules/src/breaking-change-exclamation-mark.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import message from "@commitlint/message";
2+
import { SyncRule } from "@commitlint/types";
3+
4+
export const breakingChangeExclamationMark: SyncRule = (
5+
parsed,
6+
when = "always",
7+
) => {
8+
const header = parsed.header;
9+
const footer = parsed.footer;
10+
11+
// It is the correct behavior to return true only when both the header and footer are empty,
12+
// but still run the usual checks if one or neither are empty.
13+
// The reasoning is that if one is empty and the other contains a breaking change marker,
14+
// then the check fails as it is not possible for the empty one to indicate a breaking change.
15+
if (!header && !footer) {
16+
return [true];
17+
}
18+
19+
const hasExclamationMark =
20+
!!header && /^(\w*)(?:\((.*)\))?!: (.*)$/.test(header);
21+
const hasBreakingChange = !!footer && /^BREAKING[ -]CHANGE:/m.test(footer);
22+
23+
const negated = when === "never";
24+
const check = hasExclamationMark === hasBreakingChange;
25+
26+
return [
27+
negated ? !check : check,
28+
message([
29+
"breaking changes",
30+
negated ? "must not" : "must",
31+
"have both an exclamation mark in the header",
32+
"and BREAKING CHANGE in the footer",
33+
"to identify a breaking change",
34+
]),
35+
];
36+
};

‎@commitlint/rules/src/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { breakingChangeExclamationMark } from "./breaking-change-exclamation-mark.js";
12
import { bodyCase } from "./body-case.js";
23
import { bodyEmpty } from "./body-empty.js";
34
import { bodyFullStop } from "./body-full-stop.js";
@@ -43,6 +44,7 @@ export default {
4344
"body-max-length": bodyMaxLength,
4445
"body-max-line-length": bodyMaxLineLength,
4546
"body-min-length": bodyMinLength,
47+
"breaking-change-exclamation-mark": breakingChangeExclamationMark,
4648
"footer-empty": footerEmpty,
4749
"footer-leading-blank": footerLeadingBlank,
4850
"footer-max-length": footerMaxLength,
@@ -62,10 +64,10 @@ export default {
6264
"signed-off-by": signedOffBy,
6365
"subject-case": subjectCase,
6466
"subject-empty": subjectEmpty,
67+
"subject-exclamation-mark": subjectExclamationMark,
6568
"subject-full-stop": subjectFullStop,
6669
"subject-max-length": subjectMaxLength,
6770
"subject-min-length": subjectMinLength,
68-
"subject-exclamation-mark": subjectExclamationMark,
6971
"trailer-exists": trailerExists,
7072
"type-case": typeCase,
7173
"type-empty": typeEmpty,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const subjectExclamationMark: SyncRule = (parsed, when = "always") => {
88
}
99

1010
const negated = when === "never";
11-
const hasExclamationMark = /!:/.test(input);
11+
const hasExclamationMark = /^(\w*)(?:\((.*)\))?!: (.*)$/.test(input);
1212

1313
return [
1414
negated ? !hasExclamationMark : hasExclamationMark,

‎@commitlint/types/src/rules.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ export type RulesConfig<V = RuleConfigQuality.User> = {
9797
"body-max-length": LengthRuleConfig<V>;
9898
"body-max-line-length": LengthRuleConfig<V>;
9999
"body-min-length": LengthRuleConfig<V>;
100+
"breaking-change-exclamation-mark": CaseRuleConfig<V>;
100101
"footer-empty": RuleConfig<V>;
101102
"footer-leading-blank": RuleConfig<V>;
102103
"footer-max-length": LengthRuleConfig<V>;

0 commit comments

Comments
 (0)