Skip to content

Commit 9ba1d21

Browse files
committed
Auto merge of #77519 - jyn514:track-doc-er, r=GuillaumeGomez
Resolve intra-doc links on additional documentation for re-exports in lexical scope Fixes #77254. - Preserve the parent module of `DocFragment`s + Add `parent_module` to `DocFragment` + Require the `parent_module` of the item being inlined + Preserve the hir_id for ExternCrates so rustdoc can find the parent module later + Take an optional `parent_module` for `build_impl` and `merge_attrs`. Preserve the difference between parent modules for each doc-comment. + Support a single additional re-exports in from_ast. Originally this took a vec but I ended up not using it. + Don't require the parent_module for all `impl`s, just inlined items In particular, this will be `None` whenever the attribute is not on a re-export. + Only store the parent_module, not the HirId When re-exporting a re-export, the HirId is not available. Fortunately, `collect_intra_doc_links` doesn't actually need all the info from a HirId, just the parent module. - Introduce `Divider` This distinguishes between documentation on the original from docs on the re-export. - Use the new module information for intra-doc links + Make the parent module conditional on whether the docs are on a re-export + Make `resolve_link` take `&Item` instead of `&mut Item` Previously the borrow checker gave an error about multiple mutable borrows, because `dox` borrowed from `item`. + Fix `crate::` for re-exports `crate` means something different depending on where the attribute came from. + Make it work for `#[doc]` attributes too This required combining several attributes as one so they would keep the links. r? `@GuillaumeGomez`
2 parents 03ef8a0 + e39a860 commit 9ba1d21

10 files changed

+235
-112
lines changed

src/librustdoc/clean/inline.rs

+40-21
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_span::hygiene::MacroKind;
1515
use rustc_span::symbol::{sym, Symbol};
1616
use rustc_span::Span;
1717

18-
use crate::clean::{self, GetDefId, ToSource, TypeKind};
18+
use crate::clean::{self, Attributes, GetDefId, ToSource, TypeKind};
1919
use crate::core::DocContext;
2020
use crate::doctree;
2121

@@ -35,8 +35,11 @@ type Attrs<'hir> = rustc_middle::ty::Attributes<'hir>;
3535
///
3636
/// The returned value is `None` if the definition could not be inlined,
3737
/// and `Some` of a vector of items if it was successfully expanded.
38+
///
39+
/// `parent_module` refers to the parent of the *re-export*, not the original item.
3840
pub fn try_inline(
3941
cx: &DocContext<'_>,
42+
parent_module: DefId,
4043
res: Res,
4144
name: Symbol,
4245
attrs: Option<Attrs<'_>>,
@@ -48,12 +51,13 @@ pub fn try_inline(
4851
}
4952
let mut ret = Vec::new();
5053

54+
debug!("attrs={:?}", attrs);
5155
let attrs_clone = attrs;
5256

5357
let inner = match res {
5458
Res::Def(DefKind::Trait, did) => {
5559
record_extern_fqn(cx, did, clean::TypeKind::Trait);
56-
ret.extend(build_impls(cx, did, attrs));
60+
ret.extend(build_impls(cx, Some(parent_module), did, attrs));
5761
clean::TraitItem(build_external_trait(cx, did))
5862
}
5963
Res::Def(DefKind::Fn, did) => {
@@ -62,27 +66,27 @@ pub fn try_inline(
6266
}
6367
Res::Def(DefKind::Struct, did) => {
6468
record_extern_fqn(cx, did, clean::TypeKind::Struct);
65-
ret.extend(build_impls(cx, did, attrs));
69+
ret.extend(build_impls(cx, Some(parent_module), did, attrs));
6670
clean::StructItem(build_struct(cx, did))
6771
}
6872
Res::Def(DefKind::Union, did) => {
6973
record_extern_fqn(cx, did, clean::TypeKind::Union);
70-
ret.extend(build_impls(cx, did, attrs));
74+
ret.extend(build_impls(cx, Some(parent_module), did, attrs));
7175
clean::UnionItem(build_union(cx, did))
7276
}
7377
Res::Def(DefKind::TyAlias, did) => {
7478
record_extern_fqn(cx, did, clean::TypeKind::Typedef);
75-
ret.extend(build_impls(cx, did, attrs));
79+
ret.extend(build_impls(cx, Some(parent_module), did, attrs));
7680
clean::TypedefItem(build_type_alias(cx, did), false)
7781
}
7882
Res::Def(DefKind::Enum, did) => {
7983
record_extern_fqn(cx, did, clean::TypeKind::Enum);
80-
ret.extend(build_impls(cx, did, attrs));
84+
ret.extend(build_impls(cx, Some(parent_module), did, attrs));
8185
clean::EnumItem(build_enum(cx, did))
8286
}
8387
Res::Def(DefKind::ForeignTy, did) => {
8488
record_extern_fqn(cx, did, clean::TypeKind::Foreign);
85-
ret.extend(build_impls(cx, did, attrs));
89+
ret.extend(build_impls(cx, Some(parent_module), did, attrs));
8690
clean::ForeignTypeItem
8791
}
8892
// Never inline enum variants but leave them shown as re-exports.
@@ -117,7 +121,7 @@ pub fn try_inline(
117121
};
118122

119123
let target_attrs = load_attrs(cx, did);
120-
let attrs = merge_attrs(cx, target_attrs, attrs_clone);
124+
let attrs = merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone);
121125

122126
cx.renderinfo.borrow_mut().inlined.insert(did);
123127
ret.push(clean::Item {
@@ -291,40 +295,52 @@ pub fn build_ty(cx: &DocContext<'_>, did: DefId) -> Option<clean::Type> {
291295
}
292296

293297
/// Builds all inherent implementations of an ADT (struct/union/enum) or Trait item/path/reexport.
294-
pub fn build_impls(cx: &DocContext<'_>, did: DefId, attrs: Option<Attrs<'_>>) -> Vec<clean::Item> {
298+
pub fn build_impls(
299+
cx: &DocContext<'_>,
300+
parent_module: Option<DefId>,
301+
did: DefId,
302+
attrs: Option<Attrs<'_>>,
303+
) -> Vec<clean::Item> {
295304
let tcx = cx.tcx;
296305
let mut impls = Vec::new();
297306

298307
// for each implementation of an item represented by `did`, build the clean::Item for that impl
299308
for &did in tcx.inherent_impls(did).iter() {
300-
build_impl(cx, did, attrs, &mut impls);
309+
build_impl(cx, parent_module, did, attrs, &mut impls);
301310
}
302311

303312
impls
304313
}
305314

315+
/// `parent_module` refers to the parent of the re-export, not the original item
306316
fn merge_attrs(
307317
cx: &DocContext<'_>,
308-
attrs: Attrs<'_>,
309-
other_attrs: Option<Attrs<'_>>,
318+
parent_module: Option<DefId>,
319+
old_attrs: Attrs<'_>,
320+
new_attrs: Option<Attrs<'_>>,
310321
) -> clean::Attributes {
311322
// NOTE: If we have additional attributes (from a re-export),
312323
// always insert them first. This ensure that re-export
313324
// doc comments show up before the original doc comments
314325
// when we render them.
315-
let merged_attrs = if let Some(inner) = other_attrs {
316-
let mut both = inner.to_vec();
317-
both.extend_from_slice(attrs);
318-
both
326+
if let Some(inner) = new_attrs {
327+
if let Some(new_id) = parent_module {
328+
let diag = cx.sess().diagnostic();
329+
Attributes::from_ast(diag, old_attrs, Some((inner, new_id)))
330+
} else {
331+
let mut both = inner.to_vec();
332+
both.extend_from_slice(old_attrs);
333+
both.clean(cx)
334+
}
319335
} else {
320-
attrs.to_vec()
321-
};
322-
merged_attrs.clean(cx)
336+
old_attrs.clean(cx)
337+
}
323338
}
324339

325340
/// Builds a specific implementation of a type. The `did` could be a type method or trait method.
326341
pub fn build_impl(
327342
cx: &DocContext<'_>,
343+
parent_module: impl Into<Option<DefId>>,
328344
did: DefId,
329345
attrs: Option<Attrs<'_>>,
330346
ret: &mut Vec<clean::Item>,
@@ -333,7 +349,8 @@ pub fn build_impl(
333349
return;
334350
}
335351

336-
let attrs = merge_attrs(cx, load_attrs(cx, did), attrs);
352+
let attrs = merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs);
353+
debug!("merged_attrs={:?}", attrs);
337354

338355
let tcx = cx.tcx;
339356
let associated_trait = tcx.impl_trait_ref(did);
@@ -499,7 +516,9 @@ fn build_module(cx: &DocContext<'_>, did: DefId, visited: &mut FxHashSet<DefId>)
499516
},
500517
)),
501518
});
502-
} else if let Some(i) = try_inline(cx, item.res, item.ident.name, None, visited) {
519+
} else if let Some(i) =
520+
try_inline(cx, did, item.res, item.ident.name, None, visited)
521+
{
503522
items.extend(i)
504523
}
505524
}

src/librustdoc/clean/mod.rs

+17-7
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ impl Clean<Item> for doctree::Module<'_> {
284284

285285
impl Clean<Attributes> for [ast::Attribute] {
286286
fn clean(&self, cx: &DocContext<'_>) -> Attributes {
287-
Attributes::from_ast(cx.sess().diagnostic(), self)
287+
Attributes::from_ast(cx.sess().diagnostic(), self, None)
288288
}
289289
}
290290

@@ -2205,9 +2205,14 @@ impl Clean<Vec<Item>> for doctree::ExternCrate<'_> {
22052205

22062206
let res = Res::Def(DefKind::Mod, DefId { krate: self.cnum, index: CRATE_DEF_INDEX });
22072207

2208-
if let Some(items) =
2209-
inline::try_inline(cx, res, self.name, Some(self.attrs), &mut visited)
2210-
{
2208+
if let Some(items) = inline::try_inline(
2209+
cx,
2210+
cx.tcx.parent_module(self.hir_id).to_def_id(),
2211+
res,
2212+
self.name,
2213+
Some(self.attrs),
2214+
&mut visited,
2215+
) {
22112216
return items;
22122217
}
22132218
}
@@ -2268,9 +2273,14 @@ impl Clean<Vec<Item>> for doctree::Import<'_> {
22682273
}
22692274
if !denied {
22702275
let mut visited = FxHashSet::default();
2271-
if let Some(items) =
2272-
inline::try_inline(cx, path.res, name, Some(self.attrs), &mut visited)
2273-
{
2276+
if let Some(items) = inline::try_inline(
2277+
cx,
2278+
cx.tcx.parent_module(self.id).to_def_id(),
2279+
path.res,
2280+
name,
2281+
Some(self.attrs),
2282+
&mut visited,
2283+
) {
22742284
return items;
22752285
}
22762286
}

src/librustdoc/clean/types.rs

+85-42
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,11 @@ impl<I: IntoIterator<Item = ast::NestedMetaItem>> NestedAttributesExt for I {
373373
pub struct DocFragment {
374374
pub line: usize,
375375
pub span: rustc_span::Span,
376+
/// The module this doc-comment came from.
377+
///
378+
/// This allows distinguishing between the original documentation and a pub re-export.
379+
/// If it is `None`, the item was not re-exported.
380+
pub parent_module: Option<DefId>,
376381
pub doc: String,
377382
pub kind: DocFragmentKind,
378383
}
@@ -386,6 +391,24 @@ pub enum DocFragmentKind {
386391
/// A doc fragment created from a `#[doc(include="filename")]` attribute. Contains both the
387392
/// given filename and the file contents.
388393
Include { filename: String },
394+
/// A doc fragment used to distinguish between documentation in different modules.
395+
///
396+
/// In particular, this prevents `collapse_docs` from turning all documentation comments
397+
/// into a single giant attributes even when the item is re-exported with documentation on the re-export.
398+
Divider,
399+
}
400+
401+
impl DocFragment {
402+
/// Creates a dummy doc-fragment which divides earlier and later fragments.
403+
fn divider() -> Self {
404+
DocFragment {
405+
line: 0,
406+
span: DUMMY_SP,
407+
parent_module: None,
408+
doc: String::new(),
409+
kind: DocFragmentKind::Divider,
410+
}
411+
}
389412
}
390413

391414
impl<'a> FromIterator<&'a DocFragment> for String {
@@ -521,57 +544,77 @@ impl Attributes {
521544
false
522545
}
523546

524-
pub fn from_ast(diagnostic: &::rustc_errors::Handler, attrs: &[ast::Attribute]) -> Attributes {
525-
let mut doc_strings = vec![];
547+
pub fn from_ast(
548+
diagnostic: &::rustc_errors::Handler,
549+
attrs: &[ast::Attribute],
550+
additional_attrs: Option<(&[ast::Attribute], DefId)>,
551+
) -> Attributes {
552+
let doc_strings = RefCell::new(vec![]);
526553
let mut sp = None;
527554
let mut cfg = Cfg::True;
528555
let mut doc_line = 0;
529556

530-
let other_attrs = attrs
531-
.iter()
532-
.filter_map(|attr| {
533-
if let Some(value) = attr.doc_str() {
534-
let value = beautify_doc_string(value);
535-
let kind = if attr.is_doc_comment() {
536-
DocFragmentKind::SugaredDoc
537-
} else {
538-
DocFragmentKind::RawDoc
539-
};
540-
541-
let line = doc_line;
542-
doc_line += value.lines().count();
543-
doc_strings.push(DocFragment { line, span: attr.span, doc: value, kind });
544-
545-
if sp.is_none() {
546-
sp = Some(attr.span);
547-
}
548-
None
557+
let clean_attr = |(attr, parent_module): (&ast::Attribute, _)| {
558+
if let Some(value) = attr.doc_str() {
559+
trace!("got doc_str={:?}", value);
560+
let value = beautify_doc_string(value);
561+
let kind = if attr.is_doc_comment() {
562+
DocFragmentKind::SugaredDoc
549563
} else {
550-
if attr.has_name(sym::doc) {
551-
if let Some(mi) = attr.meta() {
552-
if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
553-
// Extracted #[doc(cfg(...))]
554-
match Cfg::parse(cfg_mi) {
555-
Ok(new_cfg) => cfg &= new_cfg,
556-
Err(e) => diagnostic.span_err(e.span, e.msg),
557-
}
558-
} else if let Some((filename, contents)) =
559-
Attributes::extract_include(&mi)
560-
{
561-
let line = doc_line;
562-
doc_line += contents.lines().count();
563-
doc_strings.push(DocFragment {
564-
line,
565-
span: attr.span,
566-
doc: contents,
567-
kind: DocFragmentKind::Include { filename },
568-
});
564+
DocFragmentKind::RawDoc
565+
};
566+
567+
let line = doc_line;
568+
doc_line += value.lines().count();
569+
doc_strings.borrow_mut().push(DocFragment {
570+
line,
571+
span: attr.span,
572+
doc: value,
573+
kind,
574+
parent_module,
575+
});
576+
577+
if sp.is_none() {
578+
sp = Some(attr.span);
579+
}
580+
None
581+
} else {
582+
if attr.has_name(sym::doc) {
583+
if let Some(mi) = attr.meta() {
584+
if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
585+
// Extracted #[doc(cfg(...))]
586+
match Cfg::parse(cfg_mi) {
587+
Ok(new_cfg) => cfg &= new_cfg,
588+
Err(e) => diagnostic.span_err(e.span, e.msg),
569589
}
590+
} else if let Some((filename, contents)) = Attributes::extract_include(&mi)
591+
{
592+
let line = doc_line;
593+
doc_line += contents.lines().count();
594+
doc_strings.borrow_mut().push(DocFragment {
595+
line,
596+
span: attr.span,
597+
doc: contents,
598+
kind: DocFragmentKind::Include { filename },
599+
parent_module: parent_module,
600+
});
570601
}
571602
}
572-
Some(attr.clone())
573603
}
604+
Some(attr.clone())
605+
}
606+
};
607+
608+
// Additional documentation should be shown before the original documentation
609+
let other_attrs = additional_attrs
610+
.into_iter()
611+
.map(|(attrs, id)| {
612+
doc_strings.borrow_mut().push(DocFragment::divider());
613+
attrs.iter().map(move |attr| (attr, Some(id)))
574614
})
615+
.flatten()
616+
.chain(attrs.iter().map(|attr| (attr, None)))
617+
.filter_map(clean_attr)
575618
.collect();
576619

577620
// treat #[target_feature(enable = "feat")] attributes as if they were
@@ -597,7 +640,7 @@ impl Attributes {
597640
.map_or(true, |a| a.style == AttrStyle::Inner);
598641

599642
Attributes {
600-
doc_strings,
643+
doc_strings: doc_strings.into_inner(),
601644
other_attrs,
602645
cfg: if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) },
603646
span: sp,

src/librustdoc/clean/utils.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V
361361
let primitive = match *target {
362362
ResolvedPath { did, .. } if did.is_local() => continue,
363363
ResolvedPath { did, .. } => {
364-
ret.extend(inline::build_impls(cx, did, None));
364+
ret.extend(inline::build_impls(cx, None, did, None));
365365
continue;
366366
}
367367
_ => match target.primitive_type() {
@@ -371,7 +371,7 @@ pub fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut V
371371
};
372372
for &did in primitive.impls(tcx) {
373373
if !did.is_local() {
374-
inline::build_impl(cx, did, None, ret);
374+
inline::build_impl(cx, None, did, None, ret);
375375
}
376376
}
377377
}

src/librustdoc/doctest.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
927927
sp: Span,
928928
nested: F,
929929
) {
930-
let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs);
930+
let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs, None);
931931
if let Some(ref cfg) = attrs.cfg {
932932
if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) {
933933
return;

0 commit comments

Comments
 (0)