Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Generate links to definition in rustdoc source code pages #84176

Merged
merged 22 commits into from
Aug 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
023231a
Add links on source types to go to definition
GuillaumeGomez Apr 5, 2021
2104bf2
Add an option for the source code link generation
GuillaumeGomez Apr 13, 2021
83dcd30
Ensure that --generate-link-to-definition is only used with HTML outp…
GuillaumeGomez May 4, 2021
b689ced
Generate links for methods as well
GuillaumeGomez Apr 13, 2021
71763a5
Add test for source code pages URLs
GuillaumeGomez Apr 13, 2021
1abb7fa
Generate links for modules as well
GuillaumeGomez Apr 13, 2021
89bdc33
Update rustdoc tests
GuillaumeGomez Apr 13, 2021
b5c27b4
Underline source code links on hover
GuillaumeGomez Apr 14, 2021
2a3b71a
* Rename 'move_span' into 'local_span_to_global_span'
GuillaumeGomez Apr 20, 2021
38444f6
* Rename Span::from_rustc_span to Span::new
GuillaumeGomez Apr 22, 2021
c5c927d
Improve code readability
GuillaumeGomez May 4, 2021
e8869cb
Wrap the span_map tuple index into a type called "LightSpan"
GuillaumeGomez May 4, 2021
dffc9c0
Move extra arguments for highlight URL generation into a new ContextI…
GuillaumeGomez May 5, 2021
f233a70
Use rustdoc Span in LinkFromSrc directly
GuillaumeGomez May 5, 2021
ef0d909
formatting
GuillaumeGomez May 7, 2021
b336f28
Fix invalid generation of HTML in highlight
GuillaumeGomez May 12, 2021
1a48d1a
Add documentation and FIXME
GuillaumeGomez May 20, 2021
fd69fa8
Add missing root_path when generating links using href
GuillaumeGomez Jun 12, 2021
0799528
* Rename LightSpan::empty into LightSpan::dummy
GuillaumeGomez Jul 13, 2021
5cf300d
Remove warnings/errors from compiler when using typeck_body in rustdo…
GuillaumeGomez Jul 21, 2021
dfe4fec
Remove LightSpan and use Span directly
GuillaumeGomez Aug 2, 2021
ba11dc7
Fix URL conflict for std type
GuillaumeGomez Aug 5, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ struct HandlerInner {
deduplicated_warn_count: usize,

future_breakage_diagnostics: Vec<Diagnostic>,

/// If set to `true`, no warning or error will be emitted.
quiet: bool,
}

/// A key denoting where from a diagnostic was stashed.
Expand Down Expand Up @@ -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, F: FnOnce() -> 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 {
Expand Down Expand Up @@ -818,7 +830,7 @@ impl HandlerInner {
}

fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) {
if diagnostic.cancelled() {
if diagnostic.cancelled() || self.quiet {
return;
}

Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ impl Session {
&self.parse_sess.span_diagnostic
}

pub fn with_disabled_diagnostic<T, F: FnOnce() -> 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>(
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ impl Clean<Item> 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());
Expand Down
11 changes: 6 additions & 5 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -1943,10 +1943,11 @@ crate enum Variant {
crate struct Span(rustc_span::Span);

impl Span {
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)
/// 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 new(sp: rustc_span::Span) -> Self {
Self(sp.source_callsite())
}

Expand Down
12 changes: 12 additions & 0 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ crate struct RenderOptions {
crate show_type_layout: bool,
crate unstable_features: rustc_feature::UnstableFeatures,
crate emit: Vec<EmitType>,
/// If `true`, HTML source pages will generate links for items to their definition.
crate generate_link_to_definition: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally all these bools would instead be new enums. So far there aren't that many, but I'd prefer it to avoid issues in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

They don't exclude each others, so I don't think enums would be helpful here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant newtyping the bools for each field, so that someone doesn't accidentally sets generate_link_to_definition to true, when they meant to set show_type_layout, although with named fields this is less of a concern.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum... I'm not convinced but I'll open an issue just in case.

}

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -655,6 +657,15 @@ 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");

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",
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
)
.emit();
return Err(1);
}

let (lint_opts, describe_lints, lint_cap) =
get_cmd_lint_options(matches, error_format, &debugging_opts);
Expand Down Expand Up @@ -721,6 +732,7 @@ impl Options {
crate_name.as_deref(),
),
emit,
generate_link_to_definition,
},
crate_name,
output_format,
Expand Down
18 changes: 17 additions & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,11 @@ crate enum HrefError {
NotInExternalCache,
}

crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
crate fn href_with_root_path(
did: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<String>), HrefError> {
let cache = &cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
Expand All @@ -495,6 +499,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
return Err(HrefError::Private);
}

let mut is_remote = false;
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
Some(&(ref fqp, shortty)) => (fqp, shortty, {
let module_fqp = to_module_fqp(shortty, fqp);
Expand All @@ -508,6 +513,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
shortty,
match cache.extern_locations[&did.krate] {
ExternalLocation::Remote(ref s) => {
is_remote = true;
let s = s.trim_end_matches('/');
let mut s = vec![s];
s.extend(module_fqp[..].iter().map(String::as_str));
Expand All @@ -522,6 +528,12 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
}
}
};
if !is_remote {
if let Some(root_path) = root_path {
let root = root_path.trim_end_matches('/');
url_parts.insert(0, root);
}
}
let last = &fqp.last().unwrap()[..];
let filename;
match shortty {
Expand All @@ -536,6 +548,10 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Str
Ok((url_parts.join("/"), shortty, fqp.to_vec()))
}

crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), 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.
Expand Down
Loading