rustdoc: Add `CrateName` newtype (#13056)

Marshall Bowers created

This PR adds a `CrateName` newtype used to represent crate names.

This makes the code a bit more self-descriptive and prevents confusing
other string values for a crate name.

It also changes the internal representation from a `String` to an
`Arc<str>` for cheaper clones.

Release Notes:

- N/A

Change summary

Cargo.lock                                            |  1 
crates/assistant/src/slash_command/rustdoc_command.rs | 13 +++---
crates/rustdoc/Cargo.toml                             |  1 
crates/rustdoc/src/indexer.rs                         | 14 ++++---
crates/rustdoc/src/store.rs                           | 26 +++++++++---
5 files changed, 35 insertions(+), 20 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -8714,6 +8714,7 @@ dependencies = [
  "anyhow",
  "async-trait",
  "collections",
+ "derive_more",
  "fs",
  "futures 0.3.28",
  "fuzzy",

crates/assistant/src/slash_command/rustdoc_command.rs 🔗

@@ -10,8 +10,8 @@ use gpui::{AppContext, Model, Task, WeakView};
 use http::{AsyncBody, HttpClient, HttpClientWithUrl};
 use language::LspAdapterDelegate;
 use project::{Project, ProjectPath};
-use rustdoc::LocalProvider;
 use rustdoc::{convert_rustdoc_to_markdown, RustdocStore};
+use rustdoc::{CrateName, LocalProvider};
 use ui::{prelude::*, ButtonLike, ElevationIndex};
 use util::{maybe, ResultExt};
 use workspace::Workspace;
@@ -30,14 +30,14 @@ impl RustdocSlashCommand {
     async fn build_message(
         fs: Arc<dyn Fs>,
         http_client: Arc<HttpClientWithUrl>,
-        crate_name: String,
+        crate_name: CrateName,
         module_path: Vec<String>,
         path_to_cargo_toml: Option<&Path>,
     ) -> Result<(RustdocSource, String)> {
         let cargo_workspace_root = path_to_cargo_toml.and_then(|path| path.parent());
         if let Some(cargo_workspace_root) = cargo_workspace_root {
             let mut local_cargo_doc_path = cargo_workspace_root.join("target/doc");
-            local_cargo_doc_path.push(&crate_name);
+            local_cargo_doc_path.push(crate_name.as_ref());
             if !module_path.is_empty() {
                 local_cargo_doc_path.push(module_path.join("/"));
             }
@@ -144,7 +144,7 @@ impl SlashCommand for RustdocSlashCommand {
                         let provider = Box::new(LocalProvider::new(fs, cargo_workspace_root));
                         // We don't need to hold onto this task, as the `RustdocStore` will hold it
                         // until it completes.
-                        let _ = store.clone().index(crate_name.to_string(), provider);
+                        let _ = store.clone().index(crate_name.into(), provider);
                     }
                 }
             }
@@ -178,7 +178,7 @@ impl SlashCommand for RustdocSlashCommand {
             .next()
             .ok_or_else(|| anyhow!("missing crate name"))
         {
-            Ok(crate_name) => crate_name.to_string(),
+            Ok(crate_name) => CrateName::from(crate_name),
             Err(err) => return Task::ready(Err(err)),
         };
         let item_path = path_components.map(ToString::to_string).collect::<Vec<_>>();
@@ -207,7 +207,6 @@ impl SlashCommand for RustdocSlashCommand {
             }
         });
 
-        let crate_name = SharedString::from(crate_name);
         let module_path = if item_path.is_empty() {
             None
         } else {
@@ -242,7 +241,7 @@ struct RustdocPlaceholder {
     pub id: ElementId,
     pub unfold: Arc<dyn Fn(&mut WindowContext)>,
     pub source: RustdocSource,
-    pub crate_name: SharedString,
+    pub crate_name: CrateName,
     pub module_path: Option<SharedString>,
 }
 

crates/rustdoc/Cargo.toml 🔗

@@ -15,6 +15,7 @@ path = "src/rustdoc.rs"
 anyhow.workspace = true
 async-trait.workspace = true
 collections.workspace = true
+derive_more.workspace = true
 fs.workspace = true
 futures.workspace = true
 fuzzy.workspace = true

crates/rustdoc/src/indexer.rs 🔗

@@ -8,7 +8,9 @@ use fs::Fs;
 use futures::AsyncReadExt;
 use http::{AsyncBody, HttpClient, HttpClientWithUrl};
 
-use crate::{convert_rustdoc_to_markdown, RustdocDatabase, RustdocItem, RustdocItemKind};
+use crate::{
+    convert_rustdoc_to_markdown, CrateName, RustdocDatabase, RustdocItem, RustdocItemKind,
+};
 
 #[derive(Debug, Clone, Copy)]
 pub enum RustdocSource {
@@ -22,7 +24,7 @@ pub enum RustdocSource {
 pub trait RustdocProvider {
     async fn fetch_page(
         &self,
-        crate_name: &str,
+        crate_name: &CrateName,
         item: Option<&RustdocItem>,
     ) -> Result<Option<String>>;
 }
@@ -45,11 +47,11 @@ impl LocalProvider {
 impl RustdocProvider for LocalProvider {
     async fn fetch_page(
         &self,
-        crate_name: &str,
+        crate_name: &CrateName,
         item: Option<&RustdocItem>,
     ) -> Result<Option<String>> {
         let mut local_cargo_doc_path = self.cargo_workspace_root.join("target/doc");
-        local_cargo_doc_path.push(&crate_name);
+        local_cargo_doc_path.push(crate_name.as_ref());
         if let Some(item) = item {
             local_cargo_doc_path.push(item.url_path());
         } else {
@@ -78,7 +80,7 @@ impl DocsDotRsProvider {
 impl RustdocProvider for DocsDotRsProvider {
     async fn fetch_page(
         &self,
-        crate_name: &str,
+        crate_name: &CrateName,
         item: Option<&RustdocItem>,
     ) -> Result<Option<String>> {
         let version = "latest";
@@ -138,7 +140,7 @@ impl RustdocIndexer {
     }
 
     /// Indexes the crate with the given name.
-    pub async fn index(&self, crate_name: String) -> Result<()> {
+    pub async fn index(&self, crate_name: CrateName) -> Result<()> {
         let Some(crate_root_content) = self.provider.fetch_page(&crate_name, None).await? else {
             return Ok(());
         };

crates/rustdoc/src/store.rs 🔗

@@ -4,6 +4,7 @@ use std::sync::Arc;
 
 use anyhow::{anyhow, Result};
 use collections::HashMap;
+use derive_more::{Deref, Display};
 use futures::future::{self, BoxFuture, Shared};
 use futures::FutureExt;
 use fuzzy::StringMatchCandidate;
@@ -18,6 +19,16 @@ use util::ResultExt;
 use crate::indexer::{RustdocIndexer, RustdocProvider};
 use crate::{RustdocItem, RustdocItemKind};
 
+/// The name of a crate.
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Deref, Display)]
+pub struct CrateName(Arc<str>);
+
+impl From<&str> for CrateName {
+    fn from(value: &str) -> Self {
+        Self(value.into())
+    }
+}
+
 struct GlobalRustdocStore(Arc<RustdocStore>);
 
 impl Global for GlobalRustdocStore {}
@@ -25,7 +36,8 @@ impl Global for GlobalRustdocStore {}
 pub struct RustdocStore {
     executor: BackgroundExecutor,
     database_future: Shared<BoxFuture<'static, Result<Arc<RustdocDatabase>, Arc<anyhow::Error>>>>,
-    indexing_tasks_by_crate: RwLock<HashMap<String, Shared<Task<Result<(), Arc<anyhow::Error>>>>>>,
+    indexing_tasks_by_crate:
+        RwLock<HashMap<CrateName, Shared<Task<Result<(), Arc<anyhow::Error>>>>>>,
 }
 
 impl RustdocStore {
@@ -61,7 +73,7 @@ impl RustdocStore {
 
     pub async fn load(
         &self,
-        crate_name: String,
+        crate_name: CrateName,
         item_path: Option<String>,
     ) -> Result<RustdocDatabaseEntry> {
         self.database_future
@@ -74,7 +86,7 @@ impl RustdocStore {
 
     pub fn index(
         self: Arc<Self>,
-        crate_name: String,
+        crate_name: CrateName,
         provider: Box<dyn RustdocProvider + Send + Sync + 'static>,
     ) -> Shared<Task<Result<(), Arc<anyhow::Error>>>> {
         let indexing_task = self
@@ -215,7 +227,7 @@ impl RustdocDatabase {
 
     pub fn load(
         &self,
-        crate_name: String,
+        crate_name: CrateName,
         item_path: Option<String>,
     ) -> Task<Result<RustdocDatabaseEntry>> {
         let env = self.env.clone();
@@ -223,7 +235,7 @@ impl RustdocDatabase {
         let item_path = if let Some(item_path) = item_path {
             format!("{crate_name}::{item_path}")
         } else {
-            crate_name
+            crate_name.to_string()
         };
 
         self.executor.spawn(async move {
@@ -236,7 +248,7 @@ impl RustdocDatabase {
 
     pub fn insert(
         &self,
-        crate_name: String,
+        crate_name: CrateName,
         item: Option<&RustdocItem>,
         docs: String,
     ) -> Task<Result<()>> {
@@ -251,7 +263,7 @@ impl RustdocDatabase {
                 },
             )
         } else {
-            (crate_name, RustdocDatabaseEntry::Crate { docs })
+            (crate_name.to_string(), RustdocDatabaseEntry::Crate { docs })
         };
 
         self.executor.spawn(async move {