bug: footer-leading-blank complains with the number sign in the commit body

[e18r]

Expected Behavior

The following commit message should pass the footer-leading-blank rule:

test: footer-leading-blank bug

hello
see issue #1234

Current Behavior

The above commit message, however, fails with the following:

⧗   input: test: footer-leading-blank bug
⚠   footer must have leading blank line [footer-leading-blank]

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

This happens when the following conditions are true:

1. The # symbol is present in the commit body
2. It is immediately followed by a number
3. It is not at the beginning of a line
4. It doesn't appear in the first line of the commit body

For instance, the following example, taken from the spec, also triggers this bug:

fix: correct minor typos in code

see the issue for details

on typos fixed.

Reviewed-by: Z
Refs #133

Affected packages

• cli
• core
• prompt
• config-angular

Steps to Reproduce

1. Create a repo with the below config file
2. In this repo, create a commit such as the above examples
3. Run commitlint --edit
4. An error such as the above appears

commitlint.config.js

module.exports = {
  extends: ['@commitlint/config-conventional'],
  rules: {
    'type-enum': [
      1,
      'always',
      [
        'build',
        'chore',
        'ci',
        'docs',
        'feat',
        'fix',
        'perf',
        'refactor',
        'revert',
        'style',
        'test'
      ]
    ]
  }
}

Context

I was referencing a GitHub issue in the standard GitHub format, which is #1234. This was inside a very detailed commit message with multiple lines.

Your Environment

Executable	Version
commitlint --version	8.3.4
git --version	2.20.1
node --version	10.17.0

[byCedric]

Thanks for reporting! I can see it's something related to parsing the commit. I think both Reviewed-by: Z and Refs #133 should be parsed as footer, right?

Currently it's parsed to:

{
  type: 'fix',
  scope: null,
  subject: 'correct minor typos in code',
  merge: null,
  header: 'fix: correct minor typos in code',
  body: 'see the issue for details\n\non typos fixed.\n\nReviewed-by: Z',
  footer: 'Refs #133',
  notes: [],
  references: [
    {
      action: null,
      owner: null,
      repository: null,
      issue: '133',
      raw: 'Refs #133',
      prefix: '#'
    }
  ],
  mentions: [],
  revert: null,
  raw: 'fix: correct minor typos in code\n' +
    '\n' +
    'see the issue for details\n' +
    '\n' +
    'on typos fixed.\n' +
    '\n' +
    'Reviewed-by: Z\n' +
    'Refs #133\n'
}

[sepehr]

I also confirm this bug.

[CrispyDrone]

Since this concerns the parser, I think there's also an issue with the parser not stripping out comments in the footer, see #846.

[sukima]

Confirmed with the following spec complaint commit message:

test(my-package): fancy description

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eu
facilisis nulla. Nunc orci massa, consectetur ut ante sit amet. Work
around found at:
https://github.com/redacted/redacted/issues/123#issuecomment-123456789

[n-rodriguez]

I confirm this bug too with this commit message :

chore(release): fix bug with npm and commitlint

Error: Cannot find module "@commitlint/config-conventional"
See: https://github.com/conventional-changelog/commitlint/issues/613#issuecomment-482794295

[thislooksfun]

I can also confirm this with the message:

chore: use del-cli instead of rm -rf

As @OmgImAlexis points out rm -rf doesn't work on Windows. This
implements part of #14.

[ProbstDJakob]

Maybe the following ANTLR grammar can be used to parse the commit message. ANTLR can generate code to parse the commit message. JavaScript is officially supported and a community driven generator for TypeScript also exists. The grammar below is strongly based upon the conventional commits specification.

grammar ConventionalCommits;

// needed to allow carriage returns not followed by a line feed
CARRIAGE_RETURN
  : '\r'
  ;

NEWLINE
  : CARRIAGE_RETURN? '\n'
  ;

BREAKING_CHANGE_HEADER_INDICATOR
  : '!'
  ;

// needed to allow hyphen which follows word(s) but has no trailing word (e.g. abc-def-)
INCOMPLETE_FOOTER_TOKEN
  : (WORD '-')+
  ;

FOOTER_TOKEN
  : INCOMPLETE_FOOTER_TOKEN WORD
  | BREAKING_CHANGE_TOKEN
  ;

BREAKING_CHANGE_TOKEN
  : 'BREAKING' (' ' | '-') 'CHANGE'
  ;

WORD
  : [a-zA-Z][a-zA-Z0-9]*
  ;

// needed to allow colons not followed by a space
COLON
  : ':'
  ;

// needed to allow spaces not followed by a hashtag
SPACE
  : ' '
  ;

COLON_SPACE
  : COLON SPACE
  ;

SPACE_HASHTAG
  : SPACE '#'
  ;

commit
  : header (NEWLINE body)? (NEWLINE footer)? EOF
  ;

header
  : type enclosedScope? BREAKING_CHANGE_HEADER_INDICATOR? COLON_SPACE summary
  ;

type
  : WORD
  ;

scope
  : WORD
  ;

enclosedScope
  : '(' scope ')'
  ;

summary
  : nonEmptyLine
  ;

body
  : nonEmptyNonFooterLine (NEWLINE* nonEmptyNonFooterLine)*?
  ;

line
  : (~NEWLINE)*? NEWLINE
  ;

nonEmptyLine
  : ~NEWLINE line
  ;

// a line which does not match a footer line (line != footer-token + footer-separator + footer-value)
nonEmptyNonFooterLine
  : ~(FOOTER_TOKEN | WORD | NEWLINE) line
  | (FOOTER_TOKEN | WORD) NEWLINE
  | (FOOTER_TOKEN | WORD) ~(COLON_SPACE | SPACE_HASHTAG | NEWLINE) line
  | (FOOTER_TOKEN | WORD) (COLON_SPACE | SPACE_HASHTAG) NEWLINE
  ;

footer
  : footerEntry+?
  ;

footerEntry
  : (FOOTER_TOKEN | WORD) (COLON_SPACE | SPACE_HASHTAG) footerValue
  ;

footerValue
  :  nonEmptyLine (NEWLINE* nonEmptyNonFooterLine)*?
  ;

[melink14]

I think the idea in #896 (comment) is a good idea since it is very explicit about the grammar. Of course, I think a similar less explicit solution could also be built using regex directly.

Do the maintainers have any opinion in case someone wants to submit a PR?

[melink14]

I think this would need to be fixed in https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-commits-parser but they've not been that open about matching the conventional commit spec.

I had forgotten but looked into this as part of conventional-changelog/conventional-changelog#415 (comment)

If commitlint passes through a config to the parser then the work around there would work though ideally the parser would be updated to implement the spec as written (or maybe that's the job of an extension?)

[melink14]

Parser Opts can be passed via the parser preset configuration: https://commitlint.js.org/#/reference-configuration?id=parser-presets

[escapedcat]

I think this would need to be fixed in https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-commits-parser but they've not been that open about matching the conventional commit spec.

Yeah, I agree. I think some of our issues are related to missing features in the parser. That's where they should be applied and fixed. Intead of trying to fix those in commitlint.

But it feels like the bigger issue is that conventional-changelog is that the maintainers don't have much time atm. Last updates have been done in Dec 2021.

[wtho]

Edit: This is now described in an own issue: #3120

---

Just in case someone tackles this at some point in the future:

This message also fails (due to the comment in the body, not because it is a number sign):

fix: failing example

body line 1
# comment in body breaks footer-leading-blank
body line 2

fixes #123

Try it out

[melink14]

This case might have a different root cause (though similar) since it complains about a leading blank line even if you add leading blank lines everywhere. It also correctly identifies the footer (in that demo app at least) even though it incorrectly says it's missing a leading line. If someone looks closer and root causes it, they may want to open a new issue if the root cause is not the same!

[wtho]

Alright, I opened a new issue for the body-comment bug #3120