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

Resolve intra-doc-links on Self type implicitly #59039

Open
Manishearth opened this issue Mar 9, 2019 · 13 comments
Open

Resolve intra-doc-links on Self type implicitly #59039

Manishearth opened this issue Mar 9, 2019 · 13 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Manishearth
Copy link
Member

With intra-doc-links, the following works:

/// [Self::bar]
pub struct Foo;

impl Foo {
    /// [Self::bar]
    fn bar() {}
}

but this does not:

/// [bar]
pub struct Foo;

impl Foo {
    /// [bar]
    fn bar() {}
}

For structs and impls we should try resolving things against Self anyway

@Manishearth Manishearth added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Mar 9, 2019
@Centril
Copy link
Contributor

Centril commented Mar 9, 2019

Are you saying [bar] should work? If so, what if there's a free function bar in the module that's defined next to Foo?

@petrochenkov
Copy link
Contributor

petrochenkov commented Mar 9, 2019

I'd rather stick to the language rules if possible than create a subtly different dialect working only in rustdoc links.

@Manishearth
Copy link
Member Author

Pick the function first, if it exists, and if not look for the method. You can always disambiguate with full paths.

I don't see how this is a problem, intra doc links already does a bunch of fallbacky things, this would be another.

I'd rather stick to the language rules if possible.

I was going through intra doc failures in servo's deps (debugging a rustdoc regression and in the process noticing warnings) and it's common for people to do this and expect it to work already. It's a pretty common use case; method and type docs cross link to other methods all the time. We should support it.

I don't see why intra doc links (which already have a bunch of special syntax) should stick to the language rules.

@Manishearth Manishearth changed the title Resolve intr-doc-links on Self type implicitly Resolve intra-doc-links on Self type implicitly Mar 9, 2019
@Manishearth
Copy link
Member Author

(This may need an RFC, though, but it's worth getting it on file somewhere)

@Centril
Copy link
Contributor

Centril commented Mar 9, 2019

I don't see why intra doc links (which already have a bunch of special syntax) should stick to the language rules.

I think it could create confusion as to what the language rules are for some users. Such confusion will cease when users actually try bar and find that it doesn't work but it's still a surprise. Seems like a trade-offs to make between temporarily breaking mental models vs. convenience. You could argue that not having [bar] is the surprising behavior tho per "expect it to work already". Do we have more cases of where intra-doc-links do/should diverge from the language rules?

@Manishearth
Copy link
Member Author

I disagree that intra-doc-links are fundamentally viewed as paths in the same way. The main use case is to name types and methods (so, rarely full paths, usually Type and Type::method), to me the mental model is that it's a markdown link that autoreferences the relevant type if it can find one, and being able to find one is a best-effort thing. If you type [bar] you're saying "link me to bar", and the tool should be smart enough to understand what you mean. You are not saying "resolve bar in scope", that is how it works at the moment but I don't think it has to, nor do I think this is the mental model we should push for this feature.

This is the same kind of magic as doctests, IMO: nobody sees doctests and assumes that there's a way to write Rust programs without a main(). The documentation isn't code, I don't expect to see assumptions like this leaking through to code.

And yeah, like I said, people are trying this already! (I'll have to look for the crates again)

@Manishearth
Copy link
Member Author

Actually I think the best way to look at intra doc links is that if you're doing [`blah`](somelink) you should be able to omit the somelink in as many cases as possible

@Centril
Copy link
Contributor

Centril commented Mar 9, 2019

@Manishearth You make a good case overall. I still think there's a bit of a risk for some users, but it may be small enough to not matter in the grand scheme. An RFC sounds like a good place to flesh it all out.

@Manishearth
Copy link
Member Author

Sounds good. Might open one, depending on what the docs team feels about this feature overall.

@ehuss
Copy link
Contributor

ehuss commented Apr 5, 2020

Note to anyone that might be confused, the original example of /// [Self::bar] on a method does not work. It only works on the struct definition. The example presumably does not show any warnings because the method is private. See #70802.

@jyn514

This comment has been minimized.

@rustbot rustbot added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 10, 2020
@pickfire
Copy link
Contributor

I don't know if Self would be good but I think using the parent module would be good since it is quite common to do env::arg. In the docs, this will align with the parent item import.

@clarfonthey
Copy link
Contributor

Copying over what I said in a dupe issue since despite looking over every issue tagged intra-doc-links I missed this one:

I think if there's any ambiguity, there should be a lint to request you state self::item or Self::item to resolve the ambiguity. We should probably default to the former since it's the current behaviour.

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. 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

8 participants