rustdoc is parsing email link literals in docs as intradoc links in the latest nightly

[lopopolo]

I tried this code:

/// Information about an Artichoke build.
///
/// Implementations of this trait are used to set global Ruby constants that
/// describe the current build.
pub trait ReleaseMetadata {
    /// Copyright information.
    ///
    /// This value will populate the `RUBY_COPYRIGHT` constant.
    ///
    /// # Examples
    ///
    /// > artichoke - Copyright (c) 2019-2020 Ryan Lopopolo <rjl@hyperbo.la>
    fn ruby_copyright(&self) -> &str;
}

I expected to see this happen: rustdoc parses the link as an email address and renders it as a mailto: link.

Instead, this happened:

• https://twitter.com/artichokeruby/status/1378788852261736450
• Escape email from example docs for ReleaseMetadata::ruby_copyright artichoke/artichoke#1152

error: unknown disambiguator `rjl`
  --> artichoke-core/src/release_metadata.rs:18:62
   |
18 |     /// > artichoke - Copyright (c) 2019-2020 Ryan Lopopolo <rjl@hyperbo.la>
   |                                                              ^^^
   |
   = note: requested on the command line with `-D rustdoc::broken-intra-doc-links`

Meta

/usr/share/rust/.cargo/bin/rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/runner/.rustup

info: syncing channel updates for '1.51.0-x86_64-unknown-linux-gnu'
info: latest update on 2021-03-25, rust version 1.51.0 (2fd73fabe 2021-03-23)
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: using up to 500.0 MiB of RAM to unpack components
info: installing component 'rust-std'
info: installing component 'rustc'
1.51.0-x86_64-unknown-linux-gnu (overridden by '/home/runner/work/artichoke/artichoke/rust-toolchain')
rustc 1.51.0 (2fd73fabe 2021-03-23)
/usr/share/rust/.cargo/bin/rustup -V
rustup 1.23.1 (3df2264a9 2020-11-30)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.51.0 (2fd73fabe 2021-03-23)`
Installed rustup 1.23.1 support profiles
/usr/share/rust/.cargo/bin/rustup set profile minimal
info: profile set to 'minimal'
/usr/share/rust/.cargo/bin/rustup toolchain install nightly
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: latest update on 2021-04-04, rust version 1.53.0-nightly (0b417ab5c 2021-04-03)
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: installing component 'cargo'
info: using up to 500.0 MiB of RAM to unpack components
info: installing component 'rust-std'
info: installing component 'rustc'

  nightly-x86_64-unknown-linux-gnu installed - rustc 1.53.0-nightly (0b417ab5c 2021-04-03)

info: checking for self-updates
/usr/share/rust/.cargo/bin/rustup override set nightly
info: using existing install for 'nightly-x86_64-unknown-linux-gnu'
info: override toolchain for '/home/runner/work/artichoke/artichoke' set to 'nightly-x86_64-unknown-linux-gnu'

  nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.53.0-nightly (0b417ab5c 2021-04-03)

(from a GitHub Actions run, the latest nightly won't install on my Arch box due to broken components)

[lopopolo]

This turned out to be a bug in my docs; I didn't intend for this to be rendered as a mailto: link, but I was hoping for a better error message.

[lopopolo]

I think this PR introduced this new behavior: #83543.

[jyn514]

This is weird - rustdoc should be ignoring any link that has . in it:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Line 988 in 8ad6a44

if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ":_<>, !*&;".contains(ch))) {

[jyn514]

Ah I see the issue - disambiguator_error should only be reported if the link isn't ignored. @camelid do you have time to fix it? It should be as simple as adding the same check here:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Line 983 in 8ad6a44

disambiguator_error(self.cx, &item, dox, disambiguator_range, &err_msg);

[camelid]

Assigning P-medium and removing I-prioritize as discussed in the prioritization working group.

@rustbot label: +P-medium -I-prioritize

[lopopolo]

@jyn514 @camelid can part of the fix here be to improve the error message?

"Unknown disambiguator" doesn't tell me much about what is wrong or how to fix it.

[jyn514]

@lopopolo I'm confused. After #83865 there won't be an error message, because the link shouldn't have been treated as an intra-doc link in the first place. Are you asking for a lint that warns about docs that will be rendered as mailto: links? I'm not sure how useful that is, mailto: links are normal and reasonable to write.

[lopopolo]

@jyn514 based on the UI tests that are checked in, if I actually intend to use an intradoc link with a disambiguator and fat finger mod@ as nod@, the error message is not super friendly.

[jyn514]

Hmm, what would you like to see instead? There's quite a few valid disambiguators, I'm not sure it makes sense to list them all. Maybe we could add a header to the disambiguator section of https://doc.rust-lang.org/rustdoc/linking-to-items-by-name.html and link there?

[lopopolo]

Ideally I'd like to see a suggestion to use a disambiguator that corresponds to an item that is currently in scope or otherwise resolvable via the path after the invalid disambiguator.

[jyn514]

Hmm, that seems useful, although I think it should be a separate issue. Maybe we could try resolving without a disambiguator and see if there are any results? That would need a pretty significant overhaul of how it works currently, right now all the errors are emitted eagerly which would end up with a lot more verbose output that might not be relevant.

What would you expect to see if the link isn't valid even without the disambiguator?

[lopopolo]

@jyn514 I think an extra note to more docs on intradoc link disambiguators would be helpful per your previous comment.

The idea to attempt to resolve items for all invalid disambiguators feels similar to the Ruby CLI and REPL's "did you mean" feature, which I find quite useful.

[pietroalbini]

The PR improving the diagnostics is being reverted in #84950, reopening the issue.

[camelid]

@pietroalbini I don't think this is still a regression; the diagnostics improvement was not part of the fix for the regression. (The regression was fixed in #83865.)

[jyn514]

Yeah, #83866 never had a tracking issue.

[jyn514]

#83866 was relanded in #85052.