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

chore: audit all doc links #1473

Closed
ipetkov opened this issue Aug 18, 2019 · 17 comments
Closed

chore: audit all doc links #1473

ipetkov opened this issue Aug 18, 2019 · 17 comments
Labels
A-tokio Area: The main tokio crate A-tokio-util Area: The tokio-util crate C-maintenance Category: PRs that clean code up or issues documenting cleanup. T-docs Topic: documentation

Comments

@ipetkov
Copy link
Member

ipetkov commented Aug 18, 2019

Many of the doc links currently point to arbitrary URLs or don't actually link anywhere.

We should instead rely on rustdoc's resolution feature to link to the appropriate item, and block the CI on any errors/warnings there.

@ipetkov ipetkov added the T-docs Topic: documentation label Aug 18, 2019
@ipetkov ipetkov added this to the v0.2 milestone Aug 18, 2019
@carllerche
Copy link
Member

@ipetkov that would be great. Historically, that option wasn't mature enough, but maybe that works now 👍

@ghost
Copy link

ghost commented Apr 1, 2020

Would we like to replace all static links by some kind of script?
So the script could replace for example
struct.DelayQueue.html#method.insert
with
tokio::stream::StreamMap::insert
(or whatever the correct path look like).

Potentially helpful link here
https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md

@Darksonn
Copy link
Contributor

Darksonn commented Apr 1, 2020

Using ripgrep you can quickly find a lot of these:

rg '(struct|trait|enum|macro|fn|attr|type)\.[a-zA-Z0-9_]*\.html|#method.'

@ghost
Copy link

ghost commented Apr 6, 2020

I've made some changes #2381

Next steps

  1. fix broken links in these files
    target/doc/tokio/runtime/struct.Runtime.html, line 2, col 32
    target/doc/tokio/net/struct.UdpSocket.html, line 19, col 41
    target/doc/tokio/io/struct.Error.html, line 1, col 4928
    target/doc/tokio/io/struct.Error.html, line 1, col 4983
    target/doc/tokio/io/struct.Error.html, line 1, col 5040
    target/doc/tokio/io/struct.Error.html, line 201, col 1294
    target/doc/tokio/net/struct.TcpListener.html, line 76, col 32
    target/doc/tokio/time/struct.DelayQueue.html, line 28, col 1
    target/doc/tokio/io/index.html, line 159, col 234
    target/doc/tokio/task/struct.LocalSet.html, line 27, col 37
    target/doc/tokio/time/delay_queue/struct.DelayQueue.html, line 28, col 1
    target/doc/tokio/task/struct.LocalSet.html, line 131, col 21
    target/doc/tokio/task/struct.LocalSet.html, line 132, col 31
  2. replace all left #methods reference. Would be helpful to resolve Resolve intra-doc-links on Self for method rust-lang/rust#70802 first and than fix it with a script
  3. review what left

carllerche pushed a commit that referenced this issue Apr 12, 2020
Included changes
- all simple references like `<type>.<name>.html` for these types
    - enum
    - fn
    - struct
    - trait
    - type
- simple references for methods, like struct.DelayQueue.html#method.poll

Refs: #1473
@ghost
Copy link

ghost commented Apr 13, 2020

Next steps

1. fix broken links in these files

Broken links are fixed in #2400

However, some of the links from #1473 (comment) couldn't be fixed, here's the overview

  1. links on https://docs.rs/tokio/0.2.18/tokio/io/struct.Error.html
    1. https://docs.rs/tokio/0.2.18/tokio/io/trait.Read.html
    2. https://docs.rs/tokio/0.2.18/tokio/io/trait.Write.html
    3. https://docs.rs/tokio/0.2.18/tokio/io/trait.Seek.html
    4. https://docs.rs/tokio/0.2.18/tokio/ffi/struct.NulError.html

Should be fixed in rustdoc: rust-lang/rust#65983 because they are re-exports

  1. links on https://docs.rs/tokio/0.2.18/tokio/time/struct.DelayQueue.html#implementation
    1. https://docs.rs/tokio/0.2.18/tokio/struct.Timer.html

Docs should re rephrased, because according to

git log 27e5b41067d01c0c9fac230c5addb58034201a63`
  • tokio::timer::Timer is renamed to Driver and made private.

So Timer is now Driver, but Driver shouldn't be used in public docs.

  1. links on https://docs.rs/tokio/0.2.18/tokio/task/struct.LocalSet.html#notes
    1. https://docs.rs/tokio/0.2.18/tokio/blocking/fn.in_place.html

What is in-place blocking in the docs (see below or the link above).
Perhaps it should be replaced with https://docs.rs/tokio/0.2.18/tokio/task/fn.block_in_place.html if this makes sense (can't tell for my own) or the doc should be reworded.

Since this function internally calls Runtime::block_on, and drives futures in the local task set inside that call to block_on, the local futures may not use in-place blocking.

@Darksonn
Copy link
Contributor

Hey @xliiv, thank you for doing this!

The in-place blocking link should indeed be a link to block_in_place, as you suggested.

@ghost
Copy link

ghost commented Apr 13, 2020

The in-place blocking link should indeed be a link to block_in_place, as you suggested.

Thanks, updated #2400 with 3f46ba7

@ghost
Copy link

ghost commented Apr 13, 2020

  1. links on docs.rs/tokio/0.2.18/tokio/time/struct.DelayQueue.html#implementation

    1. https://docs.rs/tokio/0.2.18/tokio/struct.Timer.html

Fixed in e19a0d4 commit and discussed in the thread starting with this comment #2400 (comment)

@Darksonn
Copy link
Contributor

Links to AsyncRead/AsyncWrite on https://docs.rs/tokio-util/0.3.1/tokio_util/codec/length_delimited/index.html are broken.

damienrg added a commit to damienrg/tokio that referenced this issue Apr 21, 2020
The link to tokio::main was relative to tokio_macros crate in the source
directory.
This is why it worked in local build of documentation and not in doc.rs.

Related to tokio-rs#1473.
damienrg added a commit to damienrg/tokio that referenced this issue Apr 21, 2020
Use rustdoc automatic link resolution over relative path to have compile
time error when it could not resolve links.

Fix invalid links to AsyncRead and AsyncWrite in tokio-util.

Related to tokio-rs#1473.
@ghost
Copy link

ghost commented Apr 21, 2020

Links to AsyncRead/AsyncWrite on docs.rs/tokio-util/0.3.1/tokio_util/codec/length_delimited/index.html are broken.

Thanks, I'll fix it as 3rd step of #1473 (comment) if it's not handled earlier

@Darksonn
Copy link
Contributor

Note that #2423 was just opened.

Darksonn pushed a commit that referenced this issue Apr 21, 2020
The link to tokio::main was relative to tokio_macros crate in the source
directory. This is why it worked in local build of documentation and not
in doc.rs.

Refs: #1473
damienrg added a commit to damienrg/tokio that referenced this issue Apr 21, 2020
The link to tokio::main was relative to tokio_macros crate in the source
directory.
This is why it worked in local build of documentation and not in doc.rs.

Refs: tokio-rs#1473
damienrg added a commit to damienrg/tokio that referenced this issue Apr 21, 2020
Use rustdoc automatic link resolution over relative path to have compile
time error when it could not resolve links.

Fix invalid links to AsyncRead and AsyncWrite in tokio-util.

Refs: tokio-rs#1473
@Darksonn Darksonn added A-tokio Area: The main tokio crate A-tokio-util Area: The tokio-util crate C-maintenance Category: PRs that clean code up or issues documenting cleanup. labels Apr 29, 2020
@Darksonn Darksonn removed this from the v0.2 milestone Apr 29, 2020
@Darksonn
Copy link
Contributor

Is there more work to do on this issue?

@ghost
Copy link

ghost commented Apr 30, 2020

In short yes.

I would still change all #method.<name>. This can be done two ways

> rg '(struct|trait|enum|macro|fn|attr|type)\.[a-zA-Z0-9_]*\.html|#method.' -t rust -g '!target' -g '!tokio-test'|rg '#method'|wc -l
48
  1. manually
  2. by a script (the script way is easy if this issue is resolved Resolve intra-doc-links on Self for method rust-lang/rust#70802)

My plan is to finish resolved rust-lang/rust#70802 issue and get back to this issue.
If resolving 70802 issue takes longer i will fallback to point 1.
Ok?

@Darksonn
Copy link
Contributor

Sounds good! No rush, I was just going through issues to figure out the status.

@ghost
Copy link

ghost commented May 5, 2020

I need more time because, .. you know life. ;)

@ghost
Copy link

ghost commented May 31, 2020

Status update..

In Short

With this MR #2575
we're fixing all easily fixable links and IMO we're good enough to close this issue.

In Long

Other non intra-links still exist. This is the complete list

  1. http links

    1. missing dependency
    > rg '(struct|trait|enum|macro|fn|attr|type)\.[a-zA-Z0-9_]*\.html|#method.' -t rust -g '!target' -g '!tokio-test'|rg http
    tokio-util/src/codec/framed.rs:    /// [`split`]: https://docs.rs/futures/0.3/futures/stream/trait.StreamExt.html#method.split
    tokio-util/src/codec/framed.rs:    /// [`split`]: https://docs.rs/futures/0.3/futures/stream/trait.StreamExt.html#method.split
    tokio-util/src/codec/framed.rs:    /// [`split`]: https://docs.rs/futures/0.3/futures/stream/trait.StreamExt.html#method.split

    Here, we miss future dependency in tokio/cargo.toml. It's in dependency-dev section to be exact. When I moved future to dependency section, i was able to make the intra-links work. Though, adding future to deps, because of docs is not a solution :).

    1. recurrsion dependency
    tokio/src/time/delay_queue.rs:/// [`Stream`]: https://docs.rs/futures/0.1/futures/stream/trait.Stream.html
    tokio/src/io/mod.rs://! [`Encoder`]: https://docs.rs/tokio-util/0.3/tokio_util/codec/trait.Encoder.html
    tokio/src/io/mod.rs://! [`Decoder`]: https://docs.rs/tokio-util/0.3/tokio_util/codec/trait.Decoder.html
    tokio/src/io/mod.rs://! [`Sink`]: https://docs.rs/futures/0.3/futures/sink/trait.Sink.html
    tokio/src/fs/mod.rs://! [`AsyncRead`]: https://docs.rs/tokio-io/0.1/tokio_io/trait.AsyncRead.html

    Similar to one above, here we miss tokio_util depedency in cargo.toml.
    However, we can't add tokio_util to tokio deps., because tokio_util depends on tokio so it would be cyclic dependency.
    Perhaps we should keep regular links here as they are.

  2. non-http links

tokio/src/lib.rs://! [main]: attr.main.html
tokio/src/runtime/mod.rs://! [`tokio::main`]: ../attr.main.html
tokio/src/runtime/mod.rs:    /// [main]: ../attr.main.html

This doesn't work. My guess, it's because of this? Maybe?

tokio/tokio/src/lib.rs

Lines 397 to 430 in a5c1a7d

cfg_macros! {
/// Implementation detail of the `select!` macro. This macro is **not**
/// intended to be used as part of the public API and is permitted to
/// change.
#[doc(hidden)]
pub use tokio_macros::select_priv_declare_output_enum;
doc_rt_core! {
cfg_rt_threaded! {
// This is the docs.rs case (with all features) so make sure macros
// is included in doc(cfg).
#[cfg(not(test))] // Work around for rust-lang/rust#62127
#[cfg_attr(docsrs, doc(cfg(feature = "macros")))]
pub use tokio_macros::main_threaded as main;
#[cfg_attr(docsrs, doc(cfg(feature = "macros")))]
pub use tokio_macros::test_threaded as test;
}
cfg_not_rt_threaded! {
#[cfg(not(test))] // Work around for rust-lang/rust#62127
pub use tokio_macros::main_basic as main;
pub use tokio_macros::test_basic as test;
}
}
// Maintains old behavior
cfg_not_rt_core! {
#[cfg(not(test))]
pub use tokio_macros::main;
pub use tokio_macros::test;
}
}

For sure, I just checked intra-links for proc macros and it works, ..

Test details
extern crate proc_macro;
use proc_macro::TokenStream;

/// [`this_proc_macro`](crate::this_proc_macro)
#[proc_macro_attribute]
pub fn this_proc_macro(_attr: TokenStream, item: TokenStream) -> TokenStream { }

so .. If anyone wants to dig it, go ahead.

@ghost
Copy link

ghost commented May 31, 2020

@Darksonn should we close it?
IMO, yes see #1473 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate A-tokio-util Area: The tokio-util crate C-maintenance Category: PRs that clean code up or issues documenting cleanup. T-docs Topic: documentation
Projects
None yet
Development

No branches or pull requests

3 participants