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

Support proc macros in intra doc link resolution #73183

Merged
merged 5 commits into from
Jun 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 72 additions & 28 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty;
use rustc_resolve::ParentScope;
use rustc_session::lint;
use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::Ident;
use rustc_span::symbol::Symbol;
use rustc_span::DUMMY_SP;
Expand Down Expand Up @@ -122,6 +123,42 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
}

/// Resolves a string as a macro.
fn macro_resolve(&self, path_str: &str, parent_id: Option<hir::HirId>) -> Option<Res> {
let cx = self.cx;
let path = ast::Path::from_ident(Ident::from_str(path_str));
cx.enter_resolver(|resolver| {
if let Ok((Some(ext), res)) = resolver.resolve_macro_path(
&path,
None,
&ParentScope::module(resolver.graph_root()),
false,
false,
) {
if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind {
return Some(res.map_id(|_| panic!("unexpected id")));
}
}
if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
return Some(res.map_id(|_| panic!("unexpected id")));
}
if let Some(module_id) = parent_id.or(self.mod_ids.last().cloned()) {
let module_id = cx.tcx.hir().local_def_id(module_id);
if let Ok((_, res)) =
resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id)
{
// don't resolve builtins like `#[derive]`
if let Res::Def(..) = res {
let res = res.map_id(|_| panic!("unexpected node_id"));
return Some(res);
}
}
} else {
debug!("attempting to resolve item without parent module: {}", path_str);
}
None
})
}
/// Resolves a string as a path within a particular namespace. Also returns an optional
/// URL fragment in the case of variants and methods.
fn resolve(
Expand Down Expand Up @@ -371,6 +408,22 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
}

/// Check for resolve collisions between a trait and its derive
///
/// These are common and we should just resolve to the trait in that case
fn is_derive_trait_collision<T>(ns: &PerNS<Option<(Res, T)>>) -> bool {
if let PerNS {
type_ns: Some((Res::Def(DefKind::Trait, _), _)),
macro_ns: Some((Res::Def(DefKind::Macro(MacroKind::Derive), _), _)),
..
} = *ns
{
true
} else {
false
}
}

impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
fn fold_item(&mut self, mut item: Item) -> Option<Item> {
let item_hir_id = if item.is_mod() {
Expand Down Expand Up @@ -532,6 +585,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
} else if link.starts_with("macro@") {
kind = Some(MacroNS);
link.trim_start_matches("macro@")
} else if link.starts_with("derive@") {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering: is there any reason behind supporting derive@ too? (Not against it, just curious)

Copy link
Member Author

Choose a reason for hiding this comment

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

People don't typically consider custom derives to be macros, so asking them to use macro@ seems weird. macro@ still will work (this is true of all of the disambiguators, they don't do anything other than selecting the namespace)

kind = Some(MacroNS);
link.trim_start_matches("derive@")
} else if link.ends_with('!') {
kind = Some(MacroNS);
link.trim_end_matches('!')
Expand Down Expand Up @@ -614,8 +670,9 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
}
None => {
// Try everything!
let candidates = PerNS {
macro_ns: macro_resolve(cx, path_str)
let mut candidates = PerNS {
macro_ns: self
.macro_resolve(path_str, base_node)
.map(|res| (res, extra_fragment.clone())),
type_ns: match self.resolve(
path_str,
Expand Down Expand Up @@ -668,10 +725,16 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
continue;
}

let is_unambiguous = candidates.clone().present_items().count() == 1;
if is_unambiguous {
let len = candidates.clone().present_items().count();

if len == 1 {
candidates.present_items().next().unwrap()
} else if len == 2 && is_derive_trait_collision(&candidates) {
candidates.type_ns.unwrap()
} else {
if is_derive_trait_collision(&candidates) {
candidates.macro_ns = None;
}
ambiguity_error(
cx,
&item,
Expand All @@ -684,7 +747,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
}
}
Some(MacroNS) => {
if let Some(res) = macro_resolve(cx, path_str) {
if let Some(res) = self.macro_resolve(path_str, base_node) {
(res, extra_fragment)
} else {
resolution_failure(cx, &item, path_str, &dox, link_range);
Expand Down Expand Up @@ -727,28 +790,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
}
}

/// Resolves a string as a macro.
fn macro_resolve(cx: &DocContext<'_>, path_str: &str) -> Option<Res> {
let path = ast::Path::from_ident(Ident::from_str(path_str));
cx.enter_resolver(|resolver| {
if let Ok((Some(ext), res)) = resolver.resolve_macro_path(
&path,
None,
&ParentScope::module(resolver.graph_root()),
false,
false,
) {
if let SyntaxExtensionKind::LegacyBang { .. } = ext.kind {
return Some(res.map_id(|_| panic!("unexpected id")));
}
}
if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
return Some(res.map_id(|_| panic!("unexpected id")));
}
None
})
}

fn build_diagnostic(
cx: &DocContext<'_>,
item: &Item,
Expand Down Expand Up @@ -916,7 +957,7 @@ fn ambiguity_error(
Res::Def(DefKind::AssocFn | DefKind::Fn, _) => {
("add parentheses", format!("{}()", path_str))
}
Res::Def(DefKind::Macro(..), _) => {
Res::Def(DefKind::Macro(MacroKind::Bang), _) => {
("add an exclamation mark", format!("{}!", path_str))
}
_ => {
Expand All @@ -930,6 +971,9 @@ fn ambiguity_error(
(Res::Def(DefKind::Mod, _), _) => "module",
(_, TypeNS) => "type",
(_, ValueNS) => "value",
(Res::Def(DefKind::Macro(MacroKind::Derive), _), MacroNS) => {
"derive"
}
(_, MacroNS) => "macro",
};

Expand Down
35 changes: 35 additions & 0 deletions src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// force-host
// no-prefer-dynamic
// compile-flags: --crate-type proc-macro

#![crate_type="proc-macro"]
#![crate_name="intra_link_proc_macro_macro"]

extern crate proc_macro;

use proc_macro::TokenStream;

#[proc_macro_derive(DeriveA)]
pub fn a_derive(input: TokenStream) -> TokenStream {
input
}

#[proc_macro_derive(DeriveB)]
pub fn b_derive(input: TokenStream) -> TokenStream {
input
}

#[proc_macro_derive(DeriveTrait)]
pub fn trait_derive(input: TokenStream) -> TokenStream {
input
}

#[proc_macro_attribute]
pub fn attr_a(input: TokenStream, _args: TokenStream) -> TokenStream {
input
}

#[proc_macro_attribute]
pub fn attr_b(input: TokenStream, _args: TokenStream) -> TokenStream {
input
}
27 changes: 27 additions & 0 deletions src/test/rustdoc/intra-link-proc-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// aux-build:intra-link-proc-macro-macro.rs
// build-aux-docs
#![deny(intra_doc_link_resolution_failure)]

extern crate intra_link_proc_macro_macro;


pub use intra_link_proc_macro_macro::{DeriveA, attr_a};
use intra_link_proc_macro_macro::{DeriveB, attr_b};

// @has intra_link_proc_macro/struct.Foo.html
// @has - '//a/@href' '../intra_link_proc_macro/derive.DeriveA.html'
// @has - '//a/@href' '../intra_link_proc_macro/attr.attr_a.html'
// @has - '//a/@href' '../intra_link_proc_macro/trait.DeriveTrait.html'
// @has - '//a/@href' '../intra_link_proc_macro_macro/derive.DeriveB.html'
// @has - '//a/@href' '../intra_link_proc_macro_macro/attr.attr_b.html'
/// Link to [DeriveA], [attr_a], [DeriveB], [attr_b], [DeriveTrait]
Manishearth marked this conversation as resolved.
Show resolved Hide resolved
pub struct Foo;

// @has intra_link_proc_macro/struct.Bar.html
// @has - '//a/@href' '../intra_link_proc_macro/derive.DeriveA.html'
// @has - '//a/@href' '../intra_link_proc_macro/attr.attr_a.html'
/// Link to [deriveA](derive@DeriveA) [attr](macro@attr_a)
pub struct Bar;

// this should not cause ambiguity errors
pub trait DeriveTrait {}