diff options
| author | bors <bors@rust-lang.org> | 2022-05-01 20:28:10 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-05-01 20:28:10 +0000 |
| commit | 4dd8b420c027001e47b0d811a7e55e2fe1de1395 (patch) | |
| tree | b72e34a99c82b4b63f8a7f3d991ca19f00f60ac5 /src | |
| parent | a933de83989471ac444a13d62996d30621542654 (diff) | |
| parent | 6083db7c4ef72ee5435e8157ba5b3f5397da1080 (diff) | |
| download | rust-4dd8b420c027001e47b0d811a7e55e2fe1de1395.tar.gz rust-4dd8b420c027001e47b0d811a7e55e2fe1de1395.zip | |
Auto merge of #96521 - petrochenkov:docrules, r=notriddle,GuillaumeGomez
rustdoc: Resolve doc links referring to `macro_rules` items cc https://github.com/rust-lang/rust/issues/81633 UPD: the fallback to considering *all* `macro_rules` in the crate for unresolved names is not removed in this PR, it will be removed separately and will be run through crater.
Diffstat (limited to 'src')
| -rw-r--r-- | src/librustdoc/passes/collect_intra_doc_links.rs | 5 | ||||
| -rw-r--r-- | src/librustdoc/passes/collect_intra_doc_links/early.rs | 105 | ||||
| -rw-r--r-- | src/test/rustdoc-ui/intra-doc/macro-rules-error.rs | 27 | ||||
| -rw-r--r-- | src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr | 22 | ||||
| -rw-r--r-- | src/test/rustdoc-ui/intra-doc/macro-rules.rs | 15 |
5 files changed, 147 insertions, 27 deletions
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 42e87f3f961..84512fab269 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, CRATE_DEF_ID}; use rustc_hir::Mutability; use rustc_middle::ty::{DefIdTree, Ty, TyCtxt}; use rustc_middle::{bug, span_bug, ty}; +use rustc_resolve::ParentScope; use rustc_session::lint::Lint; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{sym, Ident, Symbol}; @@ -564,7 +565,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .copied() .unwrap_or_else(|| { self.cx.enter_resolver(|resolver| { - resolver.resolve_rustdoc_path(path_str, ns, module_id) + let parent_scope = + ParentScope::module(resolver.expect_module(module_id), resolver); + resolver.resolve_rustdoc_path(path_str, ns, parent_scope) }) }) .and_then(|res| res.try_into().ok()) diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs index 68e10e3a18c..3858c1cb056 100644 --- a/src/librustdoc/passes/collect_intra_doc_links/early.rs +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -1,7 +1,7 @@ use crate::clean::Attributes; use crate::core::ResolverCaches; use crate::passes::collect_intra_doc_links::preprocessed_markdown_links; -use crate::passes::collect_intra_doc_links::PreprocessedMarkdownLink; +use crate::passes::collect_intra_doc_links::{Disambiguator, PreprocessedMarkdownLink}; use rustc_ast::visit::{self, AssocCtxt, Visitor}; use rustc_ast::{self as ast, ItemKind}; @@ -9,12 +9,13 @@ use rustc_ast_lowering::ResolverAstLowering; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::Namespace::*; use rustc_hir::def::{DefKind, Namespace, Res}; -use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LocalDefId, CRATE_DEF_ID}; +use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, CRATE_DEF_ID}; use rustc_hir::TraitCandidate; use rustc_middle::ty::{DefIdTree, Visibility}; use rustc_resolve::{ParentScope, Resolver}; use rustc_session::config::Externs; use rustc_session::Session; +use rustc_span::symbol::sym; use rustc_span::{Symbol, SyntaxContext}; use std::collections::hash_map::Entry; @@ -27,10 +28,12 @@ crate fn early_resolve_intra_doc_links( externs: Externs, document_private_items: bool, ) -> ResolverCaches { + let parent_scope = + ParentScope::module(resolver.expect_module(CRATE_DEF_ID.to_def_id()), resolver); let mut link_resolver = EarlyDocLinkResolver { resolver, sess, - current_mod: CRATE_DEF_ID, + parent_scope, visited_mods: Default::default(), markdown_links: Default::default(), doc_link_resolutions: Default::default(), @@ -52,7 +55,7 @@ crate fn early_resolve_intra_doc_links( // DO NOT REMOVE THIS without first testing on the reproducer in // https://github.com/jyn514/objr/commit/edcee7b8124abf0e4c63873e8422ff81beb11ebb for (extern_name, _) in externs.iter().filter(|(_, entry)| entry.add_prelude) { - link_resolver.resolver.resolve_rustdoc_path(extern_name, TypeNS, CRATE_DEF_ID.to_def_id()); + link_resolver.resolver.resolve_rustdoc_path(extern_name, TypeNS, parent_scope); } ResolverCaches { @@ -72,7 +75,7 @@ fn doc_attrs<'a>(attrs: impl Iterator<Item = &'a ast::Attribute>) -> Attributes struct EarlyDocLinkResolver<'r, 'ra> { resolver: &'r mut Resolver<'ra>, sess: &'r Session, - current_mod: LocalDefId, + parent_scope: ParentScope<'ra>, visited_mods: DefIdSet, markdown_links: FxHashMap<String, Vec<PreprocessedMarkdownLink>>, doc_link_resolutions: FxHashMap<(Symbol, Namespace, DefId), Option<Res<ast::NodeId>>>, @@ -82,7 +85,7 @@ struct EarlyDocLinkResolver<'r, 'ra> { document_private_items: bool, } -impl EarlyDocLinkResolver<'_, '_> { +impl<'ra> EarlyDocLinkResolver<'_, 'ra> { fn add_traits_in_scope(&mut self, def_id: DefId) { // Calls to `traits_in_scope` are expensive, so try to avoid them if only possible. // Keys in the `traits_in_scope` cache are always module IDs. @@ -205,34 +208,64 @@ impl EarlyDocLinkResolver<'_, '_> { if !attrs.iter().any(|attr| attr.may_have_doc_links()) { return; } - let module_id = self.current_mod.to_def_id(); - self.resolve_doc_links(doc_attrs(attrs.iter()), module_id); + self.resolve_doc_links(doc_attrs(attrs.iter()), self.parent_scope); } - fn resolve_doc_links(&mut self, attrs: Attributes, module_id: DefId) { + fn resolve_and_cache( + &mut self, + path_str: &str, + ns: Namespace, + parent_scope: &ParentScope<'ra>, + ) -> bool { + // FIXME: This caching may be incorrect in case of multiple `macro_rules` + // items with the same name in the same module. + self.doc_link_resolutions + .entry((Symbol::intern(path_str), ns, parent_scope.module.def_id())) + .or_insert_with_key(|(path, ns, _)| { + self.resolver.resolve_rustdoc_path(path.as_str(), *ns, *parent_scope) + }) + .is_some() + } + + fn resolve_doc_links(&mut self, attrs: Attributes, parent_scope: ParentScope<'ra>) { let mut need_traits_in_scope = false; for (doc_module, doc) in attrs.prepare_to_doc_link_resolution() { assert_eq!(doc_module, None); - let links = self - .markdown_links - .entry(doc) - .or_insert_with_key(|doc| preprocessed_markdown_links(doc)); + let mut tmp_links = mem::take(&mut self.markdown_links); + let links = + tmp_links.entry(doc).or_insert_with_key(|doc| preprocessed_markdown_links(doc)); for PreprocessedMarkdownLink(pp_link, _) in links { if let Ok(pinfo) = pp_link { - // FIXME: Resolve the path in all namespaces and resolve its prefixes too. - let ns = TypeNS; - self.doc_link_resolutions - .entry((Symbol::intern(&pinfo.path_str), ns, module_id)) - .or_insert_with_key(|(path, ns, module_id)| { - self.resolver.resolve_rustdoc_path(path.as_str(), *ns, *module_id) - }); - need_traits_in_scope = true; + // The logic here is a conservative approximation for path resolution in + // `resolve_with_disambiguator`. + if let Some(ns) = pinfo.disambiguator.map(Disambiguator::ns) { + if self.resolve_and_cache(&pinfo.path_str, ns, &parent_scope) { + continue; + } + } + + // Resolve all namespaces due to no disambiguator or for diagnostics. + let mut any_resolved = false; + let mut need_assoc = false; + for ns in [TypeNS, ValueNS, MacroNS] { + if self.resolve_and_cache(&pinfo.path_str, ns, &parent_scope) { + any_resolved = true; + } else if ns != MacroNS { + need_assoc = true; + } + } + + // FIXME: Resolve all prefixes for type-relative resolution or for diagnostics. + if (need_assoc || !any_resolved) && pinfo.path_str.contains("::") { + need_traits_in_scope = true; + } } } + self.markdown_links = tmp_links; } if need_traits_in_scope { - self.add_traits_in_scope(module_id); + self.add_traits_in_scope(parent_scope.module.def_id()); } } @@ -274,19 +307,33 @@ impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> { fn visit_item(&mut self, item: &ast::Item) { self.resolve_doc_links_local(&item.attrs); // Outer attribute scope if let ItemKind::Mod(..) = item.kind { - let old_mod = mem::replace(&mut self.current_mod, self.resolver.local_def_id(item.id)); + let module_def_id = self.resolver.local_def_id(item.id).to_def_id(); + let module = self.resolver.expect_module(module_def_id); + let old_module = mem::replace(&mut self.parent_scope.module, module); + let old_macro_rules = self.parent_scope.macro_rules; self.resolve_doc_links_local(&item.attrs); // Inner attribute scope - self.process_module_children_or_reexports(self.current_mod.to_def_id()); + self.process_module_children_or_reexports(module_def_id); visit::walk_item(self, item); - self.current_mod = old_mod; + if item + .attrs + .iter() + .all(|attr| !attr.has_name(sym::macro_use) && !attr.has_name(sym::macro_escape)) + { + self.parent_scope.macro_rules = old_macro_rules; + } + self.parent_scope.module = old_module; } else { - match item.kind { + match &item.kind { ItemKind::Trait(..) => { self.all_traits.push(self.resolver.local_def_id(item.id).to_def_id()); } ItemKind::Impl(box ast::Impl { of_trait: Some(..), .. }) => { self.all_trait_impls.push(self.resolver.local_def_id(item.id).to_def_id()); } + ItemKind::MacroDef(macro_def) if macro_def.macro_rules => { + self.parent_scope.macro_rules = + self.resolver.macro_rules_scope(self.resolver.local_def_id(item.id)); + } _ => {} } visit::walk_item(self, item); @@ -313,6 +360,12 @@ impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> { visit::walk_field_def(self, field) } + fn visit_block(&mut self, block: &ast::Block) { + let old_macro_rules = self.parent_scope.macro_rules; + visit::walk_block(self, block); + self.parent_scope.macro_rules = old_macro_rules; + } + // NOTE: if doc-comments are ever allowed on other nodes (e.g. function parameters), // then this will have to implement other visitor methods too. } diff --git a/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs b/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs new file mode 100644 index 00000000000..84d63c20aa8 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs @@ -0,0 +1,27 @@ +// `macro_rules` scopes are respected during doc link resolution. + +// compile-flags: --document-private-items + +#![deny(rustdoc::broken_intra_doc_links)] + +mod no_escape { + macro_rules! before_but_limited_to_module { + () => {}; + } +} + +/// [before_but_limited_to_module] FIXME: This error should be reported +// ERROR unresolved link to `before_but_limited_to_module` +/// [after] FIXME: This error should be reported +// ERROR unresolved link to `after` +/// [str] FIXME: This error shouldn not be reported +//~^ ERROR `str` is both a builtin type and a macro +fn check() {} + +macro_rules! after { + () => {}; +} + +macro_rules! str { + () => {}; +} diff --git a/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr b/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr new file mode 100644 index 00000000000..4b984f4f6c0 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr @@ -0,0 +1,22 @@ +error: `str` is both a builtin type and a macro + --> $DIR/macro-rules-error.rs:17:6 + | +LL | /// [str] FIXME: This error shouldn not be reported + | ^^^ ambiguous link + | +note: the lint level is defined here + --> $DIR/macro-rules-error.rs:5:9 + | +LL | #![deny(rustdoc::broken_intra_doc_links)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to link to the builtin type, prefix with `prim@` + | +LL | /// [prim@str] FIXME: This error shouldn not be reported + | +++++ +help: to link to the macro, add an exclamation mark + | +LL | /// [str!] FIXME: This error shouldn not be reported + | + + +error: aborting due to previous error + diff --git a/src/test/rustdoc-ui/intra-doc/macro-rules.rs b/src/test/rustdoc-ui/intra-doc/macro-rules.rs index a14e4bdf1d7..3aeb370ef6d 100644 --- a/src/test/rustdoc-ui/intra-doc/macro-rules.rs +++ b/src/test/rustdoc-ui/intra-doc/macro-rules.rs @@ -7,3 +7,18 @@ macro_rules! foo { /// [foo!] pub fn baz() {} + +#[macro_use] +mod macros { + macro_rules! escaping { + () => {}; + } +} + +pub mod inner { + /// [foo!] + /// [escaping] + pub fn baz() { + foo!(); + } +} |
