Convert LSP URIs into custom URIs

This introduces a custom URI type in core meant to be extended later
if we want to support other schemes. For now it's just a wrapper over a
PathBuf. We use this new URI type to firewall `lsp::Url`. This was
previously done in 8141a4a but using a custom URI type is more flexible
and will improve the way Pickers handle paths for previews in the child
commit(s).

Co-authored-by: soqb <cb.setho@gmail.com>

Cargo.lock

@@ -1331,6 +1331,7 @@ dependencies = [
  "unicode-general-category",
  "unicode-segmentation",
  "unicode-width",
+ "url",
 ]
 
 [[package]]

helix-core/Cargo.toml

@@ -34,6 +34,7 @@ bitflags = "2.6"
 ahash = "0.8.11"
 hashbrown = { version = "0.14.5", features = ["raw"] }
 dunce = "1.0"
+url = "2.5.0"
 
 log = "0.4"
 serde = { version = "1.0", features = ["derive"] }

helix-core/src/lib.rs

@@ -27,6 +27,7 @@ pub mod test;
 pub mod text_annotations;
 pub mod textobject;
 mod transaction;
+pub mod uri;
 pub mod wrap;
 
 pub mod unicode {
@@ -66,3 +67,5 @@ pub use diagnostic::Diagnostic;
 
 pub use line_ending::{LineEnding, NATIVE_LINE_ENDING};
 pub use transaction::{Assoc, Change, ChangeSet, Deletion, Operation, Transaction};
+
+pub use uri::Uri;

helix-core/src/uri.rs

@@ -0,0 +1,122 @@
+use std::path::{Path, PathBuf};
+
+/// A generic pointer to a file location.
+///
+/// Currently this type only supports paths to local files.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[non_exhaustive]
+pub enum Uri {
+    File(PathBuf),
+}
+
+impl Uri {
+    // This clippy allow mirrors url::Url::from_file_path
+    #[allow(clippy::result_unit_err)]
+    pub fn to_url(&self) -> Result<url::Url, ()> {
+        match self {
+            Uri::File(path) => url::Url::from_file_path(path),
+        }
+    }
+
+    pub fn as_path(&self) -> Option<&Path> {
+        match self {
+            Self::File(path) => Some(path),
+        }
+    }
+
+    pub fn as_path_buf(self) -> Option<PathBuf> {
+        match self {
+            Self::File(path) => Some(path),
+        }
+    }
+}
+
+impl From<PathBuf> for Uri {
+    fn from(path: PathBuf) -> Self {
+        Self::File(path)
+    }
+}
+
+impl TryFrom<Uri> for PathBuf {
+    type Error = ();
+
+    fn try_from(uri: Uri) -> Result<Self, Self::Error> {
+        match uri {
+            Uri::File(path) => Ok(path),
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct UrlConversionError {
+    source: url::Url,
+    kind: UrlConversionErrorKind,
+}
+
+#[derive(Debug)]
+pub enum UrlConversionErrorKind {
+    UnsupportedScheme,
+    UnableToConvert,
+}
+
+impl std::fmt::Display for UrlConversionError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self.kind {
+            UrlConversionErrorKind::UnsupportedScheme => {
+                write!(f, "unsupported scheme in URL: {}", self.source.scheme())
+            }
+            UrlConversionErrorKind::UnableToConvert => {
+                write!(f, "unable to convert URL to file path: {}", self.source)
+            }
+        }
+    }
+}
+
+impl std::error::Error for UrlConversionError {}
+
+fn convert_url_to_uri(url: &url::Url) -> Result<Uri, UrlConversionErrorKind> {
+    if url.scheme() == "file" {
+        url.to_file_path()
+            .map(|path| Uri::File(helix_stdx::path::normalize(path)))
+            .map_err(|_| UrlConversionErrorKind::UnableToConvert)
+    } else {
+        Err(UrlConversionErrorKind::UnsupportedScheme)
+    }
+}
+
+impl TryFrom<url::Url> for Uri {
+    type Error = UrlConversionError;
+
+    fn try_from(url: url::Url) -> Result<Self, Self::Error> {
+        convert_url_to_uri(&url).map_err(|kind| Self::Error { source: url, kind })
+    }
+}
+
+impl TryFrom<&url::Url> for Uri {
+    type Error = UrlConversionError;
+
+    fn try_from(url: &url::Url) -> Result<Self, Self::Error> {
+        convert_url_to_uri(url).map_err(|kind| Self::Error {
+            source: url.clone(),
+            kind,
+        })
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use url::Url;
+
+    #[test]
+    fn unknown_scheme() {
+        let url = Url::parse("csharp:/metadata/foo/bar/Baz.cs").unwrap();
+        assert!(matches!(
+            Uri::try_from(url),
+            Err(UrlConversionError {
+                kind: UrlConversionErrorKind::UnsupportedScheme,
+                ..
+            })
+        ));
+    }
+}

helix-term/src/application.rs

@@ -735,10 +735,10 @@ impl Application {
                         }
                     }
                     Notification::PublishDiagnostics(mut params) => {
-                        let path = match params.uri.to_file_path() {
+                        let uri = match helix_core::Uri::try_from(params.uri) {
-                            Ok(path) => helix_stdx::path::normalize(path),
+                            Ok(uri) => uri,
-                            Err(_) => {
+                            Err(err) => {
-                                log::error!("Unsupported file URI: {}", params.uri);
+                                log::error!("{err}");
                                 return;
                             }
                         };
@@ -749,11 +749,11 @@ impl Application {
                         }
                         // have to inline the function because of borrow checking...
                         let doc = self.editor.documents.values_mut()
-                            .find(|doc| doc.path().map(|p| p == &path).unwrap_or(false))
+                            .find(|doc| doc.uri().is_some_and(|u| u == uri))
                             .filter(|doc| {
                                 if let Some(version) = params.version {
                                     if version != doc.version() {
-                                        log::info!("Version ({version}) is out of date for {path:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
+                                        log::info!("Version ({version}) is out of date for {uri:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
                                         return false;
                                     }
                                 }
@@ -765,7 +765,7 @@ impl Application {
                             let lang_conf = doc.language.clone();
 
                             if let Some(lang_conf) = &lang_conf {
-                                if let Some(old_diagnostics) = self.editor.diagnostics.get(&path) {
+                                if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) {
                                     if !lang_conf.persistent_diagnostic_sources.is_empty() {
                                         // Sort diagnostics first by severity and then by line numbers.
                                         // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
@@ -798,7 +798,7 @@ impl Application {
                         // Insert the original lsp::Diagnostics here because we may have no open document
                         // for diagnosic message and so we can't calculate the exact position.
                         // When using them later in the diagnostics picker, we calculate them on-demand.
-                        let diagnostics = match self.editor.diagnostics.entry(path) {
+                        let diagnostics = match self.editor.diagnostics.entry(uri) {
                             Entry::Occupied(o) => {
                                 let current_diagnostics = o.into_mut();
                                 // there may entries of other language servers, which is why we can't overwrite the whole entry
@@ -1132,20 +1132,22 @@ impl Application {
             ..
         } = params;
 
-        let path = match uri.to_file_path() {
+        let uri = match helix_core::Uri::try_from(uri) {
-            Ok(path) => path,
+            Ok(uri) => uri,
             Err(err) => {
-                log::error!("unsupported file URI: {}: {:?}", uri, err);
+                log::error!("{err}");
                 return lsp::ShowDocumentResult { success: false };
             }
         };
+        // If `Uri` gets another variant other than `Path` this may not be valid.
+        let path = uri.as_path().expect("URIs are valid paths");
 
         let action = match take_focus {
             Some(true) => helix_view::editor::Action::Replace,
             _ => helix_view::editor::Action::VerticalSplit,
         };
 
-        let doc_id = match self.editor.open(&path, action) {
+        let doc_id = match self.editor.open(path, action) {
             Ok(id) => id,
             Err(err) => {
                 log::error!("failed to open path: {:?}: {:?}", uri, err);

helix-term/src/commands/lsp.rs

@@ -13,7 +13,9 @@ use tui::{text::Span, widgets::Row};
 
 use super::{align_view, push_jump, Align, Context, Editor};
 
-use helix_core::{syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection};
+use helix_core::{
+    syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection, Uri,
+};
 use helix_stdx::path;
 use helix_view::{
     document::{DocumentInlayHints, DocumentInlayHintsId},
@@ -34,7 +36,7 @@ use std::{
     collections::{BTreeMap, HashSet},
     fmt::Write,
     future::Future,
-    path::{Path, PathBuf},
+    path::Path,
 };
 
 /// Gets the first language server that is attached to a document which supports a specific feature.
@@ -72,7 +74,7 @@ struct DiagnosticStyles {
 }
 
 struct PickerDiagnostic {
-    path: PathBuf,
+    uri: Uri,
     diag: lsp::Diagnostic,
     offset_encoding: OffsetEncoding,
 }
@@ -183,20 +185,20 @@ type DiagnosticsPicker = Picker<PickerDiagnostic, DiagnosticStyles>;
 
 fn diag_picker(
     cx: &Context,
-    diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+    diagnostics: BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
     format: DiagnosticsFormat,
 ) -> DiagnosticsPicker {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
 
     // flatten the map to a vec of (url, diag) pairs
     let mut flat_diag = Vec::new();
-    for (path, diags) in diagnostics {
+    for (uri, diags) in diagnostics {
         flat_diag.reserve(diags.len());
 
         for (diag, ls) in diags {
             if let Some(ls) = cx.editor.language_server_by_id(ls) {
                 flat_diag.push(PickerDiagnostic {
-                    path: path.clone(),
+                    uri: uri.clone(),
                     diag,
                     offset_encoding: ls.offset_encoding(),
                 });
@@ -243,8 +245,14 @@ fn diag_picker(
             // between message code and message
             2,
             ui::PickerColumn::new("path", |item: &PickerDiagnostic, _| {
-                let path = path::get_truncated_path(&item.path);
+                if let Some(path) = item.uri.as_path() {
-                path.to_string_lossy().to_string().into()
+                    path::get_truncated_path(path)
+                        .to_string_lossy()
+                        .to_string()
+                        .into()
+                } else {
+                    Default::default()
+                }
             }),
         );
         primary_column += 1;
@@ -257,17 +265,20 @@ fn diag_picker(
         styles,
         move |cx,
               PickerDiagnostic {
-                  path,
+                  uri,
                   diag,
                   offset_encoding,
               },
               action| {
+            let Some(path) = uri.as_path() else {
+                return;
+            };
             jump_to_position(cx.editor, path, diag.range, *offset_encoding, action)
         },
     )
-    .with_preview(move |_editor, PickerDiagnostic { path, diag, .. }| {
+    .with_preview(move |_editor, PickerDiagnostic { uri, diag, .. }| {
         let line = Some((diag.range.start.line as usize, diag.range.end.line as usize));
-        Some((path.clone().into(), line))
+        Some((uri.clone().as_path_buf()?.into(), line))
     })
     .truncate_start(false)
 }
@@ -456,12 +467,17 @@ pub fn workspace_symbol_picker(cx: &mut Context) {
         })
         .without_filtering(),
         ui::PickerColumn::new("path", |item: &SymbolInformationItem, _| {
-            match item.symbol.location.uri.to_file_path() {
+            if let Ok(uri) = Uri::try_from(&item.symbol.location.uri) {
-                Ok(path) => path::get_relative_path(path.as_path())
+                if let Some(path) = uri.as_path() {
+                    path::get_relative_path(path)
                         .to_string_lossy()
                         .to_string()
-                    .into(),
+                        .into()
-                Err(_) => item.symbol.location.uri.to_string().into(),
+                } else {
+                    item.symbol.location.uri.to_string().into()
+                }
+            } else {
+                item.symbol.location.uri.to_string().into()
             }
         }),
     ];
@@ -489,16 +505,11 @@ pub fn workspace_symbol_picker(cx: &mut Context) {
 
 pub fn diagnostics_picker(cx: &mut Context) {
     let doc = doc!(cx.editor);
-    if let Some(current_path) = doc.path() {
+    if let Some(uri) = doc.uri() {
-        let diagnostics = cx
+        let diagnostics = cx.editor.diagnostics.get(&uri).cloned().unwrap_or_default();
-            .editor
-            .diagnostics
-            .get(current_path)
-            .cloned()
-            .unwrap_or_default();
         let picker = diag_picker(
             cx,
-            [(current_path.clone(), diagnostics)].into(),
+            [(uri, diagnostics)].into(),
             DiagnosticsFormat::HideSourcePath,
         );
         cx.push_layer(Box::new(overlaid(picker)));
@@ -842,6 +853,8 @@ fn goto_impl(
                         // With the preallocation above and UTF-8 paths already, this closure will do one (1)
                         // allocation, for `to_file_path`, else there will be two (2), with `to_string_lossy`.
                         if let Ok(path) = item.uri.to_file_path() {
+                            // We don't convert to a `helix_core::Uri` here because we've already checked the scheme.
+                            // This path won't be normalized but it's only used for display.
                             res.push_str(
                                 &path.strip_prefix(cwdir).unwrap_or(&path).to_string_lossy(),
                             );

helix-view/src/document.rs

@@ -1741,6 +1741,10 @@ impl Document {
         Url::from_file_path(self.path()?).ok()
     }
 
+    pub fn uri(&self) -> Option<helix_core::Uri> {
+        Some(self.path()?.clone().into())
+    }
+
     #[inline]
     pub fn text(&self) -> &Rope {
         &self.text

helix-view/src/editor.rs

@@ -44,7 +44,7 @@ pub use helix_core::diagnostic::Severity;
 use helix_core::{
     auto_pairs::AutoPairs,
     syntax::{self, AutoPairConfig, IndentationHeuristic, LanguageServerFeature, SoftWrap},
-    Change, LineEnding, Position, Range, Selection, NATIVE_LINE_ENDING,
+    Change, LineEnding, Position, Range, Selection, Uri, NATIVE_LINE_ENDING,
 };
 use helix_dap as dap;
 use helix_lsp::lsp;
@@ -1022,7 +1022,7 @@ pub struct Editor {
     pub macro_recording: Option<(char, Vec<KeyEvent>)>,
     pub macro_replaying: Vec<char>,
     pub language_servers: helix_lsp::Registry,
-    pub diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+    pub diagnostics: BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
     pub diff_providers: DiffProviderRegistry,
 
     pub debugger: Option<dap::Client>,
@@ -1929,7 +1929,7 @@ impl Editor {
     /// Returns all supported diagnostics for the document
     pub fn doc_diagnostics<'a>(
         language_servers: &'a helix_lsp::Registry,
-        diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+        diagnostics: &'a BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
         document: &Document,
     ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
         Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true)
@@ -1939,15 +1939,15 @@ impl Editor {
     /// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from
     pub fn doc_diagnostics_with_filter<'a>(
         language_servers: &'a helix_lsp::Registry,
-        diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
+        diagnostics: &'a BTreeMap<Uri, Vec<(lsp::Diagnostic, LanguageServerId)>>,
         document: &Document,
         filter: impl Fn(&lsp::Diagnostic, LanguageServerId) -> bool + 'a,
     ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
         let text = document.text().clone();
         let language_config = document.language.clone();
         document
-            .path()
+            .uri()
-            .and_then(|path| diagnostics.get(path))
+            .and_then(|uri| diagnostics.get(&uri))
             .map(|diags| {
                 diags.iter().filter_map(move |(diagnostic, lsp_id)| {
                     let ls = language_servers.get_by_id(*lsp_id)?;

helix-view/src/handlers/lsp.rs

@@ -1,6 +1,7 @@
 use crate::editor::Action;
 use crate::Editor;
 use crate::{DocumentId, ViewId};
+use helix_core::Uri;
 use helix_lsp::util::generate_transaction_from_edits;
 use helix_lsp::{lsp, OffsetEncoding};
 
@@ -54,18 +55,30 @@ pub struct ApplyEditError {
 pub enum ApplyEditErrorKind {
     DocumentChanged,
     FileNotFound,
-    UnknownURISchema,
+    InvalidUrl(helix_core::uri::UrlConversionError),
     IoError(std::io::Error),
     // TODO: check edits before applying and propagate failure
     // InvalidEdit,
 }
 
+impl From<std::io::Error> for ApplyEditErrorKind {
+    fn from(err: std::io::Error) -> Self {
+        ApplyEditErrorKind::IoError(err)
+    }
+}
+
+impl From<helix_core::uri::UrlConversionError> for ApplyEditErrorKind {
+    fn from(err: helix_core::uri::UrlConversionError) -> Self {
+        ApplyEditErrorKind::InvalidUrl(err)
+    }
+}
+
 impl ToString for ApplyEditErrorKind {
     fn to_string(&self) -> String {
         match self {
             ApplyEditErrorKind::DocumentChanged => "document has changed".to_string(),
             ApplyEditErrorKind::FileNotFound => "file not found".to_string(),
-            ApplyEditErrorKind::UnknownURISchema => "URI schema not supported".to_string(),
+            ApplyEditErrorKind::InvalidUrl(err) => err.to_string(),
             ApplyEditErrorKind::IoError(err) => err.to_string(),
         }
     }
@@ -74,25 +87,28 @@ impl ToString for ApplyEditErrorKind {
 impl Editor {
     fn apply_text_edits(
         &mut self,
-        uri: &helix_lsp::Url,
+        url: &helix_lsp::Url,
         version: Option<i32>,
         text_edits: Vec<lsp::TextEdit>,
         offset_encoding: OffsetEncoding,
     ) -> Result<(), ApplyEditErrorKind> {
-        let path = match uri.to_file_path() {
+        let uri = match Uri::try_from(url) {
-            Ok(path) => path,
+            Ok(uri) => uri,
-            Err(_) => {
+            Err(err) => {
-                let err = format!("unable to convert URI to filepath: {}", uri);
+                log::error!("{err}");
-                log::error!("{}", err);
+                return Err(err.into());
-                self.set_error(err);
-                return Err(ApplyEditErrorKind::UnknownURISchema);
             }
         };
+        let path = uri.as_path().expect("URIs are valid paths");
 
-        let doc_id = match self.open(&path, Action::Load) {
+        let doc_id = match self.open(path, Action::Load) {
             Ok(doc_id) => doc_id,
             Err(err) => {
-                let err = format!("failed to open document: {}: {}", uri, err);
+                let err = format!(
+                    "failed to open document: {}: {}",
+                    path.to_string_lossy(),
+                    err
+                );
                 log::error!("{}", err);
                 self.set_error(err);
                 return Err(ApplyEditErrorKind::FileNotFound);
@@ -158,9 +174,9 @@ impl Editor {
                     for (i, operation) in operations.iter().enumerate() {
                         match operation {
                             lsp::DocumentChangeOperation::Op(op) => {
-                                self.apply_document_resource_op(op).map_err(|io| {
+                                self.apply_document_resource_op(op).map_err(|err| {
                                     ApplyEditError {
-                                        kind: ApplyEditErrorKind::IoError(io),
+                                        kind: err,
                                         failed_change_idx: i,
                                     }
                                 })?;
@@ -214,12 +230,18 @@ impl Editor {
         Ok(())
     }
 
-    fn apply_document_resource_op(&mut self, op: &lsp::ResourceOp) -> std::io::Result<()> {
+    fn apply_document_resource_op(
+        &mut self,
+        op: &lsp::ResourceOp,
+    ) -> Result<(), ApplyEditErrorKind> {
         use lsp::ResourceOp;
         use std::fs;
+        // NOTE: If `Uri` gets another variant than `Path`, the below `expect`s
+        // may no longer be valid.
         match op {
             ResourceOp::Create(op) => {
-                let path = op.uri.to_file_path().unwrap();
+                let uri = Uri::try_from(&op.uri)?;
+                let path = uri.as_path_buf().expect("URIs are valid paths");
                 let ignore_if_exists = op.options.as_ref().map_or(false, |options| {
                     !options.overwrite.unwrap_or(false) && options.ignore_if_exists.unwrap_or(false)
                 });
@@ -236,7 +258,8 @@ impl Editor {
                 }
             }
             ResourceOp::Delete(op) => {
-                let path = op.uri.to_file_path().unwrap();
+                let uri = Uri::try_from(&op.uri)?;
+                let path = uri.as_path_buf().expect("URIs are valid paths");
                 if path.is_dir() {
                     let recursive = op
                         .options
@@ -251,17 +274,19 @@ impl Editor {
                     }
                     self.language_servers.file_event_handler.file_changed(path);
                 } else if path.is_file() {
-                    fs::remove_file(&path)?;
+                    fs::remove_file(path)?;
                 }
             }
             ResourceOp::Rename(op) => {
-                let from = op.old_uri.to_file_path().unwrap();
+                let from_uri = Uri::try_from(&op.old_uri)?;
-                let to = op.new_uri.to_file_path().unwrap();
+                let from = from_uri.as_path().expect("URIs are valid paths");
+                let to_uri = Uri::try_from(&op.new_uri)?;
+                let to = to_uri.as_path().expect("URIs are valid paths");
                 let ignore_if_exists = op.options.as_ref().map_or(false, |options| {
                     !options.overwrite.unwrap_or(false) && options.ignore_if_exists.unwrap_or(false)
                 });
                 if !ignore_if_exists || !to.exists() {
-                    self.move_path(&from, &to)?;
+                    self.move_path(from, to)?;
                 }
             }
         }