Skip to content
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

rustdoc: introduce glob shadowing rules to rustdoc #83872

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
40 changes: 25 additions & 15 deletions src/librustdoc/doctree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ crate struct Item<'hir> {
crate hir_item: &'hir hir::Item<'hir>,
/// the explicit renamed name
crate renamed_name: Option<Symbol>,
/// the [`Namespace`] this Item belongs to
crate namespace: Option<hir::def::Namespace>,
longfangsong marked this conversation as resolved.
Show resolved Hide resolved
/// whether the item is from a glob import
/// if `from_glob` is true and we see another item with same name,
/// 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,
}
Expand All @@ -21,9 +23,10 @@ impl<'hir> Item<'hir> {
pub(crate) fn new(
hir_item: &'hir hir::Item<'hir>,
renamed_name: Option<Symbol>,
namespace: Option<hir::def::Namespace>,
from_glob: bool,
) -> Self {
Self { hir_item, renamed_name, from_glob }
Self { hir_item, renamed_name, namespace, from_glob }
}

pub(crate) fn name(&self) -> Symbol {
Expand All @@ -43,7 +46,7 @@ crate struct Module<'hir> {
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 item can be replaced with that one
/// then this module can be replaced with that one
crate from_glob: bool,
}

Expand All @@ -64,25 +67,29 @@ impl Module<'hir> {
}

pub(crate) fn push_item(&mut self, new_item: Item<'hir>) {
if !new_item.name().is_empty() {
// todo: also check namespace
if let Some(existing_item) =
self.items.iter_mut().find(|item| item.name() == new_item.name())
if !new_item.name().is_empty() && new_item.namespace.is_some() {
if let Some(existing_item) = self
.items
.iter_mut()
.find(|item| item.name() == new_item.name() && item.namespace == new_item.namespace)
{
match (existing_item.from_glob, new_item.from_glob) {
(true, _) => {
// `existing_item` is from glob, no matter whether `new_item` is from glob
// `existing_item` is from glob, no matter whether `new_item` is from glob,
// `new_item` should always 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
// `existing_item` is not from glob but `new_item` is,
// just keep `existing_item` and return at once
return;
}
(false, false) => unreachable!() // todo: how to handle this?
(false, false) => {
// should report "defined multiple time" error before reach this
unreachable!()
}
}
}
}
Expand All @@ -94,18 +101,21 @@ impl Module<'hir> {
if let Some(existing_mod) = self.mods.iter_mut().find(|mod_| mod_.name == new_mod.name) {
match (existing_mod.from_glob, new_mod.from_glob) {
(true, _) => {
// `existing_mod` is from glob, no matter whether `new_mod` is from glob
// `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;
},
}
(false, true) => {
// `existing_mod` is not from glob but `new_mod` is
// `existing_mod` is not from glob but `new_mod` is,
// just keep `existing_mod` and return at once
return;
},
(false, false) => unreachable!(),
}
(false, false) => {
// should report "defined multiple time" error before reach this
unreachable!()
}
}
}
// no mod with same name exists, just collect `new_mod`
Expand Down
28 changes: 24 additions & 4 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,12 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
}
}

om.push_item(Item::new(item, renamed, from_glob))
om.push_item(Item::new(
item,
renamed,
self.cx.tcx.def_kind(item.def_id).ns(),
from_glob,
))
}
hir::ItemKind::Mod(ref m) => {
om.push_mod(self.visit_mod_contents(
Expand All @@ -334,19 +339,34 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
| hir::ItemKind::OpaqueTy(..)
| hir::ItemKind::Static(..)
| hir::ItemKind::Trait(..)
| hir::ItemKind::TraitAlias(..) => om.push_item(Item::new(item, renamed, from_glob)),
| hir::ItemKind::TraitAlias(..) => om.push_item(Item::new(
item,
renamed,
self.cx.tcx.def_kind(item.def_id).ns(),
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.push_item(Item::new(item, renamed, from_glob));
om.push_item(Item::new(
item,
renamed,
self.cx.tcx.def_kind(item.def_id).ns(),
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.push_item(Item::new(item, None, from_glob));
om.push_item(Item::new(
item,
None,
self.cx.tcx.def_kind(item.def_id).ns(),
from_glob,
));
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/test/rustdoc/glob-shadowing.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// @has 'glob_shadowing/index.html'
// @count - '//tr[@class="module-item"]' 4
// @has - '//tr[@class="module-item"]' 'mod::prelude'
// @!has - '//tr[@class="module-item"]' 'sub1::describe'
// @has - '//tr[@class="module-item"]' 'mod::prelude'
// @has - '//tr[@class="module-item"]' 'sub2::describe'
// @has - '//tr[@class="module-item"]' 'sub1::Foo (struct)'
// @has - '//tr[@class="module-item"]' 'mod::Foo (function)'

// @has 'glob_shadowing/fn.describe.html'
// @has - '//div[@class="docblock"]' 'sub2::describe'
Expand Down