Skip to content
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

Doc links should count as a "use" with respect to the unused_imports lint #79542

Closed
sfackler opened this issue Nov 29, 2020 · 12 comments
Closed
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@sfackler
Copy link
Member

sfackler commented Nov 29, 2020

use std::collections::HashMap;

/// [`HashMap`]
pub fn foo() {}

Compiling this will warn about the HashMap import being unused, but it is required for the doc link to work. It would be nice if rustc handled this like Java's compiler does, where a doc link acts as a use of an import even when not building documentation. You can work around this with [HashMap](std::collections::HashMap) but long imports in the doc block can get a bit gross.

@sfackler sfackler added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Nov 29, 2020
@jyn514 jyn514 added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 29, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

This is tricky because the unused_import lint runs outside of rustdoc (and in general, the interaction between rustdoc and resolve is kind of a mess). I don't know if this is possible without first fixing #68427 and #72381.

@jyn514 jyn514 added E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Nov 29, 2020
@Mark-Simulacrum
Copy link
Member

I think this is likely unfixable in the general case without pulling all of the doc-comment parsing and evaluation machinery into rustc and running it.

That said I think you can work around this with e.g. cfg(doc) or whatever the cfg is?

@sfackler
Copy link
Member Author

Yeah, it would require inlining the doc-comment parsing, but that'd be nice as well to get the broken_intra_doc_links lint into normal builds rather than just doc builds. Definitely appreciate that there's a ton of work in the way of doing that though!

At one point IIRC there was an idea of having rustdoc just talk to rust-analyzer which seems like it could potentially unblock this as well?

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

That said I think you can work around this with e.g. cfg(doc) or whatever the cfg is?

This breaks cross-crate re-exports: #75855.

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

At one point IIRC there was an idea of having rustdoc just talk to rust-analyzer which seems like it could potentially unblock this as well?

I would be very nervous of regressions doing this, especially for associated items. IIRC rust-analyzer has some trouble with associated items still.

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

that'd be nice as well to get the broken_intra_doc_links lint into normal builds rather than just doc builds

I would want to improve the performance a lot more before doing this by default. #78761

@Lonami
Copy link
Contributor

Lonami commented Nov 30, 2020

Not a solution, but note you can work around:

You can work around this with [HashMap](std::collections::HashMap)

With the following instead:

/// [`HashMap`]
///
/// [`HashMap`]: std::collections::HashMap
pub fn foo() {}

Which is cleaner (rather than use you type out the link name again, but it's about the same amount of typing), and doesn't clutter the actual documentation that much.

@petrochenkov
Copy link
Contributor

pulling all of the doc-comment parsing and evaluation machinery into rustc and running it

Please, no. Rustdoc's link resolution is an ad-hoc mess very remotely related to path resolution rules in the language.
I don't want to pull it into the compiler and provide the compiler's stability guarantees for it.

@jyn514
Copy link
Member

jyn514 commented Nov 30, 2020

I agree with that actually - there are still some edge cases I'm hoping to change around primitives (on the theory that no one is using prim@ outside the standard library - maybe that should have been feature gated?) and it's easier to make changes like that in rustdoc than in rustc, since it will never break a working build.
#77875 (comment)

@jyn514
Copy link
Member

jyn514 commented Jan 10, 2021

I think I would want to see at least an MCP for this, it would be quite a lot of work (and like petrochenkov mentions, affect stability guarentees) for IMO not much benefit.

@dhardy
Copy link
Contributor

dhardy commented Mar 5, 2021

A shame this is rejected (for now), but understandable.


Regarding @Lonami's suggestion:

/// [`HashMap`]
///
/// [`HashMap`]: std::collections::HashMap
pub fn foo() {}

Actually, this becomes a mess as soon as you start to link to the same item from multiple doc-comments, or to methods/sub-paths such as HashMap::insert, HashMap::get, etc.

The cleaner solution often ends up being this (at risk of forgetting whether the import is actually needed):

#[allow(unused)]
use std::collections::HashMap;

/// [`HashMap`]
pub fn foo() {}

@ctron
Copy link

ctron commented Mar 3, 2023

You can also use:

/// This is a link to [`bar`](crate::some::module::bar).
fn foo() {}

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 A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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

7 participants