From 44f66aa426b6f641539ab24ffca948f2f2228a8f Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Fri, 14 Jun 2024 12:21:03 -0400 Subject: [PATCH] rustdoc: Add `CrateName` newtype (#13056) 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` for cheaper clones. Release Notes: - N/A --- Cargo.lock | 1 + .../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(-) diff --git a/Cargo.lock b/Cargo.lock index 3aeb7dede6..ed9a82fa75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8714,6 +8714,7 @@ dependencies = [ "anyhow", "async-trait", "collections", + "derive_more", "fs", "futures 0.3.28", "fuzzy", diff --git a/crates/assistant/src/slash_command/rustdoc_command.rs b/crates/assistant/src/slash_command/rustdoc_command.rs index 592bfcb20b..b11bb8d54d 100644 --- a/crates/assistant/src/slash_command/rustdoc_command.rs +++ b/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, http_client: Arc, - crate_name: String, + crate_name: CrateName, module_path: Vec, 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::>(); @@ -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, pub source: RustdocSource, - pub crate_name: SharedString, + pub crate_name: CrateName, pub module_path: Option, } diff --git a/crates/rustdoc/Cargo.toml b/crates/rustdoc/Cargo.toml index e99f485636..f60fda46f8 100644 --- a/crates/rustdoc/Cargo.toml +++ b/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 diff --git a/crates/rustdoc/src/indexer.rs b/crates/rustdoc/src/indexer.rs index 2a7e973196..79c24629da 100644 --- a/crates/rustdoc/src/indexer.rs +++ b/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>; } @@ -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> { 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> { 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(()); }; diff --git a/crates/rustdoc/src/store.rs b/crates/rustdoc/src/store.rs index 8c5d2615c2..50ebf83510 100644 --- a/crates/rustdoc/src/store.rs +++ b/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); + +impl From<&str> for CrateName { + fn from(value: &str) -> Self { + Self(value.into()) + } +} + struct GlobalRustdocStore(Arc); impl Global for GlobalRustdocStore {} @@ -25,7 +36,8 @@ impl Global for GlobalRustdocStore {} pub struct RustdocStore { executor: BackgroundExecutor, database_future: Shared, Arc>>>, - indexing_tasks_by_crate: RwLock>>>>>, + indexing_tasks_by_crate: + RwLock>>>>>, } impl RustdocStore { @@ -61,7 +73,7 @@ impl RustdocStore { pub async fn load( &self, - crate_name: String, + crate_name: CrateName, item_path: Option, ) -> Result { self.database_future @@ -74,7 +86,7 @@ impl RustdocStore { pub fn index( self: Arc, - crate_name: String, + crate_name: CrateName, provider: Box, ) -> Shared>>> { let indexing_task = self @@ -215,7 +227,7 @@ impl RustdocDatabase { pub fn load( &self, - crate_name: String, + crate_name: CrateName, item_path: Option, ) -> Task> { 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> { @@ -251,7 +263,7 @@ impl RustdocDatabase { }, ) } else { - (crate_name, RustdocDatabaseEntry::Crate { docs }) + (crate_name.to_string(), RustdocDatabaseEntry::Crate { docs }) }; self.executor.spawn(async move {