fix: keep duplicate link definitions in the tokens - #4132

Marked version: 18.1.0 (6a57018)

Markdown flavor: CommonMark|GitHub Flavored Markdown

Description

When a label is defined twice, the lexer stores only the first definition in links, as CommonMark says, but it also leaves the later def token out of the token list. Its text is then in no token, so the top-level raw values no longer join back to the source, although the docs describe raw as all of the text the token consumes. Spec examples #204 and #544 show it:

marked.lexer('[foo]: /url1\n\n[foo]: /url2\n\n[bar][foo]\n').map(t => t.raw).join('')
// '[foo]: /url1\n\n\n\n[bar][foo]\n', `[foo]: /url2` is in no token

marked.lexer('[foo]\n\n[foo]: first\n[foo]: second\n')
// the def token has raw '[foo]: first\n\n', and `[foo]: second` is in no token

The newline after a dropped definition is attached to the token before it, or becomes a space token when there is none, which can make a list item loose:

Expectation

[foo]: /a

- [foo]: /b
  [foo]: /c
  [foo]

<ul>
<li><a href="/a">foo</a></li>
</ul>

as commonmark.js renders it.

Result

<ul>
<li><p><a href="/a">foo</a></p>
</li>
</ul>

What was attempted

Found by checking that the raw values of the top-level tokens join back to the source: of the 652 CommonMark and 28 GFM spec examples, only #204 and #544 fail, both for this reason.

Now the def token is pushed for every definition, and links still keeps only the first one of a label. renderer.def returns an empty string, so the HTML changes only where a dropped definition had changed the structure around it. In my tests that was list items like the one above, and lazy lines after such a definition in a blockquote, which ended the blockquote. On 20,000 distinct generated documents with gfm: false, 54 render differently: 47 now match commonmark.js and did not before, 7 match it neither before nor after, and none stopped matching. The spec examples render the same as before. Custom def renderers and walkTokens now also get the later definitions.

(Edited: my first numbers counted some generated documents twice. These are from 20,000 distinct documents.)

The new duplicate label lexer test and test/specs/new/def_duplicate_label both fail on master. Specs, unit, umd, cjs, types and lint pass locally, except the five bin exec tests, which fail on master too because my checkout path has a space in it.

Contributor

• Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
• no tests required for this PR.
• If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

• CI is green (no forced merge required).
• Squash and Merge PR following conventional commit guidelines.