docs: use intra-RustDoc links rather than URLs

[hawkw]

Feature Request

Crates

• all

Motivation

Currently, all tracing crates use URL-based links for all RustDoc links, including internal links within the crate. However, switching to intra-RustDoc links has some advantages over URL-based links. In particular, we can now get a compiler warning (or error) when links are broken, using #[warn(intra_doc_link_resolution_failure)]. This would help prevent issues like #935.

Proposal

Intra-RustDoc links are currently available on nightly only, although they are planned for stabilization in Rust 1.47. However, we already build docs on nightly on both docs.rs and Netlify, so that we can use other nightly-only features like doc(cfg). We have a special cfg attribute that we tell docs.rs to pass when building the documentation

tracing/tracing/Cargo.toml

Lines 75 to 77 in b358201

	[package.metadata.docs.rs]
	all-features = true
	rustdoc-args = ["--cfg", "docsrs"]

and use it to guard nightly-only RustDoc features:

tracing/tracing/src/lib.rs

Line 819 in b358201

#![cfg_attr(docsrs, feature(doc_cfg))]

Therefore, we could add a

#![cfg_attr(docsrs, deny(intra_doc_link_resolution_failure))]

to get nice errors when links are broken, and switch all internal links to use intra-RustDoc links.

Alternatives

Making this change would have one major downside: it would mean that building the docs locally on a stable compiler would result in none of the links working. This is different from the current use of doc(cfg) attributes: when built on stable, the docs are still basically usable, they're just missing some additional information about feature flags. With links broken, the docs are significantly less usable. Therefore, we might want to leave things the way they are currently.

However, the rendered docs on Netlify (for branches) and on docs.rs (for published releases) would be rendered correctly, and these can be used instead in most cases. For the handful of users who do need to be able to build docs locally (such as when working on tracing crates offline), we could add a note to the CONTRIBUTING.md documentation on building docs locally, and state that docs should be built using RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc ....

It's worth noting that a majority of other major crates, including Tokio have determined that it's acceptable to require nightly for building docs...

[hawkw]

@davidbarsky what do you think about this?

[davidbarsky]

@hawkw Highly, highly in favor of this change. Requiring nightly to build docs isn't a concern for me nor is it a new line to be crossed.

[jyn514]

FYI the lint name has changed to broken_intra_doc_lints in recent nightlies: https://doc.rust-lang.org/nightly/rustdoc/lints.html#broken_intra_doc_links. +1 to more intra doc links though ❤️

#![cfg_attr(docsrs, deny(intra_doc_link_resolution_failure))]

Not that will only deny the lint on docs.rs, so you won't see it in CI. So your crate might fail to build on docs.rs even though it works locally. Maybe you want cfg_attr(doc, ...) instead?

[jyn514]

I should mention that intra-doc links are on by default in nightly, you don't need to enable them with a feature. deny is just about the level of lint to use.

[hawkw]

FYI the lint name has changed to broken_intra_doc_lints in recent nightlies: https://doc.rust-lang.org/nightly/rustdoc/lints.html#broken_intra_doc_links. +1 to more intra doc links though ❤️

Oh, thanks for letting us know!

#![cfg_attr(docsrs, deny(intra_doc_link_resolution_failure))]

Not that will only deny the lint on docs.rs, so you won't see it in CI. So your crate might fail to build on docs.rs even though it works locally. Maybe you want cfg_attr(doc, ...) instead?

Our CI job for building docs passes the docsrs cfg :) It's really just to enable nightly-only RustDoc features (IIRC the deny will fail to compile on stable, right?)

[jyn514]

It seems that the deny will actually completely be ignored on stable ... that seems like a rustdoc bug, it should warn the same way that rustc does 😅 I'll open an issue.

$ cat broken.rs 
#![deny(broken_intra_doc_links)]
/// [oops]
pub fn f() {}
$ rustdoc +nightly broken.rs 
error: unresolved link to `oops`
 --> broken.rs:2:6
  |
2 | /// [oops]
  |      ^^^^ unresolved link
  |
note: the lint level is defined here
 --> broken.rs:1:9
  |
1 | #![deny(broken_intra_doc_links)]
  |         ^^^^^^^^^^^^^^^^^^^^^^
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
$ rustdoc broken.rs  # no output
$ rustc broken.rs 
warning: unknown lint: `broken_intra_doc_links`
 --> broken.rs:1:9
  |
1 | #![deny(broken_intra_doc_links)]
  |         ^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unknown_lints)]` on by default

[TaKO8Ki]

Can I tackle this one?

[jyn514]

Should this have been closed? #981 gave warnings for broken intra-doc links, but most of the links are still using URLs.

[hawkw]

Whoops, I think you're right --- that PR shouldn't have included a "Closes" header for this issue. Thanks for catching that!

[jyn514]

I wrote up mentoring instructions for switching these in the standard library, you should be able to adapt them to tracing without much trouble: rust-lang/rust#75080

In particular, this should bring up most of the links:

rg '\[.*\]: .*(trait|struct|enum|fn)\..*\.html*'

[jyn514]

You can get 80% of the way there with this cursed regex:

rg -l '\[.*\]: .*(trait|struct|enum|fn)\..*\.html*' | grep '\.rs$' | xargs sed -i 's#\.\./\(.*\.html\)#super::\1#g; s#\([a-z]\)/\([a-z]\)#\1::\2#g; s#[a-z]\+\.\([a-zA-Z]\+\)\.html#\1#; s/\([a-z]\)#\(ty\)\?method./\1::/; s/#\(ty\)\?method./Self::/'