Merge pull request #349 from grantlemons/ls-error-improvements

refactor: Harper-ls Error-Handling Improvements

harper-ls/src/backend.rs

@@ -2,7 +2,7 @@ use std::collections::HashMap;
 use std::path::{Component, PathBuf};
 use std::sync::Arc;
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
 use harper_comments::CommentParser;
 use harper_core::linting::{LintGroup, Linter};
 use harper_core::parsers::{CollapseIdentifiers, IsolateEnglish, Markdown, Parser, PlainEnglish};
@@ -55,58 +55,70 @@ impl Backend {
 
     /// Rewrites a path to a filename using the same conventions as
     /// [Neovim's undo-files](https://neovim.io/doc/user/options.html#'undodir').
-    fn file_dict_name(url: &Url) -> Option<PathBuf> {
+    fn file_dict_name(url: &Url) -> anyhow::Result<PathBuf> {
         let mut rewritten = String::new();
 
         // We assume all URLs are local files and have a base.
-        for seg in url.to_file_path().ok()?.components() {
+        for seg in url
+            .to_file_path()
+            .map_err(|_| anyhow!("Unable to convert URL to file path."))?
+            .components()
+        {
             if !matches!(seg, Component::RootDir) {
                 rewritten.push_str(&seg.as_os_str().to_string_lossy());
                 rewritten.push('%');
             }
         }
 
-        Some(rewritten.into())
+        Ok(rewritten.into())
     }
 
     /// Get the location of the file's specific dictionary
-    async fn get_file_dict_path(&self, url: &Url) -> Option<PathBuf> {
+    async fn get_file_dict_path(&self, url: &Url) -> anyhow::Result<PathBuf> {
         let config = self.config.read().await;
 
-        Some(config.file_dict_path.join(Self::file_dict_name(url)?))
+        Ok(config.file_dict_path.join(Self::file_dict_name(url)?))
     }
 
-    /// Load a speCific file's dictionary
-    async fn load_file_dictionary(&self, url: &Url) -> Option<FullDictionary> {
-        match load_dict(self.get_file_dict_path(url).await?).await {
-            Ok(dict) => Some(dict),
-            Err(_err) => Some(FullDictionary::new()),
-        }
+    /// Load a specific file's dictionary
+    async fn load_file_dictionary(&self, url: &Url) -> anyhow::Result<FullDictionary> {
+        let path = self
+            .get_file_dict_path(url)
+            .await
+            .context("Unable to get the file path.")?;
+
+        load_dict(path)
+            .await
+            .map_err(|err| info!("{err}"))
+            .or(Ok(FullDictionary::new()))
     }
 
     async fn save_file_dictionary(&self, url: &Url, dict: impl Dictionary) -> Result<()> {
-        Ok(save_dict(
+        save_dict(
             self.get_file_dict_path(url)
                 .await
-                .ok_or(anyhow!("Could not compute dictionary path."))?,
+                .context("Unable to get the file path.")?,
             dict,
         )
-        .await?)
+        .await
+        .context("Unable to save the dictionary to path.")
     }
 
     async fn load_user_dictionary(&self) -> FullDictionary {
         let config = self.config.read().await;
 
-        match load_dict(&config.user_dict_path).await {
-            Ok(dict) => dict,
-            Err(_err) => FullDictionary::new(),
-        }
+        load_dict(&config.user_dict_path)
+            .await
+            .map_err(|err| info!("{err}"))
+            .unwrap_or(FullDictionary::new())
     }
 
     async fn save_user_dictionary(&self, dict: impl Dictionary) -> Result<()> {
         let config = self.config.read().await;
 
-        Ok(save_dict(&config.user_dict_path, dict).await?)
+        save_dict(&config.user_dict_path, dict)
+            .await
+            .map_err(|err| anyhow!("Unable to save the dictionary to file: {err}"))
     }
 
     async fn generate_global_dictionary(&self) -> Result<MergedDictionary> {
@@ -123,29 +135,22 @@ impl Backend {
             self.load_file_dictionary(url)
         );
 
-        let Some(file_dictionary) = file_dictionary else {
-            return Err(anyhow!("Unable to compute dictionary path for {url}."));
-        };
-
-        let mut global_dictionary = global_dictionary?;
-        global_dictionary.add_dictionary(Arc::new(file_dictionary));
+        let mut global_dictionary =
+            global_dictionary.context("Unable to load the global dictionary.")?;
+        global_dictionary.add_dictionary(Arc::new(
+            file_dictionary.context("Unable to load the file dictionary.")?,
+        ));
 
         Ok(global_dictionary)
     }
 
     async fn update_document_from_file(&self, url: &Url, language_id: Option<&str>) -> Result<()> {
-        let content = match tokio::fs::read_to_string(
+        let content = tokio::fs::read_to_string(
             url.to_file_path()
-                .map_err(|_| anyhow::format_err!("Could not extract file path."))?,
+                .map_err(|_| anyhow!("Unable to convert URL to file path."))?,
         )
         .await
-        {
-            Ok(content) => content,
-            Err(err) => {
-                error!("Error updating document from file: {}", err);
-                return Ok(());
-            }
-        };
+        .with_context(|| format!("Unable to read from file {:?}", url))?;
 
         self.update_document(url, &content, language_id).await
     }
@@ -161,7 +166,11 @@ impl Backend {
         let mut doc_lock = self.doc_state.lock().await;
         let config_lock = self.config.read().await;
 
-        let dict = Arc::new(self.generate_file_dictionary(url).await?);
+        let dict = Arc::new(
+            self.generate_file_dictionary(url)
+                .await
+                .context("Unable to generate the file dictionary.")?,
+        );
 
         let doc_state = doc_lock.entry(url.clone()).or_insert(DocumentState {
             linter: LintGroup::new(config_lock.lint_config, dict.clone()),
@@ -345,15 +354,7 @@ impl Backend {
     /// Update the configuration of the server and publish document updates that
     /// match it.
     async fn update_config_from_obj(&self, json_obj: Value) {
-        let new_config = match Config::from_lsp_config(json_obj) {
-            Ok(new_config) => new_config,
-            Err(err) => {
-                error!("Unable to change config: {}", err);
-                return;
-            }
-        };
-
-        {
+        if let Ok(new_config) = Config::from_lsp_config(json_obj).map_err(|err| error!("{err}")) {
             let mut config = self.config.write().await;
             *config = new_config;
         }
@@ -434,21 +435,23 @@ impl LanguageServer for Backend {
     }
 
     async fn did_save(&self, params: DidSaveTextDocumentParams) {
-        let _ = self
-            .update_document_from_file(&params.text_document.uri, None)
-            .await;
+        self.update_document_from_file(&params.text_document.uri, None)
+            .await
+            .map_err(|err| error!("{err}"))
+            .err();
 
         self.publish_diagnostics(&params.text_document.uri).await;
     }
 
     async fn did_open(&self, params: DidOpenTextDocumentParams) {
-        let _ = self
-            .update_document(
-                &params.text_document.uri,
-                &params.text_document.text,
-                Some(&params.text_document.language_id),
-            )
-            .await;
+        self.update_document(
+            &params.text_document.uri,
+            &params.text_document.text,
+            Some(&params.text_document.language_id),
+        )
+        .await
+        .map_err(|err| error!("{err}"))
+        .err();
 
         self.publish_diagnostics(&params.text_document.uri).await;
     }
@@ -526,8 +529,14 @@ impl LanguageServer for Backend {
 
                 let mut dict = self.load_user_dictionary().await;
                 dict.append_word(word, WordMetadata::default());
-                let _ = self.save_user_dictionary(dict).await;
-                let _ = self.update_document_from_file(&file_url, None).await;
+                self.save_user_dictionary(dict)
+                    .await
+                    .map_err(|err| error!("{err}"))
+                    .err();
+                self.update_document_from_file(&file_url, None)
+                    .await
+                    .map_err(|err| error!("{err}"))
+                    .err();
                 self.publish_diagnostics(&file_url).await;
             }
             "HarperAddToFileDict" => {
@@ -539,14 +548,26 @@ impl LanguageServer for Backend {
 
                 let file_url = second.parse().unwrap();
 
-                let Some(mut dict) = self.load_file_dictionary(&file_url).await else {
-                    error!("Unable resolve dictionary path: {file_url}");
-                    return Ok(None);
+                let mut dict = match self
+                    .load_file_dictionary(&file_url)
+                    .await
+                    .map_err(|err| error!("{err}"))
+                {
+                    Ok(dict) => dict,
+                    Err(_) => {
+                        return Ok(None);
+                    }
                 };
                 dict.append_word(word, WordMetadata::default());
 
-                let _ = self.save_file_dictionary(&file_url, dict).await;
-                let _ = self.update_document_from_file(&file_url, None).await;
+                self.save_file_dictionary(&file_url, dict)
+                    .await
+                    .map_err(|err| error!("{err}"))
+                    .err();
+                self.update_document_from_file(&file_url, None)
+                    .await
+                    .map_err(|err| error!("{err}"))
+                    .err();
                 self.publish_diagnostics(&file_url).await;
             }
             "HarperOpen" => match open::that(&first) {
@@ -585,7 +606,10 @@ impl LanguageServer for Backend {
         };
 
         for url in urls {
-            let _ = self.update_document_from_file(&url, None).await;
+            self.update_document_from_file(&url, None)
+                .await
+                .map_err(|err| error!("{err}"))
+                .err();
             self.publish_diagnostics(&url).await;
         }
     }

harper-ls/src/config.rs

@@ -1,5 +1,6 @@
 use std::path::PathBuf;
 
+use anyhow::{bail, Result};
 use dirs::{config_dir, data_local_dir};
 use harper_core::linting::LintGroupConfig;
 use resolve_path::PathResolveExt;
@@ -42,18 +43,16 @@ pub struct CodeActionConfig {
 }
 
 impl CodeActionConfig {
-    pub fn from_lsp_config(value: Value) -> anyhow::Result<Self> {
+    pub fn from_lsp_config(value: Value) -> Result<Self> {
         let mut base = CodeActionConfig::default();
 
         let Value::Object(value) = value else {
-            return Err(anyhow::format_err!(
-                "The code action configuration must be an object."
-            ));
+            bail!("The code action configuration must be an object.");
         };
 
         if let Some(force_stable_val) = value.get("forceStable") {
             let Value::Bool(force_stable) = force_stable_val else {
-                return Err(anyhow::format_err!("forceStable must be a boolean value."));
+                bail!("forceStable must be a boolean value.");
             };
             base.force_stable = *force_stable;
         };
@@ -73,32 +72,30 @@ pub struct Config {
 }
 
 impl Config {
-    pub fn from_lsp_config(value: Value) -> anyhow::Result<Self> {
+    pub fn from_lsp_config(value: Value) -> Result<Self> {
         let mut base = Config::default();
 
         let Value::Object(value) = value else {
-            return Err(anyhow::format_err!("Settings must be an object."));
+            bail!("Settings must be an object.");
         };
 
         let Some(Value::Object(value)) = value.get("harper-ls") else {
-            return Err(anyhow::format_err!(
-                "Settings must contain a \"harper-ls\" key."
-            ));
+            bail!("Settings must contain a \"harper-ls\" key.");
         };
 
         if let Some(v) = value.get("userDictPath") {
             if let Value::String(path) = v {
                 base.user_dict_path = path.try_resolve()?.to_path_buf();
             } else {
-                return Err(anyhow::format_err!("userDict path must be a string."));
+                bail!("userDict path must be a string.");
             }
         }
 
         if let Some(v) = value.get("fileDictPath") {
             if let Value::String(path) = v {
                 base.file_dict_path = path.try_resolve()?.to_path_buf();
             } else {
-                return Err(anyhow::format_err!("fileDict path must be a string."));
+                bail!("fileDict path must be a string.");
             }
         }
 
@@ -118,9 +115,7 @@ impl Config {
             if let Value::Bool(v) = v {
                 base.isolate_english = *v;
             } else {
-                return Err(anyhow::format_err!(
-                    "isolateEnglish path must be a boolean."
-                ));
+                bail!("isolateEnglish path must be a boolean.");
             }
         }
 

harper-ls/src/dictionary_io.rs

@@ -2,11 +2,11 @@ use std::path::Path;
 
 use harper_core::{Dictionary, FullDictionary, WordMetadata};
 use tokio::fs::{self, File};
-use tokio::io::{self, AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt, BufReader, BufWriter};
+use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt, BufReader, BufWriter, Result};
 
 /// Save the contents of a dictionary to a file.
 /// Ensures that the path to the destination exists.
-pub async fn save_dict(path: impl AsRef<Path>, dict: impl Dictionary) -> io::Result<()> {
+pub async fn save_dict(path: impl AsRef<Path>, dict: impl Dictionary) -> Result<()> {
     if let Some(parent) = path.as_ref().parent() {
         fs::create_dir_all(parent).await?;
     }
@@ -15,13 +15,12 @@ pub async fn save_dict(path: impl AsRef<Path>, dict: impl Dictionary) -> io::Res
     let mut write = BufWriter::new(file);
 
     write_word_list(dict, &mut write).await?;
-
     write.flush().await?;
 
     Ok(())
 }
 
-async fn write_word_list(dict: impl Dictionary, mut w: impl AsyncWrite + Unpin) -> io::Result<()> {
+async fn write_word_list(dict: impl Dictionary, mut w: impl AsyncWrite + Unpin) -> Result<()> {
     let mut cur_str = String::new();
 
     for word in dict.words_iter() {
@@ -35,7 +34,7 @@ async fn write_word_list(dict: impl Dictionary, mut w: impl AsyncWrite + Unpin)
     Ok(())
 }
 
-pub async fn load_dict(path: impl AsRef<Path>) -> io::Result<FullDictionary> {
+pub async fn load_dict(path: impl AsRef<Path>) -> Result<FullDictionary> {
     let file = File::open(path.as_ref()).await?;
     let read = BufReader::new(file);
 
@@ -44,7 +43,7 @@ pub async fn load_dict(path: impl AsRef<Path>) -> io::Result<FullDictionary> {
 
 /// This function could definitely be optimized to use less memory.
 /// Right now it isn't an issue.
-async fn dict_from_word_list(mut r: impl AsyncRead + Unpin) -> io::Result<FullDictionary> {
+async fn dict_from_word_list(mut r: impl AsyncRead + Unpin) -> Result<FullDictionary> {
     let mut str = String::new();
 
     r.read_to_string(&mut str).await?;