Avoid panics in LSP store path handling (#42117)

Lay Sheth created 3 days ago

Release Notes:

- Fixed incorrect journal paths handling

Change summary

crates/journal/src/journal.rs   | 73 +++++++++++++++++++++++++++++++++-
crates/project/src/lsp_store.rs |  5 -
docs/src/configuring-zed.md     |  3 
3 files changed, 74 insertions(+), 7 deletions(-)

Detailed changes

crates/journal/src/journal.rs

@@ -173,9 +173,15 @@ pub fn new_journal_entry(workspace: &Workspace, window: &mut Window, cx: &mut Ap
 }
 
 fn journal_dir(path: &str) -> Option<PathBuf> {
-    shellexpand::full(path) //TODO handle this better
-        .ok()
-        .map(|dir| Path::new(&dir.to_string()).to_path_buf().join("journal"))
+    let expanded = shellexpand::full(path).ok()?;
+    let base_path = Path::new(expanded.as_ref());
+    let absolute_path = if base_path.is_absolute() {
+        base_path.to_path_buf()
+    } else {
+        log::warn!("Invalid journal path {path:?} (not absolute), falling back to home directory",);
+        std::env::home_dir()?
+    };
+    Some(absolute_path.join("journal"))
 }
 
 fn heading_entry(now: NaiveTime, hour_format: &HourFormat) -> String {
@@ -224,4 +230,65 @@ mod tests {
             assert_eq!(actual_heading_entry, expected_heading_entry);
         }
     }
+
+    mod journal_dir_tests {
+        use super::super::*;
+
+        #[test]
+        #[cfg(target_family = "unix")]
+        fn test_absolute_unix_path() {
+            let result = journal_dir("/home/user");
+            assert!(result.is_some());
+            let path = result.unwrap();
+            assert!(path.is_absolute());
+            assert_eq!(path, PathBuf::from("/home/user/journal"));
+        }
+
+        #[test]
+        fn test_tilde_expansion() {
+            let result = journal_dir("~/documents");
+            assert!(result.is_some());
+            let path = result.unwrap();
+
+            assert!(path.is_absolute(), "Tilde should expand to absolute path");
+
+            if let Some(home) = std::env::home_dir() {
+                assert_eq!(path, home.join("documents").join("journal"));
+            }
+        }
+
+        #[test]
+        fn test_relative_path_falls_back_to_home() {
+            for relative_path in ["relative/path", "NONEXT/some/path", "../some/path"] {
+                let result = journal_dir(relative_path);
+                assert!(result.is_some(), "Failed for path: {}", relative_path);
+                let path = result.unwrap();
+
+                assert!(
+                    path.is_absolute(),
+                    "Path should be absolute for input '{}', got: {:?}",
+                    relative_path,
+                    path
+                );
+
+                if let Some(home) = std::env::home_dir() {
+                    assert_eq!(
+                        path,
+                        home.join("journal"),
+                        "Should fall back to home directory for input '{}'",
+                        relative_path
+                    );
+                }
+            }
+        }
+
+        #[test]
+        #[cfg(target_os = "windows")]
+        fn test_absolute_path_windows_style() {
+            let result = journal_dir("C:\\Users\\user\\Documents");
+            assert!(result.is_some());
+            let path = result.unwrap();
+            assert_eq!(path, PathBuf::from("C:\\Users\\user\\Documents\\journal"));
+        }
+    }
 }

crates/project/src/lsp_store.rs

@@ -7654,14 +7654,13 @@ impl LspStore {
         let uri = lsp::Uri::from_file_path(&abs_path)
             .ok()
             .with_context(|| format!("Failed to convert path to URI: {}", abs_path.display()))
-            .unwrap();
+            .log_err()?;
         let next_snapshot = buffer.text_snapshot();
         for language_server in language_servers {
             let language_server = language_server.clone();
 
             let buffer_snapshots = self
-                .as_local_mut()
-                .unwrap()
+                .as_local_mut()?
                 .buffer_snapshots
                 .get_mut(&buffer.remote_id())
                 .and_then(|m| m.get_mut(&language_server.server_id()))?;

docs/src/configuring-zed.md

@@ -2519,11 +2519,12 @@ Unspecified values have a `false` value, hints won't be toggled if all the modif
   "path": "~",
   "hour_format": "hour12"
 }
+
 ```
 
 ### Path
 
-- Description: The path of the directory where journal entries are stored.
+- Description: The path of the directory where journal entries are stored. If an invalid path is specified, the journal will fall back to using `~` (the home directory).
 - Setting: `path`
 - Default: `~`