rustdoc: Link references are case-insensitive

[ericseppanen]

EDIT(camelid): This is not an intra-doc links issue. See #80882 (comment).

---

I found a glitch in the std/core library docs. The docs for PartialEq contain this snippet:

/// This trait allows for partial equality, for types that do not have a full
/// equivalence relation. For example, in floating point numbers `NaN != NaN`,
/// so floating point types implement `PartialEq` but not [`Eq`].

At present, the Eq link has the wrong target, because rustdoc links it to the method eq instead.

I sent a PR that forces the correct linkage (#80864) for this instance, but perhaps rustdoc could be smarter here?

On that PR @jyn514 writes:

This is a bug in rustdoc for several reasons:

• It shouldn't treat identifiers as case-insensitive
• It should give a warning if the link is ambiguous

[jyn514]

@ericseppanen do you have time to make a smaller reproduction than the standard library? No problem if not, I just won't get around to it for a while.

[ericseppanen]

Sure, I'll give it a try!

[ericseppanen]

Everything works correctly under these cases:

• trait Foo and freestanding function foo
• trait Foo and impl Foo { fn foo() {} }
• trait Bar docs link to trait Foo, with a similarly named fn foo()

After staring at the PartialEq example in the standard library, I was able to reproduce the problem: we need an explicit link target specified at the end for the lowercase member fn. I think this is called a "link reference definition"?

This example shows the problem: the doc page for Bar has the wrong link target for Foo.

/// The `Bar` trait.
///
/// - Here is another trait: [`Foo`]
/// - Here is our similarly-named member fn: [`foo`]
///
/// [`foo`]: Bar::foo
///
pub trait Bar {
    /// The fn `foo` from the trait `Bar`
    fn foo();
}

/// This is the `Foo` trait.
pub trait Foo {}

[ericseppanen]

I can also make it broken in the opposite direction. If I replace it with

/// [`Foo`]: Foo

Then both links change. I.e. [`foo`] now links to Foo.

So the problem seems to be that link reference definitions are case-insensitive, and take over links to differently-cased names.

[camelid]

Darn, I'm pretty sure the issue is that pulldown-cmark treats link references as case-insensitive. I'm not sure what the standard is supposed to be though and I'm not sure if there's anything we can do it. We might be able to convince them to add an option to treat them as case-sensitive, but even then it might cause a lot of breakage for us.

By the way, you can see that it's not an intra-doc link issue with this code:

//! [foo]
//!
//! [FOO]: https://example.com

foo will be resolved to example.com.

[camelid]

@ericseppanen Thanks for debugging this!

[bugadani]

https://spec.commonmark.org/dingus/?text=%5Bfoo%5D%0A%0A%5BFOO%5D%3A%20https%3A%2F%2Fexample.com

The CommonMark reference implementation handles link references case insensitive.

As per the spec labels are case insensitive.

[jyn514]

I'm not sure what the standard is supposed to be though and I'm not sure if there's anything we can do it. We might be able to convince them to add an option to treat them as case-sensitive, but even then it might cause a lot of breakage for us.

That indeed looks like the issue :/ https://daringfireball.net/projects/markdown/syntax#link

Link definition names may consist of letters, numbers, spaces, and punctuation — but they are not case sensitive.

I don't know what we can do about this, other than to document that this is a footgun. @Manishearth what do you think?

[Manishearth]

I don't know what we can do about this, other than to document that this is a footgun. @Manishearth what do you think?

I don't really think there's a good way to fix this, aside from perhaps tweaking the markdown parser to preserve the original casing for unresolved link hooks

[camelid]

tweaking the markdown parser to preserve the original casing for unresolved link hooks

I don't think that would work, because the links are not treated as broken/unresolved.

[Thomasdezeeuw]

Could rustdoc maybe first look at existing paths (structs, function etc.), before using a case-insensitive match (not case-sensitive match) link. Making the following example work?

//! The function [`setup`].
//! The function again [`SETUP`].
//! The struct [`Setup`].
//!
//! [`setup`]: setup

pub struct Setup;

pub fn setup() -> Setup {
    Setup
}

Currently all three links go to the setup function. However the third link should go to the struct Setup, as that is a case-sensitive match.

[ericseppanen]

Does rustdoc follow the edition system? If the 2021 edition could declare that link references are now case-sensitive, would that help ease the pain of a breaking change?

[jyn514]

The problem is not the specification. It's very clear IMO that this is 'just a bug' and I don't think I would count fixing it as a breaking change. The problem is that this is just not how markdown works: markdown has case-insensitive links. We could ask https://github.com/raphlinus/pulldown-cmark to make a special case just for us, but I don't know if that's a coherent thing to ask for.

[ericseppanen]

The problem is that this is just not how markdown works

The reason this is so confusing to me is because the standard markdown piece (link references) have the same syntax as the non-standard piece (intra-doc links). There's no way for me to know without scrolling to the bottom whether [`Foo`] is an intra-doc link or not. Is it possible to refer to the same bit of link syntax as "standard markdown" and "nonstandard markdown", depending on context?

In the rustdoc-with-intra-doc-links flavor of markdown, it's confusing to have link references take priority over differently-cased intra-doc links. If I understand correctly, commenters above desire for link resolution to go in this order:

1. Link reference with matching case
2. Intra-doc link (with matching case)
3. Link reference with mismatched case

If that would already require special behavior from pulldown-cmark, then perhaps it would be equally valid to suggest just dropping # 3 going forward? That's the part that I was thinking of as a breaking change.

[ericseppanen]

Perhaps it would make sense to ask pulldown-cmark to implement a callback for all links, successful or broken, that allows the callback to either accept the parser's handling of the link or override it? That would be a relatively small change to pulldown-cmark, but it's powerful enough to allow rustdoc to have much better control over the link resolution process.