Rollup merge of rust-lang#156020 - GuillaumeGomez:cleanup-code, r=urgau

Improve source code for `librustdoc/visit_ast.rs`

While working on this part of rustdoc, got annoyed at the tuples and decided to transform them into types. Code is now much easier to follow. :3

r? @Urgau

Author: GuillaumeGomez

src/librustdoc/clean/mod.rs

@@ -62,12 +62,15 @@ pub(crate) use self::types::*;
 pub(crate) use self::utils::{krate, register_res, synthesize_auto_trait_and_blanket_impls};
 use crate::core::DocContext;
 use crate::formats::item_type::ItemType;
-use crate::visit_ast::Module as DocModule;
+use crate::visit_ast;
 
-pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<'tcx>) -> Item {
+pub(crate) fn clean_doc_module<'tcx>(
+    doc: &visit_ast::Module<'tcx>,
+    cx: &mut DocContext<'tcx>,
+) -> Item {
     let mut items: Vec<Item> = vec![];
     let mut inserted = FxHashSet::default();
-    items.extend(doc.foreigns.iter().map(|(item, renamed, import_id)| {
+    items.extend(doc.foreigns.iter().map(|visit_ast::Foreign { item, renamed, import_id }| {
         let item = clean_maybe_renamed_foreign_item(cx, item, *renamed, *import_id);
         if let Some(name) = item.name
             && (cx.document_hidden() || !item.is_doc_hidden())
@@ -95,52 +98,56 @@ 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(|entry| {
-        // First, lower everything other than glob imports.
-        let item = entry.item;
-        if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) {
-            return Vec::new();
-        }
-        let v = clean_maybe_renamed_item(cx, item, entry.renamed, &entry.import_ids);
-        for item in &v {
-            if let Some(name) = item.name
-                && (cx.document_hidden() || !item.is_doc_hidden())
-            {
-                inserted.insert((item.type_(), name));
+    items.extend(doc.items.values().flat_map(
+        |visit_ast::ItemEntry { item, renamed, import_ids }| {
+            // First, lower everything other than glob imports.
+            if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) {
+                return Vec::new();
             }
-        }
-        v
-    }));
-    items.extend(doc.inlined_foreigns.iter().flat_map(|((_, renamed), (res, local_import_id))| {
-        let Some(def_id) = res.opt_def_id() else { return Vec::new() };
-        let name = renamed.unwrap_or_else(|| cx.tcx.item_name(def_id));
-        let import = cx.tcx.hir_expect_item(*local_import_id);
-        match import.kind {
-            hir::ItemKind::Use(path, kind) => {
-                let hir::UsePath { segments, span, .. } = *path;
-                let path = hir::Path { segments, res: *res, span };
-                clean_use_statement_inner(
-                    import,
-                    Some(name),
-                    &path,
-                    kind,
-                    cx,
-                    &mut Default::default(),
-                )
+            let v = clean_maybe_renamed_item(cx, item, *renamed, import_ids);
+            for item in &v {
+                if let Some(name) = item.name
+                    && (cx.document_hidden() || !item.is_doc_hidden())
+                {
+                    inserted.insert((item.type_(), name));
+                }
             }
-            _ => unreachable!(),
-        }
-    }));
-    items.extend(doc.items.values().flat_map(|entry| {
-        // Now we actually lower the imports, skipping everything else.
-        let item = entry.item;
-        if let hir::ItemKind::Use(path, hir::UseKind::Glob) = item.kind {
-            clean_use_statement(item, entry.renamed, path, hir::UseKind::Glob, cx, &mut inserted)
-        } else {
-            // skip everything else
-            Vec::new()
-        }
-    }));
+            v
+        },
+    ));
+    items.extend(doc.inlined_foreigns.iter().flat_map(
+        |((_, renamed), visit_ast::InlinedForeign { res, import_id })| {
+            let Some(def_id) = res.opt_def_id() else { return Vec::new() };
+            let name = renamed.unwrap_or_else(|| cx.tcx.item_name(def_id));
+            let import = cx.tcx.hir_expect_item(*import_id);
+            match import.kind {
+                hir::ItemKind::Use(path, kind) => {
+                    let hir::UsePath { segments, span, .. } = *path;
+                    let path = hir::Path { segments, res: *res, span };
+                    clean_use_statement_inner(
+                        import,
+                        Some(name),
+                        &path,
+                        kind,
+                        cx,
+                        &mut Default::default(),
+                    )
+                }
+                _ => unreachable!(),
+            }
+        },
+    ));
+    items.extend(doc.items.values().flat_map(
+        |visit_ast::ItemEntry { item, renamed, import_ids: _ }| {
+            // Now we actually lower the imports, skipping everything else.
+            if let hir::ItemKind::Use(path, hir::UseKind::Glob) = item.kind {
+                clean_use_statement(item, *renamed, path, hir::UseKind::Glob, cx, &mut inserted)
+            } else {
+                // skip everything else
+                Vec::new()
+            }
+        },
+    ));
 
     // determine if we should display the inner contents or
     // the outer `mod` item for the source code.

src/librustdoc/visit_ast.rs

@@ -32,8 +32,7 @@ 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`.
-    /// We use `FxIndexMap` to keep the insert order.
+    /// The key is the item `ItemId`. 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:
@@ -54,17 +53,17 @@ pub(crate) struct Module<'hir> {
     /// shadowed or not.
     pub(crate) items: FxIndexMap<(LocalDefId, Option<Symbol>), ItemEntry<'hir>>,
 
-    /// (def_id, renamed) -> (res, local_import_id)
+    /// The key is `(def_id, renamed)`.
     ///
     /// `inlined_foreigns` only contains `extern` items
     /// that are cross-crate inlined.
     ///
     /// Locally inlined `extern` items are
     /// stored in `foreigns` with the `import_id` set,
     /// analogous to how `items` is.
-    pub(crate) inlined_foreigns: FxIndexMap<(DefId, Option<Symbol>), (Res, LocalDefId)>,
+    pub(crate) inlined_foreigns: FxIndexMap<(DefId, Option<Symbol>), InlinedForeign>,
     /// (item, renamed, import_id)
-    pub(crate) foreigns: Vec<(&'hir hir::ForeignItem<'hir>, Option<Symbol>, Option<LocalDefId>)>,
+    pub(crate) foreigns: Vec<Foreign<'hir>>,
 }
 
 #[derive(Debug)]
@@ -74,6 +73,19 @@ pub(crate) struct ItemEntry<'hir> {
     pub(crate) import_ids: Vec<LocalDefId>,
 }
 
+#[derive(Debug)]
+pub(crate) struct InlinedForeign {
+    pub(crate) res: Res,
+    pub(crate) import_id: LocalDefId,
+}
+
+#[derive(Debug)]
+pub(crate) struct Foreign<'hir> {
+    pub(crate) item: &'hir hir::ForeignItem<'hir>,
+    pub(crate) renamed: Option<Symbol>,
+    pub(crate) import_id: Option<LocalDefId>,
+}
+
 impl Module<'_> {
     pub(crate) fn new(
         name: Symbol,
@@ -283,7 +295,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
                 .last_mut()
                 .unwrap()
                 .inlined_foreigns
-                .insert((ori_res_did, renamed), (res, def_id));
+                .insert((ori_res_did, renamed), InlinedForeign { res, import_id: def_id });
             return true;
         };
 
@@ -575,7 +587,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
     ) {
         // If inlining we only want to include public functions.
         if !self.inlining || self.cx.tcx.visibility(item.owner_id).is_public() {
-            self.modules.last_mut().unwrap().foreigns.push((item, renamed, import_id));
+            self.modules.last_mut().unwrap().foreigns.push(Foreign { item, renamed, import_id });
         }
     }