apply some suggestions from code review

Signed-off-by: longfangsong <longfangsong@icloud.com>
rust-lang · Apr 5, 2021 · 4b934bb · 4b934bb
1 parent 09cb285
commit 4b934bb
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 22 deletions.
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -1939,7 +1939,7 @@ impl Clean<Vec<Item>> for doctree::Item<'_> {
         use hir::ItemKind;
 
         let item = self.hir_item;
-        let mut name = self.name().clone();
+        let mut name = self.name();
         let def_id = item.def_id.to_def_id();
 
         cx.with_param_env(def_id, |cx| {

diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs
@@ -26,8 +26,8 @@ impl<'hir> Item<'hir> {
         Self { hir_item, renamed_name, from_glob }
     }
 
-    pub(crate) fn name(&'hir self) -> &'hir Symbol {
-        self.renamed_name.as_ref().unwrap_or(&self.hir_item.ident.name)
+    pub(crate) fn name(&self) -> Symbol {
+        self.renamed_name.unwrap_or(self.hir_item.ident.name)
     }
 }
 
@@ -44,7 +44,7 @@ crate struct Module<'hir> {
     /// 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
-    pub(crate) from_glob: bool,
+    crate from_glob: bool,
 }
 
 impl Module<'hir> {
@@ -64,34 +64,30 @@ impl Module<'hir> {
     }
 
     pub(crate) fn push_item(&mut self, new_item: Item<'hir>) {
-        if let Some(existed_item) =
+        if let Some(existing_item) =
             self.items.iter_mut().find(|item| item.name() == new_item.name())
         {
-            if existed_item.name() == new_item.name() {
-                if existed_item.from_glob {
-                    debug!("push_item: {:?} shadowed by {:?}", *existed_item, new_item);
-                    *existed_item = new_item;
-                    return;
-                } else if new_item.from_glob {
-                    return;
-                }
+            if existing_item.from_glob {
+                debug!("push_item: {:?} shadowed by {:?}", *existing_item, new_item);
+                *existing_item = new_item;
+                return;
+            } else if new_item.from_glob {
+                return;
             }
-        } else {
-            self.items.push(new_item);
         }
+        self.items.push(new_item);
     }
 
     pub(crate) fn push_mod(&mut self, new_item: Module<'hir>) {
-        if let Some(existed_mod) = self.mods.iter_mut().find(|mod_| mod_.name == new_item.name) {
-            if existed_mod.from_glob {
-                debug!("push_mod: {:?} shadowed by {:?}", existed_mod.name, new_item.name);
-                *existed_mod = new_item;
+        if let Some(existing_mod) = self.mods.iter_mut().find(|mod_| mod_.name == new_item.name) {
+            if existing_mod.from_glob {
+                debug!("push_mod: {:?} shadowed by {:?}", existing_mod.name, new_item.name);
+                *existing_mod = new_item;
                 return;
             } else if new_item.from_glob {
                 return;
             }
-        } else {
-            self.mods.push(new_item);
         }
+        self.mods.push(new_item);
     }
 }
diff --git a/src/test/rustdoc/glob-shadowing.rs b/src/test/rustdoc/glob-shadowing.rs
@@ -1,10 +1,11 @@
 // @has 'glob_shadowing/index.html'
 // @count - '//tr[@class="module-item"]' 2
 // @has - '//tr[@class="module-item"]' 'mod::prelude'
+// @!has - '//tr[@class="module-item"]' 'sub1::describe'
 // @has - '//tr[@class="module-item"]' 'sub2::describe'
 
 // @has 'glob_shadowing/fn.describe.html'
-// @has - '//div[@class='docblock']' 'sub2::describe'
+// @has - '//div[@class="docblock"]' 'sub2::describe'
 
 mod sub1 {
     /// sub1::describe