rustdoc: Incorrect parsing of links followed by parens

[lilyball]

rustdoc seems to be parsing links incorrectly when they're followed by parens. The general theme is that part of rustdoc is right-biased in parsing links, e.g. [Foo][](bar) is parsed like [Foo] and [](bar), but part of rustdoc parses it correctly. The observed behavior here is rather confusing, and it gets worse once you nest another link in the parens.

Examples

Basic link

//! [Some][](value)

Output: Some(value)

<a href="https://doc.rust-lang.org/1.56.1/core/option/enum.Option.html#variant.Some" title="Some">Some</a>
(value)

The HTML output is correct but it emits a spurious warning:

warning: unresolved link to `value`
 --> src/lib.rs:1:14
  |
1 | //! [Some][](value)
  |              ^^^^^ no item named `value` in scope
  |
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

Nested link

If we put another link inside the parens, the warning goes away but the nested link is not parsed:

//! [Some][]([Ok][])

Output: Some([Ok][])

<a href="https://doc.rust-lang.org/1.56.1/core/option/enum.Option.html#variant.Some" title="Some">Some</a>
([Ok][])

Nested link with parens

If we put parens after the nested link, the nested link is then parsed, but parsed incorrectly. The output makes this clear. Again, no warnings:

//! [Some][]([Ok][](value))

Output: Some([Ok])

HTML Output:

<a href="https://doc.rust-lang.org/1.56.1/core/option/enum.Option.html#variant.Some" title="Some">Some</a>
([Ok]
<a href="value"></a>
)

Nested link with parens followed by link to same item

Perhaps the most bizarre behavior here is if I simply include the nested link again elsewhere in the same doc comment, it parses correctly (and with no warnings). I've placed it on the same line for simplicity in this example, but the link can be anywhere in the doc comment.

//! [Some][]([Ok][](value)) or maybe [Ok][]

Output: Some(Ok(value)) or maybe Ok

<a href="https://doc.rust-lang.org/1.56.1/core/option/enum.Option.html#variant.Some" title="Some">Some</a>
(
<a href="https://doc.rust-lang.org/1.56.1/core/result/enum.Result.html#variant.Ok" title="Ok">Ok</a>
(value)) or maybe 
<a href="https://doc.rust-lang.org/1.56.1/core/result/enum.Result.html#variant.Ok" title="Ok">Ok</a>

Interestingly, it's not the link text that has to be the same, but the link destination. This example works equally well if I replace the second link with [Nope][Ok] but breaks if I use e.g. [Ok](http://example.com).

Meta

rustc --version --verbose:

rustc 1.56.1 (59eed8a2a 2021-11-01)
binary: rustc
commit-hash: 59eed8a2aac0230a8b53e89d4e99d55912ba6b35
commit-date: 2021-11-01
host: x86_64-apple-darwin
release: 1.56.1
LLVM version: 13.0.0

This occurs on nightly too

rustc +nightly --version --verbose:

rustc 1.58.0-nightly (82af160c2 2021-11-10)
binary: rustc
commit-hash: 82af160c2cb9c349a0373cba98d8ad7f911f0d34
commit-date: 2021-11-10
host: x86_64-apple-darwin
release: 1.58.0-nightly
LLVM version: 13.0.0

[lilyball]

Providing a link reference definition also makes the final example work:

//! [Some][]([Ok][](value))
//! 
//! [Ok]: enum@Ok

Output: Some(Ok(value))

<a href="https://doc.rust-lang.org/1.56.1/core/option/enum.Option.html#variant.Some" title="Some">Some</a>
(
<a href="enum@Ok">Ok</a>
(value))

[lilyball]

[Some][]([Some][](value)) also works, presumably for the same reason that providing the link reference definition works.

[lilyball]

Putting something that's not validly parsed as a link destination in the parens also works. [Some][]([Ok][] foo) parses corectly, and [Some][]([Ok][](value) foo) has the same behavior as [Some][](value).

My best guess here is we have two passes over the text. The first collects all link destinations, and is buggy in that it tries to parse [Some][](…) as both [Some][] and [](…) (if … matches the parsing rules for a link destination). Also because of this, it doesn't parse the contents of … and so it misses any nested links. The second pass then uses those link destinations to convert the markdown into HTML.

For [Some][](value), the first pass collects [Some][] and emits a warning on [](value). The second pass converts [Some][] into a link as the link destination is known.

For [Some][]([Ok][]), the first pass does not parse the [Ok][] for link destinations, and does not emit a warning as it's not an intra-doc link. The second pass then has no link destination for [Ok][] and so leaves it as text in the output.

For [Some][]([Ok][](value)), the first pass does the same. The second pass has no link destination for [Ok][], but it does see that [](value) matches an inline link and converts that. This second pass behavior actually appears to be spec-compliant which surprises me (apparently parsing of reference links is contingent upon the link label matching a link reference definition).

For [Some][]([Ok][](value)) or maybe [Ok][] the first pass collects the link reference definition for [Ok][] and so the second pass is able to convert [Ok][] into a link and leave (value) as text.

---

Ultimately, it appears that the first pass is simply buggy, both in that it emits a warning for [Some][](value) on the (value) even though the [Some][] is a link and thus (value) isn't, and in that it refuses to parse the contents of value here for more links, thus causing the second pass to mis-parse.

[jyn514]

Thanks for the detailed write-up, but I'm pretty sure this a duplicate of #83677, which we aren't going to fix - the markdown is ambiguous and there's two ways to interpret each of the examples you posted, it's not left associative.

[lilyball]

That doesn't look right. Or rather, it looks like it is the same issue, but the resolution there is wrong.

The issue is that [a][b](c) is ambiguous markdown syntax: it could either be

1. a link to b titled a, followed by the literal text (c), which you seemed to intend, or
   the literal text [a], with a link to c titled b.

Rustdoc is using the second meaning here, and when c fails to resolve it falls back to the first meaning.

CommonMark defines a shortcut reference link as

A shortcut reference link consists of a link label that matches a link reference definition elsewhere in the document and is not followed by [] or a link label.

That very clearly means [a] is not a shortcut reference link. So the choice is between [a][b] and [b](c). If [a][b] can be resolved as a link, then it should be, which leaves (c) as literal text.

Based on the parsing algorithm documented in the appendices, links are left-biased, meaning if the [a][b] can be parsed as a link, it must be. So [b](c) should only be considered a link if [a][b] fails to resolve to a link.

There certainly should never be a possibility of "c failed to resolve, so we fall back to [a][b] instead", both because [b](c) is an inline link rather than a link reference and so if c fails to resolve we're still left with <a href="c">b</a>, and because [a][b] has precedence.

[lilyball]

It really seems to me here that the issue is in the pass that collects intra-doc links and synthesizes link references for them. This is the pass that decides to treat (value) as part of a link and emit a warning and not collect links from inside. This is why producing a link to the same item elsewhere suddenly changes the parsing here.

In theory, rustdoc should be parsed as though every possible intra-doc link is already defined. That's impractical of course, so having a pass to collect all intra-doc links seems reasonable, it just needs to a) parse pessimistically and b) not emit warnings about references to unknown items until the full markdown conversion part has finished and actually determined whether or not that was part of a link.

By "parse pessimistically" I mean when confronted with something like [a][b](c), since it appears to not yet be able to figure out whether [a][b] is a valid link (I'm guessing it collects all names first, then looks them up at the end), it should collect the b name there, then proceed to treat [b](c) as a link (which it's doing), then, and here's the new behavior, it should recurse into c under the assumption that perhaps the [b](c) doesn't end up being a link and that c might need to be parsed as links instead.

[camelid]

I'm pretty sure all parsing decisions about what's a link happens outside of rustdoc, in pulldown-cmark. The intra-doc links pass just does name resolution on the parsing result. So, I believe this is already the case:

In theory, rustdoc should be parsed as though every possible intra-doc link is already defined.

[lilyball]

@camelid In that case an issue needs to be raised upstream with pulldown-cmark on how the broken link callback works.

[camelid]

cc @marcusklaas again :)