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

Intra-doc links cannot link to associated methods on type aliases. #86120

Closed
jonhoo opened this issue Jun 7, 2021 · 5 comments
Closed

Intra-doc links cannot link to associated methods on type aliases. #86120

jonhoo opened this issue Jun 7, 2021 · 5 comments
Assignees
Labels
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jonhoo
Copy link
Contributor

jonhoo commented Jun 7, 2021

I tried this code:

#[warn(rustdoc::all)]

pub struct Foo<C>(C);

impl<C> Foo<C> {
    pub fn new() -> Self {
        todo!()
    }
}

/// [`Bar::new`]
pub type Bar<C> = Foo<C>;

I expected to see this happen: Bar::new should be a link to Foo::new.

Instead, this happened: Bar::new was not turned into a link, and the compiler gave this warning:

warning: unresolved link to `Bar::new`
  --> src/lib.rs:11:6
   |
11 | /// [`Bar::new`]
   |      ^^^^^^^^^^ the type alias `Bar` has no associated item named `new`
   |
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: 1 warning emitted

I'm guessing that this isn't implemented because the forwarding may not always be obvious such as if there are multiple fn new for Foo for different values of C. But it'd be nice if it did work when there aren't and the choice is "clear".

EDIT: Though on second thought, that should also be an issue for

/// [`Foo::new`]

on Foo, so I feel like if that works this should also work.

Meta

rustc --version --verbose:

rustc 1.52.1 (9bc8c42bb 2021-05-09)
binary: rustc
commit-hash: 9bc8c42bb2f19e745a63f3445f1ac248fb015e53
commit-date: 2021-05-09
host: x86_64-apple-darwin
release: 1.52.1
LLVM version: 12.0.0

The same issue is present on nightly.

@jonhoo jonhoo added the C-bug Category: This is a bug. label Jun 7, 2021
@jonhoo
Copy link
Contributor Author

jonhoo commented Jun 7, 2021

This is a simplified variant of #85960.

jonhoo pushed a commit to jonhoo/smithy-rs that referenced this issue Jun 7, 2021
The standard syntax doesn't work:
rust-lang/rust#86120
@inquisitivecrystal
Copy link
Contributor

inquisitivecrystal commented Jun 8, 2021

@rustbot label +A-intra-doc-links +T-rustdoc

@rustbot rustbot added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name T-rustdoc labels Jun 8, 2021
rcoh added a commit to smithy-lang/smithy-rs that referenced this issue Jun 11, 2021
* Implement Debug for more things

* Extract out generic hyper client to smithy-hyper

* Add generic fluent client generation

* Make the bounds nicer

* Make smithy-hyper hyper dep optional

* Rename smithy-hyper to smithy-client

* Enable rustls by default

* Also warn on rust 2018 idioms

* Add type-erased middleware

* Restore old DispatchLayer tracing

* Add connection type-erasure

* Fix rustdoc link

* Split up lib.rs

* Make Builder a little nicer to use

* Make aws_hyper simply wrap smithy_client

* Make it clear that bounds:: should never be implemented

* Finish adjusting aws fluent generator

* Make clippy happy

* Also re-expose test_connection in aws_hyper

* Make ktlint happy

* No Builder::native_tls with default features

Since the function "doesn't exist", we can't link to it. Arguably, the
docs should only be tested with all features enabled, but for now just
don't try to link to `native_tls`.

* Work around rustdoc bug

rust-lang/rust#72081

* Better names for type-erase methods

* Add middleware_fn

* Better docs for client

* Fix remaining erase_connector

* Better name for service in docs

* Correct send+sync test name

* Use crate name with _ in Rust code

* Fix up relative links

The standard syntax doesn't work:
rust-lang/rust#86120

* Fix the new integration test

* Hide temporary Operation type aliases

* Don't bound middleware_fn as it also bounds C

With the extra "helpful" bound, we also end up enforcing that C
implements Service, but since we're in a builder, C may not have been
set yet, and may be (), which in turn means that it isn't Service. So
users would end up with an error if they write:

    Builder::new().middleware_fn(|r| r).https().build()

but it would work with

    Builder::new().https().middleware_fn(|r| r).build()

which is silly.

Co-authored-by: Russell Cohen <rcoh@amazon.com>
rcoh added a commit to smithy-lang/smithy-rs that referenced this issue Jun 11, 2021
…#496)

* Implement Debug for more things

* Extract out generic hyper client to smithy-hyper

* Add generic fluent client generation

* Make the bounds nicer

* Make smithy-hyper hyper dep optional

* Rename smithy-hyper to smithy-client

* Enable rustls by default

* Also warn on rust 2018 idioms

* Add type-erased middleware

* Restore old DispatchLayer tracing

* Add connection type-erasure

* Fix rustdoc link

* Split up lib.rs

* Make Builder a little nicer to use

* Make aws_hyper simply wrap smithy_client

* Make it clear that bounds:: should never be implemented

* Finish adjusting aws fluent generator

* Make clippy happy

* Also re-expose test_connection in aws_hyper

* Make ktlint happy

* No Builder::native_tls with default features

Since the function "doesn't exist", we can't link to it. Arguably, the
docs should only be tested with all features enabled, but for now just
don't try to link to `native_tls`.

* Work around rustdoc bug

rust-lang/rust#72081

* Better names for type-erase methods

* Add middleware_fn

* Better docs for client

* Fix remaining erase_connector

* Better name for service in docs

* Correct send+sync test name

* Use crate name with _ in Rust code

* Fix up relative links

The standard syntax doesn't work:
rust-lang/rust#86120

* Fix the new integration test

* Hide temporary Operation type aliases

* Don't bound middleware_fn as it also bounds C

With the extra "helpful" bound, we also end up enforcing that C
implements Service, but since we're in a builder, C may not have been
set yet, and may be (), which in turn means that it isn't Service. So
users would end up with an error if they write:

    Builder::new().middleware_fn(|r| r).https().build()

but it would work with

    Builder::new().https().middleware_fn(|r| r).build()

which is silly.

* Don't recursive infinitely

* Can only doc(inline) re-exports, not type alises

* Can only doc(inline) re-exports, not type alises

Co-authored-by: Russell Cohen <rcoh@amazon.com>
@LeSeulArtichaut
Copy link
Contributor

This can give some pretty surprising results:

pub struct Foo;

/// You should really try [`Self::bar`]
pub type Bar = Foo;

impl Bar {
    pub fn bar() {}
}

fails with

warning: unresolved link to `Bar::bar`
 --> test.rs:3:28
  |
3 | /// You should really try [`Self::bar`]
  |                            ^^^^^^^^^^^ the type alias `Bar` has no associated item named `bar`
  |
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: 1 warning emitted

@LeSeulArtichaut LeSeulArtichaut self-assigned this Jun 14, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 21, 2021
…iases, r=jyn514

Resolve type aliases to the type they point to in intra-doc links

This feels a bit sketchy, but I think it's better than just rejecting the link.
Helps with rust-lang#86120, r? `@jyn514`
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 21, 2021
…iases, r=jyn514

Resolve type aliases to the type they point to in intra-doc links

This feels a bit sketchy, but I think it's better than just rejecting the link.
Helps with rust-lang#86120, r? ``@jyn514``
@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed T-rustdoc-temp labels Jun 22, 2021
@jyn514
Copy link
Member

jyn514 commented Jul 2, 2021

@LeSeulArtichaut this was fixed by #86334, right?

@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented Jul 2, 2021

If we consider that the link not going to the type alias page is not a bug, this should be fixed

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-bug Category: This is a bug. 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