Delay ambiguous intra-doc link resolution after Cache has been populated

This commit is contained in:
Guillaume Gomez 2024-10-14 17:51:08 +02:00
parent a1eceec00b
commit 73fc00ff8c
14 changed files with 263 additions and 56 deletions

View File

@ -1228,15 +1228,14 @@ pub(crate) fn get_doc_aliases(&self) -> Box<[Symbol]> {
for attr in self.other_attrs.lists(sym::doc).filter(|a| a.has_name(sym::alias)) {
if let Some(values) = attr.meta_item_list() {
for l in values {
match l.lit().unwrap().kind {
ast::LitKind::Str(s, _) => {
aliases.insert(s);
}
_ => unreachable!(),
if let Some(lit) = l.lit()
&& let ast::LitKind::Str(s, _) = lit.kind
{
aliases.insert(s);
}
}
} else {
aliases.insert(attr.value_str().unwrap());
} else if let Some(value) = attr.value_str() {
aliases.insert(value);
}
}
aliases.into_iter().collect::<Vec<_>>().into()

View File

@ -29,8 +29,9 @@
use crate::clean::{self, ItemId};
use crate::config::{Options as RustdocOptions, OutputFormat, RenderOptions};
use crate::formats::cache::Cache;
use crate::passes;
use crate::passes::Condition::*;
use crate::passes::{self};
use crate::passes::collect_intra_doc_links::LinkCollector;
pub(crate) struct DocContext<'tcx> {
pub(crate) tcx: TyCtxt<'tcx>,
@ -427,6 +428,9 @@ fn report_deprecated_attr(name: &str, dcx: DiagCtxtHandle<'_>, sp: Span) {
info!("Executing passes");
let mut visited = FxHashMap::default();
let mut ambiguous = FxIndexMap::default();
for p in passes::defaults(show_coverage) {
let run = match p.condition {
Always => true,
@ -436,18 +440,30 @@ fn report_deprecated_attr(name: &str, dcx: DiagCtxtHandle<'_>, sp: Span) {
};
if run {
debug!("running pass {}", p.pass.name);
krate = tcx.sess.time(p.pass.name, || (p.pass.run)(krate, &mut ctxt));
if let Some(run_fn) = p.pass.run {
krate = tcx.sess.time(p.pass.name, || run_fn(krate, &mut ctxt));
} else {
let (k, LinkCollector { visited_links, ambiguous_links, .. }) =
passes::collect_intra_doc_links::collect_intra_doc_links(krate, &mut ctxt);
krate = k;
visited = visited_links;
ambiguous = ambiguous_links;
}
}
}
tcx.sess.time("check_lint_expectations", || tcx.check_expectations(Some(sym::rustdoc)));
krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate));
let mut collector =
LinkCollector { cx: &mut ctxt, visited_links: visited, ambiguous_links: ambiguous };
collector.resolve_ambiguities();
if let Some(guar) = tcx.dcx().has_errors() {
return Err(guar);
}
krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate));
Ok((krate, ctxt.render_options, ctxt.cache))
}

View File

@ -20,7 +20,7 @@
pub(crate) const CALCULATE_DOC_COVERAGE: Pass = Pass {
name: "calculate-doc-coverage",
run: calculate_doc_coverage,
run: Some(calculate_doc_coverage),
description: "counts the number of items with and without documentation",
};

View File

@ -20,7 +20,7 @@
pub(crate) const CHECK_DOC_TEST_VISIBILITY: Pass = Pass {
name: "check_doc_test_visibility",
run: check_doc_test_visibility,
run: Some(check_doc_test_visibility),
description: "run various visibility-related lints on doctests",
};

View File

@ -9,12 +9,12 @@
use pulldown_cmark::LinkType;
use rustc_ast::util::comments::may_have_doc_links;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::intern::Interned;
use rustc_errors::{Applicability, Diag, DiagMessage};
use rustc_hir::def::Namespace::*;
use rustc_hir::def::{DefKind, Namespace, PerNS};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LOCAL_CRATE};
use rustc_hir::{Mutability, Safety};
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_middle::{bug, span_bug, ty};
@ -30,23 +30,27 @@
use tracing::{debug, info, instrument, trace};
use crate::clean::utils::find_nearest_parent_module;
use crate::clean::{self, Crate, Item, ItemLink, PrimitiveType};
use crate::clean::{self, Crate, Item, ItemId, ItemLink, PrimitiveType};
use crate::core::DocContext;
use crate::html::markdown::{MarkdownLink, MarkdownLinkRange, markdown_links};
use crate::lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS};
use crate::passes::Pass;
use crate::visit::DocVisitor;
pub(crate) const COLLECT_INTRA_DOC_LINKS: Pass = Pass {
name: "collect-intra-doc-links",
run: collect_intra_doc_links,
description: "resolves intra-doc links",
};
pub(crate) const COLLECT_INTRA_DOC_LINKS: Pass =
Pass { name: "collect-intra-doc-links", run: None, description: "resolves intra-doc links" };
fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
let mut collector = LinkCollector { cx, visited_links: FxHashMap::default() };
pub(crate) fn collect_intra_doc_links<'a, 'tcx>(
krate: Crate,
cx: &'a mut DocContext<'tcx>,
) -> (Crate, LinkCollector<'a, 'tcx>) {
let mut collector = LinkCollector {
cx,
visited_links: FxHashMap::default(),
ambiguous_links: FxIndexMap::default(),
};
collector.visit_crate(&krate);
krate
(krate, collector)
}
fn filter_assoc_items_by_name_and_namespace<'a>(
@ -61,7 +65,7 @@ fn filter_assoc_items_by_name_and_namespace<'a>(
}
#[derive(Copy, Clone, Debug, Hash, PartialEq)]
enum Res {
pub(crate) enum Res {
Def(DefKind, DefId),
Primitive(PrimitiveType),
}
@ -234,7 +238,7 @@ pub(crate) fn render(&self, s: &mut String, tcx: TyCtxt<'_>) {
}
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
struct ResolutionInfo {
pub(crate) struct ResolutionInfo {
item_id: DefId,
module_id: DefId,
dis: Option<Disambiguator>,
@ -243,18 +247,63 @@ struct ResolutionInfo {
}
#[derive(Clone)]
struct DiagnosticInfo<'a> {
pub(crate) struct DiagnosticInfo<'a> {
item: &'a Item,
dox: &'a str,
ori_link: &'a str,
link_range: MarkdownLinkRange,
}
struct LinkCollector<'a, 'tcx> {
cx: &'a mut DocContext<'tcx>,
pub(crate) struct OwnedDiagnosticInfo {
item: Item,
dox: String,
ori_link: String,
link_range: MarkdownLinkRange,
}
impl From<DiagnosticInfo<'_>> for OwnedDiagnosticInfo {
fn from(f: DiagnosticInfo<'_>) -> Self {
Self {
item: f.item.clone(),
dox: f.dox.to_string(),
ori_link: f.ori_link.to_string(),
link_range: f.link_range.clone(),
}
}
}
impl OwnedDiagnosticInfo {
pub(crate) fn into_info(&self) -> DiagnosticInfo<'_> {
DiagnosticInfo {
item: &self.item,
ori_link: &self.ori_link,
dox: &self.dox,
link_range: self.link_range.clone(),
}
}
}
pub(crate) struct LinkCollector<'a, 'tcx> {
pub(crate) cx: &'a mut DocContext<'tcx>,
/// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link.
/// The link will be `None` if it could not be resolved (i.e. the error was cached).
visited_links: FxHashMap<ResolutionInfo, Option<(Res, Option<UrlFragment>)>>,
pub(crate) visited_links: FxHashMap<ResolutionInfo, Option<(Res, Option<UrlFragment>)>>,
/// These links are ambiguous. We need for the cache to have its paths filled. Unfortunately,
/// if we run the `LinkCollector` pass after `Cache::populate`, a lot of items that we need
/// to go through will be removed, making a lot of intra-doc links to not be inferred.
///
/// So instead, we store the ambiguous links and we wait for cache paths to be filled before
/// inferring them (if possible).
///
/// Key is `(item ID, path str)`.
pub(crate) ambiguous_links: FxIndexMap<(ItemId, String), Vec<AmbiguousLinks>>,
}
pub(crate) struct AmbiguousLinks {
disambiguator: Option<Disambiguator>,
link_text: Box<str>,
diag_info: OwnedDiagnosticInfo,
resolved: Vec<(Res, Option<UrlFragment>)>,
}
impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
@ -1001,6 +1050,10 @@ fn resolve_links(&mut self, item: &Item) {
}
}
pub(crate) fn save_link(&mut self, item_id: ItemId, link: ItemLink) {
self.cx.cache.intra_doc_links.entry(item_id).or_default().insert(link);
}
/// This is the entry point for resolving an intra-doc link.
///
/// FIXME(jynelson): this is way too many arguments
@ -1024,7 +1077,7 @@ fn resolve_link(
pp_link.as_ref().map_err(|err| err.report(self.cx, diag_info.clone())).ok()?;
let disambiguator = *disambiguator;
let (mut res, fragment) = self.resolve_with_disambiguator_cached(
let mut resolved = self.resolve_with_disambiguator_cached(
ResolutionInfo {
item_id,
module_id,
@ -1040,6 +1093,142 @@ fn resolve_link(
false,
)?;
if resolved.len() > 1 {
let links = AmbiguousLinks {
disambiguator,
link_text: link_text.clone(),
diag_info: diag_info.into(),
resolved,
};
self.ambiguous_links
.entry((item.item_id, path_str.to_string()))
.or_default()
.push(links);
None
} else if let Some((res, fragment)) = resolved.pop() {
self.compute_link(res, fragment, path_str, disambiguator, diag_info, link_text)
} else {
None
}
}
/// Returns `true` if a link could be generated from the given intra-doc information.
///
/// This is a very light version of `format::href_with_root_path` since we're only interested
/// about whether we can generate a link to an item or not.
///
/// * If `original_did` is local, then we check if the item is reexported or public.
/// * If `original_did` is not local, then we check if the crate it comes from is a direct
/// public dependency.
fn validate_link(&self, original_did: DefId) -> bool {
let tcx = self.cx.tcx;
let def_kind = tcx.def_kind(original_did);
let did = match def_kind {
DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant => {
// documented on their parent's page
tcx.parent(original_did)
}
// If this a constructor, we get the parent (either a struct or a variant) and then
// generate the link for this item.
DefKind::Ctor(..) => return self.validate_link(tcx.parent(original_did)),
DefKind::ExternCrate => {
// Link to the crate itself, not the `extern crate` item.
if let Some(local_did) = original_did.as_local() {
tcx.extern_mod_stmt_cnum(local_did).unwrap_or(LOCAL_CRATE).as_def_id()
} else {
original_did
}
}
_ => original_did,
};
let cache = &self.cx.cache;
if !original_did.is_local()
&& !cache.effective_visibilities.is_directly_public(tcx, did)
&& !cache.document_private
&& !cache.primitive_locations.values().any(|&id| id == did)
{
return false;
}
cache.paths.get(&did).is_some()
|| cache.external_paths.get(&did).is_some()
|| !did.is_local()
}
#[allow(rustc::potential_query_instability)]
pub(crate) fn resolve_ambiguities(&mut self) {
let mut ambiguous_links = mem::take(&mut self.ambiguous_links);
for ((item_id, path_str), info_items) in ambiguous_links.iter_mut() {
for info in info_items {
info.resolved.retain(|(res, _)| match res {
Res::Def(_, def_id) => self.validate_link(*def_id),
// Primitive types are always valid.
Res::Primitive(_) => true,
});
let diag_info = info.diag_info.into_info();
match info.resolved.len() {
1 => {
let (res, fragment) = info.resolved.pop().unwrap();
if let Some(link) = self.compute_link(
res,
fragment,
path_str,
info.disambiguator,
diag_info,
&info.link_text,
) {
self.save_link(*item_id, link);
}
}
0 => {
report_diagnostic(
self.cx.tcx,
BROKEN_INTRA_DOC_LINKS,
format!(
"all items matching `{path_str}` are either private or doc(hidden)"
),
&diag_info,
|diag, sp, _| {
if let Some(sp) = sp {
diag.span_label(sp, "unresolved link");
} else {
diag.note("unresolved link");
}
},
);
}
_ => {
let candidates = info
.resolved
.iter()
.map(|(res, fragment)| {
let def_id = if let Some(UrlFragment::Item(def_id)) = fragment {
Some(*def_id)
} else {
None
};
(*res, def_id)
})
.collect::<Vec<_>>();
ambiguity_error(self.cx, &diag_info, path_str, &candidates, true);
}
}
}
}
}
fn compute_link(
&mut self,
mut res: Res,
fragment: Option<UrlFragment>,
path_str: &str,
disambiguator: Option<Disambiguator>,
diag_info: DiagnosticInfo<'_>,
link_text: &Box<str>,
) -> Option<ItemLink> {
// Check for a primitive which might conflict with a module
// Report the ambiguity and require that the user specify which one they meant.
// FIXME: could there ever be a primitive not in the type namespace?
@ -1055,7 +1244,7 @@ fn resolve_link(
} else {
// `[char]` when a `char` module is in scope
let candidates = &[(res, res.def_id(self.cx.tcx)), (prim, None)];
ambiguity_error(self.cx, &diag_info, path_str, candidates);
ambiguity_error(self.cx, &diag_info, path_str, candidates, true);
return None;
}
}
@ -1085,7 +1274,7 @@ fn resolve_link(
}
res.def_id(self.cx.tcx).map(|page_id| ItemLink {
link: Box::<str>::from(&*ori_link.link),
link: Box::<str>::from(&*diag_info.ori_link),
link_text: link_text.clone(),
page_id,
fragment,
@ -1107,7 +1296,7 @@ fn resolve_link(
let page_id = clean::register_res(self.cx, rustc_hir::def::Res::Def(kind, id));
Some(ItemLink {
link: Box::<str>::from(&*ori_link.link),
link: Box::<str>::from(&*diag_info.ori_link),
link_text: link_text.clone(),
page_id,
fragment,
@ -1220,10 +1409,10 @@ fn resolve_with_disambiguator_cached(
// If this call is intended to be recoverable, then pass true to silence.
// This is only recoverable when path is failed to resolved.
recoverable: bool,
) -> Option<(Res, Option<UrlFragment>)> {
) -> Option<Vec<(Res, Option<UrlFragment>)>> {
if let Some(res) = self.visited_links.get(&key) {
if res.is_some() || cache_errors {
return res.clone();
return res.clone().map(|r| vec![r]);
}
}
@ -1248,13 +1437,14 @@ fn resolve_with_disambiguator_cached(
// and after removing duplicated kinds, only one remains, the `ambiguity_error` function
// won't emit an error. So at this point, we can just take the first candidate as it was
// the first retrieved and use it to generate the link.
if let [candidate, _candidate2, ..] = *candidates
&& !ambiguity_error(self.cx, &diag, &key.path_str, &candidates)
{
candidates = vec![candidate];
if let [candidate, _candidate2, ..] = *candidates {
if !ambiguity_error(self.cx, &diag, &key.path_str, &candidates, false) {
candidates = vec![candidate];
}
}
if let &[(res, def_id)] = candidates.as_slice() {
let mut out = Vec::with_capacity(candidates.len());
for (res, def_id) in candidates {
let fragment = match (&key.extra_fragment, def_id) {
(Some(_), Some(def_id)) => {
report_anchor_conflict(self.cx, diag, def_id);
@ -1264,15 +1454,14 @@ fn resolve_with_disambiguator_cached(
(None, Some(def_id)) => Some(UrlFragment::Item(def_id)),
(None, None) => None,
};
let r = Some((res, fragment));
self.visited_links.insert(key, r.clone());
return r;
out.push((res, fragment));
}
if cache_errors {
if let [r] = out.as_slice() {
self.visited_links.insert(key, Some(r.clone()));
} else if cache_errors {
self.visited_links.insert(key, None);
}
None
Some(out)
}
/// After parsing the disambiguator, resolve the main part of the link.
@ -2046,6 +2235,7 @@ fn ambiguity_error(
diag_info: &DiagnosticInfo<'_>,
path_str: &str,
candidates: &[(Res, Option<DefId>)],
emit_error: bool,
) -> bool {
let mut descrs = FxHashSet::default();
let kinds = candidates
@ -2061,6 +2251,8 @@ fn ambiguity_error(
// There is no way for users to disambiguate at this point, so better return the first
// candidate and not show a warning.
return false;
} else if !emit_error {
return true;
}
let mut msg = format!("`{path_str}` is ");

View File

@ -16,7 +16,7 @@
pub(crate) const COLLECT_TRAIT_IMPLS: Pass = Pass {
name: "collect-trait-impls",
run: collect_trait_impls,
run: Some(collect_trait_impls),
description: "retrieves trait impls for items in the crate",
};

View File

@ -14,7 +14,7 @@
use crate::visit::DocVisitor;
pub(crate) const RUN_LINTS: Pass =
Pass { name: "run-lints", run: run_lints, description: "runs some of rustdoc's lints" };
Pass { name: "run-lints", run: Some(run_lints), description: "runs some of rustdoc's lints" };
struct Linter<'a, 'tcx> {
cx: &'a mut DocContext<'tcx>,

View File

@ -47,7 +47,7 @@
#[derive(Copy, Clone)]
pub(crate) struct Pass {
pub(crate) name: &'static str,
pub(crate) run: fn(clean::Crate, &mut DocContext<'_>) -> clean::Crate,
pub(crate) run: Option<fn(clean::Crate, &mut DocContext<'_>) -> clean::Crate>,
pub(crate) description: &'static str,
}

View File

@ -13,7 +13,7 @@
pub(crate) const PROPAGATE_DOC_CFG: Pass = Pass {
name: "propagate-doc-cfg",
run: propagate_doc_cfg,
run: Some(propagate_doc_cfg),
description: "propagates `#[doc(cfg(...))]` to child items",
};

View File

@ -16,7 +16,7 @@
pub(crate) const PROPAGATE_STABILITY: Pass = Pass {
name: "propagate-stability",
run: propagate_stability,
run: Some(propagate_stability),
description: "propagates stability to child items",
};

View File

@ -8,7 +8,7 @@
pub(crate) const STRIP_ALIASED_NON_LOCAL: Pass = Pass {
name: "strip-aliased-non-local",
run: strip_aliased_non_local,
run: Some(strip_aliased_non_local),
description: "strips all non-local private aliased items from the output",
};

View File

@ -16,7 +16,7 @@
pub(crate) const STRIP_HIDDEN: Pass = Pass {
name: "strip-hidden",
run: strip_hidden,
run: Some(strip_hidden),
description: "strips all `#[doc(hidden)]` items from the output",
};

View File

@ -8,7 +8,7 @@
pub(crate) const STRIP_PRIV_IMPORTS: Pass = Pass {
name: "strip-priv-imports",
run: strip_priv_imports,
run: Some(strip_priv_imports),
description: "strips all private import statements (`use`, `extern crate`) from a crate",
};

View File

@ -8,7 +8,7 @@
pub(crate) const STRIP_PRIVATE: Pass = Pass {
name: "strip-private",
run: strip_private,
run: Some(strip_private),
description: "strips all private items from a crate which cannot be seen externally, \
implies strip-priv-imports",
};