From dd476b39e7989fcbc600bfbc366248a09976cb80 Mon Sep 17 00:00:00 2001 From: oxalica Date: Tue, 6 Sep 2022 04:41:42 +0800 Subject: [PATCH] Proper error handling for LSP --- Cargo.lock | 1 - crates/ide/src/base.rs | 8 +-- crates/ide/src/def/path.rs | 2 +- crates/ide/src/ide/mod.rs | 3 +- crates/ide/src/lib.rs | 3 +- crates/nil/Cargo.toml | 1 - crates/nil/src/convert.rs | 49 ++++++++-------- crates/nil/src/handler.rs | 55 +++++++++++------- crates/nil/src/lib.rs | 10 ++-- crates/nil/src/main.rs | 18 +++--- crates/nil/src/state.rs | 111 +++++++++++++++++++++++++------------ crates/nil/src/vfs.rs | 41 +++++++++----- 12 files changed, 186 insertions(+), 116 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8854980..ed8977c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -312,7 +312,6 @@ dependencies = [ name = "nil" version = "0.0.0" dependencies = [ - "anyhow", "crossbeam-channel", "ide", "ignore", diff --git a/crates/ide/src/base.rs b/crates/ide/src/base.rs index 87c8f0f..7347f03 100644 --- a/crates/ide/src/base.rs +++ b/crates/ide/src/base.rs @@ -99,8 +99,8 @@ impl FileSet { self.files.get(path).copied() } - pub fn get_path_for_file(&self, file: FileId) -> Option<&VfsPath> { - self.paths.get(&file) + pub fn path_for_file(&self, file: FileId) -> &VfsPath { + &self.paths[&file] } pub fn iter(&self) -> impl Iterator + '_ { @@ -130,8 +130,8 @@ impl SourceRoot { self.file_set.get_file_for_path(path) } - pub fn get_path_for_file(&self, file: FileId) -> Option<&VfsPath> { - self.file_set.get_path_for_file(file) + pub fn path_for_file(&self, file: FileId) -> &VfsPath { + self.file_set.path_for_file(file) } pub fn iter(&self) -> impl Iterator + '_ { diff --git a/crates/ide/src/def/path.rs b/crates/ide/src/def/path.rs index 78e2798..40d4fe6 100644 --- a/crates/ide/src/def/path.rs +++ b/crates/ide/src/def/path.rs @@ -24,7 +24,7 @@ impl Path { }; let sid = db.file_source_root(file); let root = db.source_root(sid); - let mut vpath = root.get_path_for_file(file)?.clone(); + let mut vpath = root.path_for_file(file).clone(); for _ in 0..(data.supers.saturating_add(1)) { vpath.pop()?; } diff --git a/crates/ide/src/ide/mod.rs b/crates/ide/src/ide/mod.rs index 9d0fed0..4456c22 100644 --- a/crates/ide/src/ide/mod.rs +++ b/crates/ide/src/ide/mod.rs @@ -8,7 +8,7 @@ use crate::base::SourceDatabaseStorage; use crate::def::DefDatabaseStorage; use crate::{Change, Diagnostic, FileId, FilePos, FileRange}; use rowan::TextRange; -use salsa::{Cancelled, Database, Durability, ParallelDatabase}; +use salsa::{Database, Durability, ParallelDatabase}; use std::fmt; pub use completion::{CompletionItem, CompletionItemKind}; @@ -20,6 +20,7 @@ pub struct NavigationTarget { pub focus_range: TextRange, } +pub use salsa::Cancelled; pub type Cancellable = Result; #[salsa::database(SourceDatabaseStorage, DefDatabaseStorage)] diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index c084f6f..4cefa6b 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -8,7 +8,8 @@ mod ide; mod tests; pub use self::ide::{ - Analysis, AnalysisHost, CompletionItem, CompletionItemKind, NavigationTarget, RootDatabase, + Analysis, AnalysisHost, Cancelled, CompletionItem, CompletionItemKind, NavigationTarget, + RootDatabase, }; pub use base::{ Change, FileId, FilePos, FileRange, FileSet, InFile, SourceDatabase, SourceRoot, SourceRootId, diff --git a/crates/nil/Cargo.toml b/crates/nil/Cargo.toml index 18270f8..8ef02b4 100644 --- a/crates/nil/Cargo.toml +++ b/crates/nil/Cargo.toml @@ -5,7 +5,6 @@ edition = "2021" license = "MIT OR Apache-2.0" [dependencies] -anyhow = "1.0.58" crossbeam-channel = "0.5.6" ide = { path = "../ide" } ignore = "0.4.18" diff --git a/crates/nil/src/convert.rs b/crates/nil/src/convert.rs index caab511..90fc743 100644 --- a/crates/nil/src/convert.rs +++ b/crates/nil/src/convert.rs @@ -1,4 +1,4 @@ -use crate::{LineMap, StateSnapshot, Vfs}; +use crate::{LineMap, Result, StateSnapshot, Vfs}; use ide::{CompletionItem, CompletionItemKind, Diagnostic, FileId, FilePos, FileRange, Severity}; use lsp_types::{ self as lsp, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, @@ -6,31 +6,31 @@ use lsp_types::{ }; use text_size::{TextRange, TextSize}; -pub(crate) fn from_file(snap: &StateSnapshot, doc: &TextDocumentIdentifier) -> Option { +pub(crate) fn from_file(snap: &StateSnapshot, doc: &TextDocumentIdentifier) -> Result { let vfs = snap.vfs.read().unwrap(); vfs.get_file_for_uri(&doc.uri) } -pub(crate) fn from_pos(snap: &StateSnapshot, file: FileId, pos: Position) -> Option { +pub(crate) fn from_pos(snap: &StateSnapshot, file: FileId, pos: Position) -> Result { let vfs = snap.vfs.read().unwrap(); - let line_map = vfs.get_line_map(file)?; + let line_map = vfs.file_line_map(file); let pos = line_map.pos(pos.line, pos.character); - Some(pos) + Ok(pos) } pub(crate) fn from_file_pos( snap: &StateSnapshot, params: &TextDocumentPositionParams, -) -> Option { +) -> Result { let file = from_file(snap, ¶ms.text_document)?; let pos = from_pos(snap, file, params.position)?; - Some(FilePos::new(file, pos)) + Ok(FilePos::new(file, pos)) } -pub(crate) fn to_location(vfs: &Vfs, frange: FileRange) -> Option { - let uri = vfs.get_uri_for_file(frange.file_id)?; - let line_map = vfs.get_line_map(frange.file_id)?; - Some(Location::new(uri, to_range(line_map, frange.range))) +pub(crate) fn to_location(vfs: &Vfs, frange: FileRange) -> Location { + let uri = vfs.uri_for_file(frange.file_id); + let line_map = vfs.file_line_map(frange.file_id); + Location::new(uri, to_range(line_map, frange.range)) } pub(crate) fn to_range(line_map: &LineMap, range: TextRange) -> Range { @@ -43,8 +43,8 @@ pub(crate) fn to_diagnostics( vfs: &Vfs, file: FileId, diags: &[Diagnostic], -) -> Option> { - let line_map = vfs.get_line_map(file)?; +) -> Vec { + let line_map = vfs.file_line_map(file); let mut ret = Vec::with_capacity(diags.len() * 2); for diag in diags { let primary_diag = lsp::Diagnostic { @@ -62,11 +62,9 @@ pub(crate) fn to_diagnostics( Some( diag.notes .iter() - .filter_map(|(frange, msg)| { - Some(DiagnosticRelatedInformation { - location: to_location(vfs, *frange)?, - message: msg.to_owned(), - }) + .map(|(frange, msg)| DiagnosticRelatedInformation { + location: to_location(vfs, *frange), + message: msg.to_owned(), }) .collect(), ) @@ -99,8 +97,7 @@ pub(crate) fn to_diagnostics( source: primary_diag.source.clone(), message: msg.into(), related_information: Some(vec![DiagnosticRelatedInformation { - location: to_location(vfs, FileRange::new(file, diag.range)) - .expect("Checked by get_line_map"), + location: to_location(vfs, FileRange::new(file, diag.range)), message: "original diagnostic".into(), }]), tags: None, @@ -110,13 +107,11 @@ pub(crate) fn to_diagnostics( ret.push(primary_diag); } - Some(ret) + + ret } -pub(crate) fn to_completion_item( - line_map: &LineMap, - item: CompletionItem, -) -> Option { +pub(crate) fn to_completion_item(line_map: &LineMap, item: CompletionItem) -> lsp::CompletionItem { let kind = match item.kind { CompletionItemKind::Keyword => lsp::CompletionItemKind::KEYWORD, CompletionItemKind::Param => lsp::CompletionItemKind::VARIABLE, @@ -126,7 +121,7 @@ pub(crate) fn to_completion_item( CompletionItemKind::BuiltinFunction => lsp::CompletionItemKind::FUNCTION, CompletionItemKind::BuiltinAttrset => lsp::CompletionItemKind::CLASS, }; - Some(lsp::CompletionItem { + lsp::CompletionItem { label: item.label.into(), kind: Some(kind), insert_text: None, @@ -139,5 +134,5 @@ pub(crate) fn to_completion_item( })), // TODO ..Default::default() - }) + } } diff --git a/crates/nil/src/handler.rs b/crates/nil/src/handler.rs index fc851f9..889b698 100644 --- a/crates/nil/src/handler.rs +++ b/crates/nil/src/handler.rs @@ -1,4 +1,4 @@ -use crate::{convert, StateSnapshot}; +use crate::{convert, Result, StateSnapshot}; use ide::FileRange; use lsp_types::{ CompletionOptions, CompletionParams, CompletionResponse, GotoDefinitionParams, @@ -31,65 +31,77 @@ pub(crate) fn server_capabilities() -> ServerCapabilities { pub(crate) fn goto_definition( snap: StateSnapshot, params: GotoDefinitionParams, -) -> Option { +) -> Result> { let fpos = convert::from_file_pos(&snap, ¶ms.text_document_position_params)?; - let targets = snap.analysis.goto_definition(fpos).ok()??; + let targets = match snap.analysis.goto_definition(fpos)? { + None => return Ok(None), + Some(targets) => targets, + }; let vfs = snap.vfs.read().unwrap(); let targets = targets .into_iter() - .filter_map(|target| { + .map(|target| { convert::to_location(&vfs, FileRange::new(target.file_id, target.focus_range)) }) .collect::>(); - Some(GotoDefinitionResponse::Array(targets)) + Ok(Some(GotoDefinitionResponse::Array(targets))) } -pub(crate) fn references(snap: StateSnapshot, params: ReferenceParams) -> Option> { +pub(crate) fn references( + snap: StateSnapshot, + params: ReferenceParams, +) -> Result>> { let fpos = convert::from_file_pos(&snap, ¶ms.text_document_position)?; - let refs = snap.analysis.references(fpos).ok()??; + let refs = match snap.analysis.references(fpos)? { + None => return Ok(None), + Some(refs) => refs, + }; let vfs = snap.vfs.read().unwrap(); let locs = refs - .iter() - .filter_map(|&frange| convert::to_location(&vfs, frange)) + .into_iter() + .map(|frange| convert::to_location(&vfs, frange)) .collect::>(); - Some(locs) + Ok(Some(locs)) } pub(crate) fn completion( snap: StateSnapshot, params: CompletionParams, -) -> Option { +) -> Result> { let fpos = convert::from_file_pos(&snap, ¶ms.text_document_position)?; - let items = snap.analysis.completions(fpos).ok()??; + let items = match snap.analysis.completions(fpos)? { + None => return Ok(None), + Some(items) => items, + }; let vfs = snap.vfs.read().unwrap(); - let line_map = vfs.get_line_map(fpos.file_id)?; + let line_map = vfs.file_line_map(fpos.file_id); let items = items .into_iter() - .filter_map(|item| convert::to_completion_item(line_map, item)) + .map(|item| convert::to_completion_item(line_map, item)) .collect::>(); - Some(CompletionResponse::Array(items)) + Ok(Some(CompletionResponse::Array(items))) } pub(crate) fn selection_range( snap: StateSnapshot, params: SelectionRangeParams, -) -> Option> { +) -> Result>> { let file = convert::from_file(&snap, ¶ms.text_document)?; - params + let ret = params .positions .iter() .map(|&pos| { let pos = convert::from_pos(&snap, file, pos)?; let frange = FileRange::new(file, TextRange::empty(pos)); - let mut ranges = snap.analysis.expand_selection(frange).ok()??; + let mut ranges = snap.analysis.expand_selection(frange)?.unwrap_or_default(); if ranges.is_empty() { ranges.push(TextRange::empty(pos)); } // FIXME: Use Arc for LineMap. let vfs = snap.vfs.read().unwrap(); - let line_map = vfs.get_line_map(file)?; + let line_map = vfs.file_line_map(file); let mut ret = SelectionRange { range: convert::to_range(line_map, *ranges.last().unwrap()), parent: None, @@ -101,7 +113,8 @@ pub(crate) fn selection_range( }; } - Some(ret) + Ok(ret) }) - .collect() + .collect::>>(); + ret.map(Some) } diff --git a/crates/nil/src/lib.rs b/crates/nil/src/lib.rs index ea38e56..5aaf6db 100644 --- a/crates/nil/src/lib.rs +++ b/crates/nil/src/lib.rs @@ -3,7 +3,6 @@ mod handler; mod state; mod vfs; -use anyhow::Result; use lsp_server::Connection; use lsp_types::InitializeParams; use std::env; @@ -12,10 +11,13 @@ use std::path::PathBuf; pub(crate) use state::{State, StateSnapshot}; pub(crate) use vfs::{LineMap, Vfs}; +pub type Error = Box; +pub type Result = std::result::Result; + pub fn main_loop(conn: Connection) -> Result<()> { let init_params = conn.initialize(serde_json::to_value(&handler::server_capabilities()).unwrap())?; - log::info!("Init params: {}", init_params); + tracing::info!("Init params: {}", init_params); let init_params = serde_json::from_value::(init_params)?; let workspace_path = (|| -> Option { @@ -25,12 +27,12 @@ pub fn main_loop(conn: Connection) -> Result<()> { if let Some(uri) = &init_params.root_uri { return uri.to_file_path().ok(); } - Some(env::current_dir().expect("Cannot get current directory")) + env::current_dir().ok() })(); let mut state = State::new(conn.sender.clone(), workspace_path); state.run(conn.receiver)?; - log::info!("Leaving main loop"); + tracing::info!("Leaving main loop"); Ok(()) } diff --git a/crates/nil/src/main.rs b/crates/nil/src/main.rs index a7ce102..cbb9bea 100644 --- a/crates/nil/src/main.rs +++ b/crates/nil/src/main.rs @@ -1,8 +1,7 @@ -use anyhow::Result; use lsp_server::Connection; use std::path::PathBuf; use std::sync::Arc; -use std::{env, fs, io}; +use std::{env, fs, io, process}; use tracing_subscriber::fmt::writer::BoxMakeWriter; use tracing_subscriber::EnvFilter; @@ -10,7 +9,7 @@ const LOG_FILTER_ENV: &str = "NIL_LOG"; const LOG_PATH_ENV: &str = "NIL_LOG_PATH"; const BACKTRACE_ENV: &str = "RUST_BACKTRACE"; -fn main() -> Result<()> { +fn main() { if env::var(BACKTRACE_ENV).is_err() { env::set_var(BACKTRACE_ENV, "short"); } @@ -21,13 +20,18 @@ fn main() -> Result<()> { let date = option_env!("CFG_DATE").unwrap_or("unknown"); let rev = option_env!("CFG_REV").unwrap_or("unknown"); println!("nil {} {}", date, rev); - return Ok(()); + return; } let (conn, io_threads) = Connection::stdio(); - nil::main_loop(conn)?; - io_threads.join()?; - Ok(()) + match nil::main_loop(conn).and_then(|()| io_threads.join().map_err(Into::into)) { + Ok(()) => {} + Err(err) => { + tracing::error!("Unexpected error: {}", err); + eprintln!("{}", err); + process::exit(101); + } + } } fn setup_logger() { diff --git a/crates/nil/src/state.rs b/crates/nil/src/state.rs index cd49658..1863587 100644 --- a/crates/nil/src/state.rs +++ b/crates/nil/src/state.rs @@ -1,10 +1,10 @@ -use crate::{convert, handler, Vfs}; -use anyhow::{bail, Result}; +use crate::{convert, handler, Result, Vfs}; use crossbeam_channel::{Receiver, Sender}; -use ide::{Analysis, AnalysisHost, VfsPath}; -use lsp_server::{ErrorCode, Message, Notification, Request, Response}; +use ide::{Analysis, AnalysisHost, Cancelled, VfsPath}; +use lsp_server::{ErrorCode, Message, Notification, Request, RequestId, Response}; use lsp_types::notification::Notification as _; use lsp_types::{notification as notif, request as req, PublishDiagnosticsParams, Url}; +use serde::Serialize; use std::collections::HashSet; use std::fs; use std::path::PathBuf; @@ -23,6 +23,8 @@ pub struct State { impl State { pub fn new(responder: Sender, workspace_root: Option) -> Self { + // Vfs root must be absolute. + let workspace_root = workspace_root.and_then(|root| root.canonicalize().ok()); let vfs = Vfs::new(workspace_root.clone().unwrap_or_else(|| PathBuf::from("/"))); Self { host: Default::default(), @@ -58,12 +60,13 @@ impl State { if notif.method == notif::Exit::METHOD { return Ok(()); } - self.dispatch_notification(notif) + self.dispatch_notification(notif)?; } Message::Response(_) => {} } } - bail!("Channel closed") + + Err("Channel closed".into()) } fn dispatch_request(&mut self, req: Request) { @@ -80,6 +83,7 @@ impl State { RequestDispatcher(self, Some(req)) .on_sync_mut::(|st, ()| { st.is_shutdown = true; + Ok(()) }) .on::(handler::goto_definition) .on::(handler::references) @@ -88,26 +92,29 @@ impl State { .finish(); } - fn dispatch_notification(&mut self, notif: Notification) { + fn dispatch_notification(&mut self, notif: Notification) -> Result<()> { NotificationDispatcher(self, Some(notif)) .on_sync_mut::(|st, params| { let uri = ¶ms.text_document.uri; st.opened_files.write().unwrap().insert(uri.clone()); - st.set_vfs_file_content(uri, Some(params.text_document.text)); - }) + st.set_vfs_file_content(uri, Some(params.text_document.text))?; + Ok(()) + })? .on_sync_mut::(|st, params| { // N.B. Don't clear text here. st.opened_files .write() .unwrap() .remove(¶ms.text_document.uri); - }) + Ok(()) + })? .on_sync_mut::(|st, params| { if let Some(chg) = params.content_changes.into_iter().next() { - st.set_vfs_file_content(¶ms.text_document.uri, Some(chg.text)); + st.set_vfs_file_content(¶ms.text_document.uri, Some(chg.text))?; } - }) - .finish(); + Ok(()) + })? + .finish() } fn send_notification(&self, params: N::Params) { @@ -123,9 +130,10 @@ impl State { } } - fn set_vfs_file_content(&mut self, uri: &Url, text: Option) { - self.vfs.write().unwrap().set_uri_content(uri, text); + fn set_vfs_file_content(&mut self, uri: &Url, text: Option) -> Result<()> { + self.vfs.write().unwrap().set_uri_content(uri, text)?; self.apply_vfs_change(); + Ok(()) } fn apply_vfs_change(&mut self) { @@ -136,22 +144,23 @@ impl State { .iter() .map(|(file, text)| (*file, text.is_some())) .collect::>(); - log::debug!("Change: {:?}", change); + tracing::debug!("Change: {:?}", change); self.host.apply_change(change); let snap = self.host.snapshot(); let opened_files = self.opened_files.read().unwrap(); for (file, has_text) in file_changes { - let uri = vfs.get_uri_for_file(file).unwrap(); + let uri = vfs.uri_for_file(file); if !opened_files.contains(&uri) { continue; } + // TODO: Error is ignored. let diagnostics = has_text .then(|| { let mut diags = snap.diagnostics(file).ok()?; diags.truncate(MAX_DIAGNOSTICS_CNT); - convert::to_diagnostics(&vfs, file, &diags) + Some(convert::to_diagnostics(&vfs, file, &diags)) }) .flatten() .unwrap_or_default(); @@ -168,25 +177,41 @@ impl State { struct RequestDispatcher<'s>(&'s mut State, Option); impl<'s> RequestDispatcher<'s> { - fn on_sync_mut(mut self, f: fn(&mut State, R::Params) -> R::Result) -> Self { + fn on_sync_mut( + mut self, + f: fn(&mut State, R::Params) -> Result, + ) -> Self { if matches!(&self.1, Some(notif) if notif.method == R::METHOD) { let req = self.1.take().unwrap(); - let params = serde_json::from_value::(req.params).unwrap(); - let resp = f(self.0, params); - let resp = Response::new_ok(req.id, serde_json::to_value(resp).unwrap()); - self.0.sender.send(resp.into()).unwrap(); + let ret = match serde_json::from_value::(req.params) { + Ok(params) => result_to_response(req.id, f(self.0, params)), + Err(err) => Ok(Response::new_err( + req.id, + ErrorCode::InvalidParams as i32, + err.to_string(), + )), + }; + if let Ok(resp) = ret { + self.0.sender.send(resp.into()).unwrap(); + } } self } - // TODO: Error handling? - fn on(mut self, f: fn(StateSnapshot, R::Params) -> R::Result) -> Self { + fn on(mut self, f: fn(StateSnapshot, R::Params) -> Result) -> Self { if matches!(&self.1, Some(notif) if notif.method == R::METHOD) { let req = self.1.take().unwrap(); - let params = serde_json::from_value::(req.params).unwrap(); - let resp = f(self.0.snapshot(), params); - let resp = Response::new_ok(req.id, serde_json::to_value(resp).unwrap()); - self.0.sender.send(resp.into()).unwrap(); + let ret = match serde_json::from_value::(req.params) { + Ok(params) => result_to_response(req.id, f(self.0.snapshot(), params)), + Err(err) => Ok(Response::new_err( + req.id, + ErrorCode::InvalidParams as i32, + err.to_string(), + )), + }; + if let Ok(resp) = ret { + self.0.sender.send(resp.into()).unwrap(); + } } self } @@ -203,17 +228,35 @@ impl<'s> RequestDispatcher<'s> { struct NotificationDispatcher<'s>(&'s mut State, Option); impl<'s> NotificationDispatcher<'s> { - fn on_sync_mut(mut self, f: fn(&mut State, N::Params)) -> Self { + fn on_sync_mut( + mut self, + f: fn(&mut State, N::Params) -> Result<()>, + ) -> Result { if matches!(&self.1, Some(notif) if notif.method == N::METHOD) { let params = serde_json::from_value::(self.1.take().unwrap().params).unwrap(); - f(self.0, params); + f(self.0, params)?; } - self + Ok(self) } - fn finish(self) { - let _ = self; + fn finish(self) -> Result<()> { + // TODO: We are not done yet. + Ok(()) + } +} + +fn result_to_response(id: RequestId, ret: Result) -> Result { + match ret { + Ok(ret) => Ok(Response::new_ok(id, ret)), + Err(err) => match err.downcast::() { + Ok(cancelled) => Err(*cancelled), + Err(err) => Ok(Response::new_err( + id, + ErrorCode::InternalError as i32, + err.to_string(), + )), + }, } } diff --git a/crates/nil/src/vfs.rs b/crates/nil/src/vfs.rs index 43bdc0b..a615680 100644 --- a/crates/nil/src/vfs.rs +++ b/crates/nil/src/vfs.rs @@ -1,3 +1,4 @@ +use crate::Result; use ide::{Change, FileId, FileSet, SourceRoot, VfsPath}; use lsp_types::Url; use std::collections::HashMap; @@ -9,6 +10,7 @@ use text_size::TextSize; pub struct Vfs { // FIXME: Currently this list is append-only. files: Vec, LineMap)>>, + /// The root directory, which must be absolute. local_root: PathBuf, local_file_set: FileSet, root_changed: bool, @@ -26,6 +28,7 @@ impl fmt::Debug for Vfs { impl Vfs { pub fn new(local_root: PathBuf) -> Self { + assert!(local_root.is_absolute()); Self { files: Vec::new(), local_root, @@ -41,15 +44,20 @@ impl Vfs { FileId(id) } - fn uri_to_vpath(&self, uri: &Url) -> Option { - let path = uri.to_file_path().ok()?; - let relative_path = path.strip_prefix(&self.local_root).ok()?; - VfsPath::from_path(relative_path) + fn uri_to_vpath(&self, uri: &Url) -> Result { + let path = uri + .to_file_path() + .map_err(|_| format!("Non-file URI: {}", uri))?; + let relative_path = path + .strip_prefix(&self.local_root) + .map_err(|_| format!("URI outside workspace: {}", uri))?; + Ok(VfsPath::from_path(relative_path).expect("URI is UTF-8")) } - pub fn set_uri_content(&mut self, uri: &Url, text: Option) -> Option { + pub fn set_uri_content(&mut self, uri: &Url, text: Option) -> Result<()> { let vpath = self.uri_to_vpath(uri)?; - self.set_path_content(vpath, text) + self.set_path_content(vpath, text); + Ok(()) } pub fn set_path_content(&mut self, path: VfsPath, text: Option) -> Option { @@ -77,16 +85,18 @@ impl Vfs { Some(file) } - pub fn get_file_for_uri(&self, uri: &Url) -> Option { + pub fn get_file_for_uri(&self, uri: &Url) -> Result { let vpath = self.uri_to_vpath(uri)?; - self.local_file_set.get_file_for_path(&vpath) + self.local_file_set + .get_file_for_path(&vpath) + .ok_or_else(|| format!("URI not found: {}", uri).into()) } - pub fn get_uri_for_file(&self, file: FileId) -> Option { - let vpath = self.local_file_set.get_path_for_file(file)?.as_str(); + pub fn uri_for_file(&self, file: FileId) -> Url { + let vpath = self.local_file_set.path_for_file(file).as_str(); assert!(!vpath.is_empty(), "Root is a directory"); - let path = self.local_root.join(vpath.strip_prefix('/')?); - Url::from_file_path(path).ok() + let path = self.local_root.join(vpath.strip_prefix('/').unwrap()); + Url::from_file_path(path).expect("Root is absolute") } pub fn take_change(&mut self) -> Change { @@ -106,8 +116,11 @@ impl Vfs { change } - pub fn get_line_map(&self, file_id: FileId) -> Option<&LineMap> { - Some(&self.files.get(file_id.0 as usize)?.as_ref()?.1) + pub fn file_line_map(&self, file_id: FileId) -> &LineMap { + &self.files[file_id.0 as usize] + .as_ref() + .expect("File must be valid") + .1 } }