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

GuillaumeGomez · GuillaumeGomez · commit 4a2709dfad77 · 2025-07-07T17:27:32.000+02:00
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
@@ -152,8 +152,12 @@ pub(crate) fn try_inline(
     };
 
     cx.inlined.insert(did.into());
+    let import_def_ids: &[LocalDefId] = match import_def_id {
+        Some(import_def_id) => &[import_def_id],
+        None => &[],
+    };
     let mut item =
-        crate::clean::generate_item_with_correct_attrs(cx, kind, did, name, import_def_id, None);
+        crate::clean::generate_item_with_correct_attrs(cx, kind, did, name, import_def_ids, 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
@@ -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())
@@ -157,12 +157,16 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
     });
 
     let kind = ModuleItem(Module { items, span });
+    let import_def_ids: &[LocalDefId] = match doc.import_id {
+        Some(import_def_id) => &[import_def_id],
+        None => &[],
+    };
     generate_item_with_correct_attrs(
         cx,
         kind,
         doc.def_id.to_def_id(),
         doc.name,
-        doc.import_id,
+        import_def_ids,
         doc.renamed,
     )
 }
@@ -182,22 +186,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 {
@@ -216,7 +227,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
 }
 
@@ -2759,7 +2771,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;
 
@@ -2831,7 +2843,7 @@ fn clean_maybe_renamed_item<'tcx>(
                     })),
                     item.owner_id.def_id.to_def_id(),
                     name,
-                    import_id,
+                    import_ids,
                     renamed,
                 ));
                 return ret;
@@ -2886,7 +2898,7 @@ fn clean_maybe_renamed_item<'tcx>(
             kind,
             item.owner_id.def_id.to_def_id(),
             name,
-            import_id,
+            import_ids,
             renamed,
         )]
     })
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
@@ -32,11 +32,28 @@ 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 both
+    /// imports to be merged.
     pub(crate) items: FxIndexMap<
         (LocalDefId, Option<Symbol>),
-        (&'hir hir::Item<'hir>, Option<Symbol>, Option<LocalDefId>),
+        (&'hir hir::Item<'hir>, Option<Symbol>, Vec<LocalDefId>),
     >,
     /// Same as for `items`.
     pub(crate) inlined_foreigns: FxIndexMap<(DefId, Option<Symbol>), (Res, LocalDefId)>,
@@ -145,7 +162,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()));
             }
         }
 
@@ -227,7 +246,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;
@@ -252,14 +270,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
@@ -307,10 +326,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);
@@ -319,13 +338,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);
                 self.inlining = prev;
@@ -369,8 +388,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`
@@ -383,11 +402,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()));
+            }
         }
     }
 
@@ -459,7 +488,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!(),
                         };
