Tracking Issue for intra-doc links in libstd

jyn514 commented 4 years ago

This is a tracking issue for switching libstd to intra-doc links (rust-lang/rfcs#1946, https://github.com/rust-lang/rust/pull/74430#issuecomment-664693080).

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Mentoring instructions (or rather, suggested workflow)

Please first leave a comment here stating that you want to work on file xxx.rs or module xxx, to make sure that this implements Sync.

For each link of the form
rewrite it as
In most cases, the type will already be in scope, in which case you can remove the reference link altogether. For an example PR, see https://github.com/rust-lang/rust/pull/74470.
Run x.py doc library/std. This should warn(broken_intra_doc_links) by default.
Fix any warnings that appear.
Once you are ready to make a PR, run ./x.py tidy, which will run rustfmt.

In case of rustdoc bugs, there may be broken links that didn't show up when you ran x.py doc. However, these will be caught by the test suite, so you don't have to check them locally.

Suggested tools

@poliorcetics wrote a python script which can automate much of the conversion: https://github.com/rust-lang/rust/issues/75080#issuecomment-670935099
@denisvasilik wrote a regex which matches links in core and std: https://github.com/rust-lang/rust/issues/75080#issuecomment-668985777
@rdambrosio016 came up with several different regexes, starting around https://github.com/rust-lang/rust/issues/75080#issuecomment-668374288

Caveats

~~Primitives that have the same name as a module in scope must use type@ to disambiguate them. For example, to link to char from std, use type@char.~~ This was changed in https://github.com/rust-lang/rust/pull/75318, char will now link to the primitive, not the module.

The following cannot use intra-doc links. If you run into an issue with them, it's ok to skip them. However, if you see an issue not mentioned here, please file a bug report!

Some primitives (notably slices and pointers): https://github.com/rust-lang/rust/issues/63351, https://github.com/rust-lang/rust/issues/74515
Links from core to std (https://github.com/rust-lang/rust/issues/74481)
Documentation is also tested with --stage 0, so you may not be able to make changes that require recently merged fixes: https://github.com/rust-lang/rust/pull/75368#issuecomment-672196852
~~Associated items anywhere other than documenting the associated item itself (https://github.com/rust-lang/rust/pull/74489)~~ Fixed
Links on default function implementations for a trait (https://github.com/rust-lang/rust/issues/73829). Note that these will not warn if they fail to resolve, so you should be careful about modifying links on traits. Fixed

TODO list

This list was generated with rg '\[.*\]: \.\./.*' library/ -l | sed 's/^/- [ ] /' and may not be complete. If you see other links not mentioned here, feel free to fix them as well.

Since most files only have a few links, it's fine to claim multiple files at the same time.

Cannot be fixed

The following links cannot yet be fixed due to limitations in rustdoc.

[ ] [Vec::get_mut] (https://github.com/rust-lang/rust/pull/75672#discussion_r472322546)

denisvasilik commented 4 years ago

Hi @jyn514, I would like to work on the following files:

library/core/src/time.rs
library/core/src/cmp.rs
library/core/src/borrow.rs

Manishearth commented 4 years ago

Go for it!

RDambrosio016 commented 4 years ago

Many of these links can be brute force replaced using a regex, for items such as ... ../../std/stuff/something.Item.html it can be brute forced with (\[.*\]: )\.\.\/\.\.\/std\/(\w*)\/(\w*)\.(\w*)\.html[^#], then replacing with $1std::$2::$4\n, many more like method ones can also be knocked out using regex replacement, that regex alone can knock out 384 links. So that might be something worth exploring so less work has to be done with replacing boring links like these

RDambrosio016 commented 4 years ago

Method links and others (207) can be caught with (\[.*\]: )\.\.\/\.\.\/std\/(\w*)\/(\w*)\.(\w*)\.html#\w+\.(\w*) and replaced with $1std::$2::$4::$5\n, although as pointed out by @jyn514, some of these are broke even if the replacement is correct, so that will have to be checked manually.

RDambrosio016 commented 4 years ago

Intrinsics furthermore has a lot (~80) of special ones which match (\[.*\])$\.\.\/\.\.\/std\/(\w*)\/(\w*)\/\w*\.(\w*)\.\w+$ which can be replaced with $1(std::$2::$3::$4). There is another small use of this but with ../../std/module/stuff which can be matched with (\[.*\])$\.\.\/\.\.\/std\/(\w*)\/\w*\.(\w*)\.\w+$ and replaced with $1(std::$2::$3)

Manishearth commented 4 years ago

Go ahead and try the regex approach, if there are conflcits we can try and resolve them.

RDambrosio016 commented 4 years ago

I ran all replacements, then ran ./x.py test src/tools/linkchecker library/std, and there were no warnings. So i assume all of them worked correctly, however from my convo with @jyn514 some should not really work because they are fundamentally broken? so i am not quite sure if that is not the case or if testing is not working for me.

Manishearth commented 4 years ago

I'm not sure which ones you think will be broken.

jyn514 commented 4 years ago

I'm not sure which ones you think will be broken.

Associated items anywhere other than documenting the associated item itself (#74489)

jyn514 commented 4 years ago

so i am not quite sure if that is not the case or if testing is not working for me.

You can check by manually introducing a broken link and see if it's caught. If so, the tests are working :) I recommend --keep-stage 0 --keep-stage 1 for that so you don't have to rebuild the compiler (edit: after building the compiler for the first time - you can't keep a stage that hasn't been built!).

denisvasilik commented 4 years ago

Hi @RDambrosio016, could you do that for time.rs cmp.rs and borrow.rs as well? It's great to have a more automated variant.

RDambrosio016 commented 4 years ago

@denisvasilik not quite sure what you mean by do it for those files, as far as i can see all of the replacements in those files work. I identified another pattern of ../../std/module/index.html, which can be matched with \.\.\/\.\.\/std\/(\w*)\/index\.html and replaced with std::$1, as well as ../../std/primitive.primitive.html (not only primitive but also macro and others) which can be matched with \.\.\/\.\.\/std\/\w*\.(\w*)\.html and replaced with std::$1. i am not aware of any other major patterns, but if you are then please post it here and make a regex replacement, or just post it and i will do it when i have time 👍

denisvasilik commented 4 years ago

I had troubles with files located under library/core. I guess it has something to do with the caveats mentioned above, which states that intra-doc links from core to std are not possible.

RDambrosio016 commented 4 years ago

Yeah a fair amount of these replacement may be broken, but the goal of this is to overall reduce the amount of changes needed to be done.

denisvasilik commented 4 years ago

Here's another regex slightly modified to match core and std. Example matches:

[`Add`]: ../../std/ops/trait.Add.html
[`GlobalAlloc`]: ../../core/alloc/trait.GlobalAlloc.html

Regex: (\[.*\]: )\.\.\/\.\.\/(core|std)\/(\w*)\/(\w*)\.(\w*)\.html[^#] Replace: $1$2::$3::$5\n

denisvasilik commented 4 years ago

I saw that I cannot reference std in time.rs. For example I tried to do the following replacements:

 [`ops`]: ../../std/ops/index.html
 [`ops`]: std::ops (results in unresolved link)
 [`ops`]: core::ops (results in unresolved link)

If I add use create::ops it works, but I get a warning, because that import is just used for documentation. Is there another way to handle this or should I just leave these kind of links as is?

jyn514 commented 4 years ago

If I add use create::ops it works, but I get a warning, because that import is just used for documentation. Is there another way to handle this or should I just leave these kind of links as is?

Use crate::ops in the link instead:

 [`ops`]: crate::ops

Intra-doc links follow (almost) exactly the same rules as normal name resolution.

chansuke commented 4 years ago

Thank you for the detailed explanation. I would like to work on:

library/core/src/mem/manually_drop.rs
library/core/src/mem/maybe_uninit.rs

poliorcetics commented 4 years ago

I'll take library/std/src/path.rs.

poliorcetics commented 4 years ago

Hello !

I wrote a Python (3.6+, tested with 3.7 and 3.8) script to help with this issue, here is the gist.

By default it will not modify the files you pass it, only showing what it would do. You can find more informations about it in the documentation of the script.

Feel free to use it to speed up the work but be aware it is not perfect and you still review the whole file as well as any changes it makes. It may miss lines or modify some that shouldn't be modified. It is not aware of uses so some modification will make the docs fail to compile but on the whole it should be good enough to help with this tedious task. 😄

The script should preserve indentation perfectly if nothing else so you shouldn't have to rerun x.py fmt <file> 🤞.

I attached an example of the output produced by the script, please check yours is similar before applying its proposed changes:

poliorcetics commented 4 years ago

Doing library/std/src/prelude/mod.rs.

I also updated the gist to handle tymethod on traits as well as links to module -- those should be heavily checked, I think they can interfere with links to the the book for example if the right (well, wrong) pattern appears.

poliorcetics commented 4 years ago

Doing library/std/src/time.rs right now.

nixphix commented 4 years ago

I'll take all the files of form library/std/src/os/*/fs.rs these files have minimal diff (one per file) I could fit them all in a PR

here's the complete list

library/std/src/os/macos/fs.rs
library/std/src/os/dragonfly/fs.rs
library/std/src/os/ios/fs.rs
library/std/src/os/android/fs.rs
library/std/src/os/redox/fs.rs
library/std/src/os/freebsd/fs.rs
library/std/src/os/emscripten/fs.rs
library/std/src/os/openbsd/fs.rs
library/std/src/os/solaris/fs.rs
library/std/src/os/illumos/fs.rs
library/std/src/os/fuchsia/fs.rs
library/std/src/os/haiku/fs.rs
library/std/src/os/vxworks/fs.rs
library/std/src/os/netbsd/fs.rs
library/std/src/os/linux/fs.rs

denisvasilik commented 4 years ago

I'll take these files:

library/core/src/any.rs
library/core/src/hint.rs
library/core/src/char/convert.rs

camelid commented 4 years ago

I'll work on:

library/std/src/io/mod.rs
library/std/src/process.rs
library/core/src/option.rs
library/core/src/result.rs

camelid commented 4 years ago

@jyn514 it says that I claimed library/std/src/os/*/fs.rs, but I think @nixphix is the one who claimed those.

jyn514 commented 4 years ago

Oops, fixed. I wish there were a way to automate updating the issue, it's getting a little unwieldy :/

poliorcetics commented 4 years ago

I'll do:

library/std/src/panic.rs
library/std/src/ascii.rs
library/std/src/lib.rs
library/std/src/fs.rs

camelid commented 4 years ago

I've used this regex:

^\s*//[!/].*: .*(#\S+|\.html)

camelid commented 4 years ago

How do I link to the Book? E.g., here's the old line:

//! [`?` operator]: ../../book/appendix-02-operators.html

poliorcetics commented 4 years ago

@camelid if you want to go faster I made a python script to help, there is a link to the relevant comment in the first comment. :)

About the book, you don't need to change the links, they are not intra doc ones.

poliorcetics commented 4 years ago

Taking library/std/ffi/*.rs

nixphix commented 4 years ago

I ll take library/std/src/os/raw/*.md

complete list

 library/std/src/os/raw/float.md
 library/std/src/os/raw/long.md
 library/std/src/os/raw/ulong.md
 library/std/src/os/raw/ushort.md
 library/std/src/os/raw/short.md
 library/std/src/os/raw/uchar.md
 library/std/src/os/raw/uint.md
 library/std/src/os/raw/ulonglong.md
 library/std/src/os/raw/char.md
 library/std/src/os/raw/longlong.md
 library/std/src/os/raw/double.md
 library/std/src/os/raw/int.md
 library/std/src/os/raw/schar.md

nixphix commented 4 years ago

Taking library/std/src/sys/vxworks/ext/*.rs

 library/std/src/sys/vxworks/ext/process.rs
 library/std/src/sys/vxworks/ext/fs.rs

poliorcetics commented 4 years ago

Doing library/std/src/net/*.rs

denisvasilik commented 4 years ago

I'll take the following:

library/core/src/intrinsics.rs
library/core/src/alloc/mod.rs
library/core/src/alloc/global.rs
library/core/src/alloc/layout.rs

kofls commented 4 years ago

I'd like to work on:

library/alloc/src/vec.rs
library/alloc/src/task.rs

nixphix commented 4 years ago

Working on library/std/src/sys/windows/ext/*.rs

 library/std/src/sys/windows/ext/fs.rs
 library/std/src/sys/windows/ext/process.rs
 library/std/src/sys/windows/ext/ffi.rs

BoxyUwU commented 4 years ago

I'd like to do:

library/std/src/error.rs
library/std/src/alloc.rs
library/std/src/env.rs

nixphix commented 4 years ago

Working on library/std/src/sys/unix/ext/*.rs

 library/std/src/sys/unix/ext/fs.rs
 library/std/src/sys/unix/ext/thread.rs
 library/std/src/sys/unix/ext/process.rs
 library/std/src/sys/unix/ext/net.rs

poliorcetics commented 4 years ago

Doing library/std/src/keyword_docs.rs

poliorcetics commented 4 years ago

Doing library/std/src/io/*.rs. (All files may not need changes)

library/std/src/io/buffered.rs
library/std/src/io/cursor.rs
library/std/src/io/error.rs
library/std/src/io/impls.rs
library/std/src/io/lazy.rs
library/std/src/io/mod.rs
library/std/src/io/prelude.rs
library/std/src/io/stdio.rs
library/std/src/io/util.rs

LeSeulArtichaut commented 4 years ago

I will work on the rest of alloc

camelid commented 4 years ago

@jyn514 By the way, the list shows that core::option and std::process are in progress, but my PRs for those were already merged.

jyn514 commented 4 years ago

@camelid should be fixed now. Let me know if there's any others I missed.

poliorcetics commented 4 years ago

I'll do library/std/src/thread/{local, mod}.rs.

LeSeulArtichaut commented 4 years ago

I'll take core::ops::*

GuillaumeGomez commented 4 years ago

Claiming library/core/src/str/pattern.rs then.

camelid commented 4 years ago

I'll take:

library/std/src/macros.rs
library/std/src/collections/mod.rs
library/std/src/os/illumos/fs.rs
library/std/src/os/openbsd/fs.rs

denisvasilik commented 4 years ago

I'll take:

library/core/src/sync/atomic.rs
library/core/src/panic.rs
library/core/src/iter/traits/iterator.rs
library/core/src/clone.rs

rust-lang / rust