big refactor: crate IDs always go alongside item IDs now

This makes it impossible to try to access an item with the wrong crate ID. Also this change makes us pass around references much more than owned things, so it cuts down on cloning a ton, which is nice.

Author: wfraser

src/analysis.rs

@@ -7,11 +7,11 @@ use std::process::Command;
 use anyhow::Context;
 use rayon::prelude::*;
 
-pub type Id = rustdoc_types::Id;
-
 /// Write the analysis data to a subdirectory under target/ with this name.
 const SUBDIR: &str = "rsbrowse";
 
+const EMPTY_ID: &rustdoc_types::Id = &rustdoc_types::Id(String::new());
+
 pub struct Analysis {
     pub crates: HashMap<String, rustdoc_types::Crate>,
 }
@@ -86,93 +86,80 @@ impl Analysis {
         Ok(Self { crates })
     }
 
-    pub fn crate_ids(&self) -> impl Iterator<Item = CrateId> + '_ {
-        /*let mut ids = vec![];
-
-        for c in self.crates.values() {
-            let name = c.index.iter()
-                .find_map(|(_id, item)| {
-                    match &item.inner {
-                        rustdoc_types::ItemEnum::Module(m) if m.is_crate && item.crate_id == 0 => {
-                            Some(item.name.clone().expect("crate module should have a name"))
-                        }
-                        _ => None
-                    }
-                })
-                .expect("should have an index item for the local crate");
-
-            ids.push(CrateId { name });
-        }*/
-
+    pub fn crate_ids(&self) -> impl Iterator<Item = ItemId> + '_ {
         self.crates
             .values()
             .flat_map(|crate_| &crate_.index)
             .filter_map(|(_id, item)| match &item.inner {
                 rustdoc_types::ItemEnum::Module(m) if m.is_crate && item.crate_id == 0 => {
-                    let name = item.name.clone().expect("crate module should have a name");
-                    Some(CrateId { name })
+                    let name = item.name.as_ref().expect("crate module should have a name");
+                    Some(ItemId::crate_root(CrateId { name }))
                 }
                 _ => None,
             })
-
-        //ids.into_iter()
     }
 
     pub fn items<'a, 'b>(
         &'a self,
-        crate_id: &'b CrateId,
-        parent_id: Option<Id>,
-    ) -> impl Iterator<Item = Item<'a>> + 'b
+        parent_id: &'b ItemId<'a>,
+    ) -> impl Iterator<Item = (ItemId<'a>, Item<'a>)> + 'b
     where
         'a: 'b,
     {
-        let parent_id = parent_id.unwrap_or(self.crates[&crate_id.name].root.clone());
-        let parent = self
-            .crates
-            .get(&crate_id.name)
-            .unwrap_or_else(|| panic!("no crate {crate_id:?}"))
-            .index
-            .get(&parent_id)
-            .unwrap_or_else(|| panic!("no id {parent_id:?} in {crate_id:?}"));
+        let ItemId(parent_crate, parent_id) = parent_id;
+        let parent = {
+            let crate_ = &self
+                .crates
+                .get(parent_crate.name)
+                .unwrap_or_else(|| panic!("no crate {parent_crate:?}"));
+            let id = if parent_id == &EMPTY_ID {
+                &crate_.root
+            } else {
+                parent_id
+            };
+            crate_
+                .index
+                .get(id)
+                .unwrap_or_else(|| panic!("no id {id:?} in {parent_crate:?}"))
+        };
 
         use rustdoc_types::ItemEnum::*;
-        let children = match &parent.inner {
-            Module(m) => m.items.clone(),
+        let children: Vec<&'a rustdoc_types::Id> = match &parent.inner {
+            Module(m) => m.items.iter().collect(),
             ExternCrate { .. } => vec![],
             Import(_) => vec![],
-            Union(u) => [&u.fields[..], &u.impls[..]].concat(),
+            Union(u) => u.fields.iter().chain(&u.impls).collect(),
             Struct(s) => {
                 let fields = match &s.kind {
                     rustdoc_types::StructKind::Unit => vec![],
                     rustdoc_types::StructKind::Tuple(t) => {
-                        t.iter().filter_map(|x| x.as_ref()).cloned().collect()
+                        t.iter().filter_map(|x| x.as_ref()).collect()
                     }
-                    rustdoc_types::StructKind::Plain { fields, .. } => fields.clone(),
+                    rustdoc_types::StructKind::Plain { fields, .. } => fields.iter().collect(),
                 };
-                [&fields[..], &s.impls[..]].concat()
+                fields.into_iter().chain(&s.impls).collect()
             }
             StructField(ty) => type_ids(ty),
-            Enum(e) => [&e.variants[..], &e.impls[..]].concat(),
+            Enum(e) => e.variants.iter().chain(&e.impls).collect(),
             Variant(v) => match &v.kind {
                 rustdoc_types::VariantKind::Plain => vec![],
                 rustdoc_types::VariantKind::Tuple(t) => {
-                    t.iter().filter_map(|id| id.clone()).collect()
+                    t.iter().filter_map(|id| id.as_ref()).collect()
                 }
-                rustdoc_types::VariantKind::Struct { fields, .. } => fields.clone(),
+                rustdoc_types::VariantKind::Struct { fields, .. } => fields.iter().collect(),
             },
             Function(_) => vec![],
             Trait(t) => {
                 // TODO: also find impls?
-                t.items.clone()
+                t.items.iter().collect()
             }
             TraitAlias(_) => vec![],
             Impl(i) => {
-                let mut items = i.items.clone();
-                // Add a reference to the trait itself too if it's not an inherent impl:
-                if let Some(trait_) = &i.trait_ {
-                    items.push(trait_.id.clone());
-                }
-                items
+                i.items
+                    .iter()
+                    // Add a reference to the trait itself too if it's not an inherent impl:
+                    .chain(i.trait_.as_ref().map(|t| &t.id))
+                    .collect()
             }
             TypeAlias(ty) => type_ids(&ty.type_),
             OpaqueTy(_) => vec![],
@@ -187,10 +174,18 @@ impl Analysis {
         };
 
         children.into_iter().filter_map(move |id| {
-            if let Some(item) = self.crates[&crate_id.name].index.get(&id) {
-                Some(Item::Item(item))
+            if let Some(item) = self.crates[parent_crate.name].index.get(id) {
+                Some((
+                    ItemId(
+                        CrateId {
+                            name: parent_crate.name,
+                        },
+                        id,
+                    ),
+                    Item::Item(item),
+                ))
             } else {
-                let summary = self.crates[&crate_id.name].paths.get(&id)?;
+                let summary = self.crates[parent_crate.name].paths.get(id)?;
                 let other_crate = &summary.path[0];
                 let other_id =
                     self.crates
@@ -205,11 +200,9 @@ impl Analysis {
                             }
                         })?;
                 let item = self.crates[other_crate].index.get(other_id)?;
-                Some(Item::Foreign(
-                    CrateId {
-                        name: other_crate.to_owned(),
-                    },
-                    item,
+                Some((
+                    ItemId(CrateId { name: other_crate }, other_id),
+                    Item::Item(item),
                 ))
             }
         })
@@ -222,11 +215,11 @@ fn parse_json(p: &Path) -> anyhow::Result<rustdoc_types::Crate> {
     Ok(data)
 }
 
-fn type_ids(ty: &rustdoc_types::Type) -> Vec<Id> {
+fn type_ids(ty: &rustdoc_types::Type) -> Vec<&rustdoc_types::Id> {
     use rustdoc_types::Type::*;
     match ty {
-        ResolvedPath(path) => vec![path.id.clone()],
-        DynTrait(dt) => dt.traits.iter().map(|t| t.trait_.id.clone()).collect(),
+        ResolvedPath(path) => vec![&path.id],
+        DynTrait(dt) => dt.traits.iter().map(|t| &t.trait_.id).collect(),
         Generic(_) => vec![],
         Primitive(_) => vec![],
         FunctionPointer(_) => vec![],
@@ -236,7 +229,7 @@ fn type_ids(ty: &rustdoc_types::Type) -> Vec<Id> {
         ImplTrait(generics) => generics
             .iter()
             .filter_map(|g| match g {
-                rustdoc_types::GenericBound::TraitBound { trait_, .. } => Some(trait_.id.clone()),
+                rustdoc_types::GenericBound::TraitBound { trait_, .. } => Some(&trait_.id),
                 rustdoc_types::GenericBound::Outlives(_) => None,
             })
             .collect(),
@@ -248,7 +241,7 @@ fn type_ids(ty: &rustdoc_types::Type) -> Vec<Id> {
         } => {
             let from_self = type_ids(self_type);
             if let Some(t) = trait_ {
-                [&from_self[..], &[t.id.clone()]].concat()
+                [&from_self[..], &[&t.id]].concat()
             } else {
                 from_self
             }
@@ -257,17 +250,28 @@ fn type_ids(ty: &rustdoc_types::Type) -> Vec<Id> {
 }
 
 #[derive(Debug, Clone)]
-pub struct CrateId {
-    pub name: String,
-    //pub id: u32,
+pub struct CrateId<'a> {
+    pub name: &'a String,
+}
+
+#[derive(Debug, Clone)]
+pub struct ItemId<'a>(CrateId<'a>, &'a rustdoc_types::Id);
+
+impl<'a> ItemId<'a> {
+    pub fn crate_root(crate_id: CrateId<'a>) -> Self {
+        Self(crate_id, EMPTY_ID)
+    }
+
+    pub fn crate_name(&self) -> &str {
+        self.0.name
+    }
 }
 
 #[allow(clippy::large_enum_variant)]
 #[derive(Debug, Clone)]
 pub enum Item<'a> {
     Root,
     Item(&'a rustdoc_types::Item),
-    Foreign(CrateId, &'a rustdoc_types::Item),
 }
 
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]

src/browser_rustdoc.rs

@@ -1,5 +1,5 @@
-use crate::analysis::{Analysis, CrateId, Item};
-use crate::browser_trait::{self, Browser};
+use crate::analysis::{Analysis, Item, ItemId};
+use crate::browser_trait::Browser;
 use std::fmt::Write;
 
 pub struct RustdocBrowser {
@@ -13,41 +13,30 @@ impl RustdocBrowser {
 }
 
 impl<'a> Browser for &'a RustdocBrowser {
-    type CrateId = CrateId;
     type Item = Item<'a>;
+    type ItemId = ItemId<'a>;
 
-    fn list_crates(&self) -> Vec<(String, CrateId)> {
+    fn list_crates(&self) -> Vec<(String, ItemId<'a>)> {
         let mut crates = self
             .analysis
             .crate_ids()
             //.filter(|c| !self.analysis.stdlib_crates.contains(c))
-            .map(|c| (crate_label(&c), c))
+            .map(|item_id| (crate_label(&item_id), item_id))
             .collect::<Vec<_>>();
 
         sort_by_label(&mut crates);
 
         crates
     }
 
-    fn list_items(&self, crate_id: &CrateId, parent: &Item<'a>) -> Vec<(String, Item<'a>)> {
-        let parent_id = match parent {
-            Item::Item(item) | Item::Foreign(_, item) => Some(item.id.clone()),
-            Item::Root => None,
-        };
-
-        let crate_id = match parent {
-            Item::Foreign(other_crate, _) => other_crate,
-            Item::Item(_) | Item::Root => crate_id,
-        };
-
+    fn list_items(&self, parent_id: &ItemId<'a>) -> Vec<(String, (ItemId<'a>, Item<'a>))> {
         let mut items = self
             .analysis
-            .items(crate_id, parent_id)
-            .filter_map(|item| {
+            .items(parent_id)
+            .filter_map(|(id, item)| {
                 let inner = match item {
                     Item::Root => return None,
                     Item::Item(item) => item,
-                    Item::Foreign(_, item) => item,
                 };
 
                 // Remove the clutter of automatically derived, blanket, and synthetic trait impls.
@@ -57,7 +46,7 @@ impl<'a> Browser for &'a RustdocBrowser {
                 }
                 match &inner.inner {
                     Impl(i) if i.blanket_impl.is_some() || i.synthetic => None,
-                    _ => Some((item_label(inner), item)),
+                    _ => Some((item_label(inner), (id, item))),
                 }
             })
             .collect::<Vec<_>>();
@@ -66,10 +55,10 @@ impl<'a> Browser for &'a RustdocBrowser {
         items
     }
 
-    fn get_info(&self, crate_id: &CrateId, item: &Item<'a>) -> String {
+    fn get_info(&self, item: &Item<'a>) -> String {
         let mut txt = String::new();
         match item {
-            Item::Item(item) | Item::Foreign(_, item) => {
+            Item::Item(item) => {
                 if let Some(docs) = &item.docs {
                     txt += &docs;
                     txt.push('\n');
@@ -84,19 +73,19 @@ impl<'a> Browser for &'a RustdocBrowser {
                 }
             }
             Item::Root => {
-                write!(txt, "crate root of {crate_id:?}").unwrap();
+                write!(txt, "crate root").unwrap();
             }
         }
         txt
     }
 
-    fn get_debug_info(&self, crate_id: &CrateId, item: &Item) -> String {
-        format!("{crate_id:?}: {item:#?}")
+    fn get_debug_info(&self, item: &Item) -> String {
+        format!("{item:#?}")
     }
 
     fn get_source(&self, item: &Item) -> (String, Option<usize>) {
         match item {
-            Item::Item(item) | Item::Foreign(_, item) => {
+            Item::Item(item) => {
                 let (txt, line) = get_source_for_item(item);
                 (txt, Some(line))
             }
@@ -134,15 +123,15 @@ fn sort_by_label<T>(slice: &mut [(String, T)]) {
     slice.sort_unstable_by(cmp_labels);
 }
 
-fn crate_label(c: &CrateId) -> String {
+fn crate_label(id: &ItemId) -> String {
     /*match c.crate_type {
         CrateType::Bin => format!("{} (bin)", c.name),
         CrateType::ProcMacro => format!("{} (proc-macro)", c.name),
         CrateType::Lib => c.name.clone(),
         CrateType::CDylib => format!("{} (cdylib)", c.name),
         CrateType::Dylib => format!("{} (dylib)", c.name),
     }*/
-    c.name.clone()
+    id.crate_name().to_owned()
 }
 
 fn item_label(item: &rustdoc_types::Item) -> String {
@@ -280,18 +269,3 @@ fn type_label(ty: &rustdoc_types::Type) -> String {
         }
     }
 }
-
-impl<'a> browser_trait::Item for Item<'a> {
-    type CrateId = CrateId;
-
-    fn crate_root() -> Self {
-        Item::Root
-    }
-
-    fn crate_id<'b>(&'b self, crate_id: &'b CrateId) -> &'b CrateId {
-        match self {
-            Item::Root | Item::Item(_) => crate_id,
-            Item::Foreign(other_crate, _) => other_crate,
-        }
-    }
-}