Skip to content

Commit fcbb219

Browse files
committed
rustdoc: parse item and reexport markdown separate
This change is a pre-requisite for LaTeX support, because the markdown parser doesn't support enabling and disabling extensions in the middle of a document, and there's no way to add that. This means string concatenating a document that has LaTeX Math disabled with a document that has it enabled can't be done. As part of this change, a bug related to intra-doc links is fixed. This shows up when the reexport and the item both have intra-doc links with the same visible path, but where they resolve to different items. The bug is demonstrated in `tests/rustdoc-html/reexport/link-with-same-name-but-different-destination.rs`. The other test case changes demonstrate that this is, technically, a breaking change. When I ran a Crater test for docs that rely on this behavior, though, it seemed most authors weren't relying on it.
1 parent feaadee commit fcbb219

71 files changed

Lines changed: 522 additions & 427 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

compiler/rustc_resolve/src/rustdoc.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,7 @@ pub fn attrs_to_doc_fragments<'a, A: AttributeExt + Clone + 'a>(
236236
///
237237
/// The last newline is not trimmed so the produced strings are reusable between
238238
/// early and late doc link resolution regardless of their position.
239-
pub fn prepare_to_doc_link_resolution(
240-
doc_fragments: &[DocFragment],
241-
) -> FxIndexMap<Option<DefId>, String> {
239+
pub fn prepare_fragments(doc_fragments: &[DocFragment]) -> FxIndexMap<Option<DefId>, String> {
242240
let mut res = FxIndexMap::default();
243241
for fragment in doc_fragments {
244242
let out_str = res.entry(fragment.item_id).or_default();
@@ -412,7 +410,7 @@ pub fn may_be_doc_link(link_type: LinkType) -> bool {
412410
pub(crate) fn attrs_to_preprocessed_links(attrs: &[ast::Attribute]) -> Vec<Box<str>> {
413411
let (doc_fragments, other_attrs) =
414412
attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), false);
415-
let doc = prepare_to_doc_link_resolution(&doc_fragments).into_values().next();
413+
let doc = prepare_fragments(&doc_fragments).into_values().next();
416414
let mut links = doc.as_deref().map(parse_links).unwrap_or_default();
417415

418416
for attr in other_attrs {

src/librustdoc/calculate_doc_coverage.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,9 @@ impl DocVisitor<'_> for CoverageCalculator<'_, '_> {
238238
let has_docs = !i.attrs.doc_strings.is_empty();
239239
let mut tests = Tests { found_tests: 0 };
240240

241-
find_testable_code(&i.doc_value(), &mut tests, ErrorCodes::No, None);
241+
for text in i.doc_values().values() {
242+
find_testable_code(&text, &mut tests, ErrorCodes::No, None);
243+
}
242244

243245
let has_doc_example = tests.found_tests != 0;
244246
let hir_id = DocContext::as_local_hir_id(self.ctx.tcx, i.item_id).unwrap();

src/librustdoc/clean/types.rs

Lines changed: 62 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use rustc_metadata::rendered_const;
2121
use rustc_middle::ty::fast_reject::SimplifiedType;
2222
use rustc_middle::ty::{self, Ty, TyCtxt, Visibility};
2323
use rustc_resolve::rustdoc::{
24-
DocFragment, add_doc_fragment, attrs_to_doc_fragments, inner_docs, span_of_fragments,
24+
DocFragment, attrs_to_doc_fragments, inner_docs, prepare_fragments, span_of_fragments,
2525
};
2626
use rustc_session::Session;
2727
use rustc_span::def_id::{CRATE_DEF_ID, ModId};
@@ -122,6 +122,42 @@ impl ItemId {
122122
| ItemId::DefId(id) => id.krate,
123123
}
124124
}
125+
126+
pub(crate) fn links(&self, cx: &Context<'_>) -> Vec<RenderedLink> {
127+
use crate::html::format::{href_with_path_check, link_tooltip};
128+
129+
let Some(links) = cx.cache().intra_doc_links.get(&self) else {
130+
return vec![];
131+
};
132+
links
133+
.iter()
134+
.filter_map(|ItemLink { link: s, link_text, page_id: id, fragment }| {
135+
debug!(?id);
136+
if let Ok(HrefInfo { mut url, .. }) = href_with_path_check(*id, cx, link_text) {
137+
debug!(?url);
138+
match fragment {
139+
Some(UrlFragment::Item(def_id)) => {
140+
write!(url, "{}", crate::html::format::fragment(*def_id, cx.tcx()))
141+
.unwrap();
142+
}
143+
Some(UrlFragment::UserWritten(raw)) => {
144+
url.push('#');
145+
url.push_str(raw);
146+
}
147+
None => {}
148+
}
149+
Some(RenderedLink {
150+
original_text: s.clone(),
151+
new_text: link_text.clone(),
152+
tooltip: link_tooltip(*id, fragment, cx, Some(link_text)).to_string(),
153+
href: url,
154+
})
155+
} else {
156+
None
157+
}
158+
})
159+
.collect()
160+
}
125161
}
126162

127163
impl From<DefId> for ItemId {
@@ -363,7 +399,7 @@ impl fmt::Debug for Item {
363399
fmt.field("attrs", &self.attrs).field("kind", &self.kind).field("cfg", &self.cfg);
364400
} else {
365401
fmt.field("kind", &self.type_());
366-
fmt.field("docs", &self.doc_value());
402+
fmt.field("docs", &self.doc_values().values().cloned().collect::<Vec<String>>());
367403
}
368404
fmt.finish()
369405
}
@@ -507,16 +543,20 @@ impl Item {
507543
.unwrap_or_else(|| self.span(tcx).map_or(DUMMY_SP, |span| span.inner()))
508544
}
509545

510-
/// Combine all doc strings into a single value handling indentation and newlines as needed.
511-
pub(crate) fn doc_value(&self) -> String {
512-
self.attrs.doc_value()
546+
/// Combine each reexport's docstrings into one docstring per item, handling indentation and
547+
/// newlines as needed.
548+
pub(crate) fn doc_values(&self) -> FxIndexMap<Option<DefId>, String> {
549+
self.attrs.doc_values()
513550
}
514551

515-
/// Combine all doc strings into a single value handling indentation and newlines as needed.
552+
/// Combine each reexport's docstrings into one docstring per item.
553+
///
516554
/// Returns `None` is there's no documentation at all, and `Some("")` if there is some
517555
/// documentation but it is empty (e.g. `#[doc = ""]`).
518-
pub(crate) fn opt_doc_value(&self) -> Option<String> {
519-
self.attrs.opt_doc_value()
556+
///
557+
/// The trailing newline is trimmed.
558+
pub(crate) fn opt_doc_values(&self) -> Option<FxIndexMap<Option<DefId>, String>> {
559+
self.attrs.opt_doc_values()
520560
}
521561

522562
pub(crate) fn from_def_id_and_parts(
@@ -559,58 +599,6 @@ impl Item {
559599
}
560600
}
561601

562-
/// If the item has doc comments from a reexport, returns the item id of that reexport,
563-
/// otherwise returns returns the item id.
564-
///
565-
/// This is used as a key for caching intra-doc link resolution,
566-
/// to prevent two reexports of the same item from using the same cache.
567-
pub(crate) fn item_or_reexport_id(&self) -> ItemId {
568-
// added documentation on a reexport is always prepended.
569-
self.attrs
570-
.doc_strings
571-
.first()
572-
.map(|x| x.item_id)
573-
.flatten()
574-
.map(ItemId::from)
575-
.unwrap_or(self.item_id)
576-
}
577-
578-
pub(crate) fn links(&self, cx: &Context<'_>) -> Vec<RenderedLink> {
579-
use crate::html::format::{href_with_path_check, link_tooltip};
580-
581-
let Some(links) = cx.cache().intra_doc_links.get(&self.item_or_reexport_id()) else {
582-
return vec![];
583-
};
584-
links
585-
.iter()
586-
.filter_map(|ItemLink { link: s, link_text, page_id: id, fragment }| {
587-
debug!(?id);
588-
if let Ok(HrefInfo { mut url, .. }) = href_with_path_check(*id, cx, link_text) {
589-
debug!(?url);
590-
match fragment {
591-
Some(UrlFragment::Item(def_id)) => {
592-
write!(url, "{}", crate::html::format::fragment(*def_id, cx.tcx()))
593-
.unwrap();
594-
}
595-
Some(UrlFragment::UserWritten(raw)) => {
596-
url.push('#');
597-
url.push_str(raw);
598-
}
599-
None => {}
600-
}
601-
Some(RenderedLink {
602-
original_text: s.clone(),
603-
new_text: link_text.clone(),
604-
tooltip: link_tooltip(*id, fragment, cx, Some(link_text)).to_string(),
605-
href: url,
606-
})
607-
} else {
608-
None
609-
}
610-
})
611-
.collect()
612-
}
613-
614602
/// Find a list of all link names, without finding their href.
615603
///
616604
/// This is used for generating summary text, which does not include
@@ -1095,21 +1083,26 @@ impl Attributes {
10951083
Attributes { doc_strings, other_attrs }
10961084
}
10971085

1098-
/// Combine all doc strings into a single value handling indentation and newlines as needed.
1099-
pub(crate) fn doc_value(&self) -> String {
1100-
self.opt_doc_value().unwrap_or_default()
1086+
/// Combine each reexport's docstrings into one docstring per item, handling indentation and
1087+
/// newlines as needed.
1088+
pub(crate) fn doc_values(&self) -> FxIndexMap<Option<DefId>, String> {
1089+
self.opt_doc_values().unwrap_or_default()
11011090
}
11021091

1103-
/// Combine all doc strings into a single value handling indentation and newlines as needed.
1092+
/// Combine each reexport's docstrings into one docstring per item.
1093+
///
11041094
/// Returns `None` is there's no documentation at all, and `Some("")` if there is some
11051095
/// documentation but it is empty (e.g. `#[doc = ""]`).
1106-
pub(crate) fn opt_doc_value(&self) -> Option<String> {
1096+
///
1097+
/// The trailing newline is trimmed.
1098+
pub(crate) fn opt_doc_values(&self) -> Option<FxIndexMap<Option<DefId>, String>> {
11071099
(!self.doc_strings.is_empty()).then(|| {
1108-
let mut res = String::new();
1109-
for frag in &self.doc_strings {
1110-
add_doc_fragment(&mut res, frag);
1100+
let mut res = prepare_fragments(&self.doc_strings);
1101+
for string in res.values_mut() {
1102+
if string.as_bytes().last() == Some(&b'\n') {
1103+
string.pop();
1104+
}
11111105
}
1112-
res.pop();
11131106
res
11141107
})
11151108
}

src/librustdoc/clean/types/tests.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ fn run_test(input: &str, expected: &str) {
2121
let mut s = create_doc_fragment(input);
2222
unindent_doc_fragments(&mut s);
2323
let attrs = Attributes { doc_strings: s, other_attrs: Default::default() };
24-
assert_eq!(attrs.doc_value(), expected);
24+
let doc_values = attrs.doc_values();
25+
assert_eq!(doc_values.len(), 1);
26+
assert_eq!(doc_values.values().next().expect("must have exactly one doc"), expected);
2527
});
2628
}
2729

src/librustdoc/core.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ pub(crate) fn run_global_ctxt(
409409

410410
let mut krate = tcx.sess.time("clean_crate", || clean::krate(&mut ctxt));
411411

412-
if krate.module.doc_value().is_empty() {
412+
if krate.module.doc_values().is_empty() {
413413
let help = format!(
414414
"The following guide may be of use:\n\
415415
{}/rustdoc/how-to-write-documentation.html",

src/librustdoc/doctest/rust.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ impl HirCollector<'_> {
211211
// The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
212212
// anything else, this will combine them for us.
213213
let attrs = Attributes::from_hir(hir_attrs);
214-
if let Some(doc) = attrs.opt_doc_value() {
214+
for doc in attrs.doc_values().values() {
215215
let span = span_of_fragments(&attrs.doc_strings).unwrap_or(sp);
216216
self.collector.position = if span.edition().at_least_rust_2024() {
217217
span

src/librustdoc/html/render/context.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,10 @@ impl<'tcx> Context<'tcx> {
254254
};
255255
title.push_str(" - Rust");
256256
let tyname = it.type_();
257-
let desc = plain_text_summary(&it.doc_value(), &it.link_names(self.cache()));
257+
let desc = plain_text_summary(
258+
&it.doc_values().values().next().map_or_default(|s| &s[..]),
259+
&it.link_names(self.cache()),
260+
);
258261
let desc = if !desc.is_empty() {
259262
desc
260263
} else if it.is_crate() {

0 commit comments

Comments
 (0)