Skip to content

Provide doc links at item definitions on source pages #89157

New issue

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

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

Already on GitHub? # to your account

Closed
Closed
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
1 change: 1 addition & 0 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
@@ -517,6 +517,7 @@ impl clean::GenericArgs {
}

// Possible errors when computing href link source for a `DefId`
#[derive(Debug)]
pub(crate) enum HrefError {
/// This item is known to rustdoc, but from a crate that does not have documentation generated.
///
17 changes: 12 additions & 5 deletions src/librustdoc/html/highlight.rs
Original file line number Diff line number Diff line change
@@ -728,18 +728,25 @@ fn string<T: Display>(
LinkFromSrc::Local(span) => context
.href_from_span(*span, true)
.map(|s| format!("{}{}", context_info.root_path, s)),
LinkFromSrc::External(def_id) => {
format::href_with_root_path(*def_id, context, Some(context_info.root_path))
.ok()
.map(|(url, _, _)| url)
}
LinkFromSrc::External(span) => context.href_from_span(*span, true).map(|s| {
if s.starts_with("http://") || s.starts_with("https://") {
s
} else {
format!("{}{}", context_info.root_path, s)
}
}),
LinkFromSrc::Primitive(prim) => format::href_with_root_path(
PrimitiveType::primitive_locations(context.tcx())[prim],
context,
Some(context_info.root_path),
)
.ok()
.map(|(url, _, _)| url),
LinkFromSrc::Doc(def_id) => {
format::href_with_root_path(*def_id, context, Some(&context_info.root_path))
.ok()
.map(|(doc_link, _, _)| doc_link)
}
}
})
{
8 changes: 6 additions & 2 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
@@ -116,7 +116,6 @@ pub(crate) struct SharedContext<'tcx> {
/// to `Some(...)`, it'll store redirections and then generate a JSON file at the top level of
/// the crate.
redirections: Option<RefCell<FxHashMap<String, String>>>,

/// Correspondance map used to link types used in the source code pages to allow to click on
/// links to jump to the type's definition.
pub(crate) span_correspondance_map: FxHashMap<rustc_span::Span, LinkFromSrc>,
@@ -336,7 +335,12 @@ impl<'tcx> Context<'tcx> {
let e = ExternalCrate { crate_num: cnum };
(e.name(self.tcx()), e.src_root(self.tcx()))
}
ExternalLocation::Unknown => return None,
ExternalLocation::Unknown => {
let e = ExternalCrate { crate_num: cnum };
let name = e.name(self.tcx());
root = name.to_string();
(name, e.src_root(self.tcx()))
}
};

sources::clean_path(&src_root, file, false, |component| {
94 changes: 72 additions & 22 deletions src/librustdoc/html/render/span_map.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use crate::clean::types::rustc_span;
use crate::clean::{self, PrimitiveType};
use crate::html::sources;

use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{ExprKind, HirId, Mod, Node};
use rustc_hir::{ExprKind, HirId, Item, ItemKind, Mod, Node};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;
@@ -22,8 +23,9 @@ use std::path::{Path, PathBuf};
#[derive(Debug)]
pub(crate) enum LinkFromSrc {
Local(clean::Span),
External(DefId),
External(clean::Span),
Primitive(PrimitiveType),
Doc(DefId),
}

/// This function will do at most two things:
@@ -64,31 +66,52 @@ struct SpanMapVisitor<'tcx> {
impl<'tcx> SpanMapVisitor<'tcx> {
/// This function is where we handle `hir::Path` elements and add them into the "span map".
fn handle_path(&mut self, path: &rustc_hir::Path<'_>, path_span: Option<Span>) {
let info = match path.res {
match path.res {
// FIXME: For now, we only handle `DefKind` if it's not `DefKind::TyParam` or
// `DefKind::Macro`. Would be nice to support them too alongside the other `DefKind`
// (such as primitive types!).
Res::Def(kind, def_id) if kind != DefKind::TyParam => {
if matches!(kind, DefKind::Macro(_)) {
return;
}
Some(def_id)
let span = rustc_span(def_id, self.tcx);
let link = if def_id.as_local().is_some() {
LinkFromSrc::Local(span)
} else {
LinkFromSrc::External(span)
};
self.matches.insert(path_span.unwrap_or(path.span), link);
}
Res::Local(_) => {
if let Some(span) = self.tcx.hir().res_span(path.res) {
self.matches.insert(
path_span.unwrap_or(path.span),
LinkFromSrc::Local(clean::Span::new(span)),
);
}
}
Res::Local(_) => None,
Res::PrimTy(p) => {
// FIXME: Doesn't handle "path-like" primitives like arrays or tuples.
let span = path_span.unwrap_or(path.span);
self.matches.insert(span, LinkFromSrc::Primitive(PrimitiveType::from(p)));
return;
}
Res::Err => return,
_ => return,
};
if let Some(span) = self.tcx.hir().res_span(path.res) {
self.matches
.insert(path_span.unwrap_or(path.span), LinkFromSrc::Local(clean::Span::new(span)));
} else if let Some(def_id) = info {
self.matches.insert(path_span.unwrap_or(path.span), LinkFromSrc::External(def_id));
Res::Err => {}
_ => {}
}
}

/// Used to generate links on items' definition to go to their documentation page.
pub(crate) fn extract_info_from_hir_id(&mut self, hir_id: HirId) {
if let Some(def_id) = self.tcx.hir().opt_local_def_id(hir_id) {
if let Some(span) = self.tcx.def_ident_span(def_id) {
let cspan = clean::Span::new(span);
let def_id = def_id.to_def_id();
// If the span isn't from the current crate, we ignore it.
if cspan.is_dummy() || cspan.cnum(self.tcx.sess) != LOCAL_CRATE {
return;
}
self.matches.insert(span, LinkFromSrc::Doc(def_id));
}
}
}
}
@@ -117,6 +140,9 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> {
LinkFromSrc::Local(clean::Span::new(m.spans.inner_span)),
);
}
} else {
// If it's a "mod foo {}", we want to look to its documentation page.
self.extract_info_from_hir_id(id);
}
intravisit::walk_mod(self, m, id);
}
@@ -134,13 +160,13 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> {
hir.maybe_body_owned_by(body_id).expect("a body which isn't a body"),
);
if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) {
self.matches.insert(
segment.ident.span,
match hir.span_if_local(def_id) {
Some(span) => LinkFromSrc::Local(clean::Span::new(span)),
None => LinkFromSrc::External(def_id),
},
);
let span = rustc_span(def_id, self.tcx);
let link = if def_id.as_local().is_some() {
LinkFromSrc::Local(span)
} else {
LinkFromSrc::External(span)
};
self.matches.insert(segment.ident.span, link);
}
}
}
@@ -151,4 +177,28 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> {
self.handle_path(path, None);
intravisit::walk_use(self, path, id);
}

fn visit_item(&mut self, item: &'tcx Item<'tcx>) {
match item.kind {
ItemKind::Static(_, _, _)
| ItemKind::Const(_, _)
| ItemKind::Fn(_, _, _)
| ItemKind::Macro(_, _)
| ItemKind::TyAlias(_, _)
| ItemKind::Enum(_, _)
| ItemKind::Struct(_, _)
| ItemKind::Union(_, _)
| ItemKind::Trait(_, _, _, _, _)
| ItemKind::TraitAlias(_, _) => self.extract_info_from_hir_id(item.hir_id()),
ItemKind::Impl(_)
| ItemKind::Use(_, _)
| ItemKind::ExternCrate(_)
| ItemKind::ForeignMod { .. }
| ItemKind::GlobalAsm(_)
| ItemKind::OpaqueTy(_)
// We already have "visit_mod" above so no need to check it here.
| ItemKind::Mod(_) => {}
}
intravisit::walk_item(self, item);
}
}
6 changes: 3 additions & 3 deletions src/test/rustdoc-gui/source-anchor-scroll.goml
Original file line number Diff line number Diff line change
@@ -7,11 +7,11 @@ size: (600, 800)
// We check that the scroll is at the top first.
assert-property: ("html", {"scrollTop": "0"})

click: '//a[text() = "barbar"]'
click: '//a[text() = "barbar" and @href="../../src/link_to_definition/lib.rs.html#4-6"]'
assert-property: ("html", {"scrollTop": "125"})
click: '//a[text() = "bar"]'
click: '//a[text() = "bar" and @href="../../src/link_to_definition/lib.rs.html#27-35"]'
assert-property: ("html", {"scrollTop": "166"})
click: '//a[text() = "sub_fn"]'
click: '//a[text() = "sub_fn" and @href="../../src/link_to_definition/lib.rs.html#1-3"]'
assert-property: ("html", {"scrollTop": "53"})

// We now check that clicking on lines doesn't change the scroll
4 changes: 2 additions & 2 deletions src/test/rustdoc/check-source-code-urls-to-def.rs
Original file line number Diff line number Diff line change
@@ -28,11 +28,11 @@ impl Foo {

fn babar() {}

// @has - '//a/@href' '/struct.String.html'
// @has - '//a/@href' '/string.rs.html'
// @has - '//a/@href' '/primitive.u32.html'
// @has - '//a/@href' '/primitive.str.html'
// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#23"]' 5
// @has - '//a[@href="../../source_code/struct.SourceCode.html"]' 'source_code::SourceCode'
// @has - '//a[@href="../../src/source_code/source_code.rs.html#1"]' 'source_code::SourceCode'
pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar, f: source_code::SourceCode) {
let x = 12;
let y: Foo = Foo;
51 changes: 51 additions & 0 deletions src/test/rustdoc/jump-to-def-doc-links.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// compile-flags: -Zunstable-options --generate-link-to-definition

#![crate_name = "foo"]

// @has 'src/foo/jump-to-def-doc-links.rs.html'

// @has - '//a[@href="../../foo/struct.Bar.html"]' 'Bar'
// @has - '//a[@href="../../foo/struct.Foo.html"]' 'Foo'
pub struct Bar; pub struct Foo;

// @has - '//a[@href="../../foo/enum.Enum.html"]' 'Enum'
pub enum Enum {
Variant1(String),
Variant2(u8),
}

// @has - '//a[@href="../../foo/struct.Struct.html"]' 'Struct'
pub struct Struct {
pub a: u8,
b: Foo,
}

impl Struct {
pub fn foo() {}
pub fn foo2(&self) {}
fn bar() {}
fn bar(&self) {}
}

// @has - '//a[@href="../../foo/trait.Trait.html"]' 'Trait'
pub trait Trait {
fn foo();
}

impl Trait for Struct {
fn foo() {}
}

// @has - '//a[@href="../../foo/union.Union.html"]' 'Union'
pub union Union {
pub a: u16,
pub f: u32,
}

// @has - '//a[@href="../../foo/fn.bar.html"]' 'bar'
pub fn bar(b: Bar) {
let x = Foo;
}

// @has - '//a[@href="../../foo/bar/index.html"]' 'bar'
pub mod bar {}