Rollup merge of rust-lang#79310 - jyn514:fold-item-cleanup, r=Guillau…

…meGomez

Make `fold_item_recur` non-nullable

This gets rid of a bunch of `unwrap()`s and makes it a little more clear
what's going on.

Originally I wanted to make `fold_item` non-nullable too, which would
have been a lot nicer to work with, but unfortunately `stripper` does
actually return `None` in some places. I might make a follow-up moving
stripper to be special and not a pass so that passes can be
non-nullable.

Found while working on rust-lang#76998.

src/librustdoc/fold.rs

@@ -16,7 +16,7 @@ impl StripItem {
 
 crate trait DocFolder: Sized {
     fn fold_item(&mut self, item: Item) -> Option<Item> {
-        self.fold_item_recur(item)
+        Some(self.fold_item_recur(item))
     }
 
     /// don't override!
@@ -71,15 +71,12 @@ crate trait DocFolder: Sized {
     }
 
     /// don't override!
-    fn fold_item_recur(&mut self, item: Item) -> Option<Item> {
-        let Item { attrs, name, source, visibility, def_id, kind, stability, deprecation } = item;
-
-        let kind = match kind {
+    fn fold_item_recur(&mut self, mut item: Item) -> Item {
+        item.kind = match item.kind {
             StrippedItem(box i) => StrippedItem(box self.fold_inner_recur(i)),
-            _ => self.fold_inner_recur(kind),
+            _ => self.fold_inner_recur(item.kind),
         };
-
-        Some(Item { attrs, name, source, kind, visibility, stability, deprecation, def_id })
+        item
     }
 
     fn fold_mod(&mut self, m: Module) -> Module {

src/librustdoc/formats/cache.rs

@@ -421,55 +421,52 @@ impl DocFolder for Cache {
 
         // Once we've recursively found all the generics, hoard off all the
         // implementations elsewhere.
-        let ret = self.fold_item_recur(item).and_then(|item| {
-            if let clean::Item { kind: clean::ImplItem(_), .. } = item {
-                // Figure out the id of this impl. This may map to a
-                // primitive rather than always to a struct/enum.
-                // Note: matching twice to restrict the lifetime of the `i` borrow.
-                let mut dids = FxHashSet::default();
-                if let clean::Item { kind: clean::ImplItem(ref i), .. } = item {
-                    match i.for_ {
-                        clean::ResolvedPath { did, .. }
-                        | clean::BorrowedRef {
-                            type_: box clean::ResolvedPath { did, .. }, ..
-                        } => {
-                            dids.insert(did);
-                        }
-                        ref t => {
-                            let did = t
-                                .primitive_type()
-                                .and_then(|t| self.primitive_locations.get(&t).cloned());
+        let item = self.fold_item_recur(item);
+        let ret = if let clean::Item { kind: clean::ImplItem(_), .. } = item {
+            // Figure out the id of this impl. This may map to a
+            // primitive rather than always to a struct/enum.
+            // Note: matching twice to restrict the lifetime of the `i` borrow.
+            let mut dids = FxHashSet::default();
+            if let clean::Item { kind: clean::ImplItem(ref i), .. } = item {
+                match i.for_ {
+                    clean::ResolvedPath { did, .. }
+                    | clean::BorrowedRef { type_: box clean::ResolvedPath { did, .. }, .. } => {
+                        dids.insert(did);
+                    }
+                    ref t => {
+                        let did = t
+                            .primitive_type()
+                            .and_then(|t| self.primitive_locations.get(&t).cloned());
 
-                            if let Some(did) = did {
-                                dids.insert(did);
-                            }
+                        if let Some(did) = did {
+                            dids.insert(did);
                         }
                     }
+                }
 
-                    if let Some(generics) = i.trait_.as_ref().and_then(|t| t.generics()) {
-                        for bound in generics {
-                            if let Some(did) = bound.def_id() {
-                                dids.insert(did);
-                            }
+                if let Some(generics) = i.trait_.as_ref().and_then(|t| t.generics()) {
+                    for bound in generics {
+                        if let Some(did) = bound.def_id() {
+                            dids.insert(did);
                         }
                     }
-                } else {
-                    unreachable!()
-                };
-                let impl_item = Impl { impl_item: item };
-                if impl_item.trait_did().map_or(true, |d| self.traits.contains_key(&d)) {
-                    for did in dids {
-                        self.impls.entry(did).or_insert(vec![]).push(impl_item.clone());
-                    }
-                } else {
-                    let trait_did = impl_item.trait_did().expect("no trait did");
-                    self.orphan_trait_impls.push((trait_did, dids, impl_item));
                 }
-                None
             } else {
-                Some(item)
+                unreachable!()
+            };
+            let impl_item = Impl { impl_item: item };
+            if impl_item.trait_did().map_or(true, |d| self.traits.contains_key(&d)) {
+                for did in dids {
+                    self.impls.entry(did).or_insert(vec![]).push(impl_item.clone());
+                }
+            } else {
+                let trait_did = impl_item.trait_did().expect("no trait did");
+                self.orphan_trait_impls.push((trait_did, dids, impl_item));
             }
-        });
+            None
+        } else {
+            Some(item)
+        };
 
         if pushed {
             self.stack.pop().expect("stack already empty");

src/librustdoc/html/sources.rs

@@ -60,7 +60,7 @@ impl<'a> DocFolder for SourceCollector<'a> {
                 }
             };
         }
-        self.fold_item_recur(item)
+        Some(self.fold_item_recur(item))
     }
 }
 

src/librustdoc/passes/calculate_doc_coverage.rs

@@ -268,6 +268,6 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> {
             }
         }
 
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }

src/librustdoc/passes/check_code_block_syntax.rs

@@ -105,7 +105,7 @@ impl<'a, 'tcx> DocFolder for SyntaxChecker<'a, 'tcx> {
             }
         }
 
-        self.fold_item_recur(item)
+        Some(self.fold_item_recur(item))
     }
 }
 

src/librustdoc/passes/collapse_docs.rs

@@ -23,7 +23,7 @@ struct Collapser;
 impl fold::DocFolder for Collapser {
     fn fold_item(&mut self, mut i: Item) -> Option<Item> {
         i.attrs.collapse_doc_comments();
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }
 

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -858,7 +858,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
             // we don't display docs on `extern crate` items anyway, so don't process them.
             clean::ExternCrateItem(..) => {
                 debug!("ignoring extern crate item {:?}", item.def_id);
-                return self.fold_item_recur(item);
+                return Some(self.fold_item_recur(item));
             }
             clean::ImportItem(Import { kind: clean::ImportKind::Simple(ref name, ..), .. }) => {
                 Some(name.clone())
@@ -958,7 +958,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
             }
         }
 
-        if item.is_mod() {
+        Some(if item.is_mod() {
             if !item.attrs.inner_docs {
                 self.mod_ids.push(item.def_id);
             }
@@ -968,7 +968,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
             ret
         } else {
             self.fold_item_recur(item)
-        }
+        })
     }
 }
 

src/librustdoc/passes/collect_trait_impls.rs

@@ -133,7 +133,7 @@ impl<'a, 'tcx> DocFolder for SyntheticImplCollector<'a, 'tcx> {
             }
         }
 
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }
 
@@ -152,7 +152,7 @@ impl DocFolder for ItemCollector {
     fn fold_item(&mut self, i: Item) -> Option<Item> {
         self.items.insert(i.def_id);
 
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }
 

src/librustdoc/passes/doc_test_lints.rs

@@ -41,7 +41,7 @@ impl<'a, 'tcx> DocFolder for PrivateItemDocTestLinter<'a, 'tcx> {
 
         look_for_tests(&cx, &dox, &item);
 
-        self.fold_item_recur(item)
+        Some(self.fold_item_recur(item))
     }
 }
 

src/librustdoc/passes/html_tags.rs

@@ -178,7 +178,7 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> {
             Some(hir_id) => hir_id,
             None => {
                 // If non-local, no need to check anything.
-                return self.fold_item_recur(item);
+                return Some(self.fold_item_recur(item));
             }
         };
         let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
@@ -223,6 +223,6 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> {
             }
         }
 
-        self.fold_item_recur(item)
+        Some(self.fold_item_recur(item))
     }
 }

src/librustdoc/passes/non_autolinks.rs

@@ -68,7 +68,7 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> {
             Some(hir_id) => hir_id,
             None => {
                 // If non-local, no need to check anything.
-                return self.fold_item_recur(item);
+                return Some(self.fold_item_recur(item));
             }
         };
         let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
@@ -133,6 +133,6 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> {
             }
         }
 
-        self.fold_item_recur(item)
+        Some(self.fold_item_recur(item))
     }
 }

src/librustdoc/passes/propagate_doc_cfg.rs

@@ -39,6 +39,6 @@ impl DocFolder for CfgPropagator {
         let result = self.fold_item_recur(item);
         self.parent_cfg = old_parent_cfg;
 
-        result
+        Some(result)
     }
 }

src/librustdoc/passes/strip_hidden.rs

@@ -47,7 +47,7 @@ impl<'a> DocFolder for Stripper<'a> {
                     // strip things like impl methods but when doing so
                     // we must not add any items to the `retained` set.
                     let old = mem::replace(&mut self.update_retained, false);
-                    let ret = StripItem(self.fold_item_recur(i).unwrap()).strip();
+                    let ret = StripItem(self.fold_item_recur(i)).strip();
                     self.update_retained = old;
                     return ret;
                 }
@@ -58,6 +58,6 @@ impl<'a> DocFolder for Stripper<'a> {
                 self.retained.insert(i.def_id);
             }
         }
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }

src/librustdoc/passes/stripper.rs

@@ -22,7 +22,7 @@ impl<'a> DocFolder for Stripper<'a> {
                 let old = mem::replace(&mut self.update_retained, false);
                 let ret = self.fold_item_recur(i);
                 self.update_retained = old;
-                return ret;
+                return Some(ret);
             }
             // These items can all get re-exported
             clean::OpaqueTyItem(..)
@@ -59,7 +59,7 @@ impl<'a> DocFolder for Stripper<'a> {
                 if i.def_id.is_local() && !i.visibility.is_public() {
                     debug!("Stripper: stripping module {:?}", i.name);
                     let old = mem::replace(&mut self.update_retained, false);
-                    let ret = StripItem(self.fold_item_recur(i).unwrap()).strip();
+                    let ret = StripItem(self.fold_item_recur(i)).strip();
                     self.update_retained = old;
                     return ret;
                 }
@@ -107,12 +107,10 @@ impl<'a> DocFolder for Stripper<'a> {
             self.fold_item_recur(i)
         };
 
-        if let Some(ref i) = i {
-            if self.update_retained {
-                self.retained.insert(i.def_id);
-            }
+        if self.update_retained {
+            self.retained.insert(i.def_id);
         }
-        i
+        Some(i)
     }
 }
 
@@ -153,7 +151,7 @@ impl<'a> DocFolder for ImplStripper<'a> {
                 }
             }
         }
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }
 
@@ -164,7 +162,7 @@ impl DocFolder for ImportStripper {
     fn fold_item(&mut self, i: Item) -> Option<Item> {
         match i.kind {
             clean::ExternCrateItem(..) | clean::ImportItem(..) if !i.visibility.is_public() => None,
-            _ => self.fold_item_recur(i),
+            _ => Some(self.fold_item_recur(i)),
         }
     }
 }

src/librustdoc/passes/unindent_comments.rs

@@ -23,7 +23,7 @@ struct CommentCleaner;
 impl fold::DocFolder for CommentCleaner {
     fn fold_item(&mut self, mut i: Item) -> Option<Item> {
         i.attrs.unindent_doc_comments();
-        self.fold_item_recur(i)
+        Some(self.fold_item_recur(i))
     }
 }