From cd3816d553762eae99e088689395c55afce0c6cc Mon Sep 17 00:00:00 2001 From: Chris Kay Date: Thu, 13 May 2021 09:14:06 +0100 Subject: [PATCH] feat(rules): add `trailer-exists` rule (#2578) This new rule behaves similarly to the existing `signed-off-by` rule, but introduces what would otherwise be breaking changes to the parser in order to resolve some issues. Rather than attempting to parse the trailers manually, this rule will use Git's `interpret-trailers` subcommand to parse trailers according to Git's rules. This allows us to properly detect all valid trailers, and does not require that the trailer to search for be the last line of the commit message (as is required by the `signed-off-by` rule). One downside to this approach is that Git will not detect trailers if they are grouped alongside other trailers with whitespace in the token (e.g. `BREAKING CHANGE`). Projects that wish to use this rule may use `BREAKING-CHANGE`, which is synonymous with `BREAKING CHANGE` but is considered a valid trailer. Co-authored-by: Ade Attwood --- @commitlint/rules/package.json | 3 +- @commitlint/rules/src/index.ts | 2 + @commitlint/rules/src/trailer-exists.test.ts | 136 +++++++++++++++++++ @commitlint/rules/src/trailer-exists.ts | 28 ++++ @commitlint/types/src/rules.ts | 1 + docs/reference-rules.md | 10 ++ 6 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 @commitlint/rules/src/trailer-exists.test.ts create mode 100644 @commitlint/rules/src/trailer-exists.ts diff --git a/@commitlint/rules/package.json b/@commitlint/rules/package.json index 59fb4345a7..d2eccf2c00 100644 --- a/@commitlint/rules/package.json +++ b/@commitlint/rules/package.json @@ -44,7 +44,8 @@ "@commitlint/ensure": "^12.1.4", "@commitlint/message": "^12.1.4", "@commitlint/to-lines": "^12.1.4", - "@commitlint/types": "^12.1.4" + "@commitlint/types": "^12.1.4", + "execa": "^5.0.0" }, "gitHead": "70f7f4688b51774e7ac5e40e896cdaa3f132b2bc" } diff --git a/@commitlint/rules/src/index.ts b/@commitlint/rules/src/index.ts index 8053f721f3..60e3239453 100644 --- a/@commitlint/rules/src/index.ts +++ b/@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 {trailerExists} from './trailer-exists'; 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, + 'trailer-exists': trailerExists, 'type-case': typeCase, 'type-empty': typeEmpty, 'type-enum': typeEnum, diff --git a/@commitlint/rules/src/trailer-exists.test.ts b/@commitlint/rules/src/trailer-exists.test.ts new file mode 100644 index 0000000000..f97ab83be1 --- /dev/null +++ b/@commitlint/rules/src/trailer-exists.test.ts @@ -0,0 +1,136 @@ +import parse from '@commitlint/parse'; +import {trailerExists} from './trailer-exists'; + +const messages = { + empty: 'test:\n', + with: `test: subject\n\nbody\n\nfooter\n\nSigned-off-by:\n\n`, + without: `test: subject\n\nbody\n\nfooter\n\n`, + inSubject: `test: subject Signed-off-by:\n\nbody\n\nfooter\n\n`, + inBody: `test: subject\n\nbody Signed-off-by:\n\nfooter\n\n`, + withSignoffAndNoise: `test: subject + +message body + +Arbitrary-trailer: +Signed-off-by: +Another-arbitrary-trailer: + +# Please enter the commit message for your changes. Lines starting +# with '#' will be ignored, and an empty message aborts the commit. +`, +}; + +const parsed = { + empty: parse(messages.empty), + with: parse(messages.with), + without: parse(messages.without), + inSubject: parse(messages.inSubject), + inBody: parse(messages.inBody), + withSignoffAndNoise: parse(messages.withSignoffAndNoise), +}; + +test('empty against "always trailer-exists" should fail', async () => { + const [actual] = trailerExists( + await parsed.empty, + 'always', + 'Signed-off-by:' + ); + + const expected = false; + expect(actual).toEqual(expected); +}); + +test('empty against "never trailer-exists" should succeed', async () => { + const [actual] = trailerExists(await parsed.empty, 'never', 'Signed-off-by:'); + const expected = true; + expect(actual).toEqual(expected); +}); + +test('with against "always trailer-exists" should succeed', async () => { + const [actual] = trailerExists(await parsed.with, 'always', 'Signed-off-by:'); + const expected = true; + expect(actual).toEqual(expected); +}); + +test('with against "never trailer-exists" should fail', async () => { + const [actual] = trailerExists(await parsed.with, 'never', 'Signed-off-by:'); + const expected = false; + expect(actual).toEqual(expected); +}); + +test('without against "always trailer-exists" should fail', async () => { + const [actual] = trailerExists( + await parsed.without, + 'always', + 'Signed-off-by:' + ); + + const expected = false; + expect(actual).toEqual(expected); +}); + +test('without against "never trailer-exists" should succeed', async () => { + const [actual] = trailerExists( + await parsed.without, + 'never', + 'Signed-off-by:' + ); + + const expected = true; + expect(actual).toEqual(expected); +}); + +test('comments and other trailers should be ignored', async () => { + const [actual] = trailerExists( + await parsed.withSignoffAndNoise, + 'always', + 'Signed-off-by:' + ); + + const expected = true; + expect(actual).toEqual(expected); +}); + +test('inSubject against "always trailer-exists" should fail', async () => { + const [actual] = trailerExists( + await parsed.inSubject, + 'always', + 'Signed-off-by:' + ); + + const expected = false; + expect(actual).toEqual(expected); +}); + +test('inSubject against "never trailer-exists" should succeed', async () => { + const [actual] = trailerExists( + await parsed.inSubject, + 'never', + 'Signed-off-by:' + ); + + const expected = true; + expect(actual).toEqual(expected); +}); + +test('inBody against "always trailer-exists" should fail', async () => { + const [actual] = trailerExists( + await parsed.inBody, + 'always', + 'Signed-off-by:' + ); + + const expected = false; + expect(actual).toEqual(expected); +}); + +test('inBody against "never trailer-exists" should succeed', async () => { + const [actual] = trailerExists( + await parsed.inBody, + 'never', + 'Signed-off-by:' + ); + + const expected = true; + expect(actual).toEqual(expected); +}); diff --git a/@commitlint/rules/src/trailer-exists.ts b/@commitlint/rules/src/trailer-exists.ts new file mode 100644 index 0000000000..2b78da87a8 --- /dev/null +++ b/@commitlint/rules/src/trailer-exists.ts @@ -0,0 +1,28 @@ +import execa from 'execa'; +import message from '@commitlint/message'; +import toLines from '@commitlint/to-lines'; +import {SyncRule} from '@commitlint/types'; + +export const trailerExists: SyncRule = ( + parsed, + when = 'always', + value = '' +) => { + const trailers = execa.sync('git', ['interpret-trailers', '--parse'], { + input: parsed.raw, + }).stdout; + + const matches = toLines(trailers).filter((ln) => ln.startsWith(value)).length; + + const negated = when === 'never'; + const hasTrailer = matches > 0; + + return [ + negated ? !hasTrailer : hasTrailer, + message([ + 'message', + negated ? 'must not' : 'must', + 'have `' + value + '` trailer', + ]), + ]; +}; diff --git a/@commitlint/types/src/rules.ts b/@commitlint/types/src/rules.ts index d729d36aba..c2b4d24c6c 100644 --- a/@commitlint/types/src/rules.ts +++ b/@commitlint/types/src/rules.ts @@ -115,6 +115,7 @@ export type RulesConfig = { 'subject-full-stop': RuleConfig; 'subject-max-length': LengthRuleConfig; 'subject-min-length': LengthRuleConfig; + 'trailer-exists': RuleConfig; 'type-case': CaseRuleConfig; 'type-empty': RuleConfig; 'type-enum': EnumRuleConfig; diff --git a/docs/reference-rules.md b/docs/reference-rules.md index a3a0385dd0..c4042dbc0b 100644 --- a/docs/reference-rules.md +++ b/docs/reference-rules.md @@ -402,3 +402,13 @@ Infinity ``` 'Signed-off-by:' ``` + +#### trailer-exists + +- **condition**: `message` has trailer `value` +- **rule**: `always` +- **value** + +``` +'Signed-off-by:' +```