From cc4a37b00673417ebd0cf8fa745ac7572fb3826c Mon Sep 17 00:00:00 2001 From: Rahul Garg Date: Sun, 15 Oct 2023 21:44:30 -0600 Subject: [PATCH 1/2] Cleanups for clippy and text_store Summary ---- - The most notable change here is the removal of unnecessary clones as suggested by clippy. Furthermore, this also makes the TextStore a wrapper type of a `HashMap` and implements Deref and DerefMut for it. - Workflow update to ensure that clippy runs in _more_ exhaustive way (this is actually how I found about the unnecessary cloning). --- .github/workflows/ci.yml | 2 +- lsp/src/handle.rs | 8 +------- lsp/src/htmx/mod.rs | 2 +- lsp/src/text_store.rs | 28 ++++++++++++++++++++-------- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d5ad52..4e201d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: uses: ./.github/actions/setup-rust-env - name: Rust clippy - run: cargo clippy -- -D warnings + run: cargo clippy -- -Dclippy::all -D warnings rustfmt: name: Format diff --git a/lsp/src/handle.rs b/lsp/src/handle.rs index 7a62f24..b83f50b 100644 --- a/lsp/src/handle.rs +++ b/lsp/src/handle.rs @@ -74,7 +74,6 @@ fn handle_didChange(noti: Notification) -> Option { .expect("text store not initialized") .lock() .expect("text store mutex poisoned") - .texts .insert(uri, text); None @@ -96,11 +95,7 @@ fn handle_didOpen(noti: Notification) -> Option { .expect("text store not initialized") .lock() .expect("text store mutex poisoned") - .texts - .insert( - text_document_changes.uri, - text_document_changes.text.to_string(), - ); + .insert(text_document_changes.uri, text_document_changes.text); None } @@ -210,7 +205,6 @@ mod tests { .expect("text store not initialized") .lock() .expect("text store mutex poisoned") - .texts .insert(file.to_string(), content.to_string()); } diff --git a/lsp/src/htmx/mod.rs b/lsp/src/htmx/mod.rs index 1da3d7e..08ea78c 100644 --- a/lsp/src/htmx/mod.rs +++ b/lsp/src/htmx/mod.rs @@ -62,7 +62,7 @@ pub fn hx_completion(text_params: TextDocumentPositionParams) -> Option Option { - let result = crate::tree_sitter::get_position_from_lsp_completion(text_params.clone())?; + let result = crate::tree_sitter::get_position_from_lsp_completion(text_params)?; debug!("handle_hover result: {:?}", result); match result { diff --git a/lsp/src/text_store.rs b/lsp/src/text_store.rs index 9839b67..27ca3e9 100644 --- a/lsp/src/text_store.rs +++ b/lsp/src/text_store.rs @@ -1,28 +1,40 @@ use std::{ collections::HashMap, + ops::{Deref, DerefMut}, sync::{Arc, Mutex, OnceLock}, }; use lsp_types::Url; -pub struct TextStore { - pub texts: HashMap, +type TxtStore = HashMap; + +pub struct TextStore(TxtStore); + +impl Deref for TextStore { + type Target = TxtStore; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for TextStore { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } } pub static TEXT_STORE: OnceLock>> = OnceLock::new(); + pub fn init_text_store() { - _ = TEXT_STORE.set(Arc::new(Mutex::new(TextStore { - texts: HashMap::new(), - }))); + _ = TEXT_STORE.set(Arc::new(Mutex::new(TextStore(HashMap::new())))); } pub fn get_text_document(uri: Url) -> Option { - return TEXT_STORE + TEXT_STORE .get() .expect("text store not initialized") .lock() .expect("text store mutex poisoned") - .texts .get(&uri.to_string()) - .cloned(); + .cloned() } From ba4e3243c82cf489f171a37d5e36e1737510e237 Mon Sep 17 00:00:00 2001 From: Rahul Garg Date: Thu, 30 Nov 2023 00:32:38 -0600 Subject: [PATCH 2/2] Review cmt: explicit return --- lsp/src/text_store.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lsp/src/text_store.rs b/lsp/src/text_store.rs index 27ca3e9..d7ed9e5 100644 --- a/lsp/src/text_store.rs +++ b/lsp/src/text_store.rs @@ -30,11 +30,11 @@ pub fn init_text_store() { } pub fn get_text_document(uri: Url) -> Option { - TEXT_STORE + return TEXT_STORE .get() .expect("text store not initialized") .lock() .expect("text store mutex poisoned") .get(&uri.to_string()) - .cloned() + .cloned(); }