rustdoc: introduce glob shadowing rules to rustdoc

src/librustdoc/clean/mod.rs

@@ -1934,13 +1934,14 @@ impl Clean<BareFunctionDecl> for hir::BareFnTy<'_> {
     }
 }
 
-impl Clean<Vec<Item>> for (&hir::Item<'_>, Option<Symbol>) {
+impl Clean<Vec<Item>> for doctree::Item<'_> {
     fn clean(&self, cx: &mut DocContext<'_>) -> Vec<Item> {
         use hir::ItemKind;
 
-        let (item, renamed) = self;
+        let item = self.hir_item;
+        let mut name = self.name();
         let def_id = item.def_id.to_def_id();
-        let mut name = renamed.unwrap_or_else(|| cx.tcx.hir().name(item.hir_id()));
+
         cx.with_param_env(def_id, |cx| {
             let kind = match item.kind {
                 ItemKind::Static(ty, mutability, body_id) => {

src/librustdoc/doctree.rs

@@ -1,20 +1,50 @@
 //! This module is used to store stuff from Rust's AST in a more convenient
 //! manner (and with prettier names) before cleaning.
+use crate::core::DocContext;
+use rustc_hir as hir;
 use rustc_span::{self, Span, Symbol};
 
-use rustc_hir as hir;
+/// A wrapper around a [`hir::Item`].
+#[derive(Debug)]
+crate struct Item<'hir> {
+    /// the wrapped item
+    crate hir_item: &'hir hir::Item<'hir>,
+    /// the explicit renamed name
+    crate renamed_name: Option<Symbol>,
+    /// whether the item is from a glob import
+    /// if `from_glob` is true and we see another item with same name and namespace,
+    /// then this item can be replaced with that one
+    crate from_glob: bool,
+}
+
+impl<'hir> Item<'hir> {
+    pub(crate) fn new(
+        hir_item: &'hir hir::Item<'hir>,
+        renamed_name: Option<Symbol>,
+        from_glob: bool,
+    ) -> Self {
+        Self { hir_item, renamed_name, from_glob }
+    }
+
+    pub(crate) fn name(&self) -> Symbol {
+        self.renamed_name.unwrap_or(self.hir_item.ident.name)
+    }
+}
 
 crate struct Module<'hir> {
     crate name: Symbol,
     crate where_outer: Span,
     crate where_inner: Span,
     crate mods: Vec<Module<'hir>>,
     crate id: hir::HirId,
-    // (item, renamed)
-    crate items: Vec<(&'hir hir::Item<'hir>, Option<Symbol>)>,
+    crate items: Vec<Item<'hir>>,
     crate foreigns: Vec<(&'hir hir::ForeignItem<'hir>, Option<Symbol>)>,
     crate macros: Vec<(&'hir hir::MacroDef<'hir>, Option<Symbol>)>,
     crate is_crate: bool,
+    /// whether the module is from a glob import
+    /// if `from_glob` is true and we see another module with same name,
+    /// then this module can be replaced with that one
+    crate from_glob: bool,
 }
 
 impl Module<'hir> {
@@ -29,6 +59,101 @@ impl Module<'hir> {
             foreigns: Vec::new(),
             macros: Vec::new(),
             is_crate: false,
+            from_glob: false,
+        }
+    }
+
+    pub(crate) fn push_item(&mut self, ctx: &DocContext<'_>, new_item: Item<'hir>) {
+        let new_item_ns = ctx.tcx.def_kind(new_item.hir_item.def_id).ns();
+        let mut to_remove = None;
+        if !new_item.name().is_empty() && new_item_ns.is_some() {
+            if let Some((index, existing_item)) =
+                self.items.iter_mut().enumerate().find(|(_, item)| {
+                    item.name() == new_item.name()
+                        && ctx.tcx.def_kind(item.hir_item.def_id).ns() == new_item_ns
+                })
+            {
+                match (existing_item.from_glob, new_item.from_glob) {
+                    (true, false) => {
+                        // `existing_item` is from glob, `new_item` is not,
+                        // `new_item` should shadow `existing_item`
+                        debug!("push_item: {:?} shadowed by {:?}", existing_item, new_item);
+                        *existing_item = new_item;
+                        return;
+                    }
+                    (false, true) => {
+                        // `existing_item` is not from glob but `new_item` is,
+                        // just keep `existing_item` and return at once
+                        return;
+                    }
+                    (true, true) => {
+                        // both `existing_item` and `new_item` are from glob
+                        if existing_item.hir_item.def_id == new_item.hir_item.def_id {
+                            // they actually point to same object, it is ok to just keep one of them
+                            return;
+                        } else {
+                            // a "glob import vs glob import in the same module" will raise when actually use these item
+                            // we should accept neither of these two items
+                            to_remove = Some(index);
+                        }
+                    }
+                    (false, false) => {
+                        error!("push_item: {:?} conflict with {:?}", existing_item, new_item);
+                        // should report "defined multiple time" error before reach this
+                        unreachable!()
+                    }
+                }
+            }
+        }
+        if let Some(index) = to_remove {
+            self.items.swap_remove(index);
+            return;
+        }
+        // no item with same name and namespace exists, just collect `new_item`
+        self.items.push(new_item);
+    }
+
+    pub(crate) fn push_mod(&mut self, new_mod: Module<'hir>) {
+        let mut to_remove = None;
+        if let Some((index, existing_mod)) =
+            self.mods.iter_mut().enumerate().find(|(_, mod_)| mod_.name == new_mod.name)
+        {
+            match (existing_mod.from_glob, new_mod.from_glob) {
+                (true, false) => {
+                    // `existing_mod` is from glob, no matter whether `new_mod` is from glob,
+                    // `new_mod` should always shadow `existing_mod`
+                    debug!("push_mod: {:?} shadowed by {:?}", existing_mod.name, new_mod.name);
+                    *existing_mod = new_mod;
+                    return;
+                }
+                (true, true) => {
+                    // both `existing_item` and `new_item` are from glob
+                    if existing_mod.id == new_mod.id {
+                        // they actually point to same object, it is ok to just keep one of them
+                        return;
+                    } else {
+                        // a "glob import vs glob import in the same module" will raise when actually use these item
+                        // we should accept neither of these two items
+                        to_remove = Some(index);
+                    }
+                }
+                (false, true) => {
+                    // `existing_mod` is not from glob but `new_mod` is,
+                    // just keep `existing_mod` and return at once
+                    return;
+                }
+                (false, false) => {
+                    error!("push_mod: {:?} conflict with {:?}", existing_mod.name, new_mod.name);
+                    // should report "defined multiple time" error before reach this
+                    unreachable!()
+                }
+            }
+        }
+        if let Some(index) = to_remove {
+            self.mods.swap_remove(index);
+            return;
         }
+        // no mod with same name exists, just collect `new_mod`
+        self.mods.push(new_mod);
     }
 }

src/librustdoc/visit_ast.rs

@@ -77,6 +77,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             hir::CRATE_HIR_ID,
             &krate.item,
             self.cx.tcx.crate_name,
+            false,
         );
         top_level_module.is_crate = true;
         // Attach the crate's exported macros to the top-level module.
@@ -134,17 +135,19 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         id: hir::HirId,
         m: &'tcx hir::Mod<'tcx>,
         name: Symbol,
+        from_glob: bool,
     ) -> Module<'tcx> {
         let mut om = Module::new(name);
         om.where_outer = span;
         om.where_inner = m.inner;
         om.id = id;
+        om.from_glob = from_glob;
         // Keep track of if there were any private modules in the path.
         let orig_inside_public_path = self.inside_public_path;
         self.inside_public_path &= vis.node.is_pub();
         for &i in m.item_ids {
             let item = self.cx.tcx.hir().item(i);
-            self.visit_item(item, None, &mut om);
+            self.visit_item(item, None, &mut om, from_glob);
         }
         self.inside_public_path = orig_inside_public_path;
         om
@@ -225,14 +228,14 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 let prev = mem::replace(&mut self.inlining, true);
                 for &i in m.item_ids {
                     let i = self.cx.tcx.hir().item(i);
-                    self.visit_item(i, None, om);
+                    self.visit_item(i, None, om, glob);
                 }
                 self.inlining = prev;
                 true
             }
             Node::Item(it) if !glob => {
                 let prev = mem::replace(&mut self.inlining, true);
-                self.visit_item(it, renamed, om);
+                self.visit_item(it, renamed, om, glob);
                 self.inlining = prev;
                 true
             }
@@ -257,6 +260,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
         item: &'tcx hir::Item<'_>,
         renamed: Option<Symbol>,
         om: &mut Module<'tcx>,
+        from_glob: bool,
     ) {
         debug!("visiting item {:?}", item);
         let name = renamed.unwrap_or(item.ident.name);
@@ -309,10 +313,13 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                     }
                 }
 
-                om.items.push((item, renamed))
+                om.push_item(self.cx, Item::new(item, renamed, from_glob))
             }
             hir::ItemKind::Mod(ref m) => {
-                om.mods.push(self.visit_mod_contents(item.span, &item.vis, item.hir_id(), m, name));
+                let mut mod_ =
+                    self.visit_mod_contents(item.span, &item.vis, item.hir_id(), m, name, false);
+                mod_.from_glob = from_glob;
+                om.push_mod(mod_);
             }
             hir::ItemKind::Fn(..)
             | hir::ItemKind::ExternCrate(..)
@@ -323,19 +330,21 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
             | hir::ItemKind::OpaqueTy(..)
             | hir::ItemKind::Static(..)
             | hir::ItemKind::Trait(..)
-            | hir::ItemKind::TraitAlias(..) => om.items.push((item, renamed)),
+            | hir::ItemKind::TraitAlias(..) => {
+                om.push_item(self.cx, Item::new(item, renamed, from_glob))
+            }
             hir::ItemKind::Const(..) => {
                 // Underscore constants do not correspond to a nameable item and
                 // so are never useful in documentation.
                 if name != kw::Underscore {
-                    om.items.push((item, renamed));
+                    om.push_item(self.cx, Item::new(item, renamed, from_glob));
                 }
             }
             hir::ItemKind::Impl(ref impl_) => {
                 // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick
                 // them up regardless of where they're located.
                 if !self.inlining && impl_.of_trait.is_none() {
-                    om.items.push((item, None));
+                    om.push_item(self.cx, Item::new(item, None, from_glob));
                 }
             }
         }

src/test/rustdoc/glob-shadowing.rs

@@ -0,0 +1,86 @@
+// @has 'glob_shadowing/index.html'
+// @count - '//tr[@class="module-item"]' 5
+// @!has - '//tr[@class="module-item"]' 'sub1::describe'
+// @has - '//tr[@class="module-item"]' 'sub2::describe'
+
+// @!has - '//tr[@class="module-item"]' 'sub1::describe2'
+
+// @!has - '//tr[@class="module-item"]' 'sub1::prelude'
+// @has - '//tr[@class="module-item"]' 'mod::prelude'
+
+// @has - '//tr[@class="module-item"]' 'sub1::Foo (struct)'
+// @has - '//tr[@class="module-item"]' 'mod::Foo (function)'
+
+// @has - '//tr[@class="module-item"]' 'sub4::inner::X'
+
+// @has 'glob_shadowing/fn.describe.html'
+// @has - '//div[@class="docblock"]' 'sub2::describe'
+
+mod sub1 {
+    // this should be shadowed by sub2::describe
+    /// sub1::describe
+    pub fn describe() -> &'static str {
+        "sub1::describe"
+    }
+
+    // this should be shadowed by mod::prelude
+    /// sub1::prelude
+    pub mod prelude {
+    }
+
+    // this should *not* be shadowed, because sub1::Foo and mod::Foo are in different namespaces
+    /// sub1::Foo (struct)
+    pub struct Foo;
+
+    // this should be shadowed, 
+    // because both sub1::describe2 and sub3::describe2 are from glob reexport
+    /// sub1::describe2
+    pub fn describe2() -> &'static str {
+        "sub1::describe2"
+    }
+}
+
+mod sub2 {
+    /// sub2::describe
+    pub fn describe() -> &'static str {
+        "sub2::describe"
+    }
+}
+
+mod sub3 {
+    // this should be shadowed
+    // because both sub1::describe2 and sub3::describe2 are from glob reexport
+    /// sub3::describe2
+    pub fn describe2() -> &'static str {
+        "sub3::describe2"
+    }
+}
+
+mod sub4 {
+    // this should be shadowed by sub4::inner::X
+    /// sub4::X
+    pub const X: usize = 0;
+    pub mod inner {
+        pub use super::*;
+        /// sub4::inner::X
+        pub const X: usize = 1;
+    }
+}
+
+/// mod::Foo (function)
+pub fn Foo() {}
+
+#[doc(inline)]
+pub use sub2::describe;
+
+#[doc(inline)]
+pub use sub1::*;
+
+#[doc(inline)]
+pub use sub3::*;
+
+#[doc(inline)]
+pub use sub4::inner::*;
+
+/// mod::prelude
+pub mod prelude {}