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.

Sign up for GitHub

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

Jump to bottom

Handle fully-qualified syntax in intra doc links #74563

Open
Manishearth opened this issue Jul 20, 2020 · 2 comments
Open

Handle fully-qualified syntax in intra doc links #74563

Manishearth opened this issue Jul 20, 2020 · 2 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. P-low Low priority T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Manishearth
Copy link
Member

Related: #62834

#74489 makes it so that Type::Item works for trait items. There currently is anchor ambiguity between multiple traits that share an item, so it's not worth disambiguating further, however if we can fix that It would be nice if <Type as Trait>::Item could link to the appropriate bit in the type's impl section.

This would involve parsing the path as two paths, perhaps just using rustc_ast and parsing it as a TyKind::Path.
@Manishearth Manishearth added the A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name label Jul 20, 2020
@Manishearth Manishearth mentioned this issue Jul 20, 2020
Merged
@jyn514 jyn514 added 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. P-low Low priority labels Jul 20, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 23, 2020
@bors
Auto merge of rust-lang#74489 - jyn514:assoc-items, r=manishearth,pet…
8fdce9b 
…rochenkov

Fix intra-doc links for associated items

@Manishearth and I found that links of the following sort are broken:
```rust
$ cat str_from.rs
/// [`String::from`]
pub fn foo() {}
$ rustdoc str_from.rs
warning: `[String::from]` cannot be resolved, ignoring it.
 --> str_from.rs:4:6
  |
4 | /// [`String::from`]
  |      ^^^^^^^^^^^^^^ cannot be resolved, ignoring
```
It turns out this is because the current implementation only looks at inherent impls (`impl Bar {}`) and traits _for the item being documented_. Note that this is not the same as the item being _linked_ to. So this code would work:

```rust
pub trait T1 {
    fn method();
}

pub struct S;
impl T1 for S {
    /// [S::method] on method
    fn method() {}
}
```

but putting the documentation on `trait T1` would not.

~~I realized that writing it up that my fix is only partially correct: It removes the inherent impls code when it should instead remove the `trait_item` code.~~ Fixed.

Additionally, I discovered while writing this there is some ambiguity: you could have multiple methods with the same name, but for different traits:

```rust
pub trait T1 {
    fn method();
}

pub trait T2 {
    fn method();
}

/// See [S::method]
pub struct S;
```

Rustdoc should give an ambiguity error here, but since there is currently no way to disambiguate the traits (rust-lang#74563) it does not (rust-lang#74489 (comment)).

There is a _third_ ambiguity that pops up: What if the trait is generic and is implemented multiple times with different generic parameters? In this case, my fix does not do very well: it thinks there is only one trait instantiated and links to that trait:

```
/// [`String::from`] -- this resolves to https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from
pub fn foo() {}
```

However, every `From` implementation has a method called `from`! So the browser picks a random one. This is not the desired behavior, but it's not clear how to avoid it.

To be consistent with the rest of intra-doc links, this only resolves associated items from traits that are in scope. This required changes to rustc_resolve to work cross-crate; the relevant commits are prefixed with `resolve: `. As a bonus, considering only traits in scope is slightly faster. To avoid re-calculating the traits over and over, rustdoc uses a cache to store the traits in scope for a given module.
@jyn514 jyn514 mentioned this issue Sep 10, 2020
Closed
This was referenced Sep 18, 2020
Open
Merged
@jyn514 jyn514 changed the title Handle UFCS in intra doc links Handle fully-qualified syntax in intra doc links Sep 21, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 2, 2020 •
edited
Loading

This should also handle <dyn T>::f (found while working on tokio-rs/tracing#940):

/// [T::f]
pub trait T {}

impl dyn T {
    pub fn f() {
    }
}
warning: unresolved link to `T::f`
 --> tmp.rs:1:6
  |
1 | /// [T::f]
  |      ^^^^ the trait `T` has no associated item named `f`

camelid added a commit to camelid/rust that referenced this issue Oct 9, 2020
@camelid
Allow generic parameters in intra-doc links
4c765f6 
The contents of the generics will be mostly ignored (except for warning
if fully-qualified syntax is used, which is currently unsupported in
intra-doc links - see issue rust-lang#74563).

* Allow links like `Vec<T>`, `Result<T, E>`, and `Option<Box<T>>`
* Allow links like `Vec::<T>::new()`
* Warn on
  * Unbalanced angle brackets (e.g. `Vec<T` or `Vec<T>>`)
  * Missing type to apply generics to (`<T>` or `<Box<T>>`)
  * Use of fully-qualified syntax (`<Vec as IntoIterator>::into_iter`)
  * Invalid path separator (`Vec:<T>:new`)
  * Too many angle brackets (`Vec<<T>>`)
  * Empty angle brackets (`Vec<>`)

Note that this implementation *does* allow some constructs that aren't
valid in the actual Rust syntax, for example `Box::<T>new()`. That may
not be supported in rustdoc in the future; it is an implementation
detail.
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 10, 2020
@bors
Auto merge of rust-lang#76934 - camelid:rustdoc-allow-generic-params,…
b1af43b 
… r=jyn514

Allow generic parameters in intra-doc links

Fixes rust-lang#62834.

---

The contents of the generics will be mostly ignored (except for warning
if fully-qualified syntax is used, which is currently unsupported in
intra-doc links - see issue rust-lang#74563).

* Allow links like `Vec<T>`, `Result<T, E>`, and `Option<Box<T>>`
* Allow links like `Vec::<T>::new()`
* Warn on
  * Unbalanced angle brackets (e.g. `Vec<T` or `Vec<T>>`)
  * Missing type to apply generics to (`<T>` or `<Box<T>>`)
  * Use of fully-qualified syntax (`<Vec as IntoIterator>::into_iter`)
  * Invalid path separator (`Vec:<T>:new`)
  * Too many angle brackets (`Vec<<T>>`)
  * Empty angle brackets (`Vec<>`)

Note that this implementation *does* allow some constructs that aren't
valid in the actual Rust syntax, for example `Box::<T>new()`. That may
not be supported in rustdoc in the future; it is an implementation
detail.
@jyn514 jyn514 mentioned this issue Nov 13, 2020
Closed
@Tamschi
Copy link

Tamschi commented Feb 22, 2021

I'd also like to see <Type as Trait> (without member) to link directly to the respective implementation.

I use these links to highlight certain important implementations of custom traits on pre-existing types.

@ids1024 ids1024 mentioned this issue Apr 30, 2021
Merged
@digama0 digama0 mentioned this issue Jun 3, 2021
Open
@jyn514 jyn514 mentioned this issue Oct 18, 2021
Closed
27 tasks
@Erigara Erigara mentioned this issue Aug 12, 2022
Merged
csnover added a commit to csnover/binrw that referenced this issue Aug 29, 2022
@csnover
Improve links in docs for generated named arguments structs
b225168 
Probably needs rust-lang/rust#74563 for
the original intended behaviour.
csnover added a commit to csnover/binrw that referenced this issue Aug 30, 2022
@csnover
Improve links in docs for generated named arguments structs
0165f21 
Probably needs rust-lang/rust#74563 for
the original intended behaviour.
csnover added a commit to csnover/binrw that referenced this issue Aug 30, 2022
@csnover
Improve links in docs for generated named arguments structs
71f0e07 
Probably needs rust-lang/rust#74563 for
the original intended behaviour.
csnover added a commit to jam1garner/binrw that referenced this issue Aug 30, 2022
@csnover
Improve links in docs for generated named arguments structs
78f427d 
Probably needs rust-lang/rust#74563 for
the original intended behaviour.
@jyn514 jyn514 mentioned this issue Mar 18, 2023
Merged
@MarijnS95 MarijnS95 mentioned this issue Apr 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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. P-low Low priority T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Tamschi @Manishearth @jyn514