diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 96199cb972a86..37b012d6d0d0c 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -152,8 +152,14 @@ pub(crate) fn try_inline( }; cx.inlined.insert(did.into()); - let mut item = - crate::clean::generate_item_with_correct_attrs(cx, kind, did, name, import_def_id, None); + let mut item = crate::clean::generate_item_with_correct_attrs( + cx, + kind, + did, + name, + import_def_id.as_slice(), + None, + ); // The visibility needs to reflect the one from the reexport and not from the "source" DefId. item.inner.inline_stmt_id = import_def_id; ret.push(item); diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index c2f3da18cd302..e6f7aef02c0ec 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -94,12 +94,12 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< // This covers the case where somebody does an import which should pull in an item, // but there's already an item with the same namespace and same name. Rust gives // priority to the not-imported one, so we should, too. - items.extend(doc.items.values().flat_map(|(item, renamed, import_id)| { + items.extend(doc.items.values().flat_map(|(item, renamed, import_ids)| { // First, lower everything other than glob imports. if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { return Vec::new(); } - let v = clean_maybe_renamed_item(cx, item, *renamed, *import_id); + let v = clean_maybe_renamed_item(cx, item, *renamed, import_ids); for item in &v { if let Some(name) = item.name && (cx.render_options.document_hidden || !item.is_doc_hidden()) @@ -162,7 +162,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< kind, doc.def_id.to_def_id(), doc.name, - doc.import_id, + doc.import_id.as_slice(), doc.renamed, ) } @@ -182,22 +182,29 @@ fn generate_item_with_correct_attrs( kind: ItemKind, def_id: DefId, name: Symbol, - import_id: Option, + import_ids: &[LocalDefId], renamed: Option, ) -> Item { let target_attrs = inline::load_attrs(cx, def_id); - let attrs = if let Some(import_id) = import_id { - // glob reexports are treated the same as `#[doc(inline)]` items. - // - // For glob re-exports the item may or may not exist to be re-exported (potentially the cfgs - // on the path up until the glob can be removed, and only cfgs on the globbed item itself - // matter), for non-inlined re-exports see #85043. - let is_inline = hir_attr_lists(inline::load_attrs(cx, import_id.to_def_id()), sym::doc) - .get_word_attr(sym::inline) - .is_some() - || (is_glob_import(cx.tcx, import_id) - && (cx.render_options.document_hidden || !cx.tcx.is_doc_hidden(def_id))); - let mut attrs = get_all_import_attributes(cx, import_id, def_id, is_inline); + let attrs = if !import_ids.is_empty() { + let mut attrs = Vec::with_capacity(import_ids.len()); + let mut is_inline = false; + + for import_id in import_ids.iter().copied() { + // glob reexports are treated the same as `#[doc(inline)]` items. + // + // For glob re-exports the item may or may not exist to be re-exported (potentially the + // cfgs on the path up until the glob can be removed, and only cfgs on the globbed item + // itself matter), for non-inlined re-exports see #85043. + let import_is_inline = + hir_attr_lists(inline::load_attrs(cx, import_id.to_def_id()), sym::doc) + .get_word_attr(sym::inline) + .is_some() + || (is_glob_import(cx.tcx, import_id) + && (cx.render_options.document_hidden || !cx.tcx.is_doc_hidden(def_id))); + attrs.extend(get_all_import_attributes(cx, import_id, def_id, is_inline)); + is_inline = is_inline || import_is_inline; + } add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None); attrs } else { @@ -216,7 +223,8 @@ fn generate_item_with_correct_attrs( let name = renamed.or(Some(name)); let mut item = Item::from_def_id_and_attrs_and_parts(def_id, name, kind, attrs, cfg); - item.inner.inline_stmt_id = import_id; + // FIXME (GuillaumeGomez): Should we also make `inline_stmt_id` a `Vec` instead of an `Option`? + item.inner.inline_stmt_id = import_ids.first().copied(); item } @@ -2754,7 +2762,7 @@ fn clean_maybe_renamed_item<'tcx>( cx: &mut DocContext<'tcx>, item: &hir::Item<'tcx>, renamed: Option, - import_id: Option, + import_ids: &[LocalDefId], ) -> Vec { use hir::ItemKind; fn get_name( @@ -2825,7 +2833,7 @@ fn clean_maybe_renamed_item<'tcx>( })), item.owner_id.def_id.to_def_id(), name, - import_id, + import_ids, renamed, )); return ret; @@ -2880,7 +2888,7 @@ fn clean_maybe_renamed_item<'tcx>( kind, item.owner_id.def_id.to_def_id(), name, - import_id, + import_ids, renamed, )] }) @@ -3151,7 +3159,7 @@ fn clean_maybe_renamed_foreign_item<'tcx>( kind, item.owner_id.def_id.to_def_id(), item.ident.name, - import_id, + import_id.as_slice(), renamed, ) }) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 2889bfae7b000..2784d7c761a84 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -32,11 +32,29 @@ pub(crate) struct Module<'hir> { pub(crate) def_id: LocalDefId, pub(crate) renamed: Option, pub(crate) import_id: Option, - /// The key is the item `ItemId` and the value is: (item, renamed, import_id). + /// The key is the item `ItemId` and the value is: (item, renamed, Vec). /// We use `FxIndexMap` to keep the insert order. + /// + /// `import_id` needs to be a `Vec` because we live in a dark world where you can have code + /// like: + /// + /// ``` + /// mod raw { + /// pub fn foo() {} + /// } + /// + /// /// Foobar + /// pub use raw::foo; + /// + /// pub use raw::*; + /// ``` + /// + /// So in this case, we don't want to have two items but just one with attributes from all + /// non-glob imports to be merged. Glob imports attributes are always ignored, whether they're + /// shadowed or not. pub(crate) items: FxIndexMap< (LocalDefId, Option), - (&'hir hir::Item<'hir>, Option, Option), + (&'hir hir::Item<'hir>, Option, Vec), >, /// (def_id, renamed) -> (res, local_import_id) @@ -154,7 +172,9 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { { let item = self.cx.tcx.hir_expect_item(local_def_id); let (ident, _, _) = item.expect_macro(); - top_level_module.items.insert((local_def_id, Some(ident.name)), (item, None, None)); + top_level_module + .items + .insert((local_def_id, Some(ident.name)), (item, None, Vec::new())); } } @@ -236,7 +256,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { ) -> bool { debug!("maybe_inline_local (renamed: {renamed:?}) res: {res:?}"); - let glob = renamed.is_none(); if renamed == Some(kw::Underscore) { // We never inline `_` reexports. return false; @@ -261,6 +280,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { return false; } + let is_glob = renamed.is_none(); let is_hidden = !document_hidden && tcx.is_doc_hidden(ori_res_did); let Some(res_did) = ori_res_did.as_local() else { // For cross-crate impl inlining we need to know whether items are @@ -268,7 +288,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // made reachable by cross-crate inlining which we're checking here. // (this is done here because we need to know this upfront). crate::visit_lib::lib_embargo_visit_item(self.cx, ori_res_did); - if is_hidden || glob { + if is_hidden || is_glob { return false; } // We store inlined foreign items otherwise, it'd mean that the `use` item would be kept @@ -316,10 +336,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // Bang macros are handled a bit on their because of how they are handled by the // compiler. If they have `#[doc(hidden)]` and the re-export doesn't have // `#[doc(inline)]`, then we don't inline it. - Node::Item(_) if is_bang_macro && !please_inline && renamed.is_some() && is_hidden => { + Node::Item(_) if is_bang_macro && !please_inline && !is_glob && is_hidden => { return false; } - Node::Item(&hir::Item { kind: hir::ItemKind::Mod(_, m), .. }) if glob => { + Node::Item(&hir::Item { kind: hir::ItemKind::Mod(_, m), .. }) if is_glob => { let prev = mem::replace(&mut self.inlining, true); for &i in m.item_ids { let i = tcx.hir_item(i); @@ -328,13 +348,13 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { self.inlining = prev; true } - Node::Item(it) if !glob => { + Node::Item(it) if !is_glob => { let prev = mem::replace(&mut self.inlining, true); self.visit_item_inner(it, renamed, Some(def_id)); self.inlining = prev; true } - Node::ForeignItem(it) if !glob => { + Node::ForeignItem(it) if !is_glob => { let prev = mem::replace(&mut self.inlining, true); self.visit_foreign_item_inner(it, renamed, Some(def_id)); self.inlining = prev; @@ -378,8 +398,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { fn add_to_current_mod( &mut self, item: &'tcx hir::Item<'_>, - renamed: Option, - parent_id: Option, + mut renamed: Option, + import_id: Option, ) { if self.is_importable_from_parent // If we're inside an item, only impl blocks and `macro_rules!` with the `macro_export` @@ -392,11 +412,21 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { _ => false, } { - self.modules - .last_mut() - .unwrap() - .items - .insert((item.owner_id.def_id, renamed), (item, renamed, parent_id)); + if renamed == item.kind.ident().map(|ident| ident.name) { + renamed = None; + } + let key = (item.owner_id.def_id, renamed); + if let Some(import_id) = import_id { + self.modules + .last_mut() + .unwrap() + .items + .entry(key) + .and_modify(|v| v.2.push(import_id)) + .or_insert_with(|| (item, renamed, vec![import_id])); + } else { + self.modules.last_mut().unwrap().items.insert(key, (item, renamed, Vec::new())); + } } } @@ -468,7 +498,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { _ => false, }); let ident = match kind { - hir::UseKind::Single(ident) => Some(renamed.unwrap_or(ident.name)), + hir::UseKind::Single(ident) => Some(ident.name), hir::UseKind::Glob => None, hir::UseKind::ListStem => unreachable!(), }; diff --git a/tests/rustdoc/reexport/merge-glob-and-non-glob.rs b/tests/rustdoc/reexport/merge-glob-and-non-glob.rs new file mode 100644 index 0000000000000..ff67859cb39af --- /dev/null +++ b/tests/rustdoc/reexport/merge-glob-and-non-glob.rs @@ -0,0 +1,41 @@ +// This test ensures that if an item is inlined from two different `use`, +// then it will use attributes from both of them. +// This is a regression test for . + +#![feature(no_core)] +#![no_core] +#![no_std] +#![crate_name = "foo"] + +// First we ensure we only have two items. +//@ has 'foo/index.html' +//@ count - '//dl[@class="item-table"]/dt' 2 +// We should also only have one section (Structs). +//@ count - '//h2[@class="section-header"]' 1 +// We now check the short docs. +//@ has - '//dl[@class="item-table"]/dd' 'Foobar Blob' +//@ has - '//dl[@class="item-table"]/dd' 'Tarte Tatin' + +//@ has 'foo/struct.Foo.html' +//@ has - '//*[@class="docblock"]' 'Foobar Blob' + +//@ has 'foo/struct.Another.html' +//@ has - '//*[@class="docblock"]' 'Tarte Tatin' + +mod raw { + /// Blob + pub struct Foo; + + /// Tatin + pub struct Another; +} + +/// Foobar +pub use raw::Foo; + +// Glob reexport attributes are ignored. +/// Baz +pub use raw::*; + +/// Tarte +pub use raw::Another as Another;