From cc33897928f97a4dd9531cc426e2332d242872ce Mon Sep 17 00:00:00 2001 From: nvarner Date: Mon, 17 Apr 2023 02:03:52 -0400 Subject: [PATCH] return errors for failed Path/URI conversion instead of panic --- src/lsp_typst_boundary/mod.rs | 35 +++++++++++++++------------------ src/lsp_typst_boundary/world.rs | 8 +++++--- src/server/lsp.rs | 5 ++++- src/workspace/font_manager.rs | 4 +++- src/workspace/source.rs | 8 ++++---- src/workspace/source_manager.rs | 12 ++++++----- 6 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/lsp_typst_boundary/mod.rs b/src/lsp_typst_boundary/mod.rs index 84869b75..92a99cad 100644 --- a/src/lsp_typst_boundary/mod.rs +++ b/src/lsp_typst_boundary/mod.rs @@ -1,9 +1,8 @@ //! Conversions between Typst and LSP types and representations use std::collections::HashMap; -use std::io; -use tower_lsp::lsp_types::{self, Url}; +use tower_lsp::lsp_types; pub mod world; @@ -54,14 +53,12 @@ pub type TypstCompletion = typst::ide::Completion; pub type TypstCompletionKind = typst::ide::CompletionKind; pub mod lsp_to_typst { - use std::path::PathBuf; - use super::*; - // TODO: these URL <-> Path functions are a quick hack to make things work. They should be - // replaced by a more comprehensive system to reliably convert `LspUri`s to `TypstPath`s - pub fn uri_to_path(lsp_uri: &LspUri) -> TypstPathOwned { - lsp_uri.to_file_path().unwrap_or_else(|_| PathBuf::new()) + pub fn uri_to_path(lsp_uri: &LspUri) -> anyhow::Result { + lsp_uri + .to_file_path() + .map_err(|()| anyhow::anyhow!("could not get path for URI {lsp_uri}")) } pub fn position_to_offset( @@ -127,7 +124,6 @@ pub mod typst_to_lsp { use tower_lsp::lsp_types::{ DiagnosticSeverity, InsertTextFormat, LanguageString, MarkedString, }; - use typst::util::PathExt; use typst::World; use typst_library::prelude::EcoString; @@ -136,12 +132,11 @@ pub mod typst_to_lsp { use super::world::WorkspaceWorld; use super::*; - // TODO: these URL <-> Path functions are a quick hack to make things work. They should be - // replaced by a more comprehensive system to reliably convert `LspUri`s to `TypstPath`s - pub fn path_to_uri(typst_path: &TypstPath) -> io::Result { - let normalized_path = typst_path.normalize(); - let lsp_uri = Url::from_file_path(normalized_path).unwrap(); - Ok(lsp_uri) + pub fn path_to_uri(typst_path: &TypstPath) -> anyhow::Result { + LspUri::from_file_path(typst_path).map_err(|()| { + let path = typst_path.to_string_lossy(); + anyhow::anyhow!("could not get URI for path {path}") + }) } pub fn offset_to_position( @@ -233,7 +228,7 @@ pub mod typst_to_lsp { typst_error: &TypstSourceError, world: &WorkspaceWorld, const_config: &ConstConfig, - ) -> (Url, LspDiagnostic) { + ) -> Option<(LspUri, LspDiagnostic)> { let typst_span = typst_error.span; let typst_source = world.source(typst_span.source()); @@ -249,9 +244,9 @@ pub mod typst_to_lsp { ..Default::default() }; - let uri = path_to_uri(typst_source.path()).unwrap(); + let uri = path_to_uri(typst_source.path()).ok()?; - (uri, diagnostic) + Some((uri, diagnostic)) } pub fn source_errors_to_diagnostics<'a>( @@ -261,7 +256,9 @@ pub mod typst_to_lsp { ) -> LspDiagnostics { errors .into_iter() - .map(|error| typst_to_lsp::source_error_to_diagnostic(error, world, const_config)) + .filter_map(|error| { + typst_to_lsp::source_error_to_diagnostic(error, world, const_config) + }) .into_group_map() } diff --git a/src/lsp_typst_boundary/world.rs b/src/lsp_typst_boundary/world.rs index 85ea0331..f9668154 100644 --- a/src/lsp_typst_boundary/world.rs +++ b/src/lsp_typst_boundary/world.rs @@ -1,6 +1,6 @@ use comemo::Prehashed; use tokio::sync::OwnedRwLockReadGuard; -use typst::diag::FileResult; +use typst::diag::{FileError, FileResult}; use typst::eval::Library; use typst::font::{Font, FontBook}; use typst::util::Buffer; @@ -37,7 +37,8 @@ impl World for WorkspaceWorld { } fn resolve(&self, typst_path: &TypstPath) -> FileResult { - let lsp_uri = typst_to_lsp::path_to_uri(typst_path).unwrap(); + let lsp_uri = typst_to_lsp::path_to_uri(typst_path) + .map_err(|_| FileError::NotFound(typst_path.to_owned()))?; self.get_workspace().sources.cache(lsp_uri).map(Into::into) } @@ -59,7 +60,8 @@ impl World for WorkspaceWorld { } fn file(&self, typst_path: &TypstPath) -> FileResult { - let lsp_uri = typst_to_lsp::path_to_uri(typst_path).unwrap(); + let lsp_uri = typst_to_lsp::path_to_uri(typst_path) + .map_err(|_| FileError::NotFound(typst_path.to_owned()))?; let mut resources = self.get_workspace().resources.write(); let lsp_resource = resources.get_or_insert_resource(lsp_uri)?; Ok(lsp_resource.into()) diff --git a/src/server/lsp.rs b/src/server/lsp.rs index 21f2b116..bd7ac4e4 100644 --- a/src/server/lsp.rs +++ b/src/server/lsp.rs @@ -89,7 +89,10 @@ impl LanguageServer for TypstServer { let text = params.text_document.text; let mut workspace = self.workspace.write().await; - workspace.sources.insert_open(&uri, text); + if let Err(error) = workspace.sources.insert_open(&uri, text) { + self.client.log_message(MessageType::ERROR, error).await; + return; + } let workspace = workspace.downgrade(); let config = self.config.read().await; diff --git a/src/workspace/font_manager.rs b/src/workspace/font_manager.rs index e5aaf517..2f4c7698 100644 --- a/src/workspace/font_manager.rs +++ b/src/workspace/font_manager.rs @@ -182,9 +182,11 @@ impl Builder { if let Ok(file) = File::open(&path) { if let Ok(mmap) = unsafe { Mmap::map(&file) } { for (i, info) in FontInfo::iter(&mmap).enumerate() { + let Ok(uri) = Url::from_file_path(&path) else { continue; }; + self.book.push(info); self.fonts.push(FontSlot { - uri: Some(Url::from_file_path(&path).unwrap()), + uri: Some(uri), index: i as u32, font: OnceCell::new(), }); diff --git a/src/workspace/source.rs b/src/workspace/source.rs index 70681b89..f5b2739b 100644 --- a/src/workspace/source.rs +++ b/src/workspace/source.rs @@ -11,12 +11,12 @@ pub struct Source { } impl Source { - pub fn new(id: SourceId, uri: &Url, text: String) -> Self { - let typst_path = lsp_to_typst::uri_to_path(uri); + pub fn new(id: SourceId, uri: &Url, text: String) -> anyhow::Result { + let typst_path = lsp_to_typst::uri_to_path(uri)?; - Self { + Ok(Self { inner: TypstSource::new(id.into(), &typst_path, text), - } + }) } pub fn new_detached() -> Self { diff --git a/src/workspace/source_manager.rs b/src/workspace/source_manager.rs index b650ccd1..39c39f64 100644 --- a/src/workspace/source_manager.rs +++ b/src/workspace/source_manager.rs @@ -87,21 +87,23 @@ impl SourceManager { SourceId(self.sources.len() as u16) } - pub fn insert_open(&mut self, uri: &Url, text: String) { + pub fn insert_open(&mut self, uri: &Url, text: String) -> anyhow::Result<()> { let next_id = self.get_next_id(); match self.ids.as_mut().entry(uri.clone()) { Entry::Occupied(entry) => { let existing_id = *entry.get(); - let source = Source::new(existing_id, uri, text); + let source = Source::new(existing_id, uri, text)?; *self.get_mut_inner_source(existing_id) = InnerSource::Open(source); } Entry::Vacant(entry) => { entry.insert(next_id); - let source = Source::new(next_id, uri, text); + let source = Source::new(next_id, uri, text)?; self.sources.push(Box::new(InnerSource::Open(source))); } } + + Ok(()) } pub fn close(&mut self, uri: &Url) { @@ -124,13 +126,13 @@ impl SourceManager { } fn read_source_from_file(id: SourceId, uri: &Url) -> FileResult { - let path = lsp_to_typst::uri_to_path(uri); + let path = lsp_to_typst::uri_to_path(uri).map_err(|_| FileError::Other)?; let text = fs::read_to_string(&path).map_err(|error| match error.kind() { io::ErrorKind::NotFound => FileError::NotFound(path), io::ErrorKind::PermissionDenied => FileError::AccessDenied, _ => FileError::Other, })?; - Ok(Source::new(id, uri, text)) + Source::new(id, uri, text).map_err(|_| FileError::Other) } pub fn cache(&self, uri: Url) -> FileResult {