From 4eda3f77bd1ec94a97229a3ff0610eee136aa0d5 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 9 Jun 2020 12:59:15 -0700 Subject: [PATCH 1/5] intra-doc macro resolution should also handle proc macros --- .../passes/collect_intra_doc_links.rs | 63 ++++++++++++------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 149480ec80f29..4514a616bd856 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -122,6 +122,42 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } } + /// Resolves a string as a macro. + fn macro_resolve(&self, path_str: &str, parent_id: Option) -> Option { + 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( @@ -615,7 +651,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { None => { // Try everything! let candidates = PerNS { - macro_ns: macro_resolve(cx, path_str) + macro_ns: self + .macro_resolve(path_str, base_node) .map(|res| (res, extra_fragment.clone())), type_ns: match self.resolve( path_str, @@ -684,7 +721,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); @@ -727,28 +764,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } -/// Resolves a string as a macro. -fn macro_resolve(cx: &DocContext<'_>, path_str: &str) -> Option { - 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, From e003c3ea059e20a0e04cc953d0d128e0ea69acd8 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 9 Jun 2020 14:13:23 -0700 Subject: [PATCH 2/5] Add test for proc macro resolution in intra doc links --- .../auxiliary/intra-link-proc-macro-macro.rs | 30 +++++++++++++++++++ src/test/rustdoc/intra-link-proc-macro.rs | 17 +++++++++++ 2 files changed, 47 insertions(+) create mode 100644 src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs create mode 100644 src/test/rustdoc/intra-link-proc-macro.rs diff --git a/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs b/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs new file mode 100644 index 0000000000000..fee92ff2e0dd1 --- /dev/null +++ b/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs @@ -0,0 +1,30 @@ +// 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_attribute] +pub fn attr_a(input: TokenStream, _args: TokenStream) -> TokenStream { + input +} + +#[proc_macro_attribute] +pub fn attr_b(input: TokenStream, _args: TokenStream) -> TokenStream { + input +} diff --git a/src/test/rustdoc/intra-link-proc-macro.rs b/src/test/rustdoc/intra-link-proc-macro.rs new file mode 100644 index 0000000000000..78ef91ab68291 --- /dev/null +++ b/src/test/rustdoc/intra-link-proc-macro.rs @@ -0,0 +1,17 @@ +// aux-build:intra-link-proc-macro-macro.rs +// build-aux-docs +// @has intra_link_proc_macro/index.html +#![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 - '//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_macro/derive.DeriveB.html' +// @has - '//a/@href' '../intra_link_proc_macro_macro/attr.attr_b.html' +/// Link to [DeriveA], [attr_a], [DeriveB], [attr_b] +pub struct Foo; From 27dcb4aab534741f747354c6a72ff29d0ccce329 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 10 Jun 2020 00:33:10 -0700 Subject: [PATCH 3/5] Avoid collisions between traits and their derive macros --- .../passes/collect_intra_doc_links.rs | 29 +++++++++++++++++-- .../auxiliary/intra-link-proc-macro-macro.rs | 5 ++++ src/test/rustdoc/intra-link-proc-macro.rs | 6 +++- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 4514a616bd856..ab4dbb87425db 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -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; @@ -407,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(ns: &PerNS>) -> 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 { let item_hir_id = if item.is_mod() { @@ -650,7 +667,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } None => { // Try everything! - let candidates = PerNS { + let mut candidates = PerNS { macro_ns: self .macro_resolve(path_str, base_node) .map(|res| (res, extra_fragment.clone())), @@ -705,10 +722,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, diff --git a/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs b/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs index fee92ff2e0dd1..04a431d99026e 100644 --- a/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs +++ b/src/test/rustdoc/auxiliary/intra-link-proc-macro-macro.rs @@ -19,6 +19,11 @@ 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 diff --git a/src/test/rustdoc/intra-link-proc-macro.rs b/src/test/rustdoc/intra-link-proc-macro.rs index 78ef91ab68291..405e08d4c07c3 100644 --- a/src/test/rustdoc/intra-link-proc-macro.rs +++ b/src/test/rustdoc/intra-link-proc-macro.rs @@ -11,7 +11,11 @@ use intra_link_proc_macro_macro::{DeriveB, attr_b}; // @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] +/// Link to [DeriveA], [attr_a], [DeriveB], [attr_b], [DeriveTrait] pub struct Foo; + +// this should not cause ambiguity errors +pub trait DeriveTrait {} From 178465516ef7571cc25a1d243902d3ea8fa220e0 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 10 Jun 2020 00:38:13 -0700 Subject: [PATCH 4/5] Don't print bang diagnostics for derives --- src/librustdoc/passes/collect_intra_doc_links.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index ab4dbb87425db..d14cd1306abbc 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -585,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@") { + kind = Some(MacroNS); + link.trim_start_matches("derive@") } else if link.ends_with('!') { kind = Some(MacroNS); link.trim_end_matches('!') @@ -954,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)) } _ => { @@ -968,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", }; From 34c6b38e68f2e88bcc6c943494a7c05d35a71f17 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 10 Jun 2020 09:11:17 -0700 Subject: [PATCH 5/5] Add tests for macro@ and derive@ --- src/test/rustdoc/intra-link-proc-macro.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/rustdoc/intra-link-proc-macro.rs b/src/test/rustdoc/intra-link-proc-macro.rs index 405e08d4c07c3..7b6ea5d60f853 100644 --- a/src/test/rustdoc/intra-link-proc-macro.rs +++ b/src/test/rustdoc/intra-link-proc-macro.rs @@ -1,6 +1,5 @@ // aux-build:intra-link-proc-macro-macro.rs // build-aux-docs -// @has intra_link_proc_macro/index.html #![deny(intra_doc_link_resolution_failure)] extern crate intra_link_proc_macro_macro; @@ -9,6 +8,7 @@ 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' @@ -17,5 +17,11 @@ use intra_link_proc_macro_macro::{DeriveB, attr_b}; /// Link to [DeriveA], [attr_a], [DeriveB], [attr_b], [DeriveTrait] 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 {}