Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/doc/rustdoc/src/scraped-examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ Rustdoc has a few techniques to ensure these examples don't overwhelm documentat

For a given item, Rustdoc sorts its examples based on the size of the example — smaller ones are shown first.

## Linking to an example source code

You can use intra-doc links to link to a scraped example source file with `example@` disambiguator:

```rust
// If your example is named "foo":
/// [example@foo]
struct Item;
```

By default, the intra-doc link will link to the file containing the `main` function. If you want to
link to another file, you can specify its path:

```rust
// If your example is named "foo":
/// [example@foo/another_file.rs]
struct Item;
```

## FAQ

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ rendered as `Foo`. The following prefixes are available: `struct`, `enum`, `trai
`mod`, `module`, `const`, `constant`, `fn`, `function`, `field`, `variant`, `method`, `derive`,
`type`, `value`, `macro`, `prim` or `primitive`.

There is another disambiguator available: `example`. If you want more information about this one,
take a look at the [scraped examples chapter](../scraped-examples.md).

You can also disambiguate for functions by adding `()` after the function name,
or for macros by adding `!` after the macro name. The macro `!` can be followed by `()`, `{}`,
or `[]`. Example:
Expand Down
9 changes: 6 additions & 3 deletions src/etc/htmldocck.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def resolve_path(self, path):
def get_absolute_path(self, path):
return os.path.join(self.root, path)

def get_file(self, path):
def get_file(self, path, need_content):
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

path = self.resolve_path(path)
if path in self.files:
return self.files[path]
Expand All @@ -359,6 +359,9 @@ def get_file(self, path):
if not(os.path.exists(abspath) and os.path.isfile(abspath)):
raise FailedCheck('File does not exist {!r}'.format(path))

if not need_content:
return None

with io.open(abspath, encoding='utf-8') as f:
data = f.read()
self.files[path] = data
Expand Down Expand Up @@ -614,15 +617,15 @@ def check_command(c, cache):
# has <path> = file existence
if len(c.args) == 1 and not regexp and 'raw' not in c.cmd:
try:
cache.get_file(c.args[0])
cache.get_file(c.args[0], False)
ret = True
except FailedCheck as err:
cerr = str(err)
ret = False
# hasraw/matchesraw <path> <pat> = string test
elif len(c.args) == 2 and 'raw' in c.cmd:
cerr = "`PATTERN` did not match"
ret = check_string(cache.get_file(c.args[0]), c.args[1], regexp)
ret = check_string(cache.get_file(c.args[0], True), c.args[1], regexp)
# has/matches <path> <pat> <match> = XML tree test
elif len(c.args) == 3 and 'raw' not in c.cmd:
cerr = "`XPATH PATTERN` did not match"
Expand Down
57 changes: 44 additions & 13 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,21 +487,38 @@ impl Item {
let Some(links) = cx.cache().intra_doc_links.get(&self.item_id) else { return vec![] };
links
.iter()
.filter_map(|ItemLink { link: s, link_text, page_id: id, ref fragment }| {
debug!(?id);
if let Ok((mut href, ..)) = href(*id, cx) {
debug!(?href);
if let Some(ref fragment) = *fragment {
fragment.render(&mut href, cx.tcx())
.filter_map(|ItemLink { link: s, link_text, ref kind, ref fragment }| match kind {
ItemLinkKind::Item { page_id: id } => {
debug!(?id);
if let Some(id) = id {
if let Ok((mut href, ..)) = href(*id, cx) {
debug!(?href);
if let Some(ref fragment) = *fragment {
fragment.render(&mut href, cx.tcx())
}
return Some(RenderedLink {
original_text: s.clone(),
new_text: link_text.clone(),
tooltip: link_tooltip(*id, fragment, cx),
href,
});
}
}
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>();
Comment on lines +511 to +512
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good point, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

href.push_str("src/");
href.push_str(file_path);
href.push_str(".html");
Some(RenderedLink {
original_text: s.clone(),
new_text: link_text.clone(),
tooltip: link_tooltip(*id, fragment, cx),
tooltip: format!("Example {example_name}"),
href,
})
} else {
None
}
})
.collect()
Expand Down Expand Up @@ -1110,6 +1127,23 @@ impl<I: Iterator<Item = ast::MetaItemInner>> NestedAttributesExt for I {
}
}

/// The kind of a link that has not yet been rendered.
///
/// It is used in [`ItemLink`].
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub(crate) enum ItemLinkKind {
Item {
/// The `DefId` of the Item whose **HTML Page** contains the item being
/// linked to. This will be different to `item_id` on item's that don't
/// have their own page, such as struct fields and enum variants.
page_id: Option<DefId>,
},
Example {
/// The path of the example file.
file_path: String,
},
}

/// A link that has not yet been rendered.
///
/// This link will be turned into a rendered link by [`Item::links`].
Expand All @@ -1122,12 +1156,9 @@ pub(crate) struct ItemLink {
/// This may not be the same as `link` if there was a disambiguator
/// in an intra-doc link (e.g. \[`fn@f`\])
pub(crate) link_text: Box<str>,
/// The `DefId` of the Item whose **HTML Page** contains the item being
/// linked to. This will be different to `item_id` on item's that don't
/// have their own page, such as struct fields and enum variants.
pub(crate) page_id: DefId,
/// The url fragment to append to the link
pub(crate) fragment: Option<UrlFragment>,
pub(crate) kind: ItemLinkKind,
}

pub struct RenderedLink {
Expand Down
8 changes: 6 additions & 2 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::html::markdown::IdMap;
use crate::html::render::StylePath;
use crate::html::static_files;
use crate::passes::{self, Condition};
use crate::scrape_examples::{AllCallLocations, ScrapeExamplesOptions};
use crate::scrape_examples::{AllCallLocations, AllExampleFiles, ScrapeExamplesOptions};
use crate::{html, opts, theme};

#[derive(Clone, Copy, PartialEq, Eq, Debug, Default)]
Expand Down Expand Up @@ -287,6 +287,8 @@ pub(crate) struct RenderOptions {
pub(crate) generate_link_to_definition: bool,
/// Set of function-call locations to include as examples
pub(crate) call_locations: AllCallLocations,
/// Set of function-call locations to include as examples
pub(crate) examples_files: AllExampleFiles,
/// If `true`, Context::init will not emit shared files.
pub(crate) no_emit_shared: bool,
/// If `true`, HTML source code pages won't be generated.
Expand Down Expand Up @@ -773,7 +775,8 @@ impl Options {

let scrape_examples_options = ScrapeExamplesOptions::new(matches, dcx);
let with_examples = matches.opt_strs("with-examples");
let call_locations = crate::scrape_examples::load_call_locations(with_examples, dcx);
let (call_locations, examples_files) =
crate::scrape_examples::load_call_locations(with_examples, dcx);

let unstable_features =
rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref());
Expand Down Expand Up @@ -846,6 +849,7 @@ impl Options {
emit,
generate_link_to_definition,
call_locations,
examples_files,
no_emit_shared: false,
html_no_source,
output_to_stdout,
Expand Down
3 changes: 3 additions & 0 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ pub(crate) struct SharedContext<'tcx> {
/// Controls whether we read / write to cci files in the doc root. Defaults read=true,
/// write=true
should_merge: ShouldMerge,
/// Paths of generated files.
pub(crate) emitted_local_sources: RefCell<FxIndexSet<String>>,
}

impl SharedContext<'_> {
Expand Down Expand Up @@ -554,6 +556,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
cache,
call_locations,
should_merge: options.should_merge,
emitted_local_sources: Default::default(),
};

let dst = output;
Expand Down
11 changes: 7 additions & 4 deletions src/librustdoc/html/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(crate) fn render(cx: &mut Context<'_>, krate: &clean::Crate) -> Result<(), E
let crate_name = crate_name.as_str();

let mut collector =
SourceCollector { dst, cx, emitted_local_sources: FxHashSet::default(), crate_name };
SourceCollector { dst, cx, crate_name, emitted_paths: FxHashSet::default() };
collector.visit_crate(krate);
Ok(())
}
Expand Down Expand Up @@ -117,9 +117,10 @@ struct SourceCollector<'a, 'tcx> {

/// Root destination to place all HTML output into
dst: PathBuf,
emitted_local_sources: FxHashSet<PathBuf>,

crate_name: &'a str,

emitted_paths: FxHashSet<PathBuf>,
}

impl DocVisitor<'_> for SourceCollector<'_, '_> {
Expand Down Expand Up @@ -182,7 +183,7 @@ impl SourceCollector<'_, '_> {
}
_ => return Ok(()),
};
if self.emitted_local_sources.contains(&*p) {
if self.emitted_paths.contains(&*p) {
// We've already emitted this source
return Ok(());
}
Expand Down Expand Up @@ -245,6 +246,7 @@ impl SourceCollector<'_, '_> {
resource_suffix: &shared.resource_suffix,
rust_logo: has_doc_flag(self.cx.tcx(), LOCAL_CRATE.as_def_id(), sym::rust_logo),
};
let file_path_s = file_path.display().to_string();
let v = layout::render(
&shared.layout,
&page,
Expand All @@ -264,7 +266,8 @@ impl SourceCollector<'_, '_> {
&shared.style_files,
);
shared.fs.write(cur, v)?;
self.emitted_local_sources.insert(p);
self.emitted_paths.insert(p);
self.cx.shared.emitted_local_sources.borrow_mut().insert(file_path_s);
Ok(())
}
}
Expand Down
24 changes: 15 additions & 9 deletions src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_span::{Pos, Symbol, sym};
use rustdoc_json_types::*;

use super::FullItemId;
use crate::clean::{self, ItemId};
use crate::clean::{self, ItemId, ItemLink, ItemLinkKind};
use crate::formats::FormatRenderer;
use crate::formats::item_type::ItemType;
use crate::json::JsonRenderer;
Expand All @@ -30,14 +30,20 @@ impl JsonRenderer<'_> {
.get(&item.item_id)
.into_iter()
.flatten()
.map(|clean::ItemLink { link, page_id, fragment, .. }| {
let id = match fragment {
Some(UrlFragment::Item(frag_id)) => *frag_id,
// FIXME: Pass the `UserWritten` segment to JSON consumer.
Some(UrlFragment::UserWritten(_)) | None => *page_id,
};

(String::from(&**link), self.id_from_item_default(id.into()))
.filter_map(|ItemLink { link, kind, fragment, .. }| {
if let ItemLinkKind::Item { page_id } = kind
&& let Some(page_id) = page_id
{
let id = match fragment {
Some(UrlFragment::Item(frag_id)) => *frag_id,
// FIXME: Pass the `UserWritten` segment to JSON consumer.
Some(UrlFragment::UserWritten(_)) | None => *page_id,
};

Some((String::from(&**link), self.id_from_item_default(id.into())))
} else {
None
}
})
.collect();
let docs = item.opt_doc_value();
Expand Down
Loading
Loading