From b4019ded77c45f9c69f1d91f5d2d3257ac58e9a4 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 15 May 2022 20:05:52 +0300 Subject: [PATCH] rustdoc: Remove doc link resolution fallback to all `macro_rules` in the crate --- .../passes/collect_intra_doc_links.rs | 52 ++++++++----------- .../rustdoc-ui/intra-doc/macro-rules-error.rs | 11 ++-- .../intra-doc/macro-rules-error.stderr | 23 ++++---- 3 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index c25a0d3b14997..95ba4ce5b06b9 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -443,21 +443,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } - /// HACK: Try to search the macro name in the list of all `macro_rules` items in the crate. - /// Used when nothing else works, may often give an incorrect result. - fn resolve_macro_rules(&self, path_str: &str, ns: Namespace) -> Option { - if ns != MacroNS { - return None; - } - - self.cx - .resolver_caches - .all_macro_rules - .get(&Symbol::intern(path_str)) - .copied() - .and_then(|res| res.try_into().ok()) - } - /// Convenience wrapper around `resolve_rustdoc_path`. /// /// This also handles resolving `true` and `false` as booleans. @@ -489,8 +474,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) }) .and_then(|res| res.try_into().ok()) - .or_else(|| resolve_primitive(path_str, ns)) - .or_else(|| self.resolve_macro_rules(path_str, ns)); + .or_else(|| resolve_primitive(path_str, ns)); debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns); result } @@ -1391,11 +1375,7 @@ impl LinkCollector<'_, '_> { } } } - resolution_failure(self, diag, path_str, disambiguator, smallvec![err]); - // This could just be a normal link or a broken link - // we could potentially check if something is - // "intra-doc-link-like" and warn in that case. - None + resolution_failure(self, diag, path_str, disambiguator, smallvec![err]) } } } @@ -1423,15 +1403,13 @@ impl LinkCollector<'_, '_> { let len = candidates.iter().filter(|res| res.is_ok()).count(); if len == 0 { - resolution_failure( + return resolution_failure( self, diag, path_str, disambiguator, candidates.into_iter().filter_map(|res| res.err()).collect(), ); - // this could just be a normal link - return None; } if len == 1 { @@ -1737,8 +1715,9 @@ fn resolution_failure( path_str: &str, disambiguator: Option, kinds: SmallVec<[ResolutionFailure<'_>; 3]>, -) { +) -> Option<(Res, Option)> { let tcx = collector.cx.tcx; + let mut recovered_res = None; report_diagnostic( tcx, BROKEN_INTRA_DOC_LINKS, @@ -1826,11 +1805,22 @@ fn resolution_failure( diag.note(¬e); } - // If the link has `::` in it, assume it was meant to be an intra-doc link. - // Otherwise, the `[]` might be unrelated. - // FIXME: don't show this for autolinks (`<>`), `()` style links, or reference links if !path_str.contains("::") { - diag.help(r#"to escape `[` and `]` characters, add '\' before them like `\[` or `\]`"#); + if disambiguator.map_or(true, |d| d.ns() == MacroNS) + && let Some(&res) = collector.cx.resolver_caches.all_macro_rules + .get(&Symbol::intern(path_str)) + { + diag.note(format!( + "`macro_rules` named `{path_str}` exists in this crate, \ + but it is not in scope at this link's location" + )); + recovered_res = res.try_into().ok().map(|res| (res, None)); + } else { + // If the link has `::` in it, assume it was meant to be an + // intra-doc link. Otherwise, the `[]` might be unrelated. + diag.help("to escape `[` and `]` characters, \ + add '\\' before them like `\\[` or `\\]`"); + } } continue; @@ -1915,6 +1905,8 @@ fn resolution_failure( } }, ); + + recovered_res } fn report_multiple_anchors(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) { diff --git a/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs b/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs index 84d63c20aa8b5..8490584c1b42b 100644 --- a/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs +++ b/src/test/rustdoc-ui/intra-doc/macro-rules-error.rs @@ -10,12 +10,11 @@ mod no_escape { } } -/// [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 +/// [before_but_limited_to_module] +//~^ ERROR unresolved link to `before_but_limited_to_module` +/// [after] +//~^ ERROR unresolved link to `after` +/// [str] fn check() {} macro_rules! after { diff --git a/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr b/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr index 4b984f4f6c016..8e17323fddebb 100644 --- a/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr +++ b/src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr @@ -1,22 +1,23 @@ -error: `str` is both a builtin type and a macro - --> $DIR/macro-rules-error.rs:17:6 +error: unresolved link to `before_but_limited_to_module` + --> $DIR/macro-rules-error.rs:13:6 | -LL | /// [str] FIXME: This error shouldn not be reported - | ^^^ ambiguous link +LL | /// [before_but_limited_to_module] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `before_but_limited_to_module` in scope | 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@` + = note: `macro_rules` named `before_but_limited_to_module` exists in this crate, but it is not in scope at this link's location + +error: unresolved link to `after` + --> $DIR/macro-rules-error.rs:15:6 | -LL | /// [prim@str] FIXME: This error shouldn not be reported - | +++++ -help: to link to the macro, add an exclamation mark +LL | /// [after] + | ^^^^^ no item named `after` in scope | -LL | /// [str!] FIXME: This error shouldn not be reported - | + + = note: `macro_rules` named `after` exists in this crate, but it is not in scope at this link's location -error: aborting due to previous error +error: aborting due to 2 previous errors