feat: add rule for `BREAKING CHANGE:` in header

[abitrolly]

Description

BREAKING CHANGE: is a footer thing

Fixed https://github.com/conventional-changelog/commitlint/issues/3810

Motivation and Context

Usage examples

echo "BREAKING CHANGE: no" | yarn run commitlint

How Has This Been Tested?

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

[abitrolly]

Test fails for some reason, and I don't know how to debug it. console.log doesn't work in this context.

Also, subject-empty triggers instead of this rule, I also don't know how to fix that.

$ echo "BREAKING CHANGE: no" | yarn run commitlint
yarn run v1.22.21
$ /workspace/commitlint/node_modules/.bin/commitlint
⧗   input: BREAKING CHANGE: no
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]

✖   found 2 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[escapedcat]

BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in Semantic Versioning). A BREAKING CHANGE can be part of commits of any type.

https://www.conventionalcommits.org/en/v1.0.0/#specification

According to the specs BREAKING CHANGE should be part of the commit-footer, not in the "first line". The tests here fail correctly I believe because BREAKING CHANGE: is a footer thing is not a valid commit according to conventionalcommits.

Does this make sense?

I usually use it like this: https://github.com/conventional-changelog/commitlint/commit/5b4aeaf4f01c2726a7bc8631a23bb34c849baad2

Update: Sorry for only giving this feedback now. Should have added this to #3810. Maybe I'm reading the specs wrong?

[abitrolly]

@escapedcat in the test https://github.com/conventional-changelog/commitlint/actions/runs/7404572868/job/20146281125?pr=3838

    Expected: false
    Received: true

But it should have received false. And when I run from command line, the test doesn't trigger at all. And console.log doesn't work.

So how to develop this without debug info?

[escapedcat]

I think my main point was that BREAKING CHANGE: is a footer thing should not work according to the spec

[abitrolly]

@escapedcat, so the test for BREAKING CHANGE: as the first line should return false, right?

[escapedcat]

@escapedcat, so the test for BREAKING CHANGE: as the first line should return false, right?

I think so, yes. It's only valid to add BREAKING CHANGE: to the commit-footer, i.e. see this example

Should have noted that when you opened #3810 but didn't notice it back then.

[abitrolly]

@escapedcat right, and the Jest test expects the test to return false, but the test returns something different and I don't know how to debug it (first time writing TypeScript).

[escapedcat]

Not sure when I have time to look into it. You can try to output the result like this maybe:

console.log(JSON.stringify(YOUR_JEST_RESULT))

[abitrolly]

console.log works no problem - I misread Jest output. :facepalm:

When I added console.log here.

	const result = parsed.subject?.startsWith('BREAKING CHANGE:');

	console.log(result);

I got this error.

 FAIL  @commitlint/rules/src/subject-breaking.test.ts
  ● Console

    console.log
      undefined

      at log (@commitlint/rules/src/subject-breaking.ts:6:10)

And thought that console.log is undefined, but it is result which is undefined.

[abitrolly]

Now the most interesting part - parsed.subject is null.

The specification doesn't define subject at all - https://www.conventionalcommits.org/en/v1.0.0/#specification

{
        type: null,
        scope: null,
        subject: null,
        merge: null,
        header: 'BREAKING CHANGE: this one',
        body: null,
        footer: null,
        notes: [],
        references: [],
        mentions: [],
        revert: null,
        raw: 'BREAKING CHANGE: this one'
      }

I should be checking header field here.

[abitrolly]

I send PR to simplify the specification text https://github.com/conventional-commits/conventionalcommits.org/pull/559

There is still no subject and header definitions in the spec - it says description instead of subject, but that change would be too big, and I am not sure it will be accepted.

[abitrolly]

Tests are passing, but I still can't trigger the rule from the command line.

$ echo "BREAKING CHANGE: no" | ./@commitlint/cli/cl
i.js
⧗   input: BREAKING CHANGE: no
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]

✖   found 2 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

[escapedcat]

I assume the issue here is that it doesn't find a subject because the type already failes? commitlint assumes BREAKING CHANGE: is the type which isn't a valid type? Not 100% sure though. A check needs to happen before they type check maybe?

[abitrolly]

A check needs to happen before they type check maybe?

That's what I thought too. How to do that?

[knocte]

From my very short understanding of commitlint so far, what might be happening here is that when commitlint's parser finds "BREAKING CHANGE:" at the beginning of the header, in theory it should try to check if the string "BREAKING CHANGE" is part of the allowed types in the configuration, however, the regExp to extract the "potential" type (the string before the colon) might not be prepared to handle spaces, therefore it already fails to parse it.

[abitrolly]

@knocte the regexp checks header content, not type.

[knocte]

Ah true, but I was actually (mistakenly) referring to subject: if type cannot be parsed, then subject cannot either.

[abitrolly]

I changed the issue title to avoid subject/header confusion. It is still not clear why it doesn't trigger.

[escapedcat]

Thanks people! We good here now?

[abitrolly]

@escapedcat no, it is not clear how to debug why the rule doesn't trigger.

[escapedcat]

Alright, let me switch this to draft for now