From f7fc8fb084e30c6846709cc4aa8cbd18fcc86a8b Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 2 Jul 2025 11:01:41 +0200 Subject: [PATCH] [ty] Request configuration from client (#18984) ## Summary This PR makes the necessary changes to the server that it can request configurations from the client using the `configuration` request. This PR doesn't make use of the request yet. It only sets up the foundation (mainly the coordination between client and server) so that future PRs could pull specific settings. I plan to use this for pulling the Python environment from the Python extension. Deno does something very similar to this. ## Test Plan Tested that diagnostics are still shown. --- crates/ruff_db/src/files.rs | 19 +- crates/ty_project/src/db.rs | 11 + crates/ty_project/src/lib.rs | 1 + crates/ty_server/src/server.rs | 12 +- crates/ty_server/src/server/api.rs | 27 +- .../ty_server/src/server/api/diagnostics.rs | 34 +-- .../src/server/api/notifications/cancel.rs | 2 +- .../server/api/notifications/did_change.rs | 2 +- .../notifications/did_change_watched_files.rs | 19 +- .../src/server/api/notifications/did_close.rs | 4 +- .../src/server/api/notifications/did_open.rs | 2 +- .../api/notifications/did_open_notebook.rs | 2 +- crates/ty_server/src/server/main_loop.rs | 88 +++++- crates/ty_server/src/server/schedule/task.rs | 4 +- crates/ty_server/src/session.rs | 268 +++++++++++++++--- crates/ty_server/src/session/client.rs | 120 ++++---- crates/ty_server/src/session/options.rs | 22 +- 17 files changed, 450 insertions(+), 187 deletions(-) diff --git a/crates/ruff_db/src/files.rs b/crates/ruff_db/src/files.rs index 34c86626eb..a5cb501f9f 100644 --- a/crates/ruff_db/src/files.rs +++ b/crates/ruff_db/src/files.rs @@ -263,12 +263,23 @@ impl Files { impl fmt::Debug for Files { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut map = f.debug_map(); + if f.alternate() { + let mut map = f.debug_map(); - for entry in self.inner.system_by_path.iter() { - map.entry(entry.key(), entry.value()); + for entry in self.inner.system_by_path.iter() { + map.entry(entry.key(), entry.value()); + } + map.finish() + } else { + f.debug_struct("Files") + .field("system_by_path", &self.inner.system_by_path.len()) + .field( + "system_virtual_by_path", + &self.inner.system_virtual_by_path.len(), + ) + .field("vendored_by_path", &self.inner.vendored_by_path.len()) + .finish() } - map.finish() } } diff --git a/crates/ty_project/src/db.rs b/crates/ty_project/src/db.rs index 2f26a93db3..9c536b2a31 100644 --- a/crates/ty_project/src/db.rs +++ b/crates/ty_project/src/db.rs @@ -1,3 +1,4 @@ +use std::fmt::Formatter; use std::panic::{AssertUnwindSafe, RefUnwindSafe}; use std::sync::Arc; use std::{cmp, fmt}; @@ -146,6 +147,16 @@ impl ProjectDatabase { } } +impl std::fmt::Debug for ProjectDatabase { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.debug_struct("ProjectDatabase") + .field("project", &self.project) + .field("files", &self.files) + .field("system", &self.system) + .finish_non_exhaustive() + } +} + /// Stores memory usage information. pub struct SalsaMemoryDump { total_fields: usize, diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index 2f503de51f..122f0010c9 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -55,6 +55,7 @@ pub fn default_lints_registry() -> LintRegistry { /// it remains the same project. That's why program is a narrowed view of the project only /// holding on to the most fundamental settings required for checking. #[salsa::input] +#[derive(Debug)] pub struct Project { /// The files that are open in the project. /// diff --git a/crates/ty_server/src/server.rs b/crates/ty_server/src/server.rs index 7271c12919..5db60c3674 100644 --- a/crates/ty_server/src/server.rs +++ b/crates/ty_server/src/server.rs @@ -136,7 +136,7 @@ impl Server { &client_capabilities, position_encoding, global_options, - &workspaces, + workspaces, )?, client_capabilities, }) @@ -227,12 +227,10 @@ impl ServerPanicHookHandler { writeln!(stderr, "{panic_info}\n{backtrace}").ok(); if let Some(client) = hook_client.upgrade() { - client - .show_message( - "The ty language server exited with a panic. See the logs for more details.", - MessageType::ERROR, - ) - .ok(); + client.show_message( + "The ty language server exited with a panic. See the logs for more details.", + MessageType::ERROR, + ); } })); diff --git a/crates/ty_server/src/server/api.rs b/crates/ty_server/src/server/api.rs index cf1ffcdaad..438bc5f754 100644 --- a/crates/ty_server/src/server/api.rs +++ b/crates/ty_server/src/server/api.rs @@ -160,7 +160,7 @@ where }; let db = match &path { - AnySystemPath::System(path) => match session.project_db_for_path(path.as_std_path()) { + AnySystemPath::System(path) => match session.project_db_for_path(path) { Some(db) => db.clone(), None => session.default_project_db().clone(), }, @@ -224,17 +224,14 @@ where request.id, request.method ); - if client.retry(request).is_ok() { - return None; - } + client.retry(request); + } else { + tracing::trace!( + "request id={} was cancelled by salsa, sending content modified", + id + ); + respond_silent_error(id.clone(), client, R::salsa_cancellation_error()); } - - tracing::trace!( - "request id={} was cancelled by salsa, sending content modified", - id - ); - - respond_silent_error(id.clone(), client, R::salsa_cancellation_error()); None } else { Some(Err(Error { @@ -343,17 +340,13 @@ fn respond( tracing::error!("An error occurred with request ID {id}: {err}"); client.show_error_message("ty encountered a problem. Check the logs for more details."); } - if let Err(err) = client.respond(id, result) { - tracing::error!("Failed to send response: {err}"); - } + client.respond(id, result); } /// Sends back an error response to the server using a [`Client`] without showing a warning /// to the user. fn respond_silent_error(id: RequestId, client: &Client, error: lsp_server::ResponseError) { - if let Err(err) = client.respond_err(id, error) { - tracing::error!("Failed to send response: {err}"); - } + client.respond_err(id, error); } /// Tries to cast a serialized request from the server into diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index 0ae55e8c3f..7a05ba8d76 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -1,4 +1,3 @@ -use lsp_server::ErrorCode; use lsp_types::notification::PublishDiagnostics; use lsp_types::{ CodeDescription, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, @@ -46,20 +45,17 @@ impl Diagnostics { /// This is done by notifying the client with an empty list of diagnostics for the document. /// For notebook cells, this clears diagnostics for the specific cell. /// For other document types, this clears diagnostics for the main document. -pub(super) fn clear_diagnostics(key: &DocumentKey, client: &Client) -> Result<()> { +pub(super) fn clear_diagnostics(key: &DocumentKey, client: &Client) { let Some(uri) = key.to_url() else { // If we can't convert to URL, we can't clear diagnostics - return Ok(()); + return; }; - client - .send_notification::(PublishDiagnosticsParams { - uri, - diagnostics: vec![], - version: None, - }) - .with_failure_code(ErrorCode::InternalError)?; - Ok(()) + client.send_notification::(PublishDiagnosticsParams { + uri, + diagnostics: vec![], + version: None, + }); } /// Publishes the diagnostics for the given document snapshot using the [publish diagnostics @@ -96,22 +92,20 @@ pub(super) fn publish_diagnostics( // Sends a notification to the client with the diagnostics for the document. let publish_diagnostics_notification = |uri: Url, diagnostics: Vec| { - client - .send_notification::(PublishDiagnosticsParams { - uri, - diagnostics, - version: Some(snapshot.query().version()), - }) - .with_failure_code(lsp_server::ErrorCode::InternalError) + client.send_notification::(PublishDiagnosticsParams { + uri, + diagnostics, + version: Some(snapshot.query().version()), + }); }; match diagnostics { Diagnostics::TextDocument(diagnostics) => { - publish_diagnostics_notification(url, diagnostics)?; + publish_diagnostics_notification(url, diagnostics); } Diagnostics::NotebookDocument(cell_diagnostics) => { for (cell_url, diagnostics) in cell_diagnostics { - publish_diagnostics_notification(cell_url, diagnostics)?; + publish_diagnostics_notification(cell_url, diagnostics); } } } diff --git a/crates/ty_server/src/server/api/notifications/cancel.rs b/crates/ty_server/src/server/api/notifications/cancel.rs index c5bcf7429e..c8f7199e75 100644 --- a/crates/ty_server/src/server/api/notifications/cancel.rs +++ b/crates/ty_server/src/server/api/notifications/cancel.rs @@ -20,7 +20,7 @@ impl SyncNotificationHandler for CancelNotificationHandler { lsp_types::NumberOrString::String(id) => id.into(), }; - let _ = client.cancel(session, id); + client.cancel(session, id); Ok(()) } diff --git a/crates/ty_server/src/server/api/notifications/did_change.rs b/crates/ty_server/src/server/api/notifications/did_change.rs index b7df7961c4..092acc7e87 100644 --- a/crates/ty_server/src/server/api/notifications/did_change.rs +++ b/crates/ty_server/src/server/api/notifications/did_change.rs @@ -39,7 +39,7 @@ impl SyncNotificationHandler for DidChangeTextDocumentHandler { match key.path() { AnySystemPath::System(path) => { - let db = match session.project_db_for_path_mut(path.as_std_path()) { + let db = match session.project_db_for_path_mut(path) { Some(db) => db, None => session.default_project_db_mut(), }; diff --git a/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs b/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs index 98c67829b1..cf8822858e 100644 --- a/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs +++ b/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs @@ -1,5 +1,4 @@ use crate::server::Result; -use crate::server::api::LSPResult; use crate::server::api::diagnostics::publish_diagnostics; use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::session::Session; @@ -45,7 +44,7 @@ impl SyncNotificationHandler for DidChangeWatchedFiles { } }; - let Some(db) = session.project_db_for_path(system_path.as_std_path()) else { + let Some(db) = session.project_db_for_path(&system_path) else { tracing::trace!( "Ignoring change event for `{system_path}` because it's not in any workspace" ); @@ -103,13 +102,11 @@ impl SyncNotificationHandler for DidChangeWatchedFiles { if project_changed { if client_capabilities.diagnostics_refresh { - client - .send_request::( - session, - (), - |_, ()| {}, - ) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; + client.send_request::( + session, + (), + |_, ()| {}, + ); } else { for key in session.text_document_keys() { publish_diagnostics(session, &key, client)?; @@ -120,9 +117,7 @@ impl SyncNotificationHandler for DidChangeWatchedFiles { } if client_capabilities.inlay_refresh { - client - .send_request::(session, (), |_, ()| {}) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; + client.send_request::(session, (), |_, ()| {}); } Ok(()) diff --git a/crates/ty_server/src/server/api/notifications/did_close.rs b/crates/ty_server/src/server/api/notifications/did_close.rs index b7b06cdc82..168c27048a 100644 --- a/crates/ty_server/src/server/api/notifications/did_close.rs +++ b/crates/ty_server/src/server/api/notifications/did_close.rs @@ -41,6 +41,8 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler { ); } - clear_diagnostics(&key, client) + clear_diagnostics(&key, client); + + Ok(()) } } diff --git a/crates/ty_server/src/server/api/notifications/did_open.rs b/crates/ty_server/src/server/api/notifications/did_open.rs index 1f61d07f7c..04a4ef8f08 100644 --- a/crates/ty_server/src/server/api/notifications/did_open.rs +++ b/crates/ty_server/src/server/api/notifications/did_open.rs @@ -41,7 +41,7 @@ impl SyncNotificationHandler for DidOpenTextDocumentHandler { match key.path() { AnySystemPath::System(system_path) => { - let db = match session.project_db_for_path_mut(system_path.as_std_path()) { + let db = match session.project_db_for_path_mut(system_path) { Some(db) => db, None => session.default_project_db_mut(), }; diff --git a/crates/ty_server/src/server/api/notifications/did_open_notebook.rs b/crates/ty_server/src/server/api/notifications/did_open_notebook.rs index ccae65a15d..644046a072 100644 --- a/crates/ty_server/src/server/api/notifications/did_open_notebook.rs +++ b/crates/ty_server/src/server/api/notifications/did_open_notebook.rs @@ -40,7 +40,7 @@ impl SyncNotificationHandler for DidOpenNotebookHandler { match &path { AnySystemPath::System(system_path) => { - let db = match session.project_db_for_path_mut(system_path.as_std_path()) { + let db = match session.project_db_for_path_mut(system_path) { Some(db) => db, None => session.default_project_db_mut(), }; diff --git a/crates/ty_server/src/server/main_loop.rs b/crates/ty_server/src/server/main_loop.rs index 6b05f39317..45187a66bd 100644 --- a/crates/ty_server/src/server/main_loop.rs +++ b/crates/ty_server/src/server/main_loop.rs @@ -1,11 +1,15 @@ use crate::server::schedule::Scheduler; use crate::server::{Server, api}; +use crate::session::ClientOptions; use crate::session::client::Client; use anyhow::anyhow; use crossbeam::select; use lsp_server::Message; use lsp_types::notification::Notification; -use lsp_types::{DidChangeWatchedFilesRegistrationOptions, FileSystemWatcher}; +use lsp_types::{ + ConfigurationParams, DidChangeWatchedFilesRegistrationOptions, FileSystemWatcher, Url, +}; +use serde_json::Value; pub(crate) type MainLoopSender = crossbeam::channel::Sender; pub(crate) type MainLoopReceiver = crossbeam::channel::Receiver; @@ -26,6 +30,10 @@ impl Server { match next_event { Event::Message(msg) => { + let Some(msg) = self.session.should_defer_message(msg) else { + continue; + }; + let client = Client::new( self.main_loop_sender.clone(), self.connection.sender.clone(), @@ -49,7 +57,7 @@ impl Server { message: "Shutdown already requested".to_owned(), data: None, }, - )?; + ); continue; } @@ -130,6 +138,9 @@ impl Server { ); } } + Action::InitializeWorkspaces(workspaces_with_options) => { + self.session.initialize_workspaces(workspaces_with_options); + } }, } } @@ -140,7 +151,25 @@ impl Server { /// Waits for the next message from the client or action. /// /// Returns `Ok(None)` if the client connection is closed. - fn next_event(&self) -> Result, crossbeam::channel::RecvError> { + fn next_event(&mut self) -> Result, crossbeam::channel::RecvError> { + // We can't queue those into the main loop because that could result in reordering if + // the `select` below picks a client message first. + if let Some(deferred) = self.session.take_deferred_messages() { + match &deferred { + Message::Request(req) => { + tracing::debug!("Processing deferred request `{}`", req.method); + } + Message::Notification(notification) => { + tracing::debug!("Processing deferred notification `{}`", notification.method); + } + Message::Response(response) => { + tracing::debug!("Processing deferred response `{}`", response.id); + } + } + + return Ok(Some(Event::Message(deferred))); + } + select!( recv(self.connection.receiver) -> msg => { // Ignore disconnect errors, they're handled by the main loop (it will exit). @@ -151,6 +180,47 @@ impl Server { } fn initialize(&mut self, client: &Client) { + let urls = self + .session + .workspaces() + .urls() + .cloned() + .collect::>(); + let items = urls + .iter() + .map(|root| lsp_types::ConfigurationItem { + scope_uri: Some(root.clone()), + section: Some("ty".to_string()), + }) + .collect(); + + tracing::debug!("Requesting workspace configuration for workspaces"); + client + .send_request::( + &self.session, + ConfigurationParams { items }, + |client, result: Vec| { + tracing::debug!("Received workspace configurations, initializing workspaces"); + assert_eq!(result.len(), urls.len()); + + let workspaces_with_options: Vec<_> = urls + .into_iter() + .zip(result) + .map(|(url, value)| { + let options: ClientOptions = serde_json::from_value(value).unwrap_or_else(|err| { + tracing::warn!("Failed to deserialize workspace options for {url}: {err}. Using default options."); + ClientOptions::default() + }); + + (url, options) + }) + .collect(); + + + client.queue_action(Action::InitializeWorkspaces(workspaces_with_options)); + }, + ); + let fs_watcher = self .client_capabilities .workspace @@ -206,17 +276,13 @@ impl Server { tracing::info!("File watcher successfully registered"); }; - if let Err(err) = client.send_request::( + client.send_request::( &self.session, lsp_types::RegistrationParams { registrations: vec![registration], }, response_handler, - ) { - tracing::error!( - "An error occurred when trying to register the configuration file watcher: {err}" - ); - } + ); } else { tracing::warn!("The client does not support file system watching."); } @@ -231,6 +297,10 @@ pub(crate) enum Action { /// Retry a request that previously failed due to a salsa cancellation. RetryRequest(lsp_server::Request), + + /// Initialize the workspace after the server received + /// the options from the client. + InitializeWorkspaces(Vec<(Url, ClientOptions)>), } #[derive(Debug)] diff --git a/crates/ty_server/src/server/schedule/task.rs b/crates/ty_server/src/server/schedule/task.rs index 024a76fe13..3d4ddb3123 100644 --- a/crates/ty_server/src/server/schedule/task.rs +++ b/crates/ty_server/src/server/schedule/task.rs @@ -83,9 +83,7 @@ impl Task { R: Serialize + Send + 'static, { Self::sync(move |_, client| { - if let Err(err) = client.respond(&id, result) { - tracing::error!("Unable to send immediate response: {err}"); - } + client.respond(&id, result); }) } diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 2f4ded2fa6..b7c365240d 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -1,22 +1,24 @@ //! Data model, state management, and configuration resolution. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, VecDeque}; use std::ops::{Deref, DerefMut}; use std::path::{Path, PathBuf}; use std::sync::Arc; -use anyhow::anyhow; +use anyhow::{Context, anyhow}; +use lsp_server::Message; use lsp_types::{ClientCapabilities, TextDocumentContentChangeEvent, Url}; use options::GlobalOptions; use ruff_db::Db; use ruff_db::files::{File, system_path_to_file}; -use ruff_db::system::SystemPath; +use ruff_db::system::{System, SystemPath, SystemPathBuf}; +use ty_project::metadata::Options; use ty_project::{ProjectDatabase, ProjectMetadata}; pub(crate) use self::capabilities::ResolvedClientCapabilities; pub use self::index::DocumentQuery; pub(crate) use self::options::{AllOptions, ClientOptions}; -use self::settings::ClientSettings; +pub(crate) use self::settings::ClientSettings; use crate::document::{DocumentKey, DocumentVersion, NotebookDocument}; use crate::session::request_queue::RequestQueue; use crate::system::{AnySystemPath, LSPSystem}; @@ -40,8 +42,13 @@ pub struct Session { /// [`index_mut`]: Session::index_mut index: Option>, - /// Maps workspace folders to their respective project databases. - projects_by_workspace_folder: BTreeMap, + /// Maps workspace folders to their respective workspace. + workspaces: Workspaces, + + /// The projects across all workspaces. + projects: BTreeMap, + + default_project: ProjectDatabase, /// The global position encoding, negotiated during LSP initialization. position_encoding: PositionEncoding, @@ -54,6 +61,8 @@ pub struct Session { /// Has the client requested the server to shutdown. shutdown_requested: bool, + + deferred_messages: VecDeque, } impl Session { @@ -61,32 +70,33 @@ impl Session { client_capabilities: &ClientCapabilities, position_encoding: PositionEncoding, global_options: GlobalOptions, - workspace_folders: &[(Url, ClientOptions)], + workspace_folders: Vec<(Url, ClientOptions)>, ) -> crate::Result { - let mut workspaces = BTreeMap::new(); let index = Arc::new(index::Index::new(global_options.into_settings())); - // TODO: Consider workspace settings - for (url, _) in workspace_folders { - let path = url - .to_file_path() - .map_err(|()| anyhow!("Workspace URL is not a file or directory: {:?}", url))?; - let system_path = SystemPath::from_std_path(&path) - .ok_or_else(|| anyhow!("Workspace path is not a valid UTF-8 path: {:?}", path))?; - let system = LSPSystem::new(index.clone()); - - // TODO(dhruvmanila): Get the values from the client settings - let mut metadata = ProjectMetadata::discover(system_path, &system)?; - metadata.apply_configuration_files(&system)?; - - // TODO(micha): Handle the case where the program settings are incorrect more gracefully. - workspaces.insert(path, ProjectDatabase::new(metadata, system)?); + let mut workspaces = Workspaces::default(); + for (url, options) in workspace_folders { + workspaces.register(url, options)?; } + let default_project = { + let system = LSPSystem::new(index.clone()); + let metadata = ProjectMetadata::from_options( + Options::default(), + system.current_directory().to_path_buf(), + None, + ) + .unwrap(); + ProjectDatabase::new(metadata, system).unwrap() + }; + Ok(Self { position_encoding, - projects_by_workspace_folder: workspaces, + workspaces, + deferred_messages: VecDeque::new(), index: Some(index), + default_project, + projects: BTreeMap::new(), resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new( client_capabilities, )), @@ -111,6 +121,52 @@ impl Session { self.shutdown_requested = requested; } + /// The LSP specification doesn't allow configuration requests during initialization, + /// but we need access to the configuration to resolve the settings in turn to create the + /// project databases. This will become more important in the future when we support + /// persistent caching. It's then crucial that we have the correct settings to select the + /// right cache. + /// + /// We work around this by queueing up all messages that arrive between the `initialized` notification + /// and the completion of workspace initialization (which waits for the client's configuration response). + /// + /// This queuing is only necessary when registering *new* workspaces. Changes to configurations + /// don't need to go through the same process because we can update the existing + /// database in place. + /// + /// See + pub(crate) fn should_defer_message(&mut self, message: Message) -> Option { + if self.workspaces.all_initialized() { + Some(message) + } else { + match &message { + Message::Request(request) => { + tracing::debug!( + "Deferring `{}` request until all workspaces are initialized", + request.method + ); + } + Message::Response(_) => { + // We still want to get client responses even during workspace initialization. + return Some(message); + } + Message::Notification(notification) => { + tracing::debug!( + "Deferring `{}` notification until all workspaces are initialized", + notification.method + ); + } + } + + self.deferred_messages.push_back(message); + None + } + } + + pub(crate) fn workspaces(&self) -> &Workspaces { + &self.workspaces + } + // TODO(dhruvmanila): Ideally, we should have a single method for `workspace_db_for_path_mut` // and `default_workspace_db_mut` but the borrow checker doesn't allow that. // https://github.com/astral-sh/ruff/pull/13041#discussion_r1726725437 @@ -119,14 +175,17 @@ impl Session { /// or the default project if no project is found for the path. pub(crate) fn project_db_or_default(&self, path: &AnySystemPath) -> &ProjectDatabase { path.as_system() - .and_then(|path| self.project_db_for_path(path.as_std_path())) + .and_then(|path| self.project_db_for_path(path)) .unwrap_or_else(|| self.default_project_db()) } /// Returns a reference to the project's [`ProjectDatabase`] corresponding to the given path, if /// any. - pub(crate) fn project_db_for_path(&self, path: impl AsRef) -> Option<&ProjectDatabase> { - self.projects_by_workspace_folder + pub(crate) fn project_db_for_path( + &self, + path: impl AsRef, + ) -> Option<&ProjectDatabase> { + self.projects .range(..=path.as_ref().to_path_buf()) .next_back() .map(|(_, db)| db) @@ -136,9 +195,9 @@ impl Session { /// path, if any. pub(crate) fn project_db_for_path_mut( &mut self, - path: impl AsRef, + path: impl AsRef, ) -> Option<&mut ProjectDatabase> { - self.projects_by_workspace_folder + self.projects .range_mut(..=path.as_ref().to_path_buf()) .next_back() .map(|(_, db)| db) @@ -147,23 +206,85 @@ impl Session { /// Returns a reference to the default project [`ProjectDatabase`]. The default project is the /// minimum root path in the project map. pub(crate) fn default_project_db(&self) -> &ProjectDatabase { - // SAFETY: Currently, ty only support a single project. - self.projects_by_workspace_folder.values().next().unwrap() + &self.default_project } /// Returns a mutable reference to the default project [`ProjectDatabase`]. pub(crate) fn default_project_db_mut(&mut self) -> &mut ProjectDatabase { - // SAFETY: Currently, ty only support a single project. - self.projects_by_workspace_folder + &mut self.default_project + } + + fn projects_mut(&mut self) -> impl Iterator + '_ { + self.projects .values_mut() - .next() - .unwrap() + .chain(std::iter::once(&mut self.default_project)) } pub(crate) fn key_from_url(&self, url: Url) -> crate::Result { self.index().key_from_url(url) } + pub(crate) fn initialize_workspaces(&mut self, workspace_settings: Vec<(Url, ClientOptions)>) { + assert!(!self.workspaces.all_initialized()); + + for (url, options) in workspace_settings { + let Some(workspace) = self.workspaces.initialize(&url, options) else { + continue; + }; + // For now, create one project database per workspace. + // In the future, index the workspace directories to find all projects + // and create a project database for each. + let system = LSPSystem::new(self.index.as_ref().unwrap().clone()); + + let Some(system_path) = SystemPath::from_std_path(workspace.root()) else { + tracing::warn!( + "Ignore workspace `{}` because it's root contains non UTF8 characters", + workspace.root().display() + ); + continue; + }; + + let root = system_path.to_path_buf(); + let project = ProjectMetadata::discover(&root, &system) + .context("Failed to find project configuration") + .and_then(|mut metadata| { + // TODO(dhruvmanila): Merge the client options with the project metadata options. + metadata + .apply_configuration_files(&system) + .context("Failed to apply configuration files")?; + ProjectDatabase::new(metadata, system) + .context("Failed to create project database") + }); + + // TODO(micha): Handle the case where the program settings are incorrect more gracefully. + // The easiest is to ignore those projects but to show a message to the user that we do so. + // Ignoring the projects has the effect that we'll use the default project for those files. + // The only challenge with this is that we need to register the project when the configuration + // becomes valid again. But that's a case we need to handle anyway for good mono repository support. + match project { + Ok(project) => { + self.projects.insert(root, project); + } + Err(err) => { + tracing::warn!("Failed to create project database for `{root}`: {err}",); + } + } + } + + assert!( + self.workspaces.all_initialized(), + "All workspaces should be initialized after calling `initialize_workspaces`" + ); + } + + pub(crate) fn take_deferred_messages(&mut self) -> Option { + if self.workspaces.all_initialized() { + self.deferred_messages.pop_front() + } else { + None + } + } + /// Creates a document snapshot with the URL referencing the document to snapshot. /// /// Returns `None` if the url can't be converted to a document key or if the document isn't open. @@ -240,7 +361,7 @@ impl Session { fn index_mut(&mut self) -> MutIndexGuard { let index = self.index.take().unwrap(); - for db in self.projects_by_workspace_folder.values_mut() { + for db in self.projects_mut() { // Remove the `index` from each database. This drops the count of `Arc` down to 1 db.system_mut() .as_any_mut() @@ -250,11 +371,11 @@ impl Session { } // There should now be exactly one reference to index which is self.index. - let index = Arc::into_inner(index); + let index = Arc::into_inner(index).unwrap(); MutIndexGuard { session: self, - index, + index: Some(index), } } @@ -289,7 +410,7 @@ impl Drop for MutIndexGuard<'_> { fn drop(&mut self) { if let Some(index) = self.index.take() { let index = Arc::new(index); - for db in self.session.projects_by_workspace_folder.values_mut() { + for db in self.session.projects_mut() { db.system_mut() .as_any_mut() .downcast_mut::() @@ -339,3 +460,72 @@ impl DocumentSnapshot { } } } + +#[derive(Debug, Default)] +pub(crate) struct Workspaces { + workspaces: BTreeMap, + uninitialized: usize, +} + +impl Workspaces { + pub(crate) fn register(&mut self, url: Url, options: ClientOptions) -> anyhow::Result<()> { + let path = url + .to_file_path() + .map_err(|()| anyhow!("Workspace URL is not a file or directory: {url:?}"))?; + + self.workspaces.insert( + url, + Workspace { + options, + root: path, + }, + ); + + self.uninitialized += 1; + + Ok(()) + } + + pub(crate) fn initialize( + &mut self, + url: &Url, + options: ClientOptions, + ) -> Option<&mut Workspace> { + if let Some(workspace) = self.workspaces.get_mut(url) { + workspace.options = options; + self.uninitialized -= 1; + Some(workspace) + } else { + None + } + } + + pub(crate) fn urls(&self) -> impl Iterator + '_ { + self.workspaces.keys() + } + + pub(crate) fn all_initialized(&self) -> bool { + self.uninitialized == 0 + } +} + +impl<'a> IntoIterator for &'a Workspaces { + type Item = (&'a Url, &'a Workspace); + type IntoIter = std::collections::btree_map::Iter<'a, Url, Workspace>; + + fn into_iter(self) -> Self::IntoIter { + self.workspaces.iter() + } +} + +#[derive(Debug)] +pub(crate) struct Workspace { + root: PathBuf, + options: ClientOptions, +} + +impl Workspace { + pub(crate) fn root(&self) -> &Path { + &self.root + } +} diff --git a/crates/ty_server/src/session/client.rs b/crates/ty_server/src/session/client.rs index f9506e375b..eb8aa406b2 100644 --- a/crates/ty_server/src/session/client.rs +++ b/crates/ty_server/src/session/client.rs @@ -1,7 +1,6 @@ use crate::Session; use crate::server::{Action, ConnectionSender}; use crate::server::{Event, MainLoopSender}; -use anyhow::{Context, anyhow}; use lsp_server::{ErrorCode, Message, Notification, RequestId, ResponseError}; use serde_json::Value; use std::any::TypeId; @@ -45,8 +44,7 @@ impl Client { session: &Session, params: R::Params, response_handler: impl FnOnce(&Client, R::Result) + Send + 'static, - ) -> crate::Result<()> - where + ) where R: lsp_types::request::Request, { let response_handler = Box::new(move |client: &Client, response: lsp_server::Response| { @@ -95,60 +93,64 @@ impl Client { .outgoing() .register(response_handler); - self.client_sender + if let Err(err) = self + .client_sender .send(Message::Request(lsp_server::Request { id, method: R::METHOD.to_string(), - params: serde_json::to_value(params).context("Failed to serialize params")?, + params: serde_json::to_value(params).expect("Params to be serializable"), })) - .with_context(|| { - format!("Failed to send request method={method}", method = R::METHOD) - })?; - - Ok(()) + { + tracing::error!( + "Failed to send request `{}` because the client sender is closed: {err}", + R::METHOD + ); + } } /// Sends a notification to the client. - pub(crate) fn send_notification(&self, params: N::Params) -> crate::Result<()> + pub(crate) fn send_notification(&self, params: N::Params) where N: lsp_types::notification::Notification, { let method = N::METHOD.to_string(); - self.client_sender - .send(lsp_server::Message::Notification(Notification::new( - method, params, - ))) - .map_err(|error| { - anyhow!( - "Failed to send notification (method={method}): {error}", - method = N::METHOD - ) - }) + if let Err(err) = + self.client_sender + .send(lsp_server::Message::Notification(Notification::new( + method, params, + ))) + { + tracing::error!( + "Failed to send notification `{}` because the client sender is closed: {err}", + N::METHOD + ); + } } /// Sends a notification without any parameters to the client. /// /// This is useful for notifications that don't require any data. #[expect(dead_code)] - pub(crate) fn send_notification_no_params(&self, method: &str) -> crate::Result<()> { - self.client_sender - .send(lsp_server::Message::Notification(Notification::new( - method.to_string(), - Value::Null, - ))) - .map_err(|error| anyhow!("Failed to send notification (method={method}): {error}",)) + pub(crate) fn send_notification_no_params(&self, method: &str) { + if let Err(err) = + self.client_sender + .send(lsp_server::Message::Notification(Notification::new( + method.to_string(), + Value::Null, + ))) + { + tracing::error!( + "Failed to send notification `{method}` because the client sender is closed: {err}", + ); + } } /// Sends a response to the client for a given request ID. /// /// The response isn't sent immediately. Instead, it's queued up in the main loop /// and checked for cancellation (each request must have exactly one response). - pub(crate) fn respond( - &self, - id: &RequestId, - result: crate::server::Result, - ) -> crate::Result<()> + pub(crate) fn respond(&self, id: &RequestId, result: crate::server::Result) where R: serde::Serialize, { @@ -161,17 +163,13 @@ impl Client { self.main_loop_sender .send(Event::Action(Action::SendResponse(response))) - .map_err(|error| anyhow!("Failed to send response for request {id}: {error}")) + .unwrap(); } /// Sends an error response to the client for a given request ID. /// /// The response isn't sent immediately. Instead, it's queued up in the main loop. - pub(crate) fn respond_err( - &self, - id: RequestId, - error: lsp_server::ResponseError, - ) -> crate::Result<()> { + pub(crate) fn respond_err(&self, id: RequestId, error: lsp_server::ResponseError) { let response = lsp_server::Response { id, result: None, @@ -180,23 +178,19 @@ impl Client { self.main_loop_sender .send(Event::Action(Action::SendResponse(response))) - .map_err(|error| anyhow!("Failed to send response: {error}")) + .unwrap(); } /// Shows a message to the user. /// /// This opens a pop up in VS Code showing `message`. - pub(crate) fn show_message( - &self, - message: impl Display, - message_type: lsp_types::MessageType, - ) -> crate::Result<()> { + pub(crate) fn show_message(&self, message: impl Display, message_type: lsp_types::MessageType) { self.send_notification::( lsp_types::ShowMessageParams { typ: message_type, message: message.to_string(), }, - ) + ); } /// Sends a request to display a warning to the client with a formatted message. The warning is @@ -204,11 +198,7 @@ impl Client { /// /// Logs an error if the message could not be sent. pub(crate) fn show_warning_message(&self, message: impl Display) { - let result = self.show_message(message, lsp_types::MessageType::WARNING); - - if let Err(err) = result { - tracing::error!("Failed to send warning message to the client: {err}"); - } + self.show_message(message, lsp_types::MessageType::WARNING); } /// Sends a request to display an error to the client with a formatted message. The error is @@ -216,23 +206,23 @@ impl Client { /// /// Logs an error if the message could not be sent. pub(crate) fn show_error_message(&self, message: impl Display) { - let result = self.show_message(message, lsp_types::MessageType::ERROR); - - if let Err(err) = result { - tracing::error!("Failed to send error message to the client: {err}"); - } + self.show_message(message, lsp_types::MessageType::ERROR); } /// Re-queues this request after a salsa cancellation for a retry. /// /// The main loop will skip the retry if the client cancelled the request in the meantime. - pub(crate) fn retry(&self, request: lsp_server::Request) -> crate::Result<()> { + pub(crate) fn retry(&self, request: lsp_server::Request) { self.main_loop_sender .send(Event::Action(Action::RetryRequest(request))) - .map_err(|error| anyhow!("Failed to send retry request: {error}")) + .unwrap(); } - pub(crate) fn cancel(&self, session: &mut Session, id: RequestId) -> crate::Result<()> { + pub(crate) fn queue_action(&self, action: Action) { + self.main_loop_sender.send(Event::Action(action)).unwrap(); + } + + pub(crate) fn cancel(&self, session: &mut Session, id: RequestId) { let method_name = session.request_queue_mut().incoming_mut().cancel(&id); if let Some(method_name) = method_name { @@ -245,14 +235,18 @@ impl Client { // Use `client_sender` here instead of `respond_err` because // `respond_err` filters out responses for canceled requests (which we just did!). - self.client_sender + if let Err(err) = self + .client_sender .send(Message::Response(lsp_server::Response { id, result: None, error: Some(error), - }))?; + })) + { + tracing::error!( + "Failed to send cancellation response for request `{method_name}` because the client sender is closed: {err}", + ); + } } - - Ok(()) } } diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index c9422f33f7..531fe583df 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -24,14 +24,7 @@ pub(crate) struct GlobalOptions { impl GlobalOptions { pub(crate) fn into_settings(self) -> ClientSettings { - ClientSettings { - disable_language_services: self - .client - .python - .and_then(|python| python.ty) - .and_then(|ty| ty.disable_language_services) - .unwrap_or_default(), - } + self.client.into_settings() } } @@ -56,6 +49,19 @@ pub(crate) struct ClientOptions { python: Option, } +impl ClientOptions { + /// Returns the client settings that are relevant to the language server. + pub(crate) fn into_settings(self) -> ClientSettings { + ClientSettings { + disable_language_services: self + .python + .and_then(|python| python.ty) + .and_then(|ty| ty.disable_language_services) + .unwrap_or_default(), + } + } +} + // TODO(dhruvmanila): We need to mirror the "python.*" namespace on the server side but ideally it // would be useful to instead use `workspace/configuration` instead. This would be then used to get // all settings and not just the ones in "python.*".