From 023231a709b88e1a73ac86076aa5e1b4b3001276 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 6 Apr 2021 00:07:46 +0200 Subject: [PATCH 01/22] Add links on source types to go to definition --- src/librustdoc/clean/types.rs | 5 + src/librustdoc/html/format.rs | 5 +- src/librustdoc/html/highlight.rs | 186 +++++++++++++++++---- src/librustdoc/html/markdown.rs | 3 + src/librustdoc/html/render/context.rs | 64 ++++--- src/librustdoc/html/render/mod.rs | 2 + src/librustdoc/html/render/print_item.rs | 5 +- src/librustdoc/html/render/span_map.rs | 110 ++++++++++++ src/librustdoc/html/render/write_shared.rs | 4 +- src/librustdoc/html/sources.rs | 176 ++++++++++++++----- 10 files changed, 454 insertions(+), 106 deletions(-) create mode 100644 src/librustdoc/html/render/span_map.rs diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 5c73d3de5b93f..85c4731c6bff7 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1950,6 +1950,11 @@ impl Span { Self(sp.source_callsite()) } + /// Unless you know what you're doing, use [`Self::from_rustc_span`] instead! + crate fn wrap(sp: rustc_span::Span) -> Span { + Self(sp) + } + crate fn inner(&self) -> rustc_span::Span { self.0 } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 8ab6aa775d2e4..8b5c147b56cae 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -494,7 +494,10 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec (fqp, shortty, { let module_fqp = to_module_fqp(shortty, fqp); diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 33b1d98313ce3..329cb3f8f09d5 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -5,16 +5,19 @@ //! //! Use the `render_with_highlighting` to highlight some rust code. +use crate::clean; use crate::html::escape::Escape; +use crate::html::render::Context; -use std::fmt::Display; +use std::fmt::{Display, Write}; use std::iter::Peekable; use rustc_lexer::{LiteralKind, TokenKind}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; -use super::format::Buffer; +use super::format::{self, Buffer}; +use super::render::LinkFromSrc; /// Highlights `src`, returning the HTML output. crate fn render_with_highlighting( @@ -25,6 +28,9 @@ crate fn render_with_highlighting( tooltip: Option<(Option, &str)>, edition: Edition, extra_content: Option, + file_span_lo: u32, + context: Option<&Context<'_>>, + root_path: &str, ) { debug!("highlighting: ================\n{}\n==============", src); if let Some((edition_info, class)) = tooltip { @@ -41,7 +47,7 @@ crate fn render_with_highlighting( } write_header(out, class, extra_content); - write_code(out, &src, edition); + write_code(out, &src, edition, file_span_lo, context, root_path); write_footer(out, playground_button); } @@ -57,12 +63,21 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option>, + root_path: &str, +) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); - Classifier::new(&src, edition).highlight(&mut |highlight| { + Classifier::new(&src, edition, file_span_lo).highlight(&mut |highlight| { match highlight { - Highlight::Token { text, class } => string(out, Escape(text), class), + Highlight::Token { text, class } => { + string(out, Escape(text), class, context, root_path) + } Highlight::EnterSpan { class } => enter_span(out, class), Highlight::ExitSpan => exit_span(out), }; @@ -82,14 +97,14 @@ enum Class { KeyWord, // Keywords that do pointer/reference stuff. RefKeyWord, - Self_, + Self_((u32, u32)), Op, Macro, MacroNonTerminal, String, Number, Bool, - Ident, + Ident((u32, u32)), Lifetime, PreludeTy, PreludeVal, @@ -105,20 +120,27 @@ impl Class { Class::Attribute => "attribute", Class::KeyWord => "kw", Class::RefKeyWord => "kw-2", - Class::Self_ => "self", + Class::Self_(_) => "self", Class::Op => "op", Class::Macro => "macro", Class::MacroNonTerminal => "macro-nonterminal", Class::String => "string", Class::Number => "number", Class::Bool => "bool-val", - Class::Ident => "ident", + Class::Ident(_) => "ident", Class::Lifetime => "lifetime", Class::PreludeTy => "prelude-ty", Class::PreludeVal => "prelude-val", Class::QuestionMark => "question-mark", } } + + fn get_span(self) -> Option<(u32, u32)> { + match self { + Self::Ident(sp) | Self::Self_(sp) => Some(sp), + _ => None, + } + } } enum Highlight<'a> { @@ -144,14 +166,23 @@ impl Iterator for TokenIter<'a> { } } -fn get_real_ident_class(text: &str, edition: Edition) -> Class { - match text { +/// Returns `None` if this is a `Class::Ident`. +fn get_real_ident_class(text: &str, edition: Edition, allow_path_keywords: bool) -> Option { + let ignore: &[&str] = + if allow_path_keywords { &["self", "Self", "super", "crate"] } else { &["self", "Self"] }; + if ignore.iter().any(|k| *k == text) { + return None; + } + Some(match text { "ref" | "mut" => Class::RefKeyWord, - "self" | "Self" => Class::Self_, "false" | "true" => Class::Bool, _ if Symbol::intern(text).is_reserved(|| edition) => Class::KeyWord, - _ => Class::Ident, - } + _ => return None, + }) +} + +fn move_span(file_span_lo: u32, start: u32, end: u32) -> (u32, u32) { + (start + file_span_lo, end + file_span_lo) } /// Processes program tokens, classifying strings of text by highlighting @@ -163,11 +194,12 @@ struct Classifier<'a> { in_macro_nonterminal: bool, edition: Edition, byte_pos: u32, + file_span_lo: u32, src: &'a str, } impl<'a> Classifier<'a> { - fn new(src: &str, edition: Edition) -> Classifier<'_> { + fn new(src: &str, edition: Edition, file_span_lo: u32) -> Classifier<'_> { let tokens = TokenIter { src }.peekable(); Classifier { tokens, @@ -176,6 +208,7 @@ impl<'a> Classifier<'a> { in_macro_nonterminal: false, edition, byte_pos: 0, + file_span_lo, src, } } @@ -201,17 +234,17 @@ impl<'a> Classifier<'a> { if has_ident { return vec![(TokenKind::Ident, start, pos), (TokenKind::Colon, pos, pos + nb)]; } else { - return vec![(TokenKind::Colon, pos, pos + nb)]; + return vec![(TokenKind::Colon, start, pos + nb)]; } } - if let Some((Class::Ident, text)) = self.tokens.peek().map(|(token, text)| { + if let Some((None, text)) = self.tokens.peek().map(|(token, text)| { if *token == TokenKind::Ident { - let class = get_real_ident_class(text, edition); + let class = get_real_ident_class(text, edition, true); (class, text) } else { // Doesn't matter which Class we put in here... - (Class::Comment, text) + (Some(Class::Comment), text) } }) { // We only "add" the colon if there is an ident behind. @@ -221,7 +254,7 @@ impl<'a> Classifier<'a> { } else if nb > 0 && has_ident { return vec![(TokenKind::Ident, start, pos), (TokenKind::Colon, pos, pos + nb)]; } else if nb > 0 { - return vec![(TokenKind::Colon, pos, pos + nb)]; + return vec![(TokenKind::Colon, start, start + nb)]; } else if has_ident { return vec![(TokenKind::Ident, start, pos)]; } else { @@ -231,10 +264,11 @@ impl<'a> Classifier<'a> { } /// Wraps the tokens iteration to ensure that the byte_pos is always correct. - fn next(&mut self) -> Option<(TokenKind, &'a str)> { + fn next(&mut self) -> Option<(TokenKind, &'a str, u32)> { if let Some((kind, text)) = self.tokens.next() { + let before = self.byte_pos; self.byte_pos += text.len() as u32; - Some((kind, text)) + Some((kind, text, before)) } else { None } @@ -254,14 +288,18 @@ impl<'a> Classifier<'a> { .unwrap_or(false) { let tokens = self.get_full_ident_path(); + let skip = !tokens.is_empty(); for (token, start, end) in tokens { let text = &self.src[start..end]; - self.advance(token, text, sink); + self.advance(token, text, sink, start as u32); self.byte_pos += text.len() as u32; } + if skip { + continue; + } } - if let Some((token, text)) = self.next() { - self.advance(token, text, sink); + if let Some((token, text, before)) = self.next() { + self.advance(token, text, sink, before); } else { break; } @@ -270,7 +308,13 @@ impl<'a> Classifier<'a> { /// Single step of highlighting. This will classify `token`, but maybe also /// a couple of following ones as well. - fn advance(&mut self, token: TokenKind, text: &'a str, sink: &mut dyn FnMut(Highlight<'a>)) { + fn advance( + &mut self, + token: TokenKind, + text: &'a str, + sink: &mut dyn FnMut(Highlight<'a>), + before: u32, + ) { let lookahead = self.peek(); let no_highlight = |sink: &mut dyn FnMut(_)| sink(Highlight::Token { text, class: None }); let class = match token { @@ -401,19 +445,30 @@ impl<'a> Classifier<'a> { sink(Highlight::Token { text, class: None }); return; } - TokenKind::Ident => match get_real_ident_class(text, self.edition) { - Class::Ident => match text { + TokenKind::Ident => match get_real_ident_class(text, self.edition, false) { + None => match text { "Option" | "Result" => Class::PreludeTy, "Some" | "None" | "Ok" | "Err" => Class::PreludeVal, _ if self.in_macro_nonterminal => { self.in_macro_nonterminal = false; Class::MacroNonTerminal } - _ => Class::Ident, + "self" | "Self" => Class::Self_(move_span( + self.file_span_lo, + before, + before + text.len() as u32, + )), + _ => Class::Ident(move_span( + self.file_span_lo, + before, + before + text.len() as u32, + )), }, - c => c, + Some(c) => c, }, - TokenKind::RawIdent | TokenKind::UnknownPrefix => Class::Ident, + TokenKind::RawIdent | TokenKind::UnknownPrefix => { + Class::Ident(move_span(self.file_span_lo, before, before + text.len() as u32)) + } TokenKind::Lifetime { .. } => Class::Lifetime, }; // Anything that didn't return above is the simple case where we the @@ -448,11 +503,74 @@ fn exit_span(out: &mut Buffer) { /// ``` /// The latter can be thought of as a shorthand for the former, which is more /// flexible. -fn string(out: &mut Buffer, text: T, klass: Option) { +fn string( + out: &mut Buffer, + text: T, + klass: Option, + context: Option<&Context<'_>>, + root_path: &str, +) { match klass { None => write!(out, "{}", text), - Some(klass) => write!(out, "{}", klass.as_html(), text), + Some(klass) => { + if let Some(def_span) = klass.get_span() { + let mut text = text.to_string(); + if text.contains("::") { + text = + text.split("::").enumerate().fold(String::new(), |mut path, (pos, t)| { + let pre = if pos != 0 { "::" } else { "" }; + match t { + "self" | "Self" => write!( + &mut path, + "{}{}", + pre, + Class::Self_((0, 0)).as_html(), + t + ), + "crate" | "super" => write!( + &mut path, + "{}{}", + pre, + Class::KeyWord.as_html(), + t + ), + t => write!(&mut path, "{}{}", pre, t), + } + .expect("Failed to build source HTML path"); + path + }); + } + if let Some(context) = context { + if let Some(href) = + context.shared.span_correspondance_map.get(&def_span).and_then(|href| { + match href { + LinkFromSrc::Local(span) => { + eprintln!("==> {:?}:{:?}", span.lo(), span.hi()); + context + .href_from_span(clean::Span::wrap(*span)) + .map(|s| format!("{}{}", root_path, s)) + }, + LinkFromSrc::External(def_id) => { + format::href(*def_id, context).map(|(url, _, _)| url) + } + } + }) + { + write!( + out, + "{}", + klass.as_html(), + href, + text + ); + return; + } + } + } + write!(out, "{}", klass.as_html(), text); + } } + write!(out, "{}", klass.as_html(), text); } #[cfg(test)] diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index b8756d2526edf..5d79ccce8e1a0 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -330,6 +330,9 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { tooltip, edition, None, + 0, + None, + "", ); Some(Event::Html(s.into_inner().into())) } diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index b6c3220901f06..33b10db1f881d 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -17,7 +17,10 @@ use rustc_span::symbol::sym; use super::cache::{build_index, ExternalLocation}; use super::print_item::{full_path, item_path, print_item}; use super::write_shared::write_shared; -use super::{print_sidebar, settings, AllTypes, NameDoc, StylePath, BASIC_KEYWORDS}; +use super::{ + collect_spans_and_sources, print_sidebar, settings, AllTypes, LinkFromSrc, NameDoc, StylePath, + BASIC_KEYWORDS, +}; use crate::clean; use crate::clean::ExternalCrate; @@ -46,7 +49,7 @@ crate struct Context<'tcx> { pub(crate) current: Vec, /// The current destination folder of where HTML artifacts should be placed. /// This changes as the context descends into the module hierarchy. - pub(super) dst: PathBuf, + crate dst: PathBuf, /// A flag, which when `true`, will render pages which redirect to the /// real location of an item. This is used to allow external links to /// publicly reused items to redirect to the right location. @@ -58,7 +61,7 @@ crate struct Context<'tcx> { /// Issue for improving the situation: [#82381][] /// /// [#82381]: https://github.com/rust-lang/rust/issues/82381 - pub(super) shared: Rc>, + crate shared: Rc>, /// The [`Cache`] used during rendering. /// /// Ideally the cache would be in [`SharedContext`], but it's mutated @@ -68,7 +71,11 @@ crate struct Context<'tcx> { /// It's immutable once in `Context`, so it's not as bad that it's not in /// `SharedContext`. // FIXME: move `cache` to `SharedContext` - pub(super) cache: Rc, + crate cache: Rc, + /// This flag indicates whether `[src]` links should be generated or not. If + /// the source files are present in the html rendering, then this will be + /// `true`. + crate include_sources: bool, } // `Context` is cloned a lot, so we don't want the size to grow unexpectedly. @@ -84,10 +91,6 @@ crate struct SharedContext<'tcx> { /// This describes the layout of each page, and is not modified after /// creation of the context (contains info like the favicon and added html). crate layout: layout::Layout, - /// This flag indicates whether `[src]` links should be generated or not. If - /// the source files are present in the html rendering, then this will be - /// `true`. - crate include_sources: bool, /// The local file sources we've emitted and their respective url-paths. crate local_sources: FxHashMap, /// Show the memory layout of types in the docs. @@ -125,6 +128,10 @@ crate struct SharedContext<'tcx> { redirections: Option>>, pub(crate) templates: tera::Tera, + + /// 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. + crate span_correspondance_map: FxHashMap<(u32, u32), LinkFromSrc>, } impl SharedContext<'_> { @@ -293,15 +300,19 @@ impl<'tcx> Context<'tcx> { /// may happen, for example, with externally inlined items where the source /// of their crate documentation isn't known. pub(super) fn src_href(&self, item: &clean::Item) -> Option { - if item.span(self.tcx()).is_dummy() { + self.href_from_span(item.span(self.tcx())) + } + + crate fn href_from_span(&self, span: clean::Span) -> Option { + if span.is_dummy() { return None; } let mut root = self.root_path(); let mut path = String::new(); - let cnum = item.span(self.tcx()).cnum(self.sess()); + let cnum = span.cnum(self.sess()); // We can safely ignore synthetic `SourceFile`s. - let file = match item.span(self.tcx()).filename(self.sess()) { + let file = match span.filename(self.sess()) { FileName::Real(ref path) => path.local_path_if_available().to_path_buf(), _ => return None, }; @@ -339,8 +350,8 @@ impl<'tcx> Context<'tcx> { (&*symbol, &path) }; - let loline = item.span(self.tcx()).lo(self.sess()).line; - let hiline = item.span(self.tcx()).hi(self.sess()).line; + let loline = span.lo(self.sess()).line; + let hiline = span.hi(self.sess()).line; let lines = if loline == hiline { loline.to_string() } else { format!("{}-{}", loline, hiline) }; Some(format!( @@ -362,9 +373,9 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { const RUN_ON_MODULE: bool = true; fn init( - mut krate: clean::Crate, + krate: clean::Crate, options: RenderOptions, - mut cache: Cache, + cache: Cache, tcx: TyCtxt<'tcx>, ) -> Result<(Self, clean::Crate), Error> { // need to save a copy of the options for rendering the index page @@ -444,13 +455,16 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { _ => {} } } + + let (mut krate, local_sources, matches) = + collect_spans_and_sources(tcx, krate, &src_root, include_sources); + let (sender, receiver) = channel(); let mut scx = SharedContext { tcx, collapsed: krate.collapsed, src_root, - include_sources, - local_sources: Default::default(), + local_sources, issue_tracker_base_url, layout, created_dirs: Default::default(), @@ -466,6 +480,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { redirections: if generate_redirect_map { Some(Default::default()) } else { None }, show_type_layout, templates, + span_correspondance_map: matches, }; // Add the default themes to the `Vec` of stylepaths @@ -483,12 +498,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { let dst = output; scx.ensure_dir(&dst)?; - if emit_crate { - krate = sources::render(&dst, &mut scx, krate)?; - } - - // Build our search index - let index = build_index(&krate, &mut cache, tcx); let mut cx = Context { current: Vec::new(), @@ -497,8 +506,16 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { id_map: RefCell::new(id_map), shared: Rc::new(scx), cache: Rc::new(cache), + include_sources, }; + if emit_crate { + krate = sources::render(&mut cx, krate)?; + } + + // Build our search index + let index = build_index(&krate, Rc::get_mut(&mut cx.cache).unwrap(), tcx); + // Write shared runs within a flock; disable thread dispatching of IO temporarily. Rc::get_mut(&mut cx.shared).unwrap().fs.set_sync_only(true); write_shared(&cx, &krate, index, &md_opts)?; @@ -514,6 +531,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { id_map: RefCell::new(IdMap::new()), shared: Rc::clone(&self.shared), cache: Rc::clone(&self.cache), + include_sources: self.include_sources, } } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index c05ea81ac1f36..fd2e18a8be77f 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -30,9 +30,11 @@ mod tests; mod context; mod print_item; +mod span_map; mod write_shared; crate use context::*; +crate use span_map::{collect_spans_and_sources, LinkFromSrc}; use std::collections::VecDeque; use std::default::Default; diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 5c30d8bbd173c..7b6bd4dd59704 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -119,7 +119,7 @@ pub(super) fn print_item(cx: &Context<'_>, item: &clean::Item, buf: &mut Buffer, // [src] link in the downstream documentation will actually come back to // this page, and this link will be auto-clicked. The `id` attribute is // used to find the link to auto-click. - if cx.shared.include_sources && !item.is_primitive() { + if cx.include_sources && !item.is_primitive() { write_srclink(cx, item, buf); } @@ -1081,6 +1081,9 @@ fn item_macro(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Mac None, it.span(cx.tcx()).inner().edition(), None, + 0, + None, + "", ); }); document(w, cx, it, None) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs new file mode 100644 index 0000000000000..c7c8fa7cc45cd --- /dev/null +++ b/src/librustdoc/html/render/span_map.rs @@ -0,0 +1,110 @@ +use crate::clean; +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::intravisit::{self, NestedVisitorMap, Visitor}; +use rustc_hir::{/*ExprKind, */ GenericParam, GenericParamKind, HirId}; +use rustc_middle::ty::TyCtxt; +use rustc_span::Span; + +#[derive(Debug)] +crate enum LinkFromSrc { + Local(Span), + External(DefId), +} + +crate fn collect_spans_and_sources( + tcx: TyCtxt<'_>, + krate: clean::Crate, + src_root: &std::path::Path, + include_sources: bool, +) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { + let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; + + if include_sources { + intravisit::walk_crate(&mut visitor, tcx.hir().krate()); + let (krate, sources) = sources::collect_local_sources(tcx, src_root, krate); + (krate, sources, visitor.matches) + } else { + (krate, Default::default(), Default::default()) + } +} + +fn span_to_tuple(span: Span) -> (u32, u32) { + (span.lo().0, span.hi().0) +} + +struct SpanMapVisitor<'tcx> { + crate tcx: TyCtxt<'tcx>, + crate matches: FxHashMap<(u32, u32), LinkFromSrc>, +} + +impl<'tcx> SpanMapVisitor<'tcx> { + fn handle_path(&mut self, path: &rustc_hir::Path<'_>, path_span: Option) -> bool { + let info = match path.res { + Res::Def(kind, def_id) if kind != DefKind::TyParam => { + if matches!(kind, DefKind::Macro(_)) { + return false; + } + Some(def_id) + } + Res::Local(_) => None, + _ => return true, + }; + if let Some(span) = self.tcx.hir().res_span(path.res) { + self.matches.insert( + path_span.map(span_to_tuple).unwrap_or_else(|| span_to_tuple(path.span)), + LinkFromSrc::Local(span), + ); + } else if let Some(def_id) = info { + self.matches.insert( + path_span.map(span_to_tuple).unwrap_or_else(|| span_to_tuple(path.span)), + LinkFromSrc::External(def_id), + ); + } + true + } +} + +impl Visitor<'tcx> for SpanMapVisitor<'tcx> { + type Map = rustc_middle::hir::map::Map<'tcx>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::All(self.tcx.hir()) + } + + fn visit_generic_param(&mut self, p: &'tcx GenericParam<'tcx>) { + if !matches!(p.kind, GenericParamKind::Type { .. }) { + return; + } + for bound in p.bounds { + if let Some(trait_ref) = bound.trait_ref() { + self.handle_path(&trait_ref.path, None); + } + } + } + + fn visit_path(&mut self, path: &'tcx rustc_hir::Path<'tcx>, _id: HirId) { + self.handle_path(path, None); + intravisit::walk_path(self, path); + } + + // fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { + // match expr.kind { + // ExprKind::MethodCall(segment, method_span, _, _) => { + // if let Some(hir_id) = segment.hir_id { + // // call https://doc.rust-lang.org/beta/nightly-rustc/rustc_middle/ty/context/struct.TypeckResults.html#method.type_dependent_def_id + // } + // } + // _ => {} + // } + // intravisit::walk_expr(self, expr); + // } + + fn visit_use(&mut self, path: &'tcx rustc_hir::Path<'tcx>, id: HirId) { + self.handle_path(path, None); + intravisit::walk_use(self, path, id); + } +} diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 4411b7771eda7..c16769c474a21 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -272,7 +272,7 @@ pub(super) fn write_shared( write_minify("search.js", static_files::SEARCH_JS)?; write_minify("settings.js", static_files::SETTINGS_JS)?; - if cx.shared.include_sources { + if cx.include_sources { write_minify("source-script.js", static_files::sidebar::SOURCE_SCRIPT)?; } @@ -398,7 +398,7 @@ pub(super) fn write_shared( } } - if cx.shared.include_sources { + if cx.include_sources { let mut hierarchy = Hierarchy::new(OsString::new()); for source in cx .shared diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 80dd7a7a952f0..24821950869e7 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -5,8 +5,10 @@ use crate::fold::DocFolder; use crate::html::format::Buffer; use crate::html::highlight; use crate::html::layout; -use crate::html::render::{SharedContext, BASIC_KEYWORDS}; +use crate::html::render::{Context, BASIC_KEYWORDS}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::def_id::LOCAL_CRATE; +use rustc_middle::ty::TyCtxt; use rustc_session::Session; use rustc_span::edition::Edition; use rustc_span::source_map::FileName; @@ -14,52 +16,116 @@ use std::ffi::OsStr; use std::fs; use std::path::{Component, Path, PathBuf}; -crate fn render( - dst: &Path, - scx: &mut SharedContext<'_>, - krate: clean::Crate, -) -> Result { +crate fn render(cx: &mut Context<'_>, krate: clean::Crate) -> Result { info!("emitting source files"); - let dst = dst.join("src").join(&*krate.name.as_str()); - scx.ensure_dir(&dst)?; - let mut folder = SourceCollector { dst, scx }; + let dst = cx.dst.join("src").join(&*krate.name.as_str()); + cx.shared.ensure_dir(&dst)?; + let mut folder = SourceCollector { dst, cx, emitted_local_sources: FxHashSet::default() }; Ok(folder.fold_crate(krate)) } +crate fn collect_local_sources<'tcx>( + tcx: TyCtxt<'tcx>, + src_root: &Path, + krate: clean::Crate, +) -> (clean::Crate, FxHashMap) { + let mut lsc = LocalSourcesCollector { tcx, local_sources: FxHashMap::default(), src_root }; + + let krate = lsc.fold_crate(krate); + (krate, lsc.local_sources) +} + +struct LocalSourcesCollector<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + local_sources: FxHashMap, + src_root: &'a Path, +} + +fn is_real_and_local(span: clean::Span, sess: &Session) -> bool { + span.filename(sess).is_real() && span.cnum(sess) == LOCAL_CRATE +} + +impl LocalSourcesCollector<'_, '_> { + fn add_local_source(&mut self, item: &clean::Item) { + let sess = self.tcx.sess; + let span = item.span(self.tcx); + // skip all synthetic "files" + if !is_real_and_local(span, sess) { + return; + } + let filename = span.filename(sess); + let p = match filename { + FileName::Real(ref file) => match file.local_path() { + Some(p) => p.to_path_buf(), + _ => return, + }, + _ => return, + }; + if self.local_sources.contains_key(&*p) { + // We've already emitted this source + return; + } + + let mut href = String::new(); + clean_path(&self.src_root, &p, false, |component| { + href.push_str(&component.to_string_lossy()); + href.push('/'); + }); + + let src_fname = p.file_name().expect("source has no filename").to_os_string(); + let mut fname = src_fname.clone(); + fname.push(".html"); + href.push_str(&fname.to_string_lossy()); + self.local_sources.insert(p, href); + } +} + +impl DocFolder for LocalSourcesCollector<'_, '_> { + fn fold_item(&mut self, item: clean::Item) -> Option { + self.add_local_source(&item); + + // FIXME: if `include_sources` isn't set and DocFolder didn't require consuming the crate by value, + // we could return None here without having to walk the rest of the crate. + Some(self.fold_item_recur(item)) + } +} + /// Helper struct to render all source code to HTML pages struct SourceCollector<'a, 'tcx> { - scx: &'a mut SharedContext<'tcx>, + cx: &'a mut Context<'tcx>, /// Root destination to place all HTML output into dst: PathBuf, + emitted_local_sources: FxHashSet, } impl DocFolder for SourceCollector<'_, '_> { fn fold_item(&mut self, item: clean::Item) -> Option { + let tcx = self.cx.tcx(); + let span = item.span(tcx); + let sess = tcx.sess; + // If we're not rendering sources, there's nothing to do. // If we're including source files, and we haven't seen this file yet, // then we need to render it out to the filesystem. - if self.scx.include_sources - // skip all synthetic "files" - && item.span(self.scx.tcx).filename(self.sess()).is_real() - // skip non-local files - && item.span(self.scx.tcx).cnum(self.sess()) == LOCAL_CRATE - { - let filename = item.span(self.scx.tcx).filename(self.sess()); + if self.cx.include_sources && is_real_and_local(span, sess) { + let filename = span.filename(sess); + let span = span.inner(); + let start_pos = sess.source_map().lookup_source_file(span.lo()).start_pos; // If it turns out that we couldn't read this file, then we probably // can't read any of the files (generating html output from json or // something like that), so just don't include sources for the // entire crate. The other option is maintaining this mapping on a // per-file basis, but that's probably not worth it... - self.scx.include_sources = match self.emit_source(&filename) { + self.cx.include_sources = match self.emit_source(&filename, start_pos.0) { Ok(()) => true, Err(e) => { - self.scx.tcx.sess.span_err( - item.span(self.scx.tcx).inner(), + self.cx.shared.tcx.sess.span_err( + span, &format!( "failed to render source code for `{}`: {}", filename.prefer_local(), - e + e, ), ); false @@ -73,12 +139,8 @@ impl DocFolder for SourceCollector<'_, '_> { } impl SourceCollector<'_, 'tcx> { - fn sess(&self) -> &'tcx Session { - &self.scx.tcx.sess - } - /// Renders the given filename into its corresponding HTML source file. - fn emit_source(&mut self, filename: &FileName) -> Result<(), Error> { + fn emit_source(&mut self, filename: &FileName, file_span_lo: u32) -> Result<(), Error> { let p = match *filename { FileName::Real(ref file) => { if let Some(local_path) = file.local_path() { @@ -89,7 +151,7 @@ impl SourceCollector<'_, 'tcx> { } _ => return Ok(()), }; - if self.scx.local_sources.contains_key(&*p) { + if self.emitted_local_sources.contains(&*p) { // We've already emitted this source return Ok(()); } @@ -107,20 +169,17 @@ impl SourceCollector<'_, 'tcx> { // Create the intermediate directories let mut cur = self.dst.clone(); let mut root_path = String::from("../../"); - let mut href = String::new(); - clean_path(&self.scx.src_root, &p, false, |component| { + clean_path(&self.cx.shared.src_root, &p, false, |component| { cur.push(component); root_path.push_str("../"); - href.push_str(&component.to_string_lossy()); - href.push('/'); }); - self.scx.ensure_dir(&cur)?; + + self.cx.shared.ensure_dir(&cur)?; let src_fname = p.file_name().expect("source has no filename").to_os_string(); let mut fname = src_fname.clone(); fname.push(".html"); cur.push(&fname); - href.push_str(&fname.to_string_lossy()); let title = format!("{} - source", src_fname.to_string_lossy()); let desc = format!("Source of the Rust file `{}`.", filename.prefer_remapped()); @@ -128,23 +187,32 @@ impl SourceCollector<'_, 'tcx> { title: &title, css_class: "source", root_path: &root_path, - static_root_path: self.scx.static_root_path.as_deref(), + static_root_path: self.cx.shared.static_root_path.as_deref(), description: &desc, keywords: BASIC_KEYWORDS, - resource_suffix: &self.scx.resource_suffix, - extra_scripts: &[&format!("source-files{}", self.scx.resource_suffix)], - static_extra_scripts: &[&format!("source-script{}", self.scx.resource_suffix)], + resource_suffix: &self.cx.shared.resource_suffix, + extra_scripts: &[&format!("source-files{}", self.cx.shared.resource_suffix)], + static_extra_scripts: &[&format!("source-script{}", self.cx.shared.resource_suffix)], }; let v = layout::render( - &self.scx.templates, - &self.scx.layout, + &self.cx.shared.templates, + &self.cx.shared.layout, &page, "", - |buf: &mut _| print_src(buf, contents, self.scx.edition()), - &self.scx.style_files, + |buf: &mut _| { + print_src( + buf, + contents, + self.cx.shared.edition(), + file_span_lo, + &self.cx, + &root_path, + ) + }, + &self.cx.shared.style_files, ); - self.scx.fs.write(&cur, v.as_bytes())?; - self.scx.local_sources.insert(p, href); + self.cx.shared.fs.write(&cur, v.as_bytes())?; + self.emitted_local_sources.insert(p); Ok(()) } } @@ -178,7 +246,14 @@ where /// Wrapper struct to render the source code of a file. This will do things like /// adding line numbers to the left-hand side. -fn print_src(buf: &mut Buffer, s: &str, edition: Edition) { +fn print_src( + buf: &mut Buffer, + s: &str, + edition: Edition, + file_span_lo: u32, + context: &Context<'_>, + root_path: &str, +) { let lines = s.lines().count(); let mut line_numbers = Buffer::empty_from(buf); let mut cols = 0; @@ -192,5 +267,16 @@ fn print_src(buf: &mut Buffer, s: &str, edition: Edition) { writeln!(line_numbers, "{0:1$}", i, cols); } line_numbers.write_str(""); - highlight::render_with_highlighting(s, buf, None, None, None, edition, Some(line_numbers)); + highlight::render_with_highlighting( + s, + buf, + None, + None, + None, + edition, + Some(line_numbers), + file_span_lo, + Some(context), + root_path, + ); } From 2104bf27d41c061bab2f10400c7b817a5f95722f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Apr 2021 15:52:41 +0200 Subject: [PATCH 02/22] Add an option for the source code link generation --- src/librustdoc/config.rs | 4 ++++ src/librustdoc/html/render/context.rs | 10 ++++++++-- src/librustdoc/html/render/span_map.rs | 5 ++++- src/librustdoc/lib.rs | 7 +++++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index abd1fd2bf39a2..99d2039b183af 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -276,6 +276,8 @@ crate struct RenderOptions { crate show_type_layout: bool, crate unstable_features: rustc_feature::UnstableFeatures, crate emit: Vec, + /// If `true`, HTML source pages will generate links for items to their definition. + crate generate_link_to_definition: bool, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -655,6 +657,7 @@ impl Options { let generate_redirect_map = matches.opt_present("generate-redirect-map"); let show_type_layout = matches.opt_present("show-type-layout"); let nocapture = matches.opt_present("nocapture"); + let generate_link_to_definition = matches.opt_present("generate-link-to-definition"); let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format, &debugging_opts); @@ -721,6 +724,7 @@ impl Options { crate_name.as_deref(), ), emit, + generate_link_to_definition, }, crate_name, output_format, diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 33b10db1f881d..c0668fcce999f 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -396,6 +396,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { unstable_features, generate_redirect_map, show_type_layout, + generate_link_to_definition, .. } = options; @@ -456,8 +457,13 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { } } - let (mut krate, local_sources, matches) = - collect_spans_and_sources(tcx, krate, &src_root, include_sources); + let (mut krate, local_sources, matches) = collect_spans_and_sources( + tcx, + krate, + &src_root, + include_sources, + generate_link_to_definition, + ); let (sender, receiver) = channel(); let mut scx = SharedContext { diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index c7c8fa7cc45cd..fa1f8954a1586 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -20,11 +20,14 @@ crate fn collect_spans_and_sources( krate: clean::Crate, src_root: &std::path::Path, include_sources: bool, + generate_link_to_definition: bool, ) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; if include_sources { - intravisit::walk_crate(&mut visitor, tcx.hir().krate()); + if generate_link_to_definition { + intravisit::walk_crate(&mut visitor, tcx.hir().krate()); + } let (krate, sources) = sources::collect_local_sources(tcx, src_root, krate); (krate, sources, visitor.matches) } else { diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index fa755777584f3..a98725e683cb6 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -607,6 +607,13 @@ fn opts() -> Vec { unstable("nocapture", |o| { o.optflag("", "nocapture", "Don't capture stdout and stderr of tests") }), + unstable("generate-link-to-definition", |o| { + o.optflag( + "", + "generate-link-to-definition", + "Make the identifiers in the HTML source code pages navigable", + ) + }), ] } From 83dcd30ee0639b4f7dc55d370953a76f21ba755f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 4 May 2021 11:51:11 +0200 Subject: [PATCH 03/22] Ensure that --generate-link-to-definition is only used with HTML output and is unstable --- src/librustdoc/config.rs | 8 ++++++++ .../generate-link-to-definition-opt-unstable.rs | 6 ++++++ .../generate-link-to-definition-opt-unstable.stderr | 2 ++ src/test/rustdoc-ui/generate-link-to-definition-opt.rs | 6 ++++++ .../rustdoc-ui/generate-link-to-definition-opt.stderr | 2 ++ src/test/rustdoc-ui/generate-link-to-definition-opt2.rs | 6 ++++++ .../rustdoc-ui/generate-link-to-definition-opt2.stderr | 2 ++ 7 files changed, 32 insertions(+) create mode 100644 src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.rs create mode 100644 src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.stderr create mode 100644 src/test/rustdoc-ui/generate-link-to-definition-opt.rs create mode 100644 src/test/rustdoc-ui/generate-link-to-definition-opt.stderr create mode 100644 src/test/rustdoc-ui/generate-link-to-definition-opt2.rs create mode 100644 src/test/rustdoc-ui/generate-link-to-definition-opt2.stderr diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 99d2039b183af..e44158bc04230 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -659,6 +659,14 @@ impl Options { let nocapture = matches.opt_present("nocapture"); let generate_link_to_definition = matches.opt_present("generate-link-to-definition"); + if generate_link_to_definition && (show_coverage || output_format != OutputFormat::Html) { + diag.struct_err( + "--generate-link-to-definition option can only be used with HTML output format", + ) + .emit(); + return Err(1); + } + let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format, &debugging_opts); diff --git a/src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.rs b/src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.rs new file mode 100644 index 0000000000000..87620d74ee645 --- /dev/null +++ b/src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.rs @@ -0,0 +1,6 @@ +// This test purpose is to check that the "--generate-link-to-definition" +// option can only be used on nightly. + +// compile-flags: --generate-link-to-definition + +pub fn f() {} diff --git a/src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.stderr b/src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.stderr new file mode 100644 index 0000000000000..a8ddf91bcbf15 --- /dev/null +++ b/src/test/rustdoc-ui/generate-link-to-definition-opt-unstable.stderr @@ -0,0 +1,2 @@ +error: the `-Z unstable-options` flag must also be passed to enable the flag `generate-link-to-definition` + diff --git a/src/test/rustdoc-ui/generate-link-to-definition-opt.rs b/src/test/rustdoc-ui/generate-link-to-definition-opt.rs new file mode 100644 index 0000000000000..8f4f561b44dcc --- /dev/null +++ b/src/test/rustdoc-ui/generate-link-to-definition-opt.rs @@ -0,0 +1,6 @@ +// This test purpose is to check that the "--generate-link-to-definition" +// option can only be used with HTML generation. + +// compile-flags: -Zunstable-options --generate-link-to-definition --output-format json + +pub fn f() {} diff --git a/src/test/rustdoc-ui/generate-link-to-definition-opt.stderr b/src/test/rustdoc-ui/generate-link-to-definition-opt.stderr new file mode 100644 index 0000000000000..4c8c607e7da23 --- /dev/null +++ b/src/test/rustdoc-ui/generate-link-to-definition-opt.stderr @@ -0,0 +1,2 @@ +error: --generate-link-to-definition option can only be used with HTML output format + diff --git a/src/test/rustdoc-ui/generate-link-to-definition-opt2.rs b/src/test/rustdoc-ui/generate-link-to-definition-opt2.rs new file mode 100644 index 0000000000000..da5142087ddee --- /dev/null +++ b/src/test/rustdoc-ui/generate-link-to-definition-opt2.rs @@ -0,0 +1,6 @@ +// This test purpose is to check that the "--generate-link-to-definition" +// option can only be used with HTML generation. + +// compile-flags: -Zunstable-options --generate-link-to-definition --show-coverage + +pub fn f() {} diff --git a/src/test/rustdoc-ui/generate-link-to-definition-opt2.stderr b/src/test/rustdoc-ui/generate-link-to-definition-opt2.stderr new file mode 100644 index 0000000000000..4c8c607e7da23 --- /dev/null +++ b/src/test/rustdoc-ui/generate-link-to-definition-opt2.stderr @@ -0,0 +1,2 @@ +error: --generate-link-to-definition option can only be used with HTML output format + From b689cedc0e44a6a1f75b0589ecb2eb4e890e71e4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Apr 2021 20:00:36 +0200 Subject: [PATCH 04/22] Generate links for methods as well --- src/librustdoc/html/highlight.rs | 6 +-- src/librustdoc/html/render/span_map.rs | 52 ++++++++++++++++++-------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 329cb3f8f09d5..579f22052adc6 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -547,9 +547,9 @@ fn string( LinkFromSrc::Local(span) => { eprintln!("==> {:?}:{:?}", span.lo(), span.hi()); context - .href_from_span(clean::Span::wrap(*span)) - .map(|s| format!("{}{}", root_path, s)) - }, + .href_from_span(clean::Span::wrap(*span)) + .map(|s| format!("{}{}", root_path, s)) + } LinkFromSrc::External(def_id) => { format::href(*def_id, context).map(|(url, _, _)| url) } diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index fa1f8954a1586..2895b4abfb294 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; -use rustc_hir::{/*ExprKind, */ GenericParam, GenericParamKind, HirId}; +use rustc_hir::{ExprKind, GenericParam, GenericParamKind, HirId}; use rustc_middle::ty::TyCtxt; use rustc_span::Span; @@ -20,14 +20,14 @@ crate fn collect_spans_and_sources( krate: clean::Crate, src_root: &std::path::Path, include_sources: bool, - generate_link_to_definition: bool, + _generate_link_to_definition: bool, ) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; if include_sources { - if generate_link_to_definition { - intravisit::walk_crate(&mut visitor, tcx.hir().krate()); - } + // if generate_link_to_definition { + intravisit::walk_crate(&mut visitor, tcx.hir().krate()); + // } let (krate, sources) = sources::collect_local_sources(tcx, src_root, krate); (krate, sources, visitor.matches) } else { @@ -94,17 +94,37 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { intravisit::walk_path(self, path); } - // fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { - // match expr.kind { - // ExprKind::MethodCall(segment, method_span, _, _) => { - // if let Some(hir_id) = segment.hir_id { - // // call https://doc.rust-lang.org/beta/nightly-rustc/rustc_middle/ty/context/struct.TypeckResults.html#method.type_dependent_def_id - // } - // } - // _ => {} - // } - // intravisit::walk_expr(self, expr); - // } + fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { + match expr.kind { + ExprKind::MethodCall(segment, method_span, _, _) => { + if let Some(hir_id) = segment.hir_id { + let hir = self.tcx.hir(); + let body_id = hir.enclosing_body_owner(hir_id); + // FIXME: this is showing error messages for parts of the code that are not + // compiled (because of cfg)! + let typeck_results = self.tcx.typeck_body( + 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) { + match hir.span_if_local(def_id) { + Some(span) => { + self.matches + .insert(span_to_tuple(method_span), LinkFromSrc::Local(span)); + } + None => { + self.matches.insert( + span_to_tuple(method_span), + LinkFromSrc::External(def_id), + ); + } + } + } + } + } + _ => {} + } + intravisit::walk_expr(self, expr); + } fn visit_use(&mut self, path: &'tcx rustc_hir::Path<'tcx>, id: HirId) { self.handle_path(path, None); From 71763a52ff221fe8f32b217e92d49b903f6826bb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Apr 2021 22:03:29 +0200 Subject: [PATCH 05/22] Add test for source code pages URLs --- src/test/rustdoc/auxiliary/source-code-bar.rs | 17 +++++++++ .../rustdoc/check-source-code-urls-to-def.rs | 38 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/test/rustdoc/auxiliary/source-code-bar.rs create mode 100644 src/test/rustdoc/check-source-code-urls-to-def.rs diff --git a/src/test/rustdoc/auxiliary/source-code-bar.rs b/src/test/rustdoc/auxiliary/source-code-bar.rs new file mode 100644 index 0000000000000..8700d688ef7e2 --- /dev/null +++ b/src/test/rustdoc/auxiliary/source-code-bar.rs @@ -0,0 +1,17 @@ +//! just some other file. :) + +use crate::Foo; + +pub struct Bar { + field: Foo, +} + +pub struct Bar2 { + field: crate::Foo, +} + +pub mod sub { + pub trait Trait { + fn tadam() {} + } +} diff --git a/src/test/rustdoc/check-source-code-urls-to-def.rs b/src/test/rustdoc/check-source-code-urls-to-def.rs new file mode 100644 index 0000000000000..32973ba3faf57 --- /dev/null +++ b/src/test/rustdoc/check-source-code-urls-to-def.rs @@ -0,0 +1,38 @@ +// compile-flags: -Zunstable-options --generate-link-to-definition + +#![crate_name = "foo"] + +#[path = "auxiliary/source-code-bar.rs"] +pub mod bar; + +// @has 'src/foo/check-source-code-urls-to-def.rs.html' + +// @count - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#5-7"]' 4 +use bar::Bar; +// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#13-17"]' 'self' +// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#14-16"]' 'Trait' +use bar::sub::{self, Trait}; + +pub struct Foo; + +impl Foo { + fn hello(&self) {} +} + +fn babar() {} + +// @has - '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html"]' 'String' +// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#16"]' 5 +pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar) { + let x = 12; + let y: Foo = Foo; + let z: Bar = bar::Bar { field: Foo }; + babar(); + // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#19"]' 'hello' + y.hello(); +} + +// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#14-16"]' 'bar::sub::Trait' +// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#14-16"]' 'Trait' +pub fn foo2(t: &T, v: &V) { +} From 1abb7faddb406d5b40ecca39a6395957ccf9c778 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Apr 2021 22:51:56 +0200 Subject: [PATCH 06/22] Generate links for modules as well --- src/librustdoc/html/render/span_map.rs | 29 +++++++++++++++---- .../rustdoc/check-source-code-urls-to-def.rs | 9 +++--- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 2895b4abfb294..6100f45e998e1 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; -use rustc_hir::{ExprKind, GenericParam, GenericParamKind, HirId}; +use rustc_hir::{ExprKind, GenericParam, GenericParamKind, HirId, Mod, Node}; use rustc_middle::ty::TyCtxt; use rustc_span::Span; @@ -20,14 +20,14 @@ crate fn collect_spans_and_sources( krate: clean::Crate, src_root: &std::path::Path, include_sources: bool, - _generate_link_to_definition: bool, + generate_link_to_definition: bool, ) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; if include_sources { - // if generate_link_to_definition { - intravisit::walk_crate(&mut visitor, tcx.hir().krate()); - // } + if generate_link_to_definition { + intravisit::walk_crate(&mut visitor, tcx.hir().krate()); + } let (krate, sources) = sources::collect_local_sources(tcx, src_root, krate); (krate, sources, visitor.matches) } else { @@ -94,6 +94,25 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { intravisit::walk_path(self, path); } + fn visit_mod(&mut self, m: &'tcx Mod<'tcx>, span: Span, id: HirId) { + // To make the difference between "mod foo {}" and "mod foo;". In case we "import" another + // file, we want to link to it. Otherwise no need to create a link. + if !span.overlaps(m.inner) { + // Now that we confirmed it's a file import, we want to get the span for the module + // name only and not all the "mod foo;". + if let Some(node) = self.tcx.hir().find(id) { + match node { + Node::Item(item) => { + self.matches + .insert(span_to_tuple(item.ident.span), LinkFromSrc::Local(m.inner)); + } + _ => {} + } + } + } + intravisit::walk_mod(self, m, id); + } + fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'tcx>) { match expr.kind { ExprKind::MethodCall(segment, method_span, _, _) => { diff --git a/src/test/rustdoc/check-source-code-urls-to-def.rs b/src/test/rustdoc/check-source-code-urls-to-def.rs index 32973ba3faf57..5caef29fede37 100644 --- a/src/test/rustdoc/check-source-code-urls-to-def.rs +++ b/src/test/rustdoc/check-source-code-urls-to-def.rs @@ -2,11 +2,12 @@ #![crate_name = "foo"] +// @has 'src/foo/check-source-code-urls-to-def.rs.html' + +// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#1-17"]' 'bar' #[path = "auxiliary/source-code-bar.rs"] pub mod bar; -// @has 'src/foo/check-source-code-urls-to-def.rs.html' - // @count - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#5-7"]' 4 use bar::Bar; // @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#13-17"]' 'self' @@ -22,13 +23,13 @@ impl Foo { fn babar() {} // @has - '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html"]' 'String' -// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#16"]' 5 +// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#17"]' 5 pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar) { let x = 12; let y: Foo = Foo; let z: Bar = bar::Bar { field: Foo }; babar(); - // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#19"]' 'hello' + // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#20"]' 'hello' y.hello(); } From 89bdc33781b36f6822c0c499fc9d15c41815103f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Apr 2021 00:52:35 +0200 Subject: [PATCH 07/22] Update rustdoc tests --- src/librustdoc/html/highlight/fixtures/sample.html | 2 +- src/librustdoc/html/highlight/tests.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/highlight/fixtures/sample.html b/src/librustdoc/html/highlight/fixtures/sample.html index 8d23477bbcb8c..866caea925609 100644 --- a/src/librustdoc/html/highlight/fixtures/sample.html +++ b/src/librustdoc/html/highlight/fixtures/sample.html @@ -23,7 +23,7 @@ assert!(self.length < N && index <= self.length); ::std::env::var("gateau").is_ok(); #[rustfmt::skip] - let s:std::path::PathBuf = std::path::PathBuf::new(); + let s:std::path::PathBuf = std::path::PathBuf::new(); let mut s = String::new(); match &s { diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs index a505865b149c4..a637d8085cb57 100644 --- a/src/librustdoc/html/highlight/tests.rs +++ b/src/librustdoc/html/highlight/tests.rs @@ -22,7 +22,7 @@ fn test_html_highlighting() { let src = include_str!("fixtures/sample.rs"); let html = { let mut out = Buffer::new(); - write_code(&mut out, src, Edition::Edition2018); + write_code(&mut out, src, Edition::Edition2018, 0, None, ""); format!("{}
{}
\n", STYLE, out.into_inner()) }; expect_file!["fixtures/sample.html"].assert_eq(&html); @@ -36,7 +36,7 @@ fn test_dos_backline() { println!(\"foo\");\r\n\ }\r\n"; let mut html = Buffer::new(); - write_code(&mut html, src, Edition::Edition2018); + write_code(&mut html, src, Edition::Edition2018, 0, None, ""); expect_file!["fixtures/dos_line.html"].assert_eq(&html.into_inner()); }); } From b5c27b49d02d5b6538e40e19e0dd1365cd7632d5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Apr 2021 13:26:49 +0200 Subject: [PATCH 08/22] Underline source code links on hover --- src/librustdoc/html/static/css/rustdoc.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 4e33eab565006..bbc48f49e63e1 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -450,6 +450,10 @@ nav.sub { border-bottom-left-radius: 5px; } +.example-wrap > pre.rust a:hover { + text-decoration: underline; +} + .rustdoc:not(.source) .example-wrap > pre:not(.line-number) { width: 100%; overflow-x: auto; From 2a3b71ae33e1b5c4f8cecf32492f69a05faa7d93 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 20 Apr 2021 17:42:06 +0200 Subject: [PATCH 09/22] * Rename 'move_span' into 'local_span_to_global_span' * Add documentation on new arguments/functions --- src/librustdoc/clean/types.rs | 10 ++-- src/librustdoc/html/format.rs | 5 +- src/librustdoc/html/highlight.rs | 70 ++++++++++++++++++++++---- src/librustdoc/html/render/span_map.rs | 17 +++++++ 4 files changed, 85 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 85c4731c6bff7..d79fcdd5fcee5 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1943,14 +1943,18 @@ crate enum Variant { crate struct Span(rustc_span::Span); impl Span { + /// Wraps a [`rustc_span::Span`]. In case this span is the result of a macro expansion, the + /// span will be updated to point to the macro invocation instead of the macro definition. + /// + /// (See rust-lang/rust#39726) crate fn from_rustc_span(sp: rustc_span::Span) -> Self { - // Get the macro invocation instead of the definition, - // in case the span is result of a macro expansion. - // (See rust-lang/rust#39726) Self(sp.source_callsite()) } /// Unless you know what you're doing, use [`Self::from_rustc_span`] instead! + /// + /// Contrary to [`Self::from_rustc_span`], this constructor wraps the span as is and don't + /// perform any operation on it, even if it's from a macro expansion. crate fn wrap(sp: rustc_span::Span) -> Span { Self(sp) } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 8b5c147b56cae..8ab6aa775d2e4 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -494,10 +494,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec (fqp, shortty, { let module_fqp = to_module_fqp(shortty, fqp); diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 579f22052adc6..62f8618b8c6e7 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -63,6 +63,24 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option Option<(u32, u32)> { match self { Self::Ident(sp) | Self::Self_(sp) => Some(sp), @@ -166,7 +186,7 @@ impl Iterator for TokenIter<'a> { } } -/// Returns `None` if this is a `Class::Ident`. +/// Classifies into identifier class; returns `None` if this is a non-keyword identifier. fn get_real_ident_class(text: &str, edition: Edition, allow_path_keywords: bool) -> Option { let ignore: &[&str] = if allow_path_keywords { &["self", "Self", "super", "crate"] } else { &["self", "Self"] }; @@ -181,7 +201,20 @@ fn get_real_ident_class(text: &str, edition: Edition, allow_path_keywords: bool) }) } -fn move_span(file_span_lo: u32, start: u32, end: u32) -> (u32, u32) { +/// Before explaining what this function does, some global explanations on rust's `Span`: +/// +/// Each source code file is stored in the source map in the compiler and has a +/// `lo` and a `hi` (lowest and highest bytes in this source map which can be seen as one huge +/// string to simplify things). So in this case, this represents the starting byte of the current +/// file. It'll be used later on to retrieve the "definition span" from the +/// `span_correspondance_map` (which is inside `context`). +/// +/// This when we transform the "span" we have from reading the input into a "span" which can be +/// used as index to the `span_correspondance_map` to get the definition of this item. +/// +/// So in here, `file_span_lo` is representing the "lo" byte in the global source map, and to make +/// our "span" works in there, we simply add `file_span_lo` to our values. +fn local_span_to_global_span(file_span_lo: u32, start: u32, end: u32) -> (u32, u32) { (start + file_span_lo, end + file_span_lo) } @@ -199,6 +232,9 @@ struct Classifier<'a> { } impl<'a> Classifier<'a> { + /// Takes as argument the source code to HTML-ify, the rust edition to use and the source code + /// file "lo" byte which we be used later on by the `span_correspondance_map`. More explanations + /// are provided in the [`local_span_to_global_span`] function documentation about how it works. fn new(src: &str, edition: Edition, file_span_lo: u32) -> Classifier<'_> { let tokens = TokenIter { src }.peekable(); Classifier { @@ -263,7 +299,10 @@ impl<'a> Classifier<'a> { } } - /// Wraps the tokens iteration to ensure that the byte_pos is always correct. + /// Wraps the tokens iteration to ensure that the `byte_pos` is always correct. + /// + /// It returns the token's kind, the token as a string and its byte position in the source + /// string. fn next(&mut self) -> Option<(TokenKind, &'a str, u32)> { if let Some((kind, text)) = self.tokens.next() { let before = self.byte_pos; @@ -306,8 +345,12 @@ impl<'a> Classifier<'a> { } } - /// Single step of highlighting. This will classify `token`, but maybe also - /// a couple of following ones as well. + /// Single step of highlighting. This will classify `token`, but maybe also a couple of + /// following ones as well. + /// + /// `before` is the position of the given token in the `source` string and is used as "lo" byte + /// in case we want to try to generate a link for this token using the + /// `span_correspondance_map`. fn advance( &mut self, token: TokenKind, @@ -453,12 +496,12 @@ impl<'a> Classifier<'a> { self.in_macro_nonterminal = false; Class::MacroNonTerminal } - "self" | "Self" => Class::Self_(move_span( + "self" | "Self" => Class::Self_(local_span_to_global_span( self.file_span_lo, before, before + text.len() as u32, )), - _ => Class::Ident(move_span( + _ => Class::Ident(local_span_to_global_span( self.file_span_lo, before, before + text.len() as u32, @@ -466,9 +509,11 @@ impl<'a> Classifier<'a> { }, Some(c) => c, }, - TokenKind::RawIdent | TokenKind::UnknownPrefix => { - Class::Ident(move_span(self.file_span_lo, before, before + text.len() as u32)) - } + TokenKind::RawIdent | TokenKind::UnknownPrefix => Class::Ident(local_span_to_global_span( + self.file_span_lo, + before, + before + text.len() as u32, + )), TokenKind::Lifetime { .. } => Class::Lifetime, }; // Anything that didn't return above is the simple case where we the @@ -501,8 +546,13 @@ fn exit_span(out: &mut Buffer) { /// enter_span(Foo), string("text", None), exit_span() /// string("text", Foo) /// ``` +/// /// The latter can be thought of as a shorthand for the former, which is more /// flexible. +/// +/// Note that if `context` is not `None` and that the given `klass` contains a `Span`, the function +/// will then try to find this `span` in the `span_correspondance_map`. If found, it'll then +/// generate a link for this element (which corresponds to where its definition is located). fn string( out: &mut Buffer, text: T, diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 6100f45e998e1..9e16c451626f7 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -9,12 +9,29 @@ use rustc_hir::{ExprKind, GenericParam, GenericParamKind, HirId, Mod, Node}; use rustc_middle::ty::TyCtxt; use rustc_span::Span; +/// This enum allows us to store two different kinds of information: +/// +/// In case the `span` definition comes from the same crate, we can simply get the `span` and use +/// it as is. +/// +/// Otherwise, we store the definition `DefId` and will generate a link to the documentation page +/// instead of the source code directly. #[derive(Debug)] crate enum LinkFromSrc { Local(Span), External(DefId), } +/// This function will do at most two things: +/// +/// 1. Generate a `span` correspondance map which links an item `span` to its definition `span`. +/// 2. Collect the source code files. +/// +/// It returns the `krate`, the source code files and the `span` correspondance map. +/// +/// Note about the `span` correspondance map: the keys are actually `(lo, hi)` of `span`s. We don't +/// need the `span` context later on, only their position, so instead of keep a whole `Span`, we +/// only keep the `lo` and `hi`. crate fn collect_spans_and_sources( tcx: TyCtxt<'_>, krate: clean::Crate, From 38444f61bb89cd8217477e2864e19966ba72acd4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 22 Apr 2021 15:14:49 +0200 Subject: [PATCH 10/22] * Rename Span::from_rustc_span to Span::new * Rename Span::wrap to Span::wrap_raw * Improve documentation for Span::wrap_raw --- src/librustdoc/clean/inline.rs | 2 +- src/librustdoc/clean/mod.rs | 3 ++- src/librustdoc/clean/types.rs | 12 ++++++------ src/librustdoc/html/highlight.rs | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index b3b89e6e673a2..43979423ae615 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -517,7 +517,7 @@ fn build_module( } } - let span = clean::Span::from_rustc_span(cx.tcx.def_span(did)); + let span = clean::Span::new(cx.tcx.def_span(did)); clean::Module { items, span } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index b3fc1e73f7885..3d65fcedaf4e5 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -95,7 +95,8 @@ impl Clean for doctree::Module<'_> { // determine if we should display the inner contents or // the outer `mod` item for the source code. - let span = Span::from_rustc_span({ + + let span = Span::new({ let where_outer = self.where_outer(cx.tcx); let sm = cx.sess().source_map(); let outer = sm.lookup_char_pos(where_outer.lo()); diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index d79fcdd5fcee5..fd4748f1f50b7 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -343,7 +343,7 @@ crate struct Item { rustc_data_structures::static_assert_size!(Item, 56); crate fn rustc_span(def_id: DefId, tcx: TyCtxt<'_>) -> Span { - Span::from_rustc_span(def_id.as_local().map_or_else( + Span::new(def_id.as_local().map_or_else( || tcx.def_span(def_id), |local| { let hir = tcx.hir(); @@ -1947,15 +1947,15 @@ impl Span { /// span will be updated to point to the macro invocation instead of the macro definition. /// /// (See rust-lang/rust#39726) - crate fn from_rustc_span(sp: rustc_span::Span) -> Self { + crate fn new(sp: rustc_span::Span) -> Self { Self(sp.source_callsite()) } - /// Unless you know what you're doing, use [`Self::from_rustc_span`] instead! + /// Unless you know what you're doing, use [`Self::new`] instead! /// - /// Contrary to [`Self::from_rustc_span`], this constructor wraps the span as is and don't - /// perform any operation on it, even if it's from a macro expansion. - crate fn wrap(sp: rustc_span::Span) -> Span { + /// This function doesn't clean the span at all. Compare with [`Self::new`]'s body to see the + /// difference. + crate fn wrap_raw(sp: rustc_span::Span) -> Span { Self(sp) } diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 62f8618b8c6e7..253343b560d07 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -597,7 +597,7 @@ fn string( LinkFromSrc::Local(span) => { eprintln!("==> {:?}:{:?}", span.lo(), span.hi()); context - .href_from_span(clean::Span::wrap(*span)) + .href_from_span(clean::Span::wrap_raw(*span)) .map(|s| format!("{}{}", root_path, s)) } LinkFromSrc::External(def_id) => { From c5c927dfda986e7370985b5a7c8c3ce452639d44 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 4 May 2021 13:52:53 +0200 Subject: [PATCH 11/22] Improve code readability --- src/librustdoc/html/highlight.rs | 97 +++++++++++--------------- src/librustdoc/html/render/span_map.rs | 6 +- 2 files changed, 46 insertions(+), 57 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 253343b560d07..edbb5dba1f375 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -560,64 +560,51 @@ fn string( context: Option<&Context<'_>>, root_path: &str, ) { - match klass { - None => write!(out, "{}", text), - Some(klass) => { - if let Some(def_span) = klass.get_span() { - let mut text = text.to_string(); - if text.contains("::") { - text = - text.split("::").enumerate().fold(String::new(), |mut path, (pos, t)| { - let pre = if pos != 0 { "::" } else { "" }; - match t { - "self" | "Self" => write!( - &mut path, - "{}{}", - pre, - Class::Self_((0, 0)).as_html(), - t - ), - "crate" | "super" => write!( - &mut path, - "{}{}", - pre, - Class::KeyWord.as_html(), - t - ), - t => write!(&mut path, "{}{}", pre, t), - } - .expect("Failed to build source HTML path"); - path - }); + let klass = match klass { + None => return write!(out, "{}", text), + Some(klass) => klass, + }; + if let Some(def_span) = klass.get_span() { + let mut text = text.to_string(); + if text.contains("::") { + text = text.split("::").intersperse("::").fold(String::new(), |mut path, t| { + match t { + "self" | "Self" => write!( + &mut path, + "{}", + Class::Self_((0, 0)).as_html(), + t + ), + "crate" | "super" => write!( + &mut path, + "{}", + Class::KeyWord.as_html(), + t + ), + t => write!(&mut path, "{}", t), } - if let Some(context) = context { - if let Some(href) = - context.shared.span_correspondance_map.get(&def_span).and_then(|href| { - match href { - LinkFromSrc::Local(span) => { - eprintln!("==> {:?}:{:?}", span.lo(), span.hi()); - context - .href_from_span(clean::Span::wrap_raw(*span)) - .map(|s| format!("{}{}", root_path, s)) - } - LinkFromSrc::External(def_id) => { - format::href(*def_id, context).map(|(url, _, _)| url) - } - } - }) - { - write!( - out, - "{}", - klass.as_html(), - href, - text - ); - return; + .expect("Failed to build source HTML path"); + path + }); + } + if let Some(context) = context { + if let Some(href) = + context.shared.span_correspondance_map.get(&def_span).and_then(|href| { + match href { + LinkFromSrc::Local(span) => { + context + .href_from_span(clean::Span::wrap_raw(*span)) + .map(|s| format!("{}{}", root_path, s)) + } + LinkFromSrc::External(def_id) => { + format::href(*def_id, context).map(|(url, _, _)| url) + } } - } + }) + { + write!(out, "{}", klass.as_html(), href, text); + return; } - write!(out, "{}", klass.as_html(), text); } } write!(out, "{}", klass.as_html(), text); diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 9e16c451626f7..9990d6f0ce6ad 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -9,6 +9,8 @@ use rustc_hir::{ExprKind, GenericParam, GenericParamKind, HirId, Mod, Node}; use rustc_middle::ty::TyCtxt; use rustc_span::Span; +use std::path::{Path, PathBuf}; + /// This enum allows us to store two different kinds of information: /// /// In case the `span` definition comes from the same crate, we can simply get the `span` and use @@ -35,10 +37,10 @@ crate enum LinkFromSrc { crate fn collect_spans_and_sources( tcx: TyCtxt<'_>, krate: clean::Crate, - src_root: &std::path::Path, + src_root: &Path, include_sources: bool, generate_link_to_definition: bool, -) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { +) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; if include_sources { From e8869cb7a7e760f39088190a724991726cd05c50 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 4 May 2021 21:41:45 +0200 Subject: [PATCH 12/22] Wrap the span_map tuple index into a type called "LightSpan" --- src/librustdoc/html/highlight.rs | 37 ++++---------- src/librustdoc/html/render/context.rs | 6 +-- src/librustdoc/html/render/mod.rs | 2 +- src/librustdoc/html/render/span_map.rs | 67 +++++++++++++++++++++----- 4 files changed, 68 insertions(+), 44 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index edbb5dba1f375..808e1ca236f71 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -17,7 +17,7 @@ use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; use super::format::{self, Buffer}; -use super::render::LinkFromSrc; +use super::render::{LightSpan, LinkFromSrc}; /// Highlights `src`, returning the HTML output. crate fn render_with_highlighting( @@ -74,7 +74,7 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option Option<(u32, u32)> { + fn get_span(self) -> Option { match self { Self::Ident(sp) | Self::Self_(sp) => Some(sp), _ => None, @@ -201,23 +201,6 @@ fn get_real_ident_class(text: &str, edition: Edition, allow_path_keywords: bool) }) } -/// Before explaining what this function does, some global explanations on rust's `Span`: -/// -/// Each source code file is stored in the source map in the compiler and has a -/// `lo` and a `hi` (lowest and highest bytes in this source map which can be seen as one huge -/// string to simplify things). So in this case, this represents the starting byte of the current -/// file. It'll be used later on to retrieve the "definition span" from the -/// `span_correspondance_map` (which is inside `context`). -/// -/// This when we transform the "span" we have from reading the input into a "span" which can be -/// used as index to the `span_correspondance_map` to get the definition of this item. -/// -/// So in here, `file_span_lo` is representing the "lo" byte in the global source map, and to make -/// our "span" works in there, we simply add `file_span_lo` to our values. -fn local_span_to_global_span(file_span_lo: u32, start: u32, end: u32) -> (u32, u32) { - (start + file_span_lo, end + file_span_lo) -} - /// Processes program tokens, classifying strings of text by highlighting /// category (`Class`). struct Classifier<'a> { @@ -234,7 +217,7 @@ struct Classifier<'a> { impl<'a> Classifier<'a> { /// Takes as argument the source code to HTML-ify, the rust edition to use and the source code /// file "lo" byte which we be used later on by the `span_correspondance_map`. More explanations - /// are provided in the [`local_span_to_global_span`] function documentation about how it works. + /// are provided in the [`LightSpan::new_in_file`] function documentation about how it works. fn new(src: &str, edition: Edition, file_span_lo: u32) -> Classifier<'_> { let tokens = TokenIter { src }.peekable(); Classifier { @@ -496,12 +479,12 @@ impl<'a> Classifier<'a> { self.in_macro_nonterminal = false; Class::MacroNonTerminal } - "self" | "Self" => Class::Self_(local_span_to_global_span( + "self" | "Self" => Class::Self_(LightSpan::new_in_file( self.file_span_lo, before, before + text.len() as u32, )), - _ => Class::Ident(local_span_to_global_span( + _ => Class::Ident(LightSpan::new_in_file( self.file_span_lo, before, before + text.len() as u32, @@ -509,7 +492,7 @@ impl<'a> Classifier<'a> { }, Some(c) => c, }, - TokenKind::RawIdent | TokenKind::UnknownPrefix => Class::Ident(local_span_to_global_span( + TokenKind::RawIdent | TokenKind::UnknownPrefix => Class::Ident(LightSpan::new_in_file( self.file_span_lo, before, before + text.len() as u32, @@ -572,7 +555,7 @@ fn string( "self" | "Self" => write!( &mut path, "{}", - Class::Self_((0, 0)).as_html(), + Class::Self_(LightSpan::empty()).as_html(), t ), "crate" | "super" => write!( diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index c0668fcce999f..36ec2cf3f7a25 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -18,8 +18,8 @@ use super::cache::{build_index, ExternalLocation}; use super::print_item::{full_path, item_path, print_item}; use super::write_shared::write_shared; use super::{ - collect_spans_and_sources, print_sidebar, settings, AllTypes, LinkFromSrc, NameDoc, StylePath, - BASIC_KEYWORDS, + collect_spans_and_sources, print_sidebar, settings, AllTypes, LightSpan, LinkFromSrc, NameDoc, + StylePath, BASIC_KEYWORDS, }; use crate::clean; @@ -131,7 +131,7 @@ crate struct SharedContext<'tcx> { /// 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. - crate span_correspondance_map: FxHashMap<(u32, u32), LinkFromSrc>, + crate span_correspondance_map: FxHashMap, } impl SharedContext<'_> { diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index fd2e18a8be77f..584afdeb280fc 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -34,7 +34,7 @@ mod span_map; mod write_shared; crate use context::*; -crate use span_map::{collect_spans_and_sources, LinkFromSrc}; +crate use span_map::{collect_spans_and_sources, LightSpan, LinkFromSrc}; use std::collections::VecDeque; use std::default::Default; diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 9990d6f0ce6ad..c7cc2250c2410 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -24,6 +24,43 @@ crate enum LinkFromSrc { External(DefId), } +/// This struct is used only as index in the `span_map`, not as [`Span`]! `Span`s contain +/// some extra information (the syntax context) we don't need. **Do not convert this type back to +/// `Span`!!!** +#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)] +crate struct LightSpan { + crate lo: u32, + crate hi: u32, +} + +impl LightSpan { + /// Before explaining what this method does, some global explanations on rust's `Span`: + /// + /// Each source code file is stored in the source map in the compiler and has a + /// `lo` and a `hi` (lowest and highest bytes in this source map which can be seen as one huge + /// string to simplify things). So in this case, this represents the starting byte of the + /// current file. It'll be used later on to retrieve the "definition span" from the + /// `span_correspondance_map` (which is inside `context`). + /// + /// This when we transform the "span" we have from reading the input into a "span" which can be + /// used as index to the `span_correspondance_map` to get the definition of this item. + /// + /// So in here, `file_span_lo` is representing the "lo" byte in the global source map, and to + /// make our "span" works in there, we simply add `file_span_lo` to our values. + crate fn new_in_file(file_span_lo: u32, lo: u32, hi: u32) -> Self { + Self { lo: lo + file_span_lo, hi: hi + file_span_lo } + } + + crate fn empty() -> Self { + Self { lo: 0, hi: 0 } + } + + /// Extra the `lo` and `hi` from the [`Span`] and discard the unused syntax context. + fn new_from_span(sp: Span) -> Self { + Self { lo: sp.lo().0, hi: sp.hi().0 } + } +} + /// This function will do at most two things: /// /// 1. Generate a `span` correspondance map which links an item `span` to its definition `span`. @@ -40,7 +77,7 @@ crate fn collect_spans_and_sources( src_root: &Path, include_sources: bool, generate_link_to_definition: bool, -) -> (clean::Crate, FxHashMap, FxHashMap<(u32, u32), LinkFromSrc>) { +) -> (clean::Crate, FxHashMap, FxHashMap) { let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; if include_sources { @@ -54,13 +91,9 @@ crate fn collect_spans_and_sources( } } -fn span_to_tuple(span: Span) -> (u32, u32) { - (span.lo().0, span.hi().0) -} - struct SpanMapVisitor<'tcx> { crate tcx: TyCtxt<'tcx>, - crate matches: FxHashMap<(u32, u32), LinkFromSrc>, + crate matches: FxHashMap, } impl<'tcx> SpanMapVisitor<'tcx> { @@ -77,12 +110,16 @@ impl<'tcx> SpanMapVisitor<'tcx> { }; if let Some(span) = self.tcx.hir().res_span(path.res) { self.matches.insert( - path_span.map(span_to_tuple).unwrap_or_else(|| span_to_tuple(path.span)), + path_span + .map(LightSpan::new_from_span) + .unwrap_or_else(|| LightSpan::new_from_span(path.span)), LinkFromSrc::Local(span), ); } else if let Some(def_id) = info { self.matches.insert( - path_span.map(span_to_tuple).unwrap_or_else(|| span_to_tuple(path.span)), + path_span + .map(LightSpan::new_from_span) + .unwrap_or_else(|| LightSpan::new_from_span(path.span)), LinkFromSrc::External(def_id), ); } @@ -122,8 +159,10 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { if let Some(node) = self.tcx.hir().find(id) { match node { Node::Item(item) => { - self.matches - .insert(span_to_tuple(item.ident.span), LinkFromSrc::Local(m.inner)); + self.matches.insert( + LightSpan::new_from_span(item.ident.span), + LinkFromSrc::Local(m.inner), + ); } _ => {} } @@ -146,12 +185,14 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) { match hir.span_if_local(def_id) { Some(span) => { - self.matches - .insert(span_to_tuple(method_span), LinkFromSrc::Local(span)); + self.matches.insert( + LightSpan::new_from_span(method_span), + LinkFromSrc::Local(span), + ); } None => { self.matches.insert( - span_to_tuple(method_span), + LightSpan::new_from_span(method_span), LinkFromSrc::External(def_id), ); } From dffc9c0a79bcf06a3abe6746056746bb11125b6a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 5 May 2021 14:19:51 +0200 Subject: [PATCH 13/22] Move extra arguments for highlight URL generation into a new ContextInfo struct for better readability --- src/librustdoc/html/highlight.rs | 70 +++++++++++++----------- src/librustdoc/html/highlight/tests.rs | 4 +- src/librustdoc/html/markdown.rs | 2 - src/librustdoc/html/render/print_item.rs | 2 - src/librustdoc/html/sources.rs | 4 +- 5 files changed, 42 insertions(+), 40 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 808e1ca236f71..d1dbfeff876df 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -19,6 +19,21 @@ use rustc_span::symbol::Symbol; use super::format::{self, Buffer}; use super::render::{LightSpan, LinkFromSrc}; +/// This type is needed in case we want to render links on items to allow to go to their definition. +crate struct ContextInfo<'a, 'b, 'c> { + crate context: &'a Context<'b>, + /// This represents the "lo" bytes of the current file we're rendering. To get a [`Span`] from + /// it, you just need to add add your current byte position in the string and its length (to get + /// the "hi" part). + /// + /// This is used to create a [`LightSpan`] which is then used as an index in the `span_map` in + /// order to retrieve the definition's [`Span`] (which is used to generate the URL). + crate file_span_lo: u32, + /// This field is used to know "how far" from the top of the directory we are to link to either + /// documentation pages or other source pages. + crate root_path: &'c str, +} + /// Highlights `src`, returning the HTML output. crate fn render_with_highlighting( src: &str, @@ -28,9 +43,7 @@ crate fn render_with_highlighting( tooltip: Option<(Option, &str)>, edition: Edition, extra_content: Option, - file_span_lo: u32, - context: Option<&Context<'_>>, - root_path: &str, + context_info: Option>, ) { debug!("highlighting: ================\n{}\n==============", src); if let Some((edition_info, class)) = tooltip { @@ -47,7 +60,7 @@ crate fn render_with_highlighting( } write_header(out, class, extra_content); - write_code(out, &src, edition, file_span_lo, context, root_path); + write_code(out, &src, edition, context_info); write_footer(out, playground_button); } @@ -69,37 +82,28 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option>, - root_path: &str, + context_info: Option>, ) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); - Classifier::new(&src, edition, file_span_lo).highlight(&mut |highlight| { - match highlight { - Highlight::Token { text, class } => { - string(out, Escape(text), class, context, root_path) - } - Highlight::EnterSpan { class } => enter_span(out, class), - Highlight::ExitSpan => exit_span(out), - }; - }); + Classifier::new(&src, edition, context_info.as_ref().map(|c| c.file_span_lo).unwrap_or(0)) + .highlight(&mut |highlight| { + match highlight { + Highlight::Token { text, class } => string(out, Escape(text), class, &context_info), + Highlight::EnterSpan { class } => enter_span(out, class), + Highlight::ExitSpan => exit_span(out), + }; + }); } fn write_footer(out: &mut Buffer, playground_button: Option<&str>) { @@ -540,8 +544,7 @@ fn string( out: &mut Buffer, text: T, klass: Option, - context: Option<&Context<'_>>, - root_path: &str, + context_info: &Option>, ) { let klass = match klass { None => return write!(out, "{}", text), @@ -570,14 +573,19 @@ fn string( path }); } - if let Some(context) = context { - if let Some(href) = - context.shared.span_correspondance_map.get(&def_span).and_then(|href| { + if let Some(context_info) = context_info { + if let Some(href) = context_info + .context + .shared + .span_correspondance_map + .get(&def_span) + .and_then(|href| { + let context = context_info.context; match href { LinkFromSrc::Local(span) => { context .href_from_span(clean::Span::wrap_raw(*span)) - .map(|s| format!("{}{}", root_path, s)) + .map(|s| format!("{}{}", context_info.root_path, s)) } LinkFromSrc::External(def_id) => { format::href(*def_id, context).map(|(url, _, _)| url) diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs index a637d8085cb57..1259a1f3a23a2 100644 --- a/src/librustdoc/html/highlight/tests.rs +++ b/src/librustdoc/html/highlight/tests.rs @@ -22,7 +22,7 @@ fn test_html_highlighting() { let src = include_str!("fixtures/sample.rs"); let html = { let mut out = Buffer::new(); - write_code(&mut out, src, Edition::Edition2018, 0, None, ""); + write_code(&mut out, src, Edition::Edition2018, None); format!("{}
{}
\n", STYLE, out.into_inner()) }; expect_file!["fixtures/sample.html"].assert_eq(&html); @@ -36,7 +36,7 @@ fn test_dos_backline() { println!(\"foo\");\r\n\ }\r\n"; let mut html = Buffer::new(); - write_code(&mut html, src, Edition::Edition2018, 0, None, ""); + write_code(&mut html, src, Edition::Edition2018, None); expect_file!["fixtures/dos_line.html"].assert_eq(&html.into_inner()); }); } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 5d79ccce8e1a0..472323daf3017 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -330,9 +330,7 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { tooltip, edition, None, - 0, None, - "", ); Some(Event::Html(s.into_inner().into())) } diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 7b6bd4dd59704..f31305c76e642 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1081,9 +1081,7 @@ fn item_macro(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Mac None, it.span(cx.tcx()).inner().edition(), None, - 0, None, - "", ); }); document(w, cx, it, None) diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 24821950869e7..9204c94bd7639 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -275,8 +275,6 @@ fn print_src( None, edition, Some(line_numbers), - file_span_lo, - Some(context), - root_path, + Some(highlight::ContextInfo { context, file_span_lo, root_path }), ); } From f233a70567a5a4f2a9f953ec4e24ae2c35d6aae1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 5 May 2021 14:48:41 +0200 Subject: [PATCH 14/22] Use rustdoc Span in LinkFromSrc directly --- src/librustdoc/clean/blanket_impl.rs | 2 +- src/librustdoc/clean/types.rs | 8 -------- src/librustdoc/html/highlight.rs | 3 +-- src/librustdoc/html/render/span_map.rs | 8 ++++---- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index 8f74a48547d88..207c89cbfe87e 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -98,7 +98,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { visibility: Inherited, def_id: ItemId::Blanket { impl_id: impl_def_id, for_: item_def_id }, kind: box ImplItem(Impl { - span: Span::from_rustc_span(self.cx.tcx.def_span(impl_def_id)), + span: Span::new(self.cx.tcx.def_span(impl_def_id)), unsafety: hir::Unsafety::Normal, generics: ( self.cx.tcx.generics_of(impl_def_id), diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index fd4748f1f50b7..22e4d21c87bdf 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1951,14 +1951,6 @@ impl Span { Self(sp.source_callsite()) } - /// Unless you know what you're doing, use [`Self::new`] instead! - /// - /// This function doesn't clean the span at all. Compare with [`Self::new`]'s body to see the - /// difference. - crate fn wrap_raw(sp: rustc_span::Span) -> Span { - Self(sp) - } - crate fn inner(&self) -> rustc_span::Span { self.0 } diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index d1dbfeff876df..9364ebc4dcfb6 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -5,7 +5,6 @@ //! //! Use the `render_with_highlighting` to highlight some rust code. -use crate::clean; use crate::html::escape::Escape; use crate::html::render::Context; @@ -584,7 +583,7 @@ fn string( match href { LinkFromSrc::Local(span) => { context - .href_from_span(clean::Span::wrap_raw(*span)) + .href_from_span(*span) .map(|s| format!("{}{}", context_info.root_path, s)) } LinkFromSrc::External(def_id) => { diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index c7cc2250c2410..6a311ac78ab2c 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -20,7 +20,7 @@ use std::path::{Path, PathBuf}; /// instead of the source code directly. #[derive(Debug)] crate enum LinkFromSrc { - Local(Span), + Local(clean::Span), External(DefId), } @@ -113,7 +113,7 @@ impl<'tcx> SpanMapVisitor<'tcx> { path_span .map(LightSpan::new_from_span) .unwrap_or_else(|| LightSpan::new_from_span(path.span)), - LinkFromSrc::Local(span), + LinkFromSrc::Local(clean::Span::new(span)), ); } else if let Some(def_id) = info { self.matches.insert( @@ -161,7 +161,7 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { Node::Item(item) => { self.matches.insert( LightSpan::new_from_span(item.ident.span), - LinkFromSrc::Local(m.inner), + LinkFromSrc::Local(clean::Span::new(m.inner)), ); } _ => {} @@ -187,7 +187,7 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { Some(span) => { self.matches.insert( LightSpan::new_from_span(method_span), - LinkFromSrc::Local(span), + LinkFromSrc::Local(clean::Span::new(span)), ); } None => { From ef0d909f26665c6c9967361c2f5d636d29168976 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 7 May 2021 13:00:53 +0200 Subject: [PATCH 15/22] formatting --- src/librustdoc/html/highlight.rs | 28 +++++++++++--------------- src/librustdoc/html/render/span_map.rs | 21 +++++++------------ 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 9364ebc4dcfb6..13c02110514ab 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -573,24 +573,20 @@ fn string( }); } if let Some(context_info) = context_info { - if let Some(href) = context_info - .context - .shared - .span_correspondance_map - .get(&def_span) - .and_then(|href| { - let context = context_info.context; - match href { - LinkFromSrc::Local(span) => { - context + if let Some(href) = + context_info.context.shared.span_correspondance_map.get(&def_span).and_then( + |href| { + let context = context_info.context; + match href { + LinkFromSrc::Local(span) => context .href_from_span(*span) - .map(|s| format!("{}{}", context_info.root_path, s)) + .map(|s| format!("{}{}", context_info.root_path, s)), + LinkFromSrc::External(def_id) => { + format::href(*def_id, context).map(|(url, _, _)| url) + } } - LinkFromSrc::External(def_id) => { - format::href(*def_id, context).map(|(url, _, _)| url) - } - } - }) + }, + ) { write!(out, "{}", klass.as_html(), href, text); return; diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 6a311ac78ab2c..daeb43b77dbb5 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -183,20 +183,13 @@ impl 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) { - match hir.span_if_local(def_id) { - Some(span) => { - self.matches.insert( - LightSpan::new_from_span(method_span), - LinkFromSrc::Local(clean::Span::new(span)), - ); - } - None => { - self.matches.insert( - LightSpan::new_from_span(method_span), - LinkFromSrc::External(def_id), - ); - } - } + self.matches.insert( + LightSpan::new_from_span(method_span), + match hir.span_if_local(def_id) { + Some(span) => LinkFromSrc::Local(clean::Span::new(span)), + None => LinkFromSrc::External(def_id), + }, + ); } } } From b336f2801c1d458990b7122bc4eba126beea37f9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 12 May 2021 17:29:51 +0200 Subject: [PATCH 16/22] Fix invalid generation of HTML in highlight --- src/librustdoc/html/highlight.rs | 83 ++++++++++--------- .../html/highlight/fixtures/highlight.html | 4 + src/librustdoc/html/highlight/tests.rs | 14 ++++ 3 files changed, 60 insertions(+), 41 deletions(-) create mode 100644 src/librustdoc/html/highlight/fixtures/highlight.html diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 13c02110514ab..9adb95fe90e43 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -313,6 +313,7 @@ impl<'a> Classifier<'a> { .unwrap_or(false) { let tokens = self.get_full_ident_path(); + // We need this variable because `tokens` is consumed in the loop. let skip = !tokens.is_empty(); for (token, start, end) in tokens { let text = &self.src[start..end]; @@ -549,51 +550,51 @@ fn string( None => return write!(out, "{}", text), Some(klass) => klass, }; - if let Some(def_span) = klass.get_span() { - let mut text = text.to_string(); - if text.contains("::") { - text = text.split("::").intersperse("::").fold(String::new(), |mut path, t| { - match t { - "self" | "Self" => write!( - &mut path, - "{}", - Class::Self_(LightSpan::empty()).as_html(), - t - ), - "crate" | "super" => write!( - &mut path, - "{}", - Class::KeyWord.as_html(), - t - ), - t => write!(&mut path, "{}", t), - } - .expect("Failed to build source HTML path"); - path - }); + let def_span = match klass.get_span() { + Some(d) => d, + None => { + write!(out, "{}", klass.as_html(), text); + return; } - if let Some(context_info) = context_info { - if let Some(href) = - context_info.context.shared.span_correspondance_map.get(&def_span).and_then( - |href| { - let context = context_info.context; - match href { - LinkFromSrc::Local(span) => context - .href_from_span(*span) - .map(|s| format!("{}{}", context_info.root_path, s)), - LinkFromSrc::External(def_id) => { - format::href(*def_id, context).map(|(url, _, _)| url) - } - } - }, - ) - { - write!(out, "{}", klass.as_html(), href, text); - return; + }; + let mut text_s = text.to_string(); + if text_s.contains("::") { + text_s = text_s.split("::").intersperse("::").fold(String::new(), |mut path, t| { + match t { + "self" | "Self" => write!( + &mut path, + "{}", + Class::Self_(LightSpan::empty()).as_html(), + t + ), + "crate" | "super" => { + write!(&mut path, "{}", Class::KeyWord.as_html(), t) + } + t => write!(&mut path, "{}", t), } + .expect("Failed to build source HTML path"); + path + }); + } + if let Some(context_info) = context_info { + if let Some(href) = + context_info.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { + let context = context_info.context; + match href { + LinkFromSrc::Local(span) => context + .href_from_span(*span) + .map(|s| format!("{}{}", context_info.root_path, s)), + LinkFromSrc::External(def_id) => { + format::href(*def_id, context).map(|(url, _, _)| url) + } + } + }) + { + write!(out, "{}", klass.as_html(), href, text_s); + return; } } - write!(out, "{}", klass.as_html(), text); + write!(out, "{}", klass.as_html(), text_s); } #[cfg(test)] diff --git a/src/librustdoc/html/highlight/fixtures/highlight.html b/src/librustdoc/html/highlight/fixtures/highlight.html new file mode 100644 index 0000000000000..abc2db1790c53 --- /dev/null +++ b/src/librustdoc/html/highlight/fixtures/highlight.html @@ -0,0 +1,4 @@ +use crate::a::foo; +use self::whatever; +let x = super::b::foo; +let y = Self::whatever; \ No newline at end of file diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs index 1259a1f3a23a2..68592ae96c187 100644 --- a/src/librustdoc/html/highlight/tests.rs +++ b/src/librustdoc/html/highlight/tests.rs @@ -40,3 +40,17 @@ fn test_dos_backline() { expect_file!["fixtures/dos_line.html"].assert_eq(&html.into_inner()); }); } + +#[test] +fn test_keyword_highlight() { + create_default_session_globals_then(|| { + let src = "use crate::a::foo; +use self::whatever; +let x = super::b::foo; +let y = Self::whatever;"; + + let mut html = Buffer::new(); + write_code(&mut html, src, Edition::Edition2018, None); + expect_file!["fixtures/highlight.html"].assert_eq(&html.into_inner()); + }); +} From 1a48d1a4de1e26e08780a026daed3c7db743ff2f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 20 May 2021 10:42:18 +0200 Subject: [PATCH 17/22] Add documentation and FIXME --- src/librustdoc/html/render/span_map.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index daeb43b77dbb5..e168e41b832a6 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -97,16 +97,20 @@ struct SpanMapVisitor<'tcx> { } impl<'tcx> SpanMapVisitor<'tcx> { - fn handle_path(&mut self, path: &rustc_hir::Path<'_>, path_span: Option) -> bool { + /// 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) { let info = 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 false; + return; } Some(def_id) } Res::Local(_) => None, - _ => return true, + _ => return, }; if let Some(span) = self.tcx.hir().res_span(path.res) { self.matches.insert( @@ -123,7 +127,6 @@ impl<'tcx> SpanMapVisitor<'tcx> { LinkFromSrc::External(def_id), ); } - true } } From fd69fa8670aaf67cde504acd097a2b4ddc74f88a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 12 Jun 2021 22:08:43 +0200 Subject: [PATCH 18/22] Add missing root_path when generating links using href --- src/librustdoc/html/format.rs | 18 +++++++++++++++++- src/librustdoc/html/highlight.rs | 9 ++++++++- src/librustdoc/html/render/span_map.rs | 1 + src/test/rustdoc/auxiliary/source_code.rs | 1 + .../rustdoc/check-source-code-urls-to-def.rs | 11 ++++++++--- 5 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 src/test/rustdoc/auxiliary/source_code.rs diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 8ab6aa775d2e4..eb7c12d13c339 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -484,7 +484,11 @@ crate enum HrefError { NotInExternalCache, } -crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec), HrefError> { +crate fn href_with_root_path( + did: DefId, + cx: &Context<'_>, + root_path: Option<&str>, +) -> Result<(String, ItemType, Vec), HrefError> { let cache = &cx.cache(); let relative_to = &cx.current; fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] { @@ -495,6 +499,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec (fqp, shortty, { let module_fqp = to_module_fqp(shortty, fqp); @@ -508,6 +513,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec { + is_remote = true; let s = s.trim_end_matches('/'); let mut s = vec![s]; s.extend(module_fqp[..].iter().map(String::as_str)); @@ -522,6 +528,12 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec) -> Result<(String, ItemType, Vec) -> Result<(String, ItemType, Vec), HrefError> { + href_with_root_path(did, cx, None) +} + /// Both paths should only be modules. /// This is because modules get their own directories; that is, `std::vec` and `std::vec::Vec` will /// both need `../iter/trait.Iterator.html` to get at the iterator trait. diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 9adb95fe90e43..47c870645ac48 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -580,12 +580,19 @@ fn string( if let Some(href) = context_info.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { let context = context_info.context; + // FIXME: later on, it'd be nice to provide two links (if possible) for all items: + // one to the documentation page and one to the source definition. + // FIXME: currently, external items only generate a link to their documentation, + // a link to their definition can be generated using this: + // https://github.com/rust-lang/rust/blob/60f1a2fc4b535ead9c85ce085fdce49b1b097531/src/librustdoc/html/render/context.rs#L315-L338 match href { LinkFromSrc::Local(span) => context .href_from_span(*span) .map(|s| format!("{}{}", context_info.root_path, s)), LinkFromSrc::External(def_id) => { - format::href(*def_id, context).map(|(url, _, _)| url) + format::href_with_root_path(*def_id, context, Some(context_info.root_path)) + .ok() + .map(|(url, _, _)| url) } } }) diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index e168e41b832a6..b60f5f433f7f7 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -110,6 +110,7 @@ impl<'tcx> SpanMapVisitor<'tcx> { Some(def_id) } Res::Local(_) => None, + Res::Err => return, _ => return, }; if let Some(span) = self.tcx.hir().res_span(path.res) { diff --git a/src/test/rustdoc/auxiliary/source_code.rs b/src/test/rustdoc/auxiliary/source_code.rs new file mode 100644 index 0000000000000..72a5c1a0ae97e --- /dev/null +++ b/src/test/rustdoc/auxiliary/source_code.rs @@ -0,0 +1 @@ +pub struct SourceCode; diff --git a/src/test/rustdoc/check-source-code-urls-to-def.rs b/src/test/rustdoc/check-source-code-urls-to-def.rs index 5caef29fede37..51c4835b5aae6 100644 --- a/src/test/rustdoc/check-source-code-urls-to-def.rs +++ b/src/test/rustdoc/check-source-code-urls-to-def.rs @@ -1,7 +1,11 @@ // compile-flags: -Zunstable-options --generate-link-to-definition +// aux-build:source_code.rs +// build-aux-docs #![crate_name = "foo"] +extern crate source_code; + // @has 'src/foo/check-source-code-urls-to-def.rs.html' // @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#1-17"]' 'bar' @@ -23,13 +27,14 @@ impl Foo { fn babar() {} // @has - '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html"]' 'String' -// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#17"]' 5 -pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar) { +// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#21"]' 5 +// @has - '//a[@href="../../source_code/struct.SourceCode.html"]' '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; let z: Bar = bar::Bar { field: Foo }; babar(); - // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#20"]' 'hello' + // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#24"]' 'hello' y.hello(); } From 0799528db785e49ad13044aac193b8be2153e202 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Jul 2021 15:28:43 +0200 Subject: [PATCH 19/22] * Rename LightSpan::empty into LightSpan::dummy * Add Classifier::new_light_span to wrap LightSpan::new_in_file constructor --- src/librustdoc/html/highlight.rs | 40 ++++++++++++-------------- src/librustdoc/html/render/span_map.rs | 2 +- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 47c870645ac48..4555d98d2bfcd 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -235,6 +235,12 @@ impl<'a> Classifier<'a> { } } + /// Convenient wrapper around [`LightSpan::new_in_file`] to prevent passing the `file_span_lo` + /// argument every time. + fn new_light_span(&self, lo: u32, hi: u32) -> LightSpan { + LightSpan::new_in_file(self.file_span_lo, lo, hi) + } + /// Concatenate colons and idents as one when possible. fn get_full_ident_path(&mut self) -> Vec<(TokenKind, usize, usize)> { let start = self.byte_pos as usize; @@ -313,14 +319,12 @@ impl<'a> Classifier<'a> { .unwrap_or(false) { let tokens = self.get_full_ident_path(); - // We need this variable because `tokens` is consumed in the loop. - let skip = !tokens.is_empty(); - for (token, start, end) in tokens { - let text = &self.src[start..end]; - self.advance(token, text, sink, start as u32); + for (token, start, end) in &tokens { + let text = &self.src[*start..*end]; + self.advance(*token, text, sink, *start as u32); self.byte_pos += text.len() as u32; } - if skip { + if !tokens.is_empty() { continue; } } @@ -483,24 +487,16 @@ impl<'a> Classifier<'a> { self.in_macro_nonterminal = false; Class::MacroNonTerminal } - "self" | "Self" => Class::Self_(LightSpan::new_in_file( - self.file_span_lo, - before, - before + text.len() as u32, - )), - _ => Class::Ident(LightSpan::new_in_file( - self.file_span_lo, - before, - before + text.len() as u32, - )), + "self" | "Self" => { + Class::Self_(self.new_light_span(before, before + text.len() as u32)) + } + _ => Class::Ident(self.new_light_span(before, before + text.len() as u32)), }, Some(c) => c, }, - TokenKind::RawIdent | TokenKind::UnknownPrefix => Class::Ident(LightSpan::new_in_file( - self.file_span_lo, - before, - before + text.len() as u32, - )), + TokenKind::RawIdent | TokenKind::UnknownPrefix => { + Class::Ident(self.new_light_span(before, before + text.len() as u32)) + } TokenKind::Lifetime { .. } => Class::Lifetime, }; // Anything that didn't return above is the simple case where we the @@ -564,7 +560,7 @@ fn string( "self" | "Self" => write!( &mut path, "{}", - Class::Self_(LightSpan::empty()).as_html(), + Class::Self_(LightSpan::dummy()).as_html(), t ), "crate" | "super" => { diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index b60f5f433f7f7..390ac88faf082 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -51,7 +51,7 @@ impl LightSpan { Self { lo: lo + file_span_lo, hi: hi + file_span_lo } } - crate fn empty() -> Self { + crate fn dummy() -> Self { Self { lo: 0, hi: 0 } } From 5cf300d695c4ac6e4bdab8fe5c48de6b05b2cd96 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 21 Jul 2021 14:53:13 +0200 Subject: [PATCH 20/22] Remove warnings/errors from compiler when using typeck_body in rustdoc span map builder --- compiler/rustc_errors/src/lib.rs | 17 ++++++++++++++++- compiler/rustc_session/src/session.rs | 4 ++++ src/librustdoc/html/render/span_map.rs | 10 +++++----- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 993a7c2c162c6..fc0924ac5f920 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -342,6 +342,9 @@ struct HandlerInner { deduplicated_warn_count: usize, future_breakage_diagnostics: Vec, + + /// If set to `true`, no warning or error will be emitted. + quiet: bool, } /// A key denoting where from a diagnostic was stashed. @@ -456,10 +459,19 @@ impl Handler { emitted_diagnostics: Default::default(), stashed_diagnostics: Default::default(), future_breakage_diagnostics: Vec::new(), + quiet: false, }), } } + pub fn with_disabled_diagnostic T>(&self, f: F) -> T { + let prev = self.inner.borrow_mut().quiet; + self.inner.borrow_mut().quiet = true; + let ret = f(); + self.inner.borrow_mut().quiet = prev; + ret + } + // This is here to not allow mutation of flags; // as of this writing it's only used in tests in librustc_middle. pub fn can_emit_warnings(&self) -> bool { @@ -818,7 +830,7 @@ impl HandlerInner { } fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) { - if diagnostic.cancelled() { + if diagnostic.cancelled() || self.quiet { return; } @@ -1035,6 +1047,9 @@ impl HandlerInner { } fn delay_as_bug(&mut self, diagnostic: Diagnostic) { + if self.quiet { + return; + } if self.flags.report_delayed_bugs { self.emit_diagnostic(&diagnostic); } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 9ab6dbb1ea906..fe87867d2996c 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -500,6 +500,10 @@ impl Session { &self.parse_sess.span_diagnostic } + pub fn with_disabled_diagnostic T>(&self, f: F) -> T { + self.parse_sess.span_diagnostic.with_disabled_diagnostic(f) + } + /// Analogous to calling methods on the given `DiagnosticBuilder`, but /// deduplicates on lint ID, span (if any), and message for this `Session` fn diag_once<'a, 'b>( diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 390ac88faf082..429673c96e787 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -181,11 +181,11 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { if let Some(hir_id) = segment.hir_id { let hir = self.tcx.hir(); let body_id = hir.enclosing_body_owner(hir_id); - // FIXME: this is showing error messages for parts of the code that are not - // compiled (because of cfg)! - let typeck_results = self.tcx.typeck_body( - hir.maybe_body_owned_by(body_id).expect("a body which isn't a body"), - ); + let typeck_results = self.tcx.sess.with_disabled_diagnostic(|| { + self.tcx.typeck_body( + 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( LightSpan::new_from_span(method_span), From dfe4fec783c657ccbcba183fe2cadbc7c1db8525 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 2 Aug 2021 17:44:04 +0200 Subject: [PATCH 21/22] Remove LightSpan and use Span directly --- src/librustdoc/html/highlight.rs | 49 +++++++++------------ src/librustdoc/html/render/context.rs | 6 +-- src/librustdoc/html/render/mod.rs | 2 +- src/librustdoc/html/render/span_map.rs | 61 ++++---------------------- src/librustdoc/html/sources.rs | 24 +++++----- 5 files changed, 44 insertions(+), 98 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 4555d98d2bfcd..3cdb1352bef95 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -14,20 +14,16 @@ use std::iter::Peekable; use rustc_lexer::{LiteralKind, TokenKind}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; +use rustc_span::{BytePos, Span, DUMMY_SP}; use super::format::{self, Buffer}; -use super::render::{LightSpan, LinkFromSrc}; +use super::render::LinkFromSrc; /// This type is needed in case we want to render links on items to allow to go to their definition. crate struct ContextInfo<'a, 'b, 'c> { crate context: &'a Context<'b>, - /// This represents the "lo" bytes of the current file we're rendering. To get a [`Span`] from - /// it, you just need to add add your current byte position in the string and its length (to get - /// the "hi" part). - /// - /// This is used to create a [`LightSpan`] which is then used as an index in the `span_map` in - /// order to retrieve the definition's [`Span`] (which is used to generate the URL). - crate file_span_lo: u32, + /// This span contains the current file we're going through. + crate file_span: Span, /// This field is used to know "how far" from the top of the directory we are to link to either /// documentation pages or other source pages. crate root_path: &'c str, @@ -86,7 +82,6 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option string(out, Escape(text), class, &context_info), @@ -118,14 +113,14 @@ enum Class { KeyWord, // Keywords that do pointer/reference stuff. RefKeyWord, - Self_(LightSpan), + Self_(Span), Op, Macro, MacroNonTerminal, String, Number, Bool, - Ident(LightSpan), + Ident(Span), Lifetime, PreludeTy, PreludeVal, @@ -158,7 +153,7 @@ impl Class { /// In case this is an item which can be converted into a link to a definition, it'll contain /// a "span" (a tuple representing `(lo, hi)` equivalent of `Span`). - fn get_span(self) -> Option { + fn get_span(self) -> Option { match self { Self::Ident(sp) | Self::Self_(sp) => Some(sp), _ => None, @@ -213,15 +208,14 @@ struct Classifier<'a> { in_macro_nonterminal: bool, edition: Edition, byte_pos: u32, - file_span_lo: u32, + file_span: Span, src: &'a str, } impl<'a> Classifier<'a> { /// Takes as argument the source code to HTML-ify, the rust edition to use and the source code - /// file "lo" byte which we be used later on by the `span_correspondance_map`. More explanations - /// are provided in the [`LightSpan::new_in_file`] function documentation about how it works. - fn new(src: &str, edition: Edition, file_span_lo: u32) -> Classifier<'_> { + /// file span which will be used later on by the `span_correspondance_map`. + fn new(src: &str, edition: Edition, file_span: Span) -> Classifier<'_> { let tokens = TokenIter { src }.peekable(); Classifier { tokens, @@ -230,15 +224,16 @@ impl<'a> Classifier<'a> { in_macro_nonterminal: false, edition, byte_pos: 0, - file_span_lo, + file_span, src, } } - /// Convenient wrapper around [`LightSpan::new_in_file`] to prevent passing the `file_span_lo` - /// argument every time. - fn new_light_span(&self, lo: u32, hi: u32) -> LightSpan { - LightSpan::new_in_file(self.file_span_lo, lo, hi) + /// Convenient wrapper to create a [`Span`] from a position in the file. + fn new_span(&self, lo: u32, text: &str) -> Span { + let hi = lo + text.len() as u32; + let file_lo = self.file_span.lo(); + self.file_span.with_lo(file_lo + BytePos(lo)).with_hi(file_lo + BytePos(hi)) } /// Concatenate colons and idents as one when possible. @@ -487,15 +482,13 @@ impl<'a> Classifier<'a> { self.in_macro_nonterminal = false; Class::MacroNonTerminal } - "self" | "Self" => { - Class::Self_(self.new_light_span(before, before + text.len() as u32)) - } - _ => Class::Ident(self.new_light_span(before, before + text.len() as u32)), + "self" | "Self" => Class::Self_(self.new_span(before, text)), + _ => Class::Ident(self.new_span(before, text)), }, Some(c) => c, }, TokenKind::RawIdent | TokenKind::UnknownPrefix => { - Class::Ident(self.new_light_span(before, before + text.len() as u32)) + Class::Ident(self.new_span(before, text)) } TokenKind::Lifetime { .. } => Class::Lifetime, }; @@ -560,7 +553,7 @@ fn string( "self" | "Self" => write!( &mut path, "{}", - Class::Self_(LightSpan::dummy()).as_html(), + Class::Self_(DUMMY_SP).as_html(), t ), "crate" | "super" => { diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 36ec2cf3f7a25..6ce0828e15940 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -18,8 +18,8 @@ use super::cache::{build_index, ExternalLocation}; use super::print_item::{full_path, item_path, print_item}; use super::write_shared::write_shared; use super::{ - collect_spans_and_sources, print_sidebar, settings, AllTypes, LightSpan, LinkFromSrc, NameDoc, - StylePath, BASIC_KEYWORDS, + collect_spans_and_sources, print_sidebar, settings, AllTypes, LinkFromSrc, NameDoc, StylePath, + BASIC_KEYWORDS, }; use crate::clean; @@ -131,7 +131,7 @@ crate struct SharedContext<'tcx> { /// 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. - crate span_correspondance_map: FxHashMap, + crate span_correspondance_map: FxHashMap, } impl SharedContext<'_> { diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 584afdeb280fc..fd2e18a8be77f 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -34,7 +34,7 @@ mod span_map; mod write_shared; crate use context::*; -crate use span_map::{collect_spans_and_sources, LightSpan, LinkFromSrc}; +crate use span_map::{collect_spans_and_sources, LinkFromSrc}; use std::collections::VecDeque; use std::default::Default; diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 429673c96e787..b35cd45dc9a88 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -24,43 +24,6 @@ crate enum LinkFromSrc { External(DefId), } -/// This struct is used only as index in the `span_map`, not as [`Span`]! `Span`s contain -/// some extra information (the syntax context) we don't need. **Do not convert this type back to -/// `Span`!!!** -#[derive(Clone, Copy, Hash, PartialEq, Eq, Debug)] -crate struct LightSpan { - crate lo: u32, - crate hi: u32, -} - -impl LightSpan { - /// Before explaining what this method does, some global explanations on rust's `Span`: - /// - /// Each source code file is stored in the source map in the compiler and has a - /// `lo` and a `hi` (lowest and highest bytes in this source map which can be seen as one huge - /// string to simplify things). So in this case, this represents the starting byte of the - /// current file. It'll be used later on to retrieve the "definition span" from the - /// `span_correspondance_map` (which is inside `context`). - /// - /// This when we transform the "span" we have from reading the input into a "span" which can be - /// used as index to the `span_correspondance_map` to get the definition of this item. - /// - /// So in here, `file_span_lo` is representing the "lo" byte in the global source map, and to - /// make our "span" works in there, we simply add `file_span_lo` to our values. - crate fn new_in_file(file_span_lo: u32, lo: u32, hi: u32) -> Self { - Self { lo: lo + file_span_lo, hi: hi + file_span_lo } - } - - crate fn dummy() -> Self { - Self { lo: 0, hi: 0 } - } - - /// Extra the `lo` and `hi` from the [`Span`] and discard the unused syntax context. - fn new_from_span(sp: Span) -> Self { - Self { lo: sp.lo().0, hi: sp.hi().0 } - } -} - /// This function will do at most two things: /// /// 1. Generate a `span` correspondance map which links an item `span` to its definition `span`. @@ -77,7 +40,7 @@ crate fn collect_spans_and_sources( src_root: &Path, include_sources: bool, generate_link_to_definition: bool, -) -> (clean::Crate, FxHashMap, FxHashMap) { +) -> (clean::Crate, FxHashMap, FxHashMap) { let mut visitor = SpanMapVisitor { tcx, matches: FxHashMap::default() }; if include_sources { @@ -93,7 +56,7 @@ crate fn collect_spans_and_sources( struct SpanMapVisitor<'tcx> { crate tcx: TyCtxt<'tcx>, - crate matches: FxHashMap, + crate matches: FxHashMap, } impl<'tcx> SpanMapVisitor<'tcx> { @@ -115,18 +78,12 @@ impl<'tcx> SpanMapVisitor<'tcx> { }; if let Some(span) = self.tcx.hir().res_span(path.res) { self.matches.insert( - path_span - .map(LightSpan::new_from_span) - .unwrap_or_else(|| LightSpan::new_from_span(path.span)), + path_span.unwrap_or_else(|| path.span), LinkFromSrc::Local(clean::Span::new(span)), ); } else if let Some(def_id) = info { - self.matches.insert( - path_span - .map(LightSpan::new_from_span) - .unwrap_or_else(|| LightSpan::new_from_span(path.span)), - LinkFromSrc::External(def_id), - ); + self.matches + .insert(path_span.unwrap_or_else(|| path.span), LinkFromSrc::External(def_id)); } } } @@ -163,10 +120,8 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { if let Some(node) = self.tcx.hir().find(id) { match node { Node::Item(item) => { - self.matches.insert( - LightSpan::new_from_span(item.ident.span), - LinkFromSrc::Local(clean::Span::new(m.inner)), - ); + self.matches + .insert(item.ident.span, LinkFromSrc::Local(clean::Span::new(m.inner))); } _ => {} } @@ -188,7 +143,7 @@ impl Visitor<'tcx> for SpanMapVisitor<'tcx> { }); if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) { self.matches.insert( - LightSpan::new_from_span(method_span), + method_span, match hir.span_if_local(def_id) { Some(span) => LinkFromSrc::Local(clean::Span::new(span)), None => LinkFromSrc::External(def_id), diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 9204c94bd7639..73916e204d942 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -111,13 +111,14 @@ impl DocFolder for SourceCollector<'_, '_> { if self.cx.include_sources && is_real_and_local(span, sess) { let filename = span.filename(sess); let span = span.inner(); - let start_pos = sess.source_map().lookup_source_file(span.lo()).start_pos; + let pos = sess.source_map().lookup_source_file(span.lo()); + let file_span = span.with_lo(pos.start_pos).with_hi(pos.end_pos); // If it turns out that we couldn't read this file, then we probably // can't read any of the files (generating html output from json or // something like that), so just don't include sources for the // entire crate. The other option is maintaining this mapping on a // per-file basis, but that's probably not worth it... - self.cx.include_sources = match self.emit_source(&filename, start_pos.0) { + self.cx.include_sources = match self.emit_source(&filename, file_span) { Ok(()) => true, Err(e) => { self.cx.shared.tcx.sess.span_err( @@ -140,7 +141,11 @@ impl DocFolder for SourceCollector<'_, '_> { impl SourceCollector<'_, 'tcx> { /// Renders the given filename into its corresponding HTML source file. - fn emit_source(&mut self, filename: &FileName, file_span_lo: u32) -> Result<(), Error> { + fn emit_source( + &mut self, + filename: &FileName, + file_span: rustc_span::Span, + ) -> Result<(), Error> { let p = match *filename { FileName::Real(ref file) => { if let Some(local_path) = file.local_path() { @@ -200,14 +205,7 @@ impl SourceCollector<'_, 'tcx> { &page, "", |buf: &mut _| { - print_src( - buf, - contents, - self.cx.shared.edition(), - file_span_lo, - &self.cx, - &root_path, - ) + print_src(buf, contents, self.cx.shared.edition(), file_span, &self.cx, &root_path) }, &self.cx.shared.style_files, ); @@ -250,7 +248,7 @@ fn print_src( buf: &mut Buffer, s: &str, edition: Edition, - file_span_lo: u32, + file_span: rustc_span::Span, context: &Context<'_>, root_path: &str, ) { @@ -275,6 +273,6 @@ fn print_src( None, edition, Some(line_numbers), - Some(highlight::ContextInfo { context, file_span_lo, root_path }), + Some(highlight::ContextInfo { context, file_span, root_path }), ); } From ba11dc7fddd58eb3559d0724e8ced0d2c2128f4a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 5 Aug 2021 23:33:43 +0200 Subject: [PATCH 22/22] Fix URL conflict for std type --- src/test/rustdoc/check-source-code-urls-to-def.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rustdoc/check-source-code-urls-to-def.rs b/src/test/rustdoc/check-source-code-urls-to-def.rs index 51c4835b5aae6..e3ae79ccdb17a 100644 --- a/src/test/rustdoc/check-source-code-urls-to-def.rs +++ b/src/test/rustdoc/check-source-code-urls-to-def.rs @@ -26,7 +26,7 @@ impl Foo { fn babar() {} -// @has - '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html"]' 'String' +// @has - '//a/@href' '/struct.String.html' // @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#21"]' 5 // @has - '//a[@href="../../source_code/struct.SourceCode.html"]' 'source_code::SourceCode' pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar, f: source_code::SourceCode) {