lsp: Use one common Error type (where possible)

tools/lsp/common.rs

@@ -5,6 +5,9 @@
 
 use std::path::{Path, PathBuf};
 
+pub type Error = Box<dyn std::error::Error>;
+pub type Result<T> = std::result::Result<T, Error>;
+
 /// API used by the LSP to talk to the Preview. The other direction uses the
 /// ServerNotifier
 pub trait PreviewApi {
@@ -13,11 +16,7 @@ pub trait PreviewApi {
     fn set_contents(&self, path: &Path, contents: &str);
     fn load_preview(&self, component: PreviewComponent, behavior: PostLoadBehavior);
     fn config_changed(&self, style: &str, include_paths: &[PathBuf]);
-    fn highlight(
+    fn highlight(&self, path: Option<PathBuf>, offset: u32) -> Result<()>;
-        &self,
-        path: Option<PathBuf>,
-        offset: u32,
-    ) -> Result<(), Box<dyn std::error::Error>>;
 }
 
 /// The Component to preview

tools/lsp/language.rs

@@ -10,7 +10,7 @@ mod semantic_tokens;
 #[cfg(test)]
 mod test;
 
-use crate::common::PreviewApi;
+use crate::common::{PreviewApi, Result};
 use crate::util::{map_node, map_range, map_token, to_lsp_diag};
 
 #[cfg(target_arch = "wasm32")]
@@ -40,8 +40,6 @@ use std::future::Future;
 use std::pin::Pin;
 use std::rc::Rc;
 
-pub type Error = Box<dyn std::error::Error>;
-
 const QUERY_PROPERTIES_COMMAND: &str = "slint/queryProperties";
 const REMOVE_BINDING_COMMAND: &str = "slint/removeBinding";
 const SHOW_PREVIEW_COMMAND: &str = "slint/showPreview";
@@ -109,7 +107,7 @@ pub struct RequestHandler(
             dyn Fn(
                 serde_json::Value,
                 Rc<Context>,
-            ) -> Pin<Box<dyn Future<Output = Result<serde_json::Value, Error>>>>,
+            ) -> Pin<Box<dyn Future<Output = Result<serde_json::Value>>>>,
         >,
     >,
 );
@@ -117,7 +115,7 @@ pub struct RequestHandler(
 impl RequestHandler {
     pub fn register<
         R: lsp_types::request::Request,
-        Fut: Future<Output = Result<R::Result, Error>> + 'static,
+        Fut: Future<Output = Result<R::Result>> + 'static,
     >(
         &mut self,
         handler: fn(R::Params, Rc<Context>) -> Fut,
@@ -386,7 +384,7 @@ pub fn register_request_handlers(rh: &mut RequestHandler) {
 }
 
 #[cfg(feature = "preview")]
-pub fn show_preview_command(params: &[serde_json::Value], ctx: &Rc<Context>) -> Result<(), Error> {
+pub fn show_preview_command(params: &[serde_json::Value], ctx: &Rc<Context>) -> Result<()> {
     let document_cache = &mut ctx.document_cache.borrow_mut();
     let config = &document_cache.documents.compiler_config;
 
@@ -415,7 +413,7 @@ pub fn show_preview_command(params: &[serde_json::Value], ctx: &Rc<Context>) ->
 }
 
 #[cfg(feature = "preview")]
-pub fn set_design_mode(params: &[serde_json::Value], ctx: &Rc<Context>) -> Result<(), Error> {
+pub fn set_design_mode(params: &[serde_json::Value], ctx: &Rc<Context>) -> Result<()> {
     let e = || "InvalidParameter";
     let enable = if let serde_json::Value::Bool(b) = params.get(0).ok_or_else(e)? {
         b
@@ -428,7 +426,7 @@ pub fn set_design_mode(params: &[serde_json::Value], ctx: &Rc<Context>) -> Resul
 }
 
 #[cfg(feature = "preview")]
-pub fn toggle_design_mode(_params: &[serde_json::Value], ctx: &Rc<Context>) -> Result<(), Error> {
+pub fn toggle_design_mode(_params: &[serde_json::Value], ctx: &Rc<Context>) -> Result<()> {
     ctx.preview.set_design_mode(!ctx.preview.design_mode());
     Ok(())
 }
@@ -436,7 +434,7 @@ pub fn toggle_design_mode(_params: &[serde_json::Value], ctx: &Rc<Context>) -> R
 pub fn query_properties_command(
     params: &[serde_json::Value],
     ctx: &Rc<Context>,
-) -> Result<serde_json::Value, Error> {
+) -> Result<serde_json::Value> {
     let document_cache = &mut ctx.document_cache.borrow_mut();
 
     let text_document_uri = serde_json::from_value::<lsp_types::TextDocumentIdentifier>(
@@ -472,7 +470,7 @@ pub fn query_properties_command(
 pub async fn set_binding_command(
     params: &[serde_json::Value],
     ctx: &Rc<Context>,
-) -> Result<serde_json::Value, Error> {
+) -> Result<serde_json::Value> {
     let text_document = serde_json::from_value::<lsp_types::OptionalVersionedTextDocumentIdentifier>(
         params.get(0).ok_or("No text document provided")?.clone(),
     )?;
@@ -563,7 +561,7 @@ pub async fn set_binding_command(
 pub async fn remove_binding_command(
     params: &[serde_json::Value],
     ctx: &Rc<Context>,
-) -> Result<serde_json::Value, Error> {
+) -> Result<serde_json::Value> {
     let text_document = serde_json::from_value::<lsp_types::OptionalVersionedTextDocumentIdentifier>(
         params.get(0).ok_or("No text document provided")?.clone(),
     )?;
@@ -692,7 +690,7 @@ pub async fn reload_document(
     uri: lsp_types::Url,
     version: i32,
     document_cache: &mut DocumentCache,
-) -> Result<(), Error> {
+) -> Result<()> {
     let lsp_diags = reload_document_impl(Some(ctx), content, uri, version, document_cache).await;
 
     for (uri, diagnostics) in lsp_diags {
@@ -1053,7 +1051,7 @@ fn find_element_id_for_highlight(
     None
 }
 
-pub async fn load_configuration(ctx: &Context) -> Result<(), Error> {
+pub async fn load_configuration(ctx: &Context) -> Result<()> {
     if !ctx
         .init_param
         .capabilities

tools/lsp/language/properties.rs

@@ -2,11 +2,11 @@
 // SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial
 
 use super::DocumentCache;
+use crate::common::{Error, Result};
 use crate::util::{
     map_node, map_node_and_url, map_position, map_range, to_lsp_diag, with_property_lookup_ctx,
     ExpressionContextInfo,
 };
-use crate::Error;
 
 use i_slint_compiler::diagnostics::{BuildDiagnostics, Spanned};
 use i_slint_compiler::langtype::{ElementType, Type};
@@ -410,7 +410,7 @@ pub(crate) fn query_properties(
     uri: &lsp_types::Url,
     source_version: i32,
     element: &ElementRc,
-) -> Result<QueryPropertyResponse, crate::Error> {
+) -> Result<QueryPropertyResponse> {
     Ok(QueryPropertyResponse {
         properties: get_properties(element),
         element: Some(get_element_information(element)),
@@ -422,7 +422,7 @@ pub(crate) fn query_properties(
 fn get_property_information(
     properties: &[PropertyInformation],
     property_name: &str,
-) -> Result<PropertyInformation, Error> {
+) -> Result<PropertyInformation> {
     if let Some(property) = properties.iter().find(|pi| pi.name == property_name) {
         Ok(property.clone())
     } else {
@@ -482,7 +482,7 @@ fn set_binding_on_existing_property(
     property: &PropertyInformation,
     new_expression: String,
     diag: &mut BuildDiagnostics,
-) -> Result<(SetBindingResponse, Option<lsp_types::WorkspaceEdit>), Error> {
+) -> Result<(SetBindingResponse, Option<lsp_types::WorkspaceEdit>)> {
     let workspace_edit = (!diag.has_error())
         .then(|| {
             create_workspace_edit_for_set_binding_on_existing_property(
@@ -599,7 +599,7 @@ fn set_binding_on_known_property(
     property_name: &str,
     new_expression: &str,
     diag: &mut BuildDiagnostics,
-) -> Result<(SetBindingResponse, Option<lsp_types::WorkspaceEdit>), Error> {
+) -> Result<(SetBindingResponse, Option<lsp_types::WorkspaceEdit>)> {
     let workspace_edit = (!diag.has_error())
         .then(|| {
             create_workspace_edit_for_set_binding_on_known_property(
@@ -643,7 +643,7 @@ pub(crate) fn set_binding(
     element: &ElementRc,
     property_name: &str,
     new_expression: String,
-) -> Result<(SetBindingResponse, Option<lsp_types::WorkspaceEdit>), Error> {
+) -> Result<(SetBindingResponse, Option<lsp_types::WorkspaceEdit>)> {
     let (mut diag, expression_node) = {
         let mut diagnostics = BuildDiagnostics::default();
 
@@ -737,7 +737,7 @@ pub(crate) fn remove_binding(
     uri: &lsp_types::Url,
     element: &ElementRc,
     property_name: &str,
-) -> Result<lsp_types::WorkspaceEdit, Error> {
+) -> Result<lsp_types::WorkspaceEdit> {
     let element = element.borrow();
     let source_file = element.node.as_ref().and_then(|n| n.source_file());
 

tools/lsp/main.rs

@@ -10,7 +10,7 @@ pub mod lsp_ext;
 mod preview;
 pub mod util;
 
-use common::PreviewApi;
+use common::{PreviewApi, Result};
 use language::*;
 
 use i_slint_compiler::CompilerConfiguration;
@@ -67,11 +67,7 @@ impl PreviewApi for Previewer {
         preview::config_changed(_style, _include_paths);
     }
 
-    fn highlight(
+    fn highlight(&self, _path: Option<std::path::PathBuf>, _offset: u32) -> Result<()> {
-        &self,
-        _path: Option<std::path::PathBuf>,
-        _offset: u32,
-    ) -> Result<(), Box<dyn std::error::Error>> {
         #[cfg(feature = "preview")]
         preview::highlight(_path, _offset);
         Ok(())
@@ -111,11 +107,7 @@ type OutgoingRequestQueue = Arc<Mutex<HashMap<RequestId, OutgoingRequest>>>;
 #[derive(Clone)]
 pub struct ServerNotifier(crossbeam_channel::Sender<Message>, OutgoingRequestQueue);
 impl ServerNotifier {
-    pub fn send_notification(
+    pub fn send_notification(&self, method: String, params: impl serde::Serialize) -> Result<()> {
-        &self,
-        method: String,
-        params: impl serde::Serialize,
-    ) -> Result<(), Error> {
         self.0.send(Message::Notification(lsp_server::Notification::new(method, params)))?;
         Ok(())
     }
@@ -123,7 +115,7 @@ impl ServerNotifier {
     pub fn send_request<T: lsp_types::request::Request>(
         &self,
         request: T::Params,
-    ) -> Result<impl Future<Output = Result<T::Result, Error>>, Error> {
+    ) -> Result<impl Future<Output = Result<T::Result>>> {
         static REQ_ID: atomic::AtomicI32 = atomic::AtomicI32::new(0);
         let id = RequestId::from(REQ_ID.fetch_add(1, atomic::Ordering::Relaxed));
         let msg =
@@ -153,11 +145,7 @@ impl ServerNotifier {
 }
 
 impl RequestHandler {
-    async fn handle_request(
+    async fn handle_request(&self, request: lsp_server::Request, ctx: &Rc<Context>) -> Result<()> {
-        &self,
-        request: lsp_server::Request,
-        ctx: &Rc<Context>,
-    ) -> Result<(), Error> {
         if let Some(x) = self.0.get(&request.method.as_str()) {
             match x(request.params, ctx.clone()).await {
                 Ok(r) => ctx
@@ -219,7 +207,7 @@ fn main() {
     }
 }
 
-pub fn run_lsp_server() -> Result<(), Error> {
+pub fn run_lsp_server() -> Result<()> {
     let (connection, io_threads) = Connection::stdio();
     let (id, params) = connection.initialize_start()?;
 
@@ -233,7 +221,7 @@ pub fn run_lsp_server() -> Result<(), Error> {
     Ok(())
 }
 
-fn main_loop(connection: Connection, init_param: InitializeParams) -> Result<(), Error> {
+fn main_loop(connection: Connection, init_param: InitializeParams) -> Result<()> {
     let mut compiler_config =
         CompilerConfiguration::new(i_slint_compiler::generator::OutputFormat::Interpreter);
 
@@ -254,7 +242,7 @@ fn main_loop(connection: Connection, init_param: InitializeParams) -> Result<(),
         preview: Box::new(Previewer { server_notifier }),
     });
 
-    let mut futures = Vec::<Pin<Box<dyn Future<Output = Result<(), Error>>>>>::new();
+    let mut futures = Vec::<Pin<Box<dyn Future<Output = Result<()>>>>>::new();
     let mut first_future = Box::pin(load_configuration(&ctx));
 
     // We are waiting in this loop for two kind of futures:
@@ -317,10 +305,7 @@ fn main_loop(connection: Connection, init_param: InitializeParams) -> Result<(),
     Ok(())
 }
 
-async fn handle_notification(
+async fn handle_notification(req: lsp_server::Notification, ctx: &Rc<Context>) -> Result<()> {
-    req: lsp_server::Notification,
-    ctx: &Rc<Context>,
-) -> Result<(), Error> {
     match &*req.method {
         DidOpenTextDocument::METHOD => {
             let params: DidOpenTextDocumentParams = serde_json::from_value(req.params)?;

tools/lsp/wasm_main.rs

@@ -11,9 +11,10 @@ mod preview;
 pub mod util;
 
 use common::PreviewApi;
+use common::{Error, Result};
 use i_slint_compiler::CompilerConfiguration;
 use js_sys::Function;
-pub use language::{Context, DocumentCache, Error, RequestHandler};
+pub use language::{Context, DocumentCache, RequestHandler};
 use serde::Serialize;
 use std::cell::RefCell;
 use std::future::Future;
@@ -22,6 +23,8 @@ use std::path::PathBuf;
 use std::rc::Rc;
 use wasm_bindgen::prelude::*;
 
+type JsResult<T> = std::result::Result<T, JsError>;
+
 pub mod wasm_prelude {
     use std::path::{Path, PathBuf};
 
@@ -70,14 +73,10 @@ impl PreviewApi for Previewer {
         // do nothing!
     }
 
-    fn highlight(
+    fn highlight(&self, path: Option<std::path::PathBuf>, offset: u32) -> Result<()> {
-        &self,
-        path: Option<std::path::PathBuf>,
-        offset: u32,
-    ) -> Result<(), Box<dyn std::error::Error>> {
         self.highlight_in_preview
             .call2(&JsValue::UNDEFINED, &to_value(&path.unwrap_or_default())?, &offset.into())
-            .map_err(|x| std::io::Error::new(ErrorKind::Other, format!("{x:?}")))?;
+            .map_err(|x| format!("{x:?}"))?;
         Ok(())
     }
 }
@@ -88,7 +87,7 @@ pub struct ServerNotifier {
     send_request: Function,
 }
 impl ServerNotifier {
-    pub fn send_notification(&self, method: String, params: impl Serialize) -> Result<(), Error> {
+    pub fn send_notification(&self, method: String, params: impl Serialize) -> Result<()> {
         self.send_notification
             .call2(&JsValue::UNDEFINED, &method.into(), &to_value(&params)?)
             .map_err(|x| format!("Error calling send_notification: {x:?}"))?;
@@ -98,7 +97,7 @@ impl ServerNotifier {
     pub fn send_request<T: lsp_types::request::Request>(
         &self,
         request: T::Params,
-    ) -> Result<impl Future<Output = Result<T::Result, Error>>, Error> {
+    ) -> Result<impl Future<Output = Result<T::Result>>> {
         let promise = self
             .send_request
             .call2(&JsValue::UNDEFINED, &T::METHOD.into(), &to_value(&request)?)
@@ -118,7 +117,7 @@ impl RequestHandler {
         method: String,
         params: JsValue,
         ctx: Rc<Context>,
-    ) -> Result<JsValue, Error> {
+    ) -> Result<JsValue> {
         if let Some(f) = self.0.get(&method.as_str()) {
             let param = serde_wasm_bindgen::from_value(params)
                 .map_err(|x| format!("invalid param to handle_request: {x:?}"))?;
@@ -207,7 +206,7 @@ pub fn create(
     send_request: SendRequestFunction,
     load_file: ImportCallbackFunction,
     highlight_in_preview: HighlightInPreviewFunction,
-) -> Result<SlintServer, JsError> {
+) -> JsResult<SlintServer> {
     console_error_panic_hook::set_once();
 
     let init_param = serde_wasm_bindgen::from_value(init_param)?;
@@ -242,7 +241,7 @@ pub fn create(
 #[wasm_bindgen]
 impl SlintServer {
     #[wasm_bindgen]
-    pub fn server_initialize_result(&self, cap: JsValue) -> Result<JsValue, JsError> {
+    pub fn server_initialize_result(&self, cap: JsValue) -> JsResult<JsValue> {
         Ok(to_value(&language::server_initialize_result(&serde_wasm_bindgen::from_value(cap)?))?)
     }
 
@@ -267,7 +266,7 @@ impl SlintServer {
     }
 
     /*  #[wasm_bindgen]
-    pub fn show_preview(&self, params: JsValue) -> Result<(), JsError> {
+    pub fn show_preview(&self, params: JsValue) -> JsResult<()> {
         language::show_preview_command(
             &serde_wasm_bindgen::from_value(params)?,
             &ServerNotifier,
@@ -289,7 +288,7 @@ impl SlintServer {
     }
 
     #[wasm_bindgen]
-    pub async fn reload_config(&self) -> Result<(), JsError> {
+    pub async fn reload_config(&self) -> JsResult<()> {
         let guard = self.reentry_guard.clone();
         let _lock = ReentryGuard::lock(guard).await;
         language::load_configuration(&self.ctx).await.map_err(|e| JsError::new(&e.to_string()))
@@ -309,6 +308,6 @@ async fn load_file(path: String, load_file: &Function) -> std::io::Result<String
 // Use a JSON friendly representation to avoid using ES maps instead of JS objects.
 fn to_value<T: serde::Serialize + ?Sized>(
     value: &T,
-) -> Result<wasm_bindgen::JsValue, serde_wasm_bindgen::Error> {
+) -> std::result::Result<wasm_bindgen::JsValue, serde_wasm_bindgen::Error> {
     value.serialize(&serde_wasm_bindgen::Serializer::json_compatible())
 }