From 09e89eb4bc34380b0d785793707b8fd9621dc0b2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 8 Dec 2021 23:45:59 +0100 Subject: [PATCH 01/14] RFC for rustdoc jump to definition --- text/000-rustdoc-jump-to-definition.md | 187 +++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 text/000-rustdoc-jump-to-definition.md diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md new file mode 100644 index 00000000000..3d2dee7dbe1 --- /dev/null +++ b/text/000-rustdoc-jump-to-definition.md @@ -0,0 +1,187 @@ +Rustdoc: jump to definition + +- Feature Name: `jump_to_definition` +- Start Date: 2021-12-09 +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#89095](https://github.com/rust-lang/rust/issues/89095) + +# Summary +[summary]: #summary + +Generate links on idents in the rustdoc source code pages which links to the item's definition and documentation (if available). + +# Motivation +[motivation]: #motivation + +This RFC proposes to generate links in the rustdoc source code pages (the ones you end up on when clicking on the `source` links) to allow you to jump to an item definition or back to its documentation page. + +The goal of this RFC is to greatly improve the browing experience of the source page (especially when looking for private items). + +Since a video is worth a thousand words: + +https://user-images.githubusercontent.com/3050060/114622354-21307b00-9cae-11eb-834d-f6d8178a37bd.mp4 + +You can also give it a try with the following docs: + + * [askama](https://rustdoc.crud.net/imperio/jump-to-def-askama/src/askama/lib.rs.html) + * [regex](https://rustdoc.crud.net/imperio/jump-to-def-regex/src/regex/lib.rs.html) + * [rand](https://rustdoc.crud.net/imperio/jump-to-def-rand/src/rand/lib.rs.html) + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +We take as given that "source view" is a valuable part of rustdoc. Sometimes, it's necessary to go beyond the documentation and look at the implementation. That's why source view is a common feature in documentation tooling for a variety of languages (like Go for example, you can access this [source code](https://cs.opensource.google/go/go/+/go1.16.5:src/database/sql/sql.go;l=44) from [here](https://pkg.go.dev/database/sql#Register), or python docs or [hexdocs.pm](https://preview.hex.pm/preview/k6/0.0.1/show/lib/k6/template/web_socket.ex) for Elixir and Erlang languages). This is not a rare feature. + +Rustdoc's source view often runs into a particular problem: to properly understand the implementation, you need to make reference to identifiers defined elsewhere. For instance, consider the common newtype pattern: + +``` +pub MyStruct(OtherStruct) +``` + +If you clicked through on `MyStruct`'s src link, expecting to see the private implementation details, you'd be disappointed: you actually need to see the contents of `OtherStruct`. However, `OtherStruct` might be in another file entirely. Tracking it down can be a tedious process: + + - Use Ctrl-F to search the page for either a definition of `OtherStruct` or a `use` statement that imports it. + - If you found a `use` statement, figure out which file it maps to. + - Open the sidebar and navigate to that file. + - Ctrl-F again to find the actual definition of `OtherStruct`. + +If `OtherStruct` itself is defined in terms of additional structs, you may need to repeat this process many times to get even a cursory understanding of the implementation of `MyStruct`. It can also be error prone. If we implement "go to definition", we can turn this whole process into a single click, and make the result reliable. + +The same applies when reading a source code when type annotations are not available: + +``` +let var = some_other_var.do_something(); +let var2 = a_function(); +``` + +It would require to look for each method in the various files, etc. + +So overall, this would greatly improve the source code browsing experience. + +The other goal is to allow to go back from the source code pages directly to the item documentation to be able to read the rendered content and not just the "raw" doc comments. + +So in short, this feature is split in two parts: + + * Jump to the definition of an ident. + * Jump to an item documentation from the source view. + +Adding this information directly into the documentation allows it to improve the documentation value: if you want to go a bit further what is written in the documentation (to see some implementation details for example), you can currently go to the source code. However, you're quickly limited to the current file if it's using private items not from this file. + +Once you found what you're looking for, having a link to an item's documentation from the source code allows to add the missing connection between the source code viewer and the doc pages. + +Another important note is that if documentation is split in multiple places or requires an external tool for the source code navigation, it's actually slowing down our users quite a lot. + +A good example where this would be very helpful is for the [rust compiler source code](https://doc.rust-lang.org/1.57.0/nightly-rustc/src/rustc_middle/hir/map/mod.rs.html#872-877): even if a fully set up IDE or github, it's very hard to go around without previous knowledge or help from someone else. It's also very common to have undocumented items or to just want to see how something is implemented. With the "jump to definition", it already improved quite a lot the browsing experience by allowing to jump super quickly by simply clicking on a item. The missing part is now to go back from the source code into the item documentation to make it complete. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +It doesn't require javascript since it's only generating links, making it work with or without javascript enabled. + +For the link generation, we use the already existing system in rustdoc. Meaning their generation will follow the same rules as the link in the documentation pages. + +All items with documentation should have a link to their documentation page (like impl blocks, methods, trait/impl associated items, etc). + +So on the technical side, there is no big issues (based on what is described in this RFC). A few parts of this RFC are implemented in the following pull requests: + + * [#84176](https://github.com/rust-lang/rust/pull/84176) + * [#88033](https://github.com/rust-lang/rust/pull/88033) + * [#91264](https://github.com/rust-lang/rust/pull/91264) + +## UI + +For the UI, we have the following constraints: + + * Must work on mobile and desktop: we cannot rely on mouse hover events. + * It's better if it works without javascript (it's important for accessibility, but also for maintenance reasons too: less JS code, less maintenance). + +Some extra explanations about the statement "less JS code, less maintenance": To have this information in the front-end, the back-end has to generate it in any case (either as a link or any other format). So deciding to use JS to handle this would mean that we still have code in the back-end but we would also have code in the front-end to treat it. + +With this in mind, the suggested UI would be like this: + + * Links generated on idents in source code will jump to the ident definition. + * Links generated on items' definition will jump to their documentation page (if any, private items don't have one by default). + * On mobile, these links will have dotted underline (because you don't have a cursor which change when hovering them). + +Another thing which would be nice: to make it more obvious if a link points to source code or to documentation, they should have different visual markers (different background/underline colors for example). + +Basically, the source code pages' UI doesn't change. + +# Drawbacks +[drawbacks]: #drawbacks + +**The biggest concern is actually about "scope expansion"**. If we add this feature, are we doing too much? Rustdoc is supposed to be about documentation, so is providing more information into the source code page really necessary? + +An answer about this is that it actually makes source code browsing much more pleasant and convenient. Multiple other documentation tools provide this feature for this reason so it doesn't seem like it would be out of scope in this regard. + +**Another concern is**: "why should we implement this in rustdoc? Aren't there already existing tools doing it?" + +A good answer to this is actually the following scenario: you are on and you're looking at a crate documentation. You want to see how something is implemented and click on the `source` link. At this point, you encounter an item used in this page but not defined in this page. Do you want to clone the crate locally to check it out or go to github/gitlab to check it there or do you prefer having the links directly available? + +**Another concern is**: "impact on the size of the generated pages" + +Since we generate links, it will increase the size of the source code pages. Here are a few examples with the currently implemented parts of this feature: + +| crate name | without the feature (in bytes) | with the feature (in bytes) | diff | +| ---------- | ------------------------------ | --------------------------- | ---- | +| std | 11.788.288 | 13.172.736 | 11.7% | +| tract_core | 4.276.224 | 5.554.176 | 29.9% | +| stm32f4 | 22.335.488 | 26.984.448 | 20.8% | +| image | 4.927.488 | 6.045.696 | 22.7% | + +The impact on the number of DOM nodes now (I took random pages with enough code for it to be significant). To compute it, I used `document.getElementsByTagName('main')[0].getElementsByTagName('*').length ` in the browser console: + +| file | without the features | with the feature | diff | +|-|-|-|-| +| askama_shared/parser.rs.html | 8414 | 8677 | 3.1% | +| regex/re_bytes.rs.html | 4386 | 4386 | 0% | +| rand/distributions/uniform.rs.html | 6838 | 6838 | 0% | + +**Last concern is about the maintenance**. + +Any feature has a maintenance cost, this one included. It was suggested to put this feature in its own crate. The only part that can be extracted is the span map builder. But it would very likely be more of a problem than a solution considering that rustdoc API doesn't allow it easily. An important reminder: this feature is less than 200 lines of code in rustdoc and doesn't require any extra dependency. + +One more argument about this is that the feature actually requires not that much code. It is split in two parts: + + * https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/render/span_map.rs (which is basically a visitor gathering `span`) + * https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/highlight.rs#L725-L753 (for generating links) + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +[Github Code navigation feature](https://docs.github.com/en/repositories/working-with-files/using-files/navigating-code-on-github): Relying on external (and private) tools doesn't seem like a good idea. If they decide to shut it down, we can't do anything about it. In addition to that, a lot of projects are not on github, so it would exclude them. It also requires an internet connection. And finally, the biggest setback: it would very likely not support linking to items from other crates. + +[Google Code Search](https://cs.opensource.google/fuchsia/fuchsia/+/main:src/factory/factory_store_providers/src/config.rs;l=62): Same issues as listed for `Github Code Navigation`. + +rust-analyzer: This is a public opensource project. However, the setup isn't super easy (but it's a minor issue) and when using it on the rust compiler source code, it's quite resource-heavy. Also, if you want to look at a crate source code on , you'd need to clone it locally to be able to use rust-analyzer on it. + +rust-analyzer in the browser: Another possibility would be to run it with wasm. However, it brings a few issues: it requires JS to work (obviously, but it's not a big problem), it would be quite heavy to run and it would complexify rustdoc build quite a lot. + +External library (like `cpan` or `tree-sitter`): They would work to generate links to another source code location. However, they bring their downsides as well: they would need to be kept up-to-date with rust syntax evolution (if any) and making links back to documentation would be very tricky because some information is only internal to rustdoc (like how to differentiate between different `impl blocks` or different blanket trait implementations like `impl Trait for T` and `impl Trait for T`). + +# Prior art +[prior-art]: #prior-art + +As mentionned above, it is actually quite common for documentation tools to have a source viewer. Implementations vary a bit between them: + +| tool/language | source code viewer | jump to definition | go back to documentation | +| ------------- | ------------------ | ------------------ | ------------------------ | +| [doxygen](https://eigen.tuxfamily.org/dox/Matrix_8h_source.html) | yes | no | yes | +| [Elixir](https://elixir.bootlin.com) (used for linux kernel) | yes | yes | no | +| [Go](https://cs.opensource.google/go/go/+/go1.16.5:src/database/sql/sql.go;l=44) | yes | yes | no | +| [hexdocs](https://preview.hex.pm/preview/k6/0.0.1/show/lib/k6/template/web_socket.ex) | yes | no | no | +| python (sphynx/pydoc) | no (links to github) | no | no | +| [ruby](https://ruby-doc.org/stdlib-3.1.2/libdoc/delegate/rdoc/Delegator.html#method-i-__raise__) | yes (directly on the page) | yes/no (it is sometimes in examples) | no | +| [perl](https://metacpan.org/dist/DBD-SQLite/source/lib/DBD/SQLite.pm) | yes | yes | yes | +| [haskell](https://devdocs.io/haskell~9/libraries/binary-0.8.9.0/data-binary-builder#v:append) | yes | yes | no | +| [opal](https://opalrb.com/docs/api/v1.4.1/stdlib/Buffer) | yes (directly on the page) | no | yes | +| [cargo-src](https://github.com/rust-dev-tools/cargo-src) | yes | yes | yes | + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +Is it the best way to make the links to go to an item documentation like this? Maybe something adding a small icon to click right besides it should be better? + +# Potential extensions + +A potential extension would be "find references". It would allow users to check where the given item is being used. We already have all the information needed for this feature, so it's mostly how to use it and show it to the end users. However, if we decide to move forward with it, it will be part of another RFC. From 389d305c3c60f8e24e372f0cb806d9c9b907e606 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 12 Aug 2022 22:02:21 +0200 Subject: [PATCH 02/14] Rework sections, improve motivations, write down all cases I thought about for links --- text/000-rustdoc-jump-to-definition.md | 145 +++++++++++++++++++++---- 1 file changed, 122 insertions(+), 23 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 3d2dee7dbe1..94630e7aa61 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -10,22 +10,20 @@ Rustdoc: jump to definition Generate links on idents in the rustdoc source code pages which links to the item's definition and documentation (if available). -# Motivation -[motivation]: #motivation +You can try a live demo with the following docs: -This RFC proposes to generate links in the rustdoc source code pages (the ones you end up on when clicking on the `source` links) to allow you to jump to an item definition or back to its documentation page. + * [askama](https://rustdoc.crud.net/imperio/jump-to-def-askama/src/askama/lib.rs.html) + * [regex](https://rustdoc.crud.net/imperio/jump-to-def-regex/src/regex/lib.rs.html) + * [rand](https://rustdoc.crud.net/imperio/jump-to-def-rand/src/rand/lib.rs.html) -The goal of this RFC is to greatly improve the browing experience of the source page (especially when looking for private items). +# Motivation +[motivation]: #motivation -Since a video is worth a thousand words: +The rustdoc source code pages (the ones you end up on when clicking on the `source` links) are a feature that allows one to easily take a look at an item implementation, whether it is by curiosity on how it works or for checking some technical details. -https://user-images.githubusercontent.com/3050060/114622354-21307b00-9cae-11eb-834d-f6d8178a37bd.mp4 +However, the Rust language uses a lot indirections, making it difficult to find an information if it is not on the page the user is already on, limiting the usefulness of the source code pages. -You can also give it a try with the following docs: - - * [askama](https://rustdoc.crud.net/imperio/jump-to-def-askama/src/askama/lib.rs.html) - * [regex](https://rustdoc.crud.net/imperio/jump-to-def-regex/src/regex/lib.rs.html) - * [rand](https://rustdoc.crud.net/imperio/jump-to-def-rand/src/rand/lib.rs.html) +This RFC proposes to generate links in the rustdoc source code pages to allow you to jump to an item definition or back to its documentation page to make it much easier to browse these pages without needing to clone the crate locally. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation @@ -73,20 +71,117 @@ Another important note is that if documentation is split in multiple places or r A good example where this would be very helpful is for the [rust compiler source code](https://doc.rust-lang.org/1.57.0/nightly-rustc/src/rustc_middle/hir/map/mod.rs.html#872-877): even if a fully set up IDE or github, it's very hard to go around without previous knowledge or help from someone else. It's also very common to have undocumented items or to just want to see how something is implemented. With the "jump to definition", it already improved quite a lot the browsing experience by allowing to jump super quickly by simply clicking on a item. The missing part is now to go back from the source code into the item documentation to make it complete. +Here is a video showing the feature in action: + +https://user-images.githubusercontent.com/3050060/114622354-21307b00-9cae-11eb-834d-f6d8178a37bd.mp4 + + # Reference-level explanation [reference-level-explanation]: #reference-level-explanation -It doesn't require javascript since it's only generating links, making it work with or without javascript enabled. +This feature **only** targets the source code pages. + +It can be implemented without JavaScript additions to Rustdoc as pure link generation; so it will work without JS enabled. Rustdoc already has an internal link generation system that can be used here. + +## Link generation + +There are two kinds of links: + + * Jump to documentation. + * Jump to definition. + +### Jump to documentation + +All items with a documentation page should have a link to it (like methods, trait/impl associated items, etc). However, fields, variants and impl blocks should not (explications below). + +The link is generated on their ident/name where they are declared, not when they are in use. For example: + +```rust +pub struct Foo; + +let x = Foo; +``` + +On `Foo` definition (`pub struct Foo`), we generate a link to its documentation page whereas on its usage (`x = Foo`), we generate a link to jump to its definition. + +This is why generating a link for the impl blocks would be complicated: on `impl Foo`, we generate a "jump to definition" link on `Foo`, leaving no space to generate a link to the impl documentation. + +For fields and variants, it's simply to limit the quantity of links generated: you can simply click on their parent item to get to their documentation page. + +### Jump to definition -For the link generation, we use the already existing system in rustdoc. Meaning their generation will follow the same rules as the link in the documentation pages. +A small preamble before listing the items we should generate links for: if an item which should get a link is preceded by a path, the whole path is used as a link. For example: -All items with documentation should have a link to their documentation page (like impl blocks, methods, trait/impl associated items, etc). +```rust +let x: b::c::D; // `b::c::D` is the link to `D`. +foo::some_fn(); // `foo::some_fn` is the link to `some_fn`. +``` + +Anything not specifically mentioned should not get a link generated for itself (such as variable name): + +```rust +fn f(mut a: String) { + a.push_str("something"); // `a` should not get a link but `push_str` should! +} +``` + +#### Primitive types + +No primitive type should get a link. + +#### Fields/variants + +They should not get a link either. + +#### Imports + +For a single import such as `use foo::bar;`, `foo::bar` would be the link. +For a multiple import such as `use foo::{a, b, c};`, `a`, `b` and `c` would be 3 different links. + +#### Types + +Whenever a type appears, we should link to its definition. For example: + +```rust +let x: String = "a".to_owned(); // we generate a link on `String`. +let y: Foo; // We generate a link on both `Foo` and `Bar`. + +fn foo(f: F, t: T) {} // We generate a link on `Display` and `Debug`. +``` -So on the technical side, there is no big issues (based on what is described in this RFC). A few parts of this RFC are implemented in the following pull requests: +#### Functions/methods - * [#84176](https://github.com/rust-lang/rust/pull/84176) - * [#88033](https://github.com/rust-lang/rust/pull/88033) - * [#91264](https://github.com/rust-lang/rust/pull/91264) +Whenever a function/method is present, we should generate a link to it. Example: + +```rust +let x: Fn() = some_fn; // We should link to `some_fn`. +some_fn(); // We should link to `some_fn`. +ty.a().b(); // We should link to `a` and to `b`. +``` + +#### Macros + +Macro calls should get a generated link: + +```rust +some_macro!(); // Should link to `some_macro!`. +``` + +#### Constants/statics + +Both these kinds of item should get a link too: + +```rust +static FOO: u32 = 0; +const BAR: u32 = 0; + +let x = FOO; // `FOO` gets a link. +let y = BAR; // `BAR` gets a link. +``` + +#### Modules + +Inline modules should never generate a "jump to definition" link, only a "jump to documentation" one. However, other modules will behave as follows: if they are private and the `--document-private-items` is not in use, they will generate a "jump to definition" link. Otherwise they will generate a "jump to documentation" link. ## UI @@ -110,15 +205,19 @@ Basically, the source code pages' UI doesn't change. # Drawbacks [drawbacks]: #drawbacks -**The biggest concern is actually about "scope expansion"**. If we add this feature, are we doing too much? Rustdoc is supposed to be about documentation, so is providing more information into the source code page really necessary? +## scope expansion + +If we add this feature, are we doing too much? Rustdoc is supposed to be about documentation, so is providing more information into the source code page really necessary? + +The source code pages have been there since the 1.0 release. Even with great documentation, looking at the item's implementation can allow to maybe have a deeper understand on how it works. It can also allow you to learn new idioms and better understand the language. -An answer about this is that it actually makes source code browsing much more pleasant and convenient. Multiple other documentation tools provide this feature for this reason so it doesn't seem like it would be out of scope in this regard. +As such, this feature isn't a scope of extension but really something to make the current situation even better by making the source code browsing much more pleasant and convenient. Multiple other documentation tools provide this feature for the same reason. -**Another concern is**: "why should we implement this in rustdoc? Aren't there already existing tools doing it?" +## Why in rustdoc? A good answer to this is actually the following scenario: you are on and you're looking at a crate documentation. You want to see how something is implemented and click on the `source` link. At this point, you encounter an item used in this page but not defined in this page. Do you want to clone the crate locally to check it out or go to github/gitlab to check it there or do you prefer having the links directly available? -**Another concern is**: "impact on the size of the generated pages" +## Impact on the generated pages' size Since we generate links, it will increase the size of the source code pages. Here are a few examples with the currently implemented parts of this feature: @@ -137,7 +236,7 @@ The impact on the number of DOM nodes now (I took random pages with enough code | regex/re_bytes.rs.html | 4386 | 4386 | 0% | | rand/distributions/uniform.rs.html | 6838 | 6838 | 0% | -**Last concern is about the maintenance**. +## Maintenance cost Any feature has a maintenance cost, this one included. It was suggested to put this feature in its own crate. The only part that can be extracted is the span map builder. But it would very likely be more of a problem than a solution considering that rustdoc API doesn't allow it easily. An important reminder: this feature is less than 200 lines of code in rustdoc and doesn't require any extra dependency. From 48babf5b90235584a1d65304a5de78bb65cf7252 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 12 Aug 2022 22:08:21 +0200 Subject: [PATCH 03/14] Add a section for local variables --- text/000-rustdoc-jump-to-definition.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 94630e7aa61..f907c4baad6 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -117,7 +117,11 @@ let x: b::c::D; // `b::c::D` is the link to `D`. foo::some_fn(); // `foo::some_fn` is the link to `some_fn`. ``` -Anything not specifically mentioned should not get a link generated for itself (such as variable name): +Anything not specifically mentioned should not get a link generated for itself. + +#### Local variables + +Local variables should not get a link: ```rust fn f(mut a: String) { @@ -129,7 +133,7 @@ fn f(mut a: String) { No primitive type should get a link. -#### Fields/variants +#### Fields They should not get a link either. From b28ca088a189bb8f1f9dcc3449896193e2fd4f20 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 12 Aug 2022 22:42:37 +0200 Subject: [PATCH 04/14] Improve motivations section --- text/000-rustdoc-jump-to-definition.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index f907c4baad6..5f112514a84 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -19,7 +19,7 @@ You can try a live demo with the following docs: # Motivation [motivation]: #motivation -The rustdoc source code pages (the ones you end up on when clicking on the `source` links) are a feature that allows one to easily take a look at an item implementation, whether it is by curiosity on how it works or for checking some technical details. +The rustdoc source code pages (the ones you end up on when clicking on the `source` links) are a feature that allows one to easily take a look at an item implementation, whether it is by curiosity on how it works or for checking some technical details. Having a great documentation doesn't prevent users to want to actually see how the code looks like. However, the Rust language uses a lot indirections, making it difficult to find an information if it is not on the page the user is already on, limiting the usefulness of the source code pages. From 521fc2f6c40b8326c6f708383bd5fe1effad73d3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Aug 2022 16:50:18 +0200 Subject: [PATCH 05/14] Python does actually have a source viewer, update link for perl docs --- text/000-rustdoc-jump-to-definition.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 5f112514a84..7a72c87a3f8 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -273,9 +273,9 @@ As mentionned above, it is actually quite common for documentation tools to have | [Elixir](https://elixir.bootlin.com) (used for linux kernel) | yes | yes | no | | [Go](https://cs.opensource.google/go/go/+/go1.16.5:src/database/sql/sql.go;l=44) | yes | yes | no | | [hexdocs](https://preview.hex.pm/preview/k6/0.0.1/show/lib/k6/template/web_socket.ex) | yes | no | no | -| python (sphynx/pydoc) | no (links to github) | no | no | +| [python](https://eff-certbot.readthedocs.io/en/stable/api/certbot.achallenges.html#certbot.achallenges.AnnotatedChallenge) (sphynx/pydoc) | yes | no | no | | [ruby](https://ruby-doc.org/stdlib-3.1.2/libdoc/delegate/rdoc/Delegator.html#method-i-__raise__) | yes (directly on the page) | yes/no (it is sometimes in examples) | no | -| [perl](https://metacpan.org/dist/DBD-SQLite/source/lib/DBD/SQLite.pm) | yes | yes | yes | +| [perl](https://metacpan.org/dist/DBD-SQLite/view/lib/DBD/SQLite/Cookbook.pod) (source link on the left side) | yes | yes | yes | | [haskell](https://devdocs.io/haskell~9/libraries/binary-0.8.9.0/data-binary-builder#v:append) | yes | yes | no | | [opal](https://opalrb.com/docs/api/v1.4.1/stdlib/Buffer) | yes (directly on the page) | no | yes | | [cargo-src](https://github.com/rust-dev-tools/cargo-src) | yes | yes | yes | From 56af431389475aa2a448537106b332d3f950d898 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Aug 2022 17:03:26 +0200 Subject: [PATCH 06/14] Change status of local variables: they should be linkified too --- text/000-rustdoc-jump-to-definition.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 7a72c87a3f8..e0f244790c3 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -121,14 +121,19 @@ Anything not specifically mentioned should not get a link generated for itself. #### Local variables -Local variables should not get a link: +Local variables should get a link: ```rust fn f(mut a: String) { - a.push_str("something"); // `a` should not get a link but `push_str` should! + a.push_str("something"); // `a` get a link just like `push_str`! } ``` +The reasons about supporting local variables are as follow: + + * We want to linkify global variables, because they may be far away or in another file. + * If you're looking at a variable reference in a function, you don't know whether it's a local variable or a global one (or one from an enclosing scope). By linkifying variables we make it easy for people to find out the answer. If we linkify only some variables (the global ones), it will appear weird that some are linkified and some are not. So for coherency, it should be done for all local variables. + #### Primitive types No primitive type should get a link. From a00ae184ea64c9c08160c216b146be9f10a535c2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Aug 2022 17:49:59 +0200 Subject: [PATCH 07/14] Add precision about local variables link --- text/000-rustdoc-jump-to-definition.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index e0f244790c3..9dd995824b6 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -121,7 +121,7 @@ Anything not specifically mentioned should not get a link generated for itself. #### Local variables -Local variables should get a link: +Local variables should get a link to their definition (not their type): ```rust fn f(mut a: String) { From 10e5626d86ad3b0aafac87a412fc5a05122d6e25 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Aug 2022 17:53:28 +0200 Subject: [PATCH 08/14] Add missing Variant handling section --- text/000-rustdoc-jump-to-definition.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 9dd995824b6..2b38a12e56b 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -142,6 +142,18 @@ No primitive type should get a link. They should not get a link either. +#### Variants + +They should get a link, but only on their usage (as already specified in the "jump to documentation" section): + +```rust +enum Enum { + Variant, // No link here. +} + +let x = Enum::Variant; // `Enum::Variant` is the link to `Variant` definition. +``` + #### Imports For a single import such as `use foo::bar;`, `foo::bar` would be the link. From d4cd0aa457d4d994d7efa1d47f3af974cf265458 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Aug 2022 18:01:03 +0200 Subject: [PATCH 09/14] Completely rewrite Motivation section (thanks @jsha!) --- text/000-rustdoc-jump-to-definition.md | 34 +++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 2b38a12e56b..756f170aef2 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -19,11 +19,39 @@ You can try a live demo with the following docs: # Motivation [motivation]: #motivation -The rustdoc source code pages (the ones you end up on when clicking on the `source` links) are a feature that allows one to easily take a look at an item implementation, whether it is by curiosity on how it works or for checking some technical details. Having a great documentation doesn't prevent users to want to actually see how the code looks like. +Documentation readers use the source link in rustdoc to perform a [variety of tasks](https://twitter.com/j4cob/status/1558175299174547457). Very often, Rust documentation readers will reach a “dead-end” trying to accomplish their task, because the source code calls functions from another source file or another crate. We should eliminate these dead ends so readers can dive into source code with confidence they will be able to complete the task they set out to do. Some of the most common tasks: -However, the Rust language uses a lot indirections, making it difficult to find an information if it is not on the page the user is already on, limiting the usefulness of the source code pages. + - bug-finding + - verifying that implementation matches documentation + - checking intentionally undocumented implementation details + - learning more about the language, idiomatic style, and useful techniques -This RFC proposes to generate links in the rustdoc source code pages to allow you to jump to an item definition or back to its documentation page to make it much easier to browse these pages without needing to clone the crate locally. +Source view is an extremely common feature in documentation tools for many languages. It is particularly useful to tie source view to documentation because many of the tasks readers want to accomplish are tied specifically to documentation they are reading. A doc page can link them directly to the implementation information for the item they are looking at. If doc pages don’t link to source, the alternative is much more painful and time consuming: the reader has to find the repository for the documentation they are looking at, clone it, open the repository in an IDE, wait for rust-analyzer to build its index, navigate to the same item they were looking at in the documentation, and finally start reading code. This can take several minutes and be very frustrating. + +Source view is useful even for the most comprehensively documented code, because the tasks readers accomplish with it are not primarily due to under-documented code. For instance, bug-finding: even the most well-documented crates have bugs, and most of those bugs are not documented because the author is unaware of them. Or finding implementation details: good documentation intentionally omits many implementation details, like the size of an object, which it can be entirely stack-resident (vs containing Boxes), or what syscalls are used to implement a function. But documentation readers sometimes need to check those details to better understand their running code, even if the wise reader knows implementation may change without notice. + +We include source view as part of rustdoc because source code, like documentation, changes over time. It’s useful to have a copy of source code that we know with 100% certainty is the same source code the documentation was generated from. When there are multiple versions of the documentation available (as on docs.rs), there can be multiple versions of the source code available too. In the particular case of docs.rs, each version of a crate is documented based on that crate’s immutable contents from crates.io. If we delegate source view to GitHub (see Alternatives), there’s no guarantee that a version of the source code corresponding to the documentation continues to exist. + +Here are some of the dead-ends a reader can encounter while reading a line of source that contains `a.foo()`: + + - “I don’t know what type `a` is” (e.g. due to type inference). + - “I know what type `a` is but not its fully-qualified type” (e.g. due to glob imports). + - “I don’t know what type `foo()` is defined on” (e.g. it could be defined on some trait). + - “I don’t know how to find `foo()`’s documentation or source” (e.g. because `foo` is private). + +Note that some of these problems are likely to occur in source view but less likely to occur in examples, because examples are only able to use public types, and because they are usually written to be clear about what types are being used. + +For any instance of `a.foo()` in source view, there is exactly one answer to the question: where is `foo` implemented? However, answering that question requires (a) doing version selection according to Cargo’s algorithms, (b) fetching all dependencies, and (c) running rustc or equivalent to process all the `use` statements and type inferences. Third-party code navigation tools (GitHub; Google Code Search) generally do not actually build the source code they are navigating. Instead, they do some amount of [syntactic processing and use search-based navigation](https://docs.github.com/en/repositories/working-with-files/using-files/navigating-code-on-github#precise-and-search-based-navigation). For instance, GitHub uses [tree-sitter](https://tree-sitter.github.io/tree-sitter/) for search-based navigation. It also uses [stack-graphs](https://github.com/github/stack-graphs) for “precise navigation” of certain languages. Neither one supports Rust yet. It is likely that even if stack-graphs acquires support for Rust, it will not be quite correct unless stack-graphs also manages to implement Rust’s type inference rules. Also, [according to GitHub](https://docs.github.com/en/repositories/working-with-files/using-files/navigating-code-on-github#troubleshooting-code-navigation): “Code navigation only works for active branches.” + +Compared to third-party source navigation tools, rustdoc and the crates.io ecosystem have a significant advantage: almost every release of every crate has its documentation built by docs.rs. Because rustdoc is tightly integrated with rustc, that documentation build can process the code exactly as rustc does, and offer the exactly correct answer for “where is `foo` implemented?” + +## Cross-crate links + +Sometimes the implementation a reader wants is in another crate. This is particularly common in Rust, where “facade crates” are often used. Source view should link to that other crate whenever possible. But linking to another crate requires (a) knowing where source code for another crate can be found, and (b) choosing an appropriate version of that crate. + +These problems are (relatively) easy to solve for crates within the crates.io ecosystem: most versions of most crates are available on docs.rs, along with source code. Outside the crates.io / docs.rs ecosystem they are much harder to solve: the mapping from crate to repository is incomplete, and so is the mapping from repository to specific versions within that repository. + +There are nevertheless some challenges to selecting the version appropriately within the crates.io ecosystem. See the Reference-level explanation for more detail (TKTK - add detail in Reference-level explanation). # Guide-level explanation [guide-level-explanation]: #guide-level-explanation From 0f11efe433b8477777878e0b547107e0b0a4cb26 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 5 Sep 2022 16:40:35 +0200 Subject: [PATCH 10/14] Fix link to github source code and add a new entry on how "Paths" should be handled. --- text/000-rustdoc-jump-to-definition.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 756f170aef2..cfd8a09eb48 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -232,6 +232,22 @@ let y = BAR; // `BAR` gets a link. Inline modules should never generate a "jump to definition" link, only a "jump to documentation" one. However, other modules will behave as follows: if they are private and the `--document-private-items` is not in use, they will generate a "jump to definition" link. Otherwise they will generate a "jump to documentation" link. +#### Paths + +Any part of a path which isn't a keyword should get a link. So for example: + +```rust +use crate::foo::bar{self, Type}; +``` + +In this one, `foo`, `bar` and `Type` should get a link. + +```rust +let x = X::Y::Z; +``` + +In this one, `X`, `Y` and `Z` should get a link. + ## UI For the UI, we have the following constraints: @@ -292,7 +308,7 @@ Any feature has a maintenance cost, this one included. It was suggested to put t One more argument about this is that the feature actually requires not that much code. It is split in two parts: * https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/render/span_map.rs (which is basically a visitor gathering `span`) - * https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/highlight.rs#L725-L753 (for generating links) + * https://github.com/rust-lang/rust/blob/6e4a9ab650b135ae0ff761e4a37d96c8bcaf7b3d/src/librustdoc/html/highlight.rs#L977-L993 (for generating links) # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives From 0a6fb7b7c1ca898f6d67fd7fcbff5d41196b5588 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 5 Sep 2022 20:53:18 +0200 Subject: [PATCH 11/14] * Update the links UI part * Mention SourceGraph in the "Rationale and alternatives" section --- text/000-rustdoc-jump-to-definition.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index cfd8a09eb48..d753d9c9119 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -259,11 +259,10 @@ Some extra explanations about the statement "less JS code, less maintenance": To With this in mind, the suggested UI would be like this: - * Links generated on idents in source code will jump to the ident definition. - * Links generated on items' definition will jump to their documentation page (if any, private items don't have one by default). - * On mobile, these links will have dotted underline (because you don't have a cursor which change when hovering them). + * By default, the links have no decoration. + * On hover, the links get an underline decoration. -Another thing which would be nice: to make it more obvious if a link points to source code or to documentation, they should have different visual markers (different background/underline colors for example). +To make it obvious whether a link is "jump to def" or "jump to doc", the underline decoration will be different between them: the "jump to doc" link underline will be dotted. Basically, the source code pages' UI doesn't change. @@ -323,6 +322,8 @@ rust-analyzer in the browser: Another possibility would be to run it with wasm. External library (like `cpan` or `tree-sitter`): They would work to generate links to another source code location. However, they bring their downsides as well: they would need to be kept up-to-date with rust syntax evolution (if any) and making links back to documentation would be very tricky because some information is only internal to rustdoc (like how to differentiate between different `impl blocks` or different blanket trait implementations like `impl Trait for T` and `impl Trait for T`). +[SourceGraph](https://sourcegraph.com/github.com/rust-lang/rustc-demangle@2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/-/blob/src/v0.rs?L65): SourceGraph uses [Language Server Index Format (LSIF)](https://github.com/Microsoft/language-server-protocol/blob/main/indexFormat/specification.md) to extract info from a language server and make it available in a static format. And rust-analyzer bindings for LSIF already exist. The problem is that it would require a live server to work, which is a blocker to be integrated into rustdoc as it must work serverless. + # Prior art [prior-art]: #prior-art From ba9881f461427a781725f7378945973948ee7b0f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 21 Oct 2022 16:10:49 +0200 Subject: [PATCH 12/14] * Add new unresolved questions "How to handle functions that do not pass type checking?" and "Adding an heuristic to automatically disable the feature on a file". * Describe the planned trajectory for the feature. * Explain more in details how cross-crate linking works. * Fix up verb tenses in the Reference section. --- text/000-rustdoc-jump-to-definition.md | 136 ++++++++++++++++++------- 1 file changed, 99 insertions(+), 37 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index d753d9c9119..ede5675f60e 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -8,7 +8,7 @@ Rustdoc: jump to definition # Summary [summary]: #summary -Generate links on idents in the rustdoc source code pages which links to the item's definition and documentation (if available). +Generate links on idents in the rustdoc source code pages which links to the item's definition and documentation (if available). Remove the nightly `--generate-link-to-definition` option, enable this feature by default and add a new option to allow to disable this feature: `--disable-jump-to-definition`. You can try a live demo with the following docs: @@ -49,9 +49,9 @@ Compared to third-party source navigation tools, rustdoc and the crates.io ecosy Sometimes the implementation a reader wants is in another crate. This is particularly common in Rust, where “facade crates” are often used. Source view should link to that other crate whenever possible. But linking to another crate requires (a) knowing where source code for another crate can be found, and (b) choosing an appropriate version of that crate. -These problems are (relatively) easy to solve for crates within the crates.io ecosystem: most versions of most crates are available on docs.rs, along with source code. Outside the crates.io / docs.rs ecosystem they are much harder to solve: the mapping from crate to repository is incomplete, and so is the mapping from repository to specific versions within that repository. +These problems are (relatively) easy to solve for crates within the `crates.io` ecosystem: most versions of most crates are available on docs.rs, along with source code. Outside the crates.io / docs.rs ecosystem they are much harder to solve: the mapping from crate to repository is incomplete, and so is the mapping from repository to specific versions within that repository. -There are nevertheless some challenges to selecting the version appropriately within the crates.io ecosystem. See the Reference-level explanation for more detail (TKTK - add detail in Reference-level explanation). +There are nevertheless some challenges to selecting the version appropriately within the crates.io ecosystem. See the Reference-level explanation for more detail. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation @@ -109,7 +109,7 @@ https://user-images.githubusercontent.com/3050060/114622354-21307b00-9cae-11eb-8 This feature **only** targets the source code pages. -It can be implemented without JavaScript additions to Rustdoc as pure link generation; so it will work without JS enabled. Rustdoc already has an internal link generation system that can be used here. +It will be implemented without JavaScript additions to Rustdoc as pure link generation; so it will work without JS enabled. Rustdoc already has an internal link generation system that can be used here. ## Link generation @@ -120,9 +120,9 @@ There are two kinds of links: ### Jump to documentation -All items with a documentation page should have a link to it (like methods, trait/impl associated items, etc). However, fields, variants and impl blocks should not (explications below). +All items with a documentation page will have a link to it (like methods, trait/impl associated items, etc). However, fields, variants and impl blocks won't (explications below). -The link is generated on their ident/name where they are declared, not when they are in use. For example: +The link will be generated on their ident/name where they are declared, not when they are in use. For example: ```rust pub struct Foo; @@ -130,26 +130,26 @@ pub struct Foo; let x = Foo; ``` -On `Foo` definition (`pub struct Foo`), we generate a link to its documentation page whereas on its usage (`x = Foo`), we generate a link to jump to its definition. +On `Foo` definition (`pub struct Foo`), we will generate a link to its documentation page whereas on its usage (`x = Foo`), we will generate a link to jump to its definition. -This is why generating a link for the impl blocks would be complicated: on `impl Foo`, we generate a "jump to definition" link on `Foo`, leaving no space to generate a link to the impl documentation. +This is why generating a link for the impl blocks would be complicated: on `impl Foo`, we will generate a "jump to definition" link on `Foo`, leaving no space to generate a link to the impl documentation. For fields and variants, it's simply to limit the quantity of links generated: you can simply click on their parent item to get to their documentation page. ### Jump to definition -A small preamble before listing the items we should generate links for: if an item which should get a link is preceded by a path, the whole path is used as a link. For example: +A small preamble before listing the items we should generate links for: if an item which will get a link is preceded by a path, each part of the path will be a link to its target. For example: ```rust -let x: b::c::D; // `b::c::D` is the link to `D`. -foo::some_fn(); // `foo::some_fn` is the link to `some_fn`. +let x: b::c::D; // `b` is a link to `b`, `c` is a link to `b::c`, `D` is a link to `b::c::D`. +foo::some_fn(); // `foo` is a link to `foo` and `some_fn` is the link to `foo::some_fn`. ``` -Anything not specifically mentioned should not get a link generated for itself. +Anything not specifically mentioned won't get a link generated for itself. #### Local variables -Local variables should get a link to their definition (not their type): +Local variables will get a link to their definition (not their type): ```rust fn f(mut a: String) { @@ -160,36 +160,36 @@ fn f(mut a: String) { The reasons about supporting local variables are as follow: * We want to linkify global variables, because they may be far away or in another file. - * If you're looking at a variable reference in a function, you don't know whether it's a local variable or a global one (or one from an enclosing scope). By linkifying variables we make it easy for people to find out the answer. If we linkify only some variables (the global ones), it will appear weird that some are linkified and some are not. So for coherency, it should be done for all local variables. + * If you're looking at a variable reference in a function, you don't know whether it's a local variable or a global one (or one from an enclosing scope). By linkifying variables we make it easy for people to find out the answer. If we linkify only some variables (the global ones), it will appear weird that some are linkified and some are not. So for coherency, it will be done for all local variables. #### Primitive types -No primitive type should get a link. +No primitive type will get a link. #### Fields -They should not get a link either. +They will not get a link either. #### Variants -They should get a link, but only on their usage (as already specified in the "jump to documentation" section): +They will get a link, but only on their usage (as already specified in the "jump to documentation" section): ```rust enum Enum { Variant, // No link here. } -let x = Enum::Variant; // `Enum::Variant` is the link to `Variant` definition. +// `Enum` links to `Enum` and `Variant` is the link to `Enum::Variant` definition. +let x = Enum::Variant; ``` #### Imports -For a single import such as `use foo::bar;`, `foo::bar` would be the link. -For a multiple import such as `use foo::{a, b, c};`, `a`, `b` and `c` would be 3 different links. +Imports will have links as well, following the [Paths](#Paths) rules. #### Types -Whenever a type appears, we should link to its definition. For example: +Whenever a type appears, we will link to its definition. For example: ```rust let x: String = "a".to_owned(); // we generate a link on `String`. @@ -200,25 +200,25 @@ fn foo(f: F, t: T) {} // We generate a link on `Displa #### Functions/methods -Whenever a function/method is present, we should generate a link to it. Example: +Whenever a function/method is present, we will generate a link to it. Example: ```rust -let x: Fn() = some_fn; // We should link to `some_fn`. -some_fn(); // We should link to `some_fn`. -ty.a().b(); // We should link to `a` and to `b`. +let x: Fn() = some_fn; // We will link to `some_fn`. +some_fn(); // We will link to `some_fn`. +ty.a().b(); // We will link to `a` and to `b`. ``` #### Macros -Macro calls should get a generated link: +Macro calls will get a link: ```rust -some_macro!(); // Should link to `some_macro!`. +some_macro!(); // Will link to `some_macro!`. ``` #### Constants/statics -Both these kinds of item should get a link too: +Both these kinds of item will get a link too: ```rust static FOO: u32 = 0; @@ -230,23 +230,33 @@ let y = BAR; // `BAR` gets a link. #### Modules -Inline modules should never generate a "jump to definition" link, only a "jump to documentation" one. However, other modules will behave as follows: if they are private and the `--document-private-items` is not in use, they will generate a "jump to definition" link. Otherwise they will generate a "jump to documentation" link. +Inline modules will never generate a "jump to definition" link, only a "jump to documentation" one. However, other modules will behave as follows: if they are private and the `--document-private-items` is not in use, they will generate a "jump to definition" link. Otherwise they will generate a "jump to documentation" link. #### Paths -Any part of a path which isn't a keyword should get a link. So for example: +Any part of a path which isn't a keyword will get a link. So for example: ```rust use crate::foo::bar{self, Type}; ``` -In this one, `foo`, `bar` and `Type` should get a link. +In this one, `foo`, `bar` and `Type` will get a link. ```rust let x = X::Y::Z; ``` -In this one, `X`, `Y` and `Z` should get a link. +In this one, `X`, `Y` and `Z` will get a link. + +## Cross-crate links + +Some issues that can appear when generate cross-crate links is to link to non-existent locations. For example, if a version your crate depends on when published is yanked, your links (not just source code links but also links in documentation) will target non-existent pages and will result in 404 or "file not found" if local. + +A discussion about this suggested to instead add a new argument in the URL (`?source=true`) which would automatically click on the source link of the item on its documentation page. The problem is that private elements (unless the documentation is generated with `--document-private-items`) don't have a documentation page, which reduces the possibilities of this solution quite a lot. This fix would also only worked on web platform implementing it as it requires a server to pick a compatible version (in the semver meaning) for this crate. Another problem is: what to do if there is no compatible version (in the semver meaning still) available? For example, if there was only a `0.15` version published, `0.14.x` and `0.16.y` are not compatible to it. Then it would very likely just don't redirect and leave the user on the 404 page. + +The idea here would be to follow the current URL strategy in use for cross-crate items: generate a URL relative to the root path. If `--extern-html-root-url` isn't used, then the root-path is the one provided to rustdoc where to build documentation. + +This is somewhat the same answer for `cargo --no-deps`: unless rustdoc actually checks if the dependencies' locations are empty or not, there is no way for rustdoc to know if they're present. As such, it should simply assume that they are present and generate links for them as it does for the foreign items in the documentation pages. ## UI @@ -257,12 +267,12 @@ For the UI, we have the following constraints: Some extra explanations about the statement "less JS code, less maintenance": To have this information in the front-end, the back-end has to generate it in any case (either as a link or any other format). So deciding to use JS to handle this would mean that we still have code in the back-end but we would also have code in the front-end to treat it. -With this in mind, the suggested UI would be like this: +With this in mind, the suggested UI will be like this: * By default, the links have no decoration. - * On hover, the links get an underline decoration. + * On hover, the links will get an underline decoration. -To make it obvious whether a link is "jump to def" or "jump to doc", the underline decoration will be different between them: the "jump to doc" link underline will be dotted. +To make it obvious whether a link is "jump to def" or "jump to doc", the underline decoration will be different between them: the "jump to doc" links underline will be dotted. Basically, the source code pages' UI doesn't change. @@ -273,7 +283,7 @@ Basically, the source code pages' UI doesn't change. If we add this feature, are we doing too much? Rustdoc is supposed to be about documentation, so is providing more information into the source code page really necessary? -The source code pages have been there since the 1.0 release. Even with great documentation, looking at the item's implementation can allow to maybe have a deeper understand on how it works. It can also allow you to learn new idioms and better understand the language. +The source code pages have been there since the 1.0 release. Even with great documentation, looking at the item's implementation can allow to maybe have a deeper understanding on how it works. It can also allow you to learn new idioms and better understand the language. As such, this feature isn't a scope of extension but really something to make the current situation even better by making the source code browsing much more pleasant and convenient. Multiple other documentation tools provide this feature for the same reason. @@ -324,6 +334,8 @@ External library (like `cpan` or `tree-sitter`): They would work to generate lin [SourceGraph](https://sourcegraph.com/github.com/rust-lang/rustc-demangle@2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/-/blob/src/v0.rs?L65): SourceGraph uses [Language Server Index Format (LSIF)](https://github.com/Microsoft/language-server-protocol/blob/main/indexFormat/specification.md) to extract info from a language server and make it available in a static format. And rust-analyzer bindings for LSIF already exist. The problem is that it would require a live server to work, which is a blocker to be integrated into rustdoc as it must work serverless. +Another argument in favour of not relying on an external tool is to have support for cross-crate links as it is a very important part of any rust project. For example, if you're browsing a crate "A" and you encounter an item coming from crate "B" (because crates often delegate significant portions of their implementations to sub-crates, for compile performance and other reasons), it seems obvious that it should be able to link to it. This means any tool that does a good job of navigating Rust source code needs to be aware of that code's dependencies, and how to link directly to source code pages within those dependencies. For instance, `askama_shared/filters/json.rs.html` needs to link to `serde::Serialize`. Rustdoc can do that because it builds a copy of all dependencies when building documentation, and it can use reliable mapping from (crate, version) to a URL pointing to the source code. GitHub or Google Code Search can't really do this mppaing while staying within their own code navigation system, because there is not a reliable mapping from (crate, version) to (repository URL, tag). + # Prior art [prior-art]: #prior-art @@ -347,6 +359,56 @@ As mentionned above, it is actually quite common for documentation tools to have Is it the best way to make the links to go to an item documentation like this? Maybe something adding a small icon to click right besides it should be better? -# Potential extensions +### How to handle functions that do not pass type checking? + +In case a function doesn't pass type checking, but is still cfg-in thanks to a `cfg` (for example, `cfg(doc)`), rustdoc will display errors that don't impact the actual documentation generation. For example, the following code: + +```rust +#[cfg(windows)] +use std::os::windows::io::AsRawHandle; + +#[cfg(any(doc, windows))] +pub fn my_fn() -> std::io::Result { + let f = std::fs::File::create("foo.txt")?; + let _handle = f.as_raw_handle(); + + // use handle with native windows bindings + Ok(f) +} +``` + +gives the following output if we're not on windows: + +``` +$ rustdoc +dev foo.rs +$ rustdoc +dev -Z unstable-options --generate-link-to-definition foo.rs +error[E0599]: no method named `as_raw_handle` found for struct `std::fs::File` in the current scope + --> foo.rs:7:21 + | +7 | let _handle = f.as_raw_handle(); + | ^^^^^^^^^^^^^ method not found in `std::fs::File` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. +``` + +Originally, in [#84176](https://github.com/rust-lang/rust/pull/84176), the error output was disabled to prevent seeing these errors. It was later reverted in [#93222](https://github.com/rust-lang/rust/pull/93222) because it was a very bad approach to solving this issue. + +A position that can be taken on this problem would be to simply keep it as is since these errors are legitimate and actually provide useful information to users. + +## Adding an heuristic to automatically disable the feature on a file + +If a file contains too many links, it could potentially worsen the browsing experience. It was suggested to add an heuristic to determine if the feature should be disabled on a file. A few of thems were suggested: + + * Disable when the file is too big: the problem with this approach is that a file could very well contain only documentation or comments and almost no links. + * Disable if the generated HTML is too big: this approach seems like the most reliable one but has a big inconvenient: it requires for the file to be generated to check its size, forcing us to re-render it in case it's too big but this time with the feature disabled. + * Disable if there are too many "span links" for this file: we have this information when we generate the source code page but we don't know how many "span links" are actually present by file unless we compute it. It would require to get the filename of each span to then increment the related counter. It would very likely be bad for performance as it requires to retrieve the file from each span we store and store increment the counter from the map. + +Another issue from all these heuristics is that it will very likely require some checks from usage before we can have a realistic value to use. As such, maybe these solutions should be visited when we have more usage of this feature in its definitive form? + +It's an issue that we expect to concern only a small subset of all the Rust crates though. + +### Potential extensions A potential extension would be "find references". It would allow users to check where the given item is being used. We already have all the information needed for this feature, so it's mostly how to use it and show it to the end users. However, if we decide to move forward with it, it will be part of another RFC. From b7a206da792fc27962227697f3997da20ea6fc06 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 24 Oct 2022 21:50:35 +0200 Subject: [PATCH 13/14] Fix terminology and improve wording --- text/000-rustdoc-jump-to-definition.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index ede5675f60e..8ed6cabdf30 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -120,7 +120,7 @@ There are two kinds of links: ### Jump to documentation -All items with a documentation page will have a link to it (like methods, trait/impl associated items, etc). However, fields, variants and impl blocks won't (explications below). +Each public [item](https://doc.rust-lang.org/reference/items.html) should have a link to its documentation (like methods, trait/impl associated items, etc). Fields, variants and impl blocks should not. The link will be generated on their ident/name where they are declared, not when they are in use. For example: @@ -138,7 +138,7 @@ For fields and variants, it's simply to limit the quantity of links generated: y ### Jump to definition -A small preamble before listing the items we should generate links for: if an item which will get a link is preceded by a path, each part of the path will be a link to its target. For example: +We will generate a link for each path segment of a path to its respective definition. For example: ```rust let x: b::c::D; // `b` is a link to `b`, `c` is a link to `b::c`, `D` is a link to `b::c::D`. @@ -189,7 +189,7 @@ Imports will have links as well, following the [Paths](#Paths) rules. #### Types -Whenever a type appears, we will link to its definition. For example: +Whenever we encounter a type path that refers to a type defined by an item (this excludes type parameters), we will link to its definition. For example: ```rust let x: String = "a".to_owned(); // we generate a link on `String`. @@ -203,7 +203,7 @@ fn foo(f: F, t: T) {} // We generate a link on `Displa Whenever a function/method is present, we will generate a link to it. Example: ```rust -let x: Fn() = some_fn; // We will link to `some_fn`. +let x: fn() = some_fn; // We will link to `some_fn`. some_fn(); // We will link to `some_fn`. ty.a().b(); // We will link to `a` and to `b`. ``` From 2dd4f6c231b6636a5970d834971252f956224b8a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 31 Oct 2022 16:16:14 +0100 Subject: [PATCH 14/14] Add clarifications and new unresolved question --- text/000-rustdoc-jump-to-definition.md | 32 ++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/text/000-rustdoc-jump-to-definition.md b/text/000-rustdoc-jump-to-definition.md index 8ed6cabdf30..5537d189b30 100644 --- a/text/000-rustdoc-jump-to-definition.md +++ b/text/000-rustdoc-jump-to-definition.md @@ -252,11 +252,11 @@ In this one, `X`, `Y` and `Z` will get a link. Some issues that can appear when generate cross-crate links is to link to non-existent locations. For example, if a version your crate depends on when published is yanked, your links (not just source code links but also links in documentation) will target non-existent pages and will result in 404 or "file not found" if local. -A discussion about this suggested to instead add a new argument in the URL (`?source=true`) which would automatically click on the source link of the item on its documentation page. The problem is that private elements (unless the documentation is generated with `--document-private-items`) don't have a documentation page, which reduces the possibilities of this solution quite a lot. This fix would also only worked on web platform implementing it as it requires a server to pick a compatible version (in the semver meaning) for this crate. Another problem is: what to do if there is no compatible version (in the semver meaning still) available? For example, if there was only a `0.15` version published, `0.14.x` and `0.16.y` are not compatible to it. Then it would very likely just don't redirect and leave the user on the 404 page. +A discussion about this suggested to instead add a new argument in the URL (`?source=true`) which would automatically click on the source link of the item on its documentation page. The problem is that hidden elements don't have a documentation page, so this would lead to another 404 error. This fix would also only worked on web platform implementing it since it requires a server to pick a compatible version (in the semver meaning) for this crate. Another problem is: what to do if there is no compatible version (in the semver meaning) available? For example, if there was only a `0.15` version published, `0.14.x` and `0.16.y` are not compatible to it. Then it would very likely just don't redirect and leave the user on the 404 page. The idea here would be to follow the current URL strategy in use for cross-crate items: generate a URL relative to the root path. If `--extern-html-root-url` isn't used, then the root-path is the one provided to rustdoc where to build documentation. -This is somewhat the same answer for `cargo --no-deps`: unless rustdoc actually checks if the dependencies' locations are empty or not, there is no way for rustdoc to know if they're present. As such, it should simply assume that they are present and generate links for them as it does for the foreign items in the documentation pages. +This is somewhat the same answer for `cargo --no-deps`: unless rustdoc actually checks if the dependencies' generated documentation folders are empty or not, there is no way for rustdoc to know if they exist or not. As such, it should simply assume that they are present and generate links for them as it does for the foreign items in the documentation pages. ## UI @@ -334,7 +334,23 @@ External library (like `cpan` or `tree-sitter`): They would work to generate lin [SourceGraph](https://sourcegraph.com/github.com/rust-lang/rustc-demangle@2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/-/blob/src/v0.rs?L65): SourceGraph uses [Language Server Index Format (LSIF)](https://github.com/Microsoft/language-server-protocol/blob/main/indexFormat/specification.md) to extract info from a language server and make it available in a static format. And rust-analyzer bindings for LSIF already exist. The problem is that it would require a live server to work, which is a blocker to be integrated into rustdoc as it must work serverless. -Another argument in favour of not relying on an external tool is to have support for cross-crate links as it is a very important part of any rust project. For example, if you're browsing a crate "A" and you encounter an item coming from crate "B" (because crates often delegate significant portions of their implementations to sub-crates, for compile performance and other reasons), it seems obvious that it should be able to link to it. This means any tool that does a good job of navigating Rust source code needs to be aware of that code's dependencies, and how to link directly to source code pages within those dependencies. For instance, `askama_shared/filters/json.rs.html` needs to link to `serde::Serialize`. Rustdoc can do that because it builds a copy of all dependencies when building documentation, and it can use reliable mapping from (crate, version) to a URL pointing to the source code. GitHub or Google Code Search can't really do this mppaing while staying within their own code navigation system, because there is not a reliable mapping from (crate, version) to (repository URL, tag). +Another argument in favour of not relying on an external tool is to have support for cross-crate links as it is a very important part of any rust project. Contrary to external tools like GitHub or Google Code Searchm it knows where the dependencies' documentations are. Here is an example with GitHub's Code Search preview: + +Visit [`servo::decoder`](https://cs.github.com/servo/servo/blob/9901cb399320e67c3ad2d62bf3a51d6ca993667b/components/net/decoder.rs#L102), part of the servo source code where it defines a gzip decoder: + +```rust +/// A gzip decoder that reads from a `libflate::gzip::Decoder` into a `BytesMut` and emits the results +/// as a `Bytes`. +struct Gzip { + inner: Box>>>, + buf: BytesMut, + reader: Arc>>, +} +``` + +How do you get to the definition of `gzip::Decoder`? If you click on it, GitHub Code Search tells you the definition is in the same file at line 69. Unforuntately, it's wrong: it's another struct that happens to share the name `Decoder`. + +But even setting aside that mistake, could GitHub Code Search find the source for `libflate::non_blocking::gzip::Decoder` and link there? Unfortunately not since it's in a different repository and it's very likely that GitHub Code Search does not have any notion of cross-repository dependencies that follows Rust's package graph. # Prior art [prior-art]: #prior-art @@ -361,7 +377,7 @@ Is it the best way to make the links to go to an item documentation like this? M ### How to handle functions that do not pass type checking? -In case a function doesn't pass type checking, but is still cfg-in thanks to a `cfg` (for example, `cfg(doc)`), rustdoc will display errors that don't impact the actual documentation generation. For example, the following code: +In case a function doesn't pass type checking, but is still cfg-in thanks to a `cfg` (for example, `cfg(doc)`), rustdoc will display errors (**that don't impact the actual documentation generation in any way! Even if errors are displayed, the documentation is generated as expected**). For example, the following code: ```rust #[cfg(windows)] @@ -397,7 +413,7 @@ Originally, in [#84176](https://github.com/rust-lang/rust/pull/84176), the error A position that can be taken on this problem would be to simply keep it as is since these errors are legitimate and actually provide useful information to users. -## Adding an heuristic to automatically disable the feature on a file +### Adding an heuristic to automatically disable the feature on a file If a file contains too many links, it could potentially worsen the browsing experience. It was suggested to add an heuristic to determine if the feature should be disabled on a file. A few of thems were suggested: @@ -409,6 +425,12 @@ Another issue from all these heuristics is that it will very likely require some It's an issue that we expect to concern only a small subset of all the Rust crates though. +### Adding a link to documentation for implementation blocks? + +Even though impl blocks are present in the documentation pages, we currently don't suggest adding a link to their documentation on them. A possibility would be to add the link on the `impl` keyword directly. + +One downside is that it wouldn't be coherent with the other links which are on the items' ident. + ### Potential extensions A potential extension would be "find references". It would allow users to check where the given item is being used. We already have all the information needed for this feature, so it's mostly how to use it and show it to the end users. However, if we decide to move forward with it, it will be part of another RFC.