[rustdoc] Add new example disambiguator for intra-doc links

[GuillaumeGomez]

Fixes #130493.

This PR adds the example disambiguator, which allows to generate a link to a scraped example source file. It works as follows: [example@example_name]. If you only provide the example name, it will link to the file containing the main function. However, if you want to link to another file of an example, you can specify a path starting with the example name: [example@example_name/file.rs].

r? @notriddle

[GuillaumeGomez]

Changes in this file were needed because tests/rustdoc/cross-crate-info/working-dir-examples/q.rs uses the @has command on the metadata file generated by --scrape-examples. However, with my changes, some of its content is not utf8 compatible anymore, breaking the test.

Now, it only read the content if we actually need to match the content of this file.

[GuillaumeGomez]

Fixed book examples.

[notriddle]

Imagine the following scenario:

• A crate in docs.rs includes an example link, let's call it crate_a. Its example can be found at https://docs.rs/crate_a/1.0/src/example/example.rs (the link is actually going to be a relative URL, something like ../src/example/example.rs, and that's fine).
• A second crate in docs.rs, second_crate, inlines that example link. That won't work, because it will try to link inside of its own source area instead of the one behind a third party. https://docs.rs/second_crate/0.1/src/example/example.rs does not exist, or, if it does, it's not the same thing.

I think example links need to carry a CrateNum, and the link-generating code needs to consult Cache::extern_locations.

[GuillaumeGomez]

Very good point, thanks!

[GuillaumeGomez]

Gave it a few tries but I don't see how it could work: the code is relying on the presence of the examples crates passed with with --with-examples. If you're inlining an item with examples intra doc link, it'll simply not work since we don't have the information provided by cargo.

[GuillaumeGomez]

What I did:

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index fc0f73df799..553cfb0f2a1 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -506,13 +506,23 @@ pub(crate) fn links(&self, cx: &Context<'_>) -> Vec<RenderedLink> {
                     }
                     None
                 }
-                ItemLinkKind::Example { file_path } => {
-                    let example_name = file_path.split('/').next().unwrap_or(file_path);
-                    let mut href =
-                        std::iter::repeat("../").take(cx.current.len()).collect::<String>();
+                ItemLinkKind::Example { file_path, krate } => {
+                    let mut href = if *krate != LOCAL_CRATE {
+                        let Some(location) = cx.cache().extern_locations.get(krate) else { return None };
+                        match location {
+                            ExternalLocation::Remote(s) => s.clone(),
+                            ExternalLocation::Local => {
+                                std::iter::repeat("../").take(cx.current.len()).collect::<String>()
+                            }
+                            ExternalLocation::Unknown => return None,
+                        }
+                    } else {
+                        std::iter::repeat("../").take(cx.current.len()).collect::<String>()
+                    };
                     href.push_str("src/");
                     href.push_str(file_path);
                     href.push_str(".html");
+                    let example_name = file_path.split('/').next().unwrap_or(file_path);
                     Some(RenderedLink {
                         original_text: s.clone(),
                         new_text: link_text.clone(),
@@ -1141,6 +1151,8 @@ pub(crate) enum ItemLinkKind {
     Example {
         /// The path of the example file.
         file_path: String,
+        /// Crate of the item containing this link.
+        krate: CrateNum,
     },
 }
 
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index b7de93d08c2..6a1b39ba1ec 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -14,7 +14,7 @@
 use rustc_errors::{Applicability, Diag, DiagMessage};
 use rustc_hir::def::Namespace::*;
 use rustc_hir::def::{DefKind, Namespace, PerNS};
-use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LOCAL_CRATE};
+use rustc_hir::def_id::{CRATE_DEF_ID, CrateNum, DefId, LOCAL_CRATE};
 use rustc_hir::{Mutability, Safety};
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_middle::{bug, span_bug, ty};
@@ -68,7 +68,7 @@ fn filter_assoc_items_by_name_and_namespace<'a>(
 pub(crate) enum Res {
     Def(DefKind, DefId),
     Primitive(PrimitiveType),
-    Example(String),
+    Example(String, CrateNum),
 }
 
 type ResolveRes = rustc_hir::def::Res<rustc_ast::NodeId>;
@@ -78,7 +78,7 @@ fn descr(&self) -> &'static str {
         match self {
             Res::Def(kind, id) => ResolveRes::Def(*kind, *id).descr(),
             Res::Primitive(_) => "primitive type",
-            Res::Example(_) => "example",
+            Res::Example(..) => "example",
         }
     }
 
@@ -86,7 +86,7 @@ fn article(&self) -> &'static str {
         match self {
             Res::Def(kind, id) => ResolveRes::Def(*kind, *id).article(),
             Res::Primitive(_) => "a",
-            Res::Example(_) => "an",
+            Res::Example(..) => "an",
         }
     }
 
@@ -94,7 +94,7 @@ fn name(&self, tcx: TyCtxt<'_>) -> Symbol {
         match self {
             Res::Def(_, id) => tcx.item_name(*id),
             Res::Primitive(prim) => prim.as_sym(),
-            Res::Example(_) => panic!("no name"),
+            Res::Example(..) => panic!("no name"),
         }
     }
 
@@ -102,7 +102,7 @@ fn def_id(&self, tcx: TyCtxt<'_>) -> Option<DefId> {
         match self {
             Res::Def(_, id) => Some(*id),
             Res::Primitive(prim) => PrimitiveType::primitive_locations(tcx).get(prim).copied(),
-            Res::Example(_) => None,
+            Res::Example(..) => None,
         }
     }
 
@@ -114,7 +114,7 @@ fn from_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> Res {
     fn disambiguator_suggestion(&self) -> Suggestion {
         let kind = match self {
             Res::Primitive(_) => return Suggestion::Prefix("prim"),
-            Res::Example(_) => return Suggestion::Prefix("example"),
+            Res::Example(..) => return Suggestion::Prefix("example"),
             Res::Def(kind, _) => kind,
         };
 
@@ -1177,7 +1177,7 @@ pub(crate) fn resolve_ambiguities(&mut self) {
                 info.resolved.retain(|(res, _)| match res {
                     Res::Def(_, def_id) => self.validate_link(*def_id),
                     // Primitive types and examples are always valid.
-                    Res::Primitive(_) | Res::Example(_) => true,
+                    Res::Primitive(_) | Res::Example(..) => true,
                 });
                 let diag_info = info.diag_info.into_info();
                 match info.resolved.len() {
@@ -1313,11 +1313,11 @@ fn compute_link(
                     kind: ItemLinkKind::Item { page_id },
                 })
             }
-            Res::Example(path) => Some(ItemLink {
+            Res::Example(path, krate) => Some(ItemLink {
                 link: Box::<str>::from(&*diag_info.ori_link),
                 link_text: link_text.clone(),
                 fragment,
-                kind: ItemLinkKind::Example { file_path: path.into() },
+                kind: ItemLinkKind::Example { file_path: path.into(), krate },
             }),
         }
     }
@@ -1483,12 +1483,13 @@ fn get_example_file(
         &self,
         path_str: &str,
         diag: DiagnosticInfo<'_>,
+        item_id: DefId,
     ) -> Vec<(Res, Option<DefId>)> {
         // If the user is referring to the example by its name:
         if let Some(files) = self.cx.render_options.examples_files.get(path_str)
             && let Some(file_path) = files.iter().next()
         {
-            return vec![(Res::Example(file_path.clone()), None)];
+            return vec![(Res::Example(file_path.clone(), item_id.krate), None)];
         }
         // If the user is referring to a specific file of the example, it'll be of this form:
         //
@@ -1497,7 +1498,7 @@ fn get_example_file(
             && let Some(files) = self.cx.render_options.examples_files.get(crate_name)
             && let Some(file_path) = files.get(path_str)
         {
-            return vec![(Res::Example(file_path.clone()), None)];
+            return vec![(Res::Example(file_path.clone(), item_id.krate), None)];
         }
         report_diagnostic(
             self.cx.tcx,
@@ -1528,7 +1529,7 @@ fn resolve_with_disambiguator(
         let module_id = key.module_id;
 
         if matches!(disambiguator, Some(Disambiguator::Example)) {
-            return self.get_example_file(path_str, diag);
+            return self.get_example_file(path_str, diag, item_id);
         }
         match disambiguator.map(Disambiguator::ns) {
             Some(expected_ns) => {
@@ -2077,7 +2078,7 @@ fn resolution_failure(
                         partial_res.as_ref().expect("None case was handled by `last_found_module`");
                     let kind_did = match res {
                         Res::Def(kind, did) => Some((kind, did)),
-                        Res::Primitive(_) | Res::Example(_) => None,
+                        Res::Primitive(_) | Res::Example(..) => None,
                     };
                     let is_struct_variant = |did| {
                         if let ty::Adt(def, _) = tcx.type_of(did).instantiate_identity().kind()

[notriddle]

It sounds like this would need to tie into https://github.com/rust-lang/rust/blob/master/compiler/rustc_resolve/src/rustdoc.rs so that the list of examples (at least their names, but not necessarily the actual line-by-line connections) would be present in the libs metadata so that they can be accessed when inlining.

[notriddle]

Why is this a GUI test?

GUI tests are slower, harder to write, require multiple test cases to live together in the same doc tree, and, in this case, I don't think it's needed. There's no custom JavaScript or CSS here. It's just a link, and we can check those with xpath-based test cases.

[GuillaumeGomez]

Problem doesn't come from the testing part but from the code generation part. I couldn't find a way with our HTML tests to generate code with the scrape-example feature. If you have an idea, I'd really like to hear it as relying on GUI tests for this is really not great (as you underlined it very well).

[notriddle]

Okay.

Could this be done with a run-make test then?

[GuillaumeGomez]

I thought about it, seemed a bit less good but if you prefer it, let's go it with a run-make test.

[rustbot]

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

[GuillaumeGomez]

I migrated the test to run-make. For inlined intra-doc links on examples, we can't do it with the information we currently have unfortunately since examples are bound to their crate.

[bors]

☔ The latest upstream changes (presumably #133561) made this pull request unmergeable. Please resolve the merge conflicts.