Skip to content

Rustdoc should warn about unused reference links #62345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
asomers opened this issue Jul 3, 2019 · 8 comments
Open

Rustdoc should warn about unused reference links #62345

asomers opened this issue Jul 3, 2019 · 8 comments
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@asomers
Copy link
Contributor

asomers commented Jul 3, 2019

rustdoc can easily generate external links by using a full URL as the target. However, this only works with inline links. It doesn't work when using footnote links, sometimes called "reference links". If you try to use a full URL as the target of a footnote link, rustdoc will throw up a intra_doc_link_resolution_failure warning. Here's an example.

src/libs.rs:

//!
//! This is an inline link to [`CStr`](https://doc.rust-lang.org/stable/std/ffi/struct.CStr.html).
//! It works!
//!
//! This is an intra-crate footnote link to [`MyType`][MyType].  It also works.
//!
//! This is a footnote link, aka "reference link", to [`OsStr`](OsStr).  It fails :( .
//!
//! [MyType]: t/struct.MyType.html
//! [OsStr]: https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html
pub mod t {
    pub struct MyType {}
}
> cargo +nightly doc --no-deps
 Documenting external_reference_link v0.1.0 (/tmp/external_reference_link)
warning: `[OsStr]` cannot be resolved, ignoring it...
 --> src/lib.rs:7:65
  |
7 | //! This is a footnote link, aka "reference link", to [`OsStr`](OsStr).  It fails :( .
  |                                                                 ^^^^^ cannot be resolved, ignoring
  |
  = note: #[warn(intra_doc_link_resolution_failure)] on by default
  = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

    Finished dev [unoptimized + debuginfo] target(s) in 1.06s
> rustup run nightly rustc --version
rustc 1.36.0-nightly (7d5aa4332 2019-05-16)

The workaround is to only use inline links for external targets. But that uglies up the code, and it requires duplication if the same comment block needs to instantiate the link multiple times.

@jonas-schievink jonas-schievink added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 3, 2019
@asomers
Copy link
Contributor Author

asomers commented Jul 3, 2019

There seem to be a few exceptions to the rule. The link tags str and cfg-if work fine. Perhaps rustdoc always interprets them as external links because the aren't valid identifiers?

asomers added a commit to asomers/mockall that referenced this issue Jul 3, 2019
There were three problems:

* I hadn't thoroughly updated Mockall's docs since the omnimacro
  project.  There were still some links to structures that are now
  autogenerated.

* Fix a few grammar issues.

* Workaround a rustdoc bug.  Rustdoc can't handle footnote links to
  external sites.  I had to convert them to internal links.

* Deny intra_doc_link_resolution_failure, so I'll catch issues like the
  Rustdoc bug faster.

rust-lang/rust#62345
@euclio
Copy link
Contributor

euclio commented Jul 3, 2019

I think you just have incorrect markdown syntax? Anything inside the parentheses will be interpreted as a link. If you use square brackets or remove the parentheses entirely it should work as expected.

@asomers
Copy link
Contributor Author

asomers commented Jul 3, 2019

Yes, I had a syntax error in the snippet that I posted. Thanks for pointing it out; it helped me narrow the problem. It looks like the real problem is that if you leave out the link name, rustdoc will try to interpret the presentation name as an intra-crate symbol, without checking whether or not a footnote exists. Oddly, if the presentation name isn't a valid identifier, then rustdoc will ignore it without printing a warning. Here's a corrected example:

/! This is an inline link to [`CStr`](https://doc.rust-lang.org/stable/std/ffi/struct.CStr.html).
//! It works!
//!
//! This is an intra-crate footnote link to [`MyType`][MyType].  It also works.
//!
//! This is a footnote link, aka "reference link", to [`OsStr`][OsStr].  It work
s.
//!
//! This is an implied footnote link to [`CString`].  It fails :( .
//!
//! This is an implied link that can't be an identifier [`some-thing`].  It fails, but doesn't
//! print a warning.
//!
//! [MyType]: t/struct.MyType.html
//! [OsStr]: https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html
//! [CString]: https://doc.rust-lang.org/stable/std/ffi/struct.CString.html
//! [some-thing]: http://some-thing.com
pub mod t {
    pub struct MyType {}
}
> cargo +nightly doc --no-deps
 Documenting t v0.1.0 (/tmp/t)
warning: `[CString]` cannot be resolved, ignoring it...
 --> src/lib.rs:8:42
  |
8 | //! This is an implied footnote link to [`CString`].  It fails :( .
  |                                          ^^^^^^^^^ cannot be resolved, ignoring
  |
  = note: #[warn(intra_doc_link_resolution_failure)] on by default
  = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

    Finished dev [unoptimized + debuginfo] target(s) in 0.96s

I feel a little dumb for not figuring this out more quickly. Would it be a reasonable feature request to ask rustc do check if a presentation name could point to a footnote instead of only being an identifier, or should I just forget about it?

@euclio
Copy link
Contributor

euclio commented Jul 4, 2019

Your markdown is still incorrect. The link is pointing to `CString` but your footnote is CString. They need to match exactly, otherwise the link will be detected as a "broken link" which triggers the check for an intra-doc link.

There's certainly room for improvement on the diagnostic, though. Maybe we could do a Levenshtein check on footnotes to see if there's a typo and suggest it?

@asomers
Copy link
Contributor Author

asomers commented Jul 4, 2019

You're right; that fixed it. Your fix would be helpful, but a simpler solution might be to warn for any unreferenced footnotes. Would that be easier?

@euclio
Copy link
Contributor

euclio commented Jul 4, 2019

Both solutions may need additional pulldown-cmark support. I'm not sure that it's possible to extract unused footnotes with the current API.

@jonas-schievink jonas-schievink added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. labels Jul 29, 2019
@jyn514

This comment has been minimized.

@rustbot rustbot added C-enhancement Category: An issue proposing an enhancement or a PR with one. S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed C-bug Category: This is a bug. labels Jul 10, 2020
@jyn514 jyn514 changed the title rustdoc treats external footnote links as intra crate links Rustdoc should warn about unused reference links Jul 18, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 23, 2020

This might be fixable with the current pulldown API using the strategy in pulldown-cmark/pulldown-cmark#466 (comment).

@jyn514 jyn514 added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants