Use of `#` in commit body causes `closes` false positive

[myii]

Notes:

• This was first opened as an issue against semantic-release.
• This may be a duplicate of #391.

---

Current behavior

Had this happen a few times now but manually fixed previously. If a hash # appears in the commit body, it is seen as a closes in the release notes and changelog. An existing example follows.

Commit https://github.com/saltstack-formulas/template-formula/commit/3a072c7:

ci(travis): prevent `release` stage running for PRs

* The `release` stage will always fail due to security reasons:
  - E.g. https://travis-ci.com/saltstack-formulas/template-formula/jobs/180068519#L466.
  - Discussed: #42 (comment).
* The `release` stage is unnecessary for PRs until the merge to `master`, in any case.

Resulted in both:

• https://github.com/saltstack-formulas/template-formula/releases/tag/v0.7.2
• https://github.com/saltstack-formulas/template-formula/blob/master/CHANGELOG.md#072-2019-02-24

Specifically:

• travis: prevent release stage running for PRs (3a072c7), closes /travis-ci.com/saltstack-formulas/template-formula/jobs/180068519#L466 /github.com/saltstack-formulas/template-formula/pull/42#issuecomment-466446324

Expected behavior

The closes shouldn't have been present:

• travis: prevent release stage running for PRs (3a072c7)

Environment

• semantic-release version: 15
• CI environment: Travis CI
• Plugins used: changelog@3, exec@3 & git@7
• semantic-release configuration: release.config.js
• CI logs: Check under the Release stage

Further example that was manually fixed

• Commit: https://github.com/saltstack-formulas/template-formula/commit/068a94d
• Manual fix: https://github.com/saltstack-formulas/template-formula/commit/be4571d

[joelmukuthu]

I also observed the same: https://github.com/semantic-release/semantic-release/issues/1129

[stevemao]

@myii are you only using angular preset or do you supply your own https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-commits-parser#referenceactions?

[hutson]

@myii and @joelmukuthu do y'all have a small example project, and release explanation, that we can use to reproduce this issue? It was really help us to better investigate this issue.

[travi]

i was able to find the following in a few minutes:

• https://github.com/travi/travis-scaffolder-javascript/releases/tag/v1.7.0
• https://github.com/travi/travis-scaffolder-javascript/releases/tag/v1.8.0
• https://github.com/travi/travis-scaffolder-javascript/releases/tag/v1.7.1

in each case the referenced issues were of the form for #<issue-number> rather than closes #<issue-number> or fixes #<issue-number>. semantic-release is used to generate the release-notes, using conventional-changelog.

i'm sure i could track down several others if it would be helpful. happy to provide additional info that would be helpful if there are more details needed.

[ybiquitous]

Hi, it looks to me that this issue is reproduced also on my repository. 🤔

The commit is https://github.com/ybiquitous/ybiq/commit/0716e880a9092a61790d7cfd745a1ec88287b479.

The following is the build log on Travis CI. https://travis-ci.org/ybiquitous/ybiq/jobs/528663636#L607

Additional information:

• Use [email protected]
• Run the command: standard-version --sign --dry-run

I would be happy if my example could help the research. 😃

[melink14]

I'm using conventionalcommits and seeing this happening as well; is there some specific information that would help root cause this bug?

I see that conventional commits has an option called issuePrefixes which defaults to just # so maybe it's expected that bare issue references would get caught as closed issues.

I assume changing it to something like 'Fixes #' or 'FIXES: #' etc would solve the problem. Or maybe the issue is that the option isn't exposed in all presets or tools?

[melink14]

Adding 'Fixes #' did not work so I assume there's some filtering or processing.

I noticed there's a concept of reference actions which matches common prefexes like fixes and closes so it could be they get parsed first and thus don't match prefix.

For log.txt:

fix: something

See #55 for more details

Closes #56

I run with the default '#' prefix first:

npx conventional-commits-parser -i "#" log.txt | jq .:

[
  {
    "type": "fix",
    "scope": null,
    "subject": "something",
    "merge": null,
    "header": "fix: something",
    "body": null,
    "footer": "See #55 for more details\n\nCloses #56",
    "notes": [],
    "references": [
      {
        "action": null,
        "owner": null,
        "repository": null,
        "issue": "55",
        "raw": "See #55",
        "prefix": "#"
      },
      {
        "action": "Closes",
        "owner": null,
        "repository": null,
        "issue": "56",
        "raw": "#56",
        "prefix": "#"
      }
    ],
    "mentions": [],
    "revert": null
  }
]

Here we see that two references were created but only one has an action. Perhaps one solution to this issue is to only use action based references.

I also ran with a prefix like 'Closes #' and got nothing:

Γ¥» npx conventional-commits-parser -i "Closes #" log.txt | jq .:

[
  {
    "type": "fix",
    "scope": null,
    "subject": "something",
    "merge": null,
    "header": "fix: something",
    "body": "See #55 for more details\n\nCloses #56",
    "footer": null,
    "notes": [],
    "references": [],
    "mentions": [],
    "revert": null
  }
]

I tried various regexes as per: https://github.com/conventional-changelog/conventional-changelog/issues/271 but nothing came up.

[melink14]

One thing to note there is that the body has been merged into the footer after parsing and there may be logic which extracts all references from footers. (though See XXX is not uncommon in footer either)

edit: it seems the presence of a reference in the body marks it as a part of the footer by definition.

[melink14]

Update:

I stepped through a bit and found the proximate cause of this problem.

The code currently searches for references on every line of the commit message; usually the reference regex requires having an action before the issue number but in order to catch references everywhere, the code falls back to matching everything if there would otherwise be no matches: https://github.com/conventional-changelog/conventional-changelog/blob/318b555b6ef3aebdb5a8d4c7c734fd7234712dd3/packages/conventional-commits-parser/lib/parser.js#L39-L41

When there is an reference action, as I surmised the regex splits them into an action keyword and the rest meaning you can't customize an issuePrefix for this unless you also set referenceAction to some random string. (Or maybe empty list)

Also as expected, there's logic which assumes we must be in the footer, not body, if there is ever a reference found which doesn't make sense given references are intentionally allowed everywhere. (https://github.com/conventional-changelog/conventional-changelog/blob/318b555b6ef3aebdb5a8d4c7c734fd7234712dd3/packages/conventional-commits-parser/lib/parser.js#L229-L233)

This was originally done because Github started adding PR references to the header and people wanted those extracted. Interestingly, the reference in the header does not get added a closed issue in the changelog which implies there's some special casing for it later since it shows up as a normal reference in the parser output!

I think extracting references from anywhere makes sense from the parser's point of view so I'd guess the overall fix should exist at write time.

Suggestions

• When writing the 'closes' section of changelog, only add references that also have an action property.
• When deciding whether a line is body or footer, choose footer if there is an action otherwise choose body. This will mimic the previous behavior before the expanded reference parsing and seems reasonable.

Workaround

• Set referenceActions to a non default unlikely to appear naturally string.
• Add issuePrefixs such that they match the action and issue that you tend to use. This will mean that there will be no action property for those references so it's unfortunately not backwards compatible with the fixes above; something to watch out for.

[melink14]

Small update: I see the logic to exclude header references from 'closes' section is specific to conventionalcommits preset: https://github.com/conventional-changelog/conventional-changelog/blob/318b555b6ef3aebdb5a8d4c7c734fd7234712dd3/packages/conventional-changelog-conventionalcommits/writer-opts.js#L146

This seems like something that should probably be core especially if we want to generalize the notion of actionable vs non-actionable reference.

Though short term, updating that code to remove non actionable references would probably be pretty straightforward...

[vialcollet]

Hi all Is there any plan to fix this issue? Is there a workaround? Thanks!

[melink14]

https://github.com/conventional-changelog/conventional-changelog/issues/415#issuecomment-881862836 has a work around if you want to try it out!

[vialcollet]

Thanks @melink14 I don't understand how to put it in action though... I am using https://github.com/semantic-release/semantic-release

[melink14]

These are options to the parser so you can pass them to the commit analyzer and release notes generator to override the behavior. See https://github.com/semantic-release/release-notes-generator#options for examples.

The workaround listed above should definite prevent false positive closes but it might also prevent detecting intentional actions though I haven't looked closely at it in a long time.

It would probably not be hard to fix but I've not heard from the maintainers if they have opinions on if my suggestions make sense to them.