Skip to content

Commit

Permalink
feat(rules): add trailer-exists rule (#2578)
Browse files Browse the repository at this point in the history
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 <code@adeattwood.co.uk>
  • Loading branch information
CJKay and AdeAttwood committed May 13, 2021
1 parent 4db4ba1 commit cd3816d
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 1 deletion.
3 changes: 2 additions & 1 deletion @commitlint/rules/package.json
Expand Up @@ -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"
}
2 changes: 2 additions & 0 deletions @commitlint/rules/src/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
136 changes: 136 additions & 0 deletions @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);
});
28 changes: 28 additions & 0 deletions @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<string> = (
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',
]),
];
};
1 change: 1 addition & 0 deletions @commitlint/types/src/rules.ts
Expand Up @@ -115,6 +115,7 @@ export type RulesConfig<V = RuleConfigQuality.User> = {
'subject-full-stop': RuleConfig<V, string>;
'subject-max-length': LengthRuleConfig<V>;
'subject-min-length': LengthRuleConfig<V>;
'trailer-exists': RuleConfig<V, string>;
'type-case': CaseRuleConfig<V>;
'type-empty': RuleConfig<V>;
'type-enum': EnumRuleConfig<V>;
Expand Down
10 changes: 10 additions & 0 deletions docs/reference-rules.md
Expand Up @@ -402,3 +402,13 @@ Infinity
```
'Signed-off-by:'
```

#### trailer-exists

- **condition**: `message` has trailer `value`
- **rule**: `always`
- **value**

```
'Signed-off-by:'
```

0 comments on commit cd3816d

Please sign in to comment.