Skip to content

Commit

Permalink
Auto merge of #96676 - petrochenkov:docrules, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
rustdoc: Remove doc link resolution fallback to all `macro_rules` in the crate

This is a deny-by-default lint detecting such fallback for crater run, as discussed in #96521.
  • Loading branch information
bors committed May 16, 2022
2 parents 56d540e + b4019de commit c52b9c1
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 47 deletions.
52 changes: 22 additions & 30 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Res> {
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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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])
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1737,8 +1715,9 @@ fn resolution_failure(
path_str: &str,
disambiguator: Option<Disambiguator>,
kinds: SmallVec<[ResolutionFailure<'_>; 3]>,
) {
) -> Option<(Res, Option<DefId>)> {
let tcx = collector.cx.tcx;
let mut recovered_res = None;
report_diagnostic(
tcx,
BROKEN_INTRA_DOC_LINKS,
Expand Down Expand Up @@ -1826,11 +1805,22 @@ fn resolution_failure(
diag.note(&note);
}

// 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;
Expand Down Expand Up @@ -1915,6 +1905,8 @@ fn resolution_failure(
}
},
);

recovered_res
}

fn report_multiple_anchors(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) {
Expand Down
11 changes: 5 additions & 6 deletions src/test/rustdoc-ui/intra-doc/macro-rules-error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
23 changes: 12 additions & 11 deletions src/test/rustdoc-ui/intra-doc/macro-rules-error.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit c52b9c1

Please sign in to comment.