Skip to content

Fix weird rustdoc output when single and glob reexport conflict on a name #143590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
50 changes: 29 additions & 21 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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,
)
}
Expand All @@ -182,22 +182,29 @@ fn generate_item_with_correct_attrs(
kind: ItemKind,
def_id: DefId,
name: Symbol,
import_id: Option<LocalDefId>,
import_ids: &[LocalDefId],
renamed: Option<Symbol>,
) -> 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 {
Expand All @@ -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
}

Expand Down Expand Up @@ -2754,7 +2762,7 @@ fn clean_maybe_renamed_item<'tcx>(
cx: &mut DocContext<'tcx>,
item: &hir::Item<'tcx>,
renamed: Option<Symbol>,
import_id: Option<LocalDefId>,
import_ids: &[LocalDefId],
) -> Vec<Item> {
use hir::ItemKind;
fn get_name(
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
)]
})
Expand Down Expand Up @@ -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,
)
})
Expand Down
64 changes: 47 additions & 17 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,29 @@ pub(crate) struct Module<'hir> {
pub(crate) def_id: LocalDefId,
pub(crate) renamed: Option<Symbol>,
pub(crate) import_id: Option<LocalDefId>,
/// 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<import_id>).
/// 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<Symbol>),
(&'hir hir::Item<'hir>, Option<Symbol>, Option<LocalDefId>),
(&'hir hir::Item<'hir>, Option<Symbol>, Vec<LocalDefId>),
>,

/// (def_id, renamed) -> (res, local_import_id)
Expand Down Expand Up @@ -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()));
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -261,14 +280,15 @@ 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
// reachable in documentation -- a previously unreachable item can be
// 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
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -378,8 +398,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
fn add_to_current_mod(
&mut self,
item: &'tcx hir::Item<'_>,
renamed: Option<Symbol>,
parent_id: Option<LocalDefId>,
mut renamed: Option<Symbol>,
import_id: Option<LocalDefId>,
) {
if self.is_importable_from_parent
// If we're inside an item, only impl blocks and `macro_rules!` with the `macro_export`
Expand All @@ -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()));
}
}
}

Expand Down Expand Up @@ -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!(),
};
Expand Down
41 changes: 41 additions & 0 deletions tests/rustdoc/reexport/merge-glob-and-non-glob.rs
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/143107>.

#![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;
Loading