diff --git a/crates/ty_server/src/capabilities.rs b/crates/ty_server/src/capabilities.rs index 81794a20a7fbf9..373a30c3f7a87a 100644 --- a/crates/ty_server/src/capabilities.rs +++ b/crates/ty_server/src/capabilities.rs @@ -37,6 +37,7 @@ bitflags::bitflags! { const COMPLETION_ITEM_LABEL_DETAILS_SUPPORT = 1 << 16; const DIAGNOSTIC_RELATED_INFORMATION = 1 << 17; const PREFER_MARKDOWN_IN_COMPLETION = 1 << 18; + const DID_CHANGE_CONFIGURATION = 1 << 19; } } @@ -81,6 +82,11 @@ impl FromStr for SupportedCommand { } impl ResolvedClientCapabilities { + /// Returns `true` if the client supports configuration change notifications. + pub(crate) const fn supports_change_conf_notifications(self) -> bool { + self.contains(Self::DID_CHANGE_CONFIGURATION) + } + /// Returns `true` if the client supports workspace diagnostic refresh. pub(crate) const fn supports_workspace_diagnostic_refresh(self) -> bool { self.contains(Self::WORKSPACE_DIAGNOSTIC_REFRESH) @@ -185,6 +191,10 @@ impl ResolvedClientCapabilities { let workspace = client_capabilities.workspace.as_ref(); let text_document = client_capabilities.text_document.as_ref(); + if workspace.is_some_and(|workspace| workspace.did_change_configuration.is_some()) { + flags |= Self::DID_CHANGE_CONFIGURATION; + } + if workspace .and_then(|workspace| workspace.diagnostics.as_ref()?.refresh_support) .unwrap_or_default() diff --git a/crates/ty_server/src/server/api.rs b/crates/ty_server/src/server/api.rs index 74637f3775a0c6..d42c4dc9f8eceb 100644 --- a/crates/ty_server/src/server/api.rs +++ b/crates/ty_server/src/server/api.rs @@ -186,6 +186,9 @@ pub(super) fn notification(notif: server::Notification) -> Task { notifications::DidChangeWorkspaceFoldersHandler::METHOD => { sync_notification_task::(notif) } + notifications::DidChangeConfiguration::METHOD => { + sync_notification_task::(notif) + } lsp_types::notification::Cancel::METHOD => { sync_notification_task::(notif) } diff --git a/crates/ty_server/src/server/api/notifications.rs b/crates/ty_server/src/server/api/notifications.rs index f0dcac92356c02..4c95dc53fdc62d 100644 --- a/crates/ty_server/src/server/api/notifications.rs +++ b/crates/ty_server/src/server/api/notifications.rs @@ -1,5 +1,6 @@ mod cancel; mod did_change; +mod did_change_configuration; mod did_change_notebook; mod did_change_watched_files; mod did_change_workspace_folders; @@ -10,6 +11,7 @@ mod did_open_notebook; pub(super) use cancel::CancelNotificationHandler; pub(super) use did_change::DidChangeTextDocumentHandler; +pub(super) use did_change_configuration::DidChangeConfiguration; pub(super) use did_change_notebook::DidChangeNotebookHandler; pub(super) use did_change_watched_files::DidChangeWatchedFiles; pub(super) use did_change_workspace_folders::DidChangeWorkspaceFoldersHandler; diff --git a/crates/ty_server/src/server/api/notifications/did_change_configuration.rs b/crates/ty_server/src/server/api/notifications/did_change_configuration.rs new file mode 100644 index 00000000000000..b6d10711c5ac80 --- /dev/null +++ b/crates/ty_server/src/server/api/notifications/did_change_configuration.rs @@ -0,0 +1,92 @@ +use crate::server::Action; +use crate::server::Result; +use crate::server::api::diagnostics::publish_diagnostics_if_needed; +use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; +use crate::session::client::Client; +use crate::session::{ClientOptions, Session}; +use lsp_types::notification as notif; +use lsp_types::{self as types, ConfigurationParams, Url}; + +pub(crate) struct DidChangeConfiguration; + +impl NotificationHandler for DidChangeConfiguration { + type NotificationType = notif::DidChangeConfiguration; +} + +impl SyncNotificationHandler for DidChangeConfiguration { + // This is implemented as the pull-based model, settings included with the notification are + // not considered. + fn run( + session: &mut Session, + client: &Client, + _params: types::DidChangeConfigurationParams, + ) -> Result<()> { + tracing::debug!("Received workspace/didChangeConfiguration"); + + let workspace_urls: Vec = session + .workspaces() + .into_iter() + .map(|(_, workspace)| workspace.url().clone()) + .collect(); + + let items: Vec = workspace_urls + .iter() + .map(|workspace| types::ConfigurationItem { + scope_uri: Some(workspace.clone()), + section: Some("ty".to_string()), + }) + .collect(); + + tracing::debug!("Sending workspace/configuration requests to client"); + client.send_request::( + session, + ConfigurationParams { items }, + |client, result: Vec| { + // This shouldn't fail because, as per the spec, the client needs to provide a + // `null` value even if it cannot provide a configuration for a workspace. + assert_eq!( + result.len(), + workspace_urls.len(), + "Mismatch in number of workspace URLs ({}) and configuration results ({})", + workspace_urls.len(), + result.len() + ); + + let workspaces_with_options: Vec<(Url, ClientOptions)> = workspace_urls + .into_iter() + .zip(result) + .map(|(url, value)| { + if value.is_null() { + tracing::debug!( + "No workspace options provided for {url}, using default options" + ); + return (url, ClientOptions::default()); + } + let options: ClientOptions = + serde_json::from_value(value).unwrap_or_else(|err| { + tracing::error!( + "Failed to deserialize workspace options for {url}: {err}. \ + Using default options" + ); + ClientOptions::default() + }); + (url, options) + }) + .collect(); + + tracing::debug!( + "Received new configuration options {:?}", + workspaces_with_options, + ); + + client.queue_action(Action::UpdateWorkspaceConfigs(workspaces_with_options)); + }, + ); + + for key in session.text_document_handles() { + publish_diagnostics_if_needed(&key, session, client); + } + + Ok(()) + } +} diff --git a/crates/ty_server/src/server/main_loop.rs b/crates/ty_server/src/server/main_loop.rs index d69ec66292ef48..b73f37b93d6cbd 100644 --- a/crates/ty_server/src/server/main_loop.rs +++ b/crates/ty_server/src/server/main_loop.rs @@ -156,6 +156,13 @@ impl Server { // paths into account. // self.try_register_file_watcher(&client); } + + Action::UpdateWorkspaceConfigs(workspaces_with_options) => { + tracing::debug!("Updating workspace configs"); + + self.session + .update_workspace_folders(&client, workspaces_with_options); + } }, } } @@ -217,6 +224,9 @@ pub(crate) enum Action { /// Initialize the workspace after the server received /// the options from the client. InitializeWorkspaces(Vec<(Url, ClientOptions)>), + + // Apply updates after pulling configuration on workspace/didChangeConfiguration + UpdateWorkspaceConfigs(Vec<(Url, ClientOptions)>), } #[derive(Debug)] diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index b34db878d6a8d0..5a9b25b87c24af 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use anyhow::{Context, anyhow}; use lsp_server::{Message, RequestId}; -use lsp_types::notification::{DidChangeWatchedFiles, Exit, Notification}; +use lsp_types::notification::{DidChangeConfiguration, DidChangeWatchedFiles, Exit, Notification}; use lsp_types::request::{ DocumentDiagnosticRequest, RegisterCapability, Request, Shutdown, UnregisterCapability, WorkspaceDiagnosticRequest, @@ -781,6 +781,38 @@ impl Session { }, ); } + // Updates workspace folders from a workspace/didChangeConfiguration request. + pub(crate) fn update_workspace_folders( + &mut self, + client: &Client, + workspace_folders: Vec<(Url, ClientOptions)>, + ) { + let mut global_options: Option = None; + + for (url, client_options) in workspace_folders { + // Like initialize_workspace_folders, the last workspace determines the global options + global_options = Some( + self.initialization_options + .options + .global + .clone() + .combine(client_options.global.clone()), + ); + + self.update_workspace(&url, client, &client_options); + } + if let Some(global_options) = global_options { + let global_settings = global_options.into_settings(); + self.global_settings = Arc::new(global_settings); + } + if let Some(check_mode) = self.global_settings.diagnostic_mode().to_check_mode() { + for project in self.projects.values_mut() { + project.db.set_check_mode(check_mode); + } + } + + self.register_capabilities(client); + } /// Removes a workspace folder at the given URL. /// @@ -888,6 +920,78 @@ impl Session { } } + // Update workspace given new client options + // + // Intended to be used when handling `workspace/didChangeConfiguration`. + // Replaces [WorkspaceSettings], and compares and replaces the [Project.settings] if the + // resolved [ProjectMetadata] does not match. + fn update_workspace(&mut self, url: &Url, client: &Client, client_options: &ClientOptions) { + let Ok(root) = url.to_file_path() else { + tracing::debug!("Ignoring workspace with non-path root: {url}"); + return; + }; + + // Realistically I don't think this can fail because we got the path from a Url + let root = match SystemPathBuf::from_path_buf(root) { + Ok(root) => root, + Err(root) => { + tracing::debug!( + "Ignoring workspace with non-UTF8 root: {root}", + root = root.display() + ); + return; + } + }; + + let options = self + .initialization_options + .options + .workspace + .clone() + .combine(client_options.workspace.clone()); + + let Some(workspace) = self.workspaces.workspaces.get_mut(&root) else { + return; + }; + + let workspace_settings = + Arc::new(options.into_settings(&root, client, &*self.native_system)); + + workspace.settings = workspace_settings.clone(); + + let db = self.project_db_mut(&AnySystemPath::System(root.clone())); + + let system = db.system(); + + let configuration_file = workspace_settings + .project_options_overrides() + .and_then(|settings| settings.config_file_override.as_ref()); + + let metadata = if let Some(configuration_file) = configuration_file { + ProjectMetadata::from_config_file(configuration_file.clone(), &root, system) + } else { + ProjectMetadata::discover(&root, system) + }; + + match metadata { + Ok(mut metadata) => { + let _ = metadata.apply_configuration_files(system); + if let Some(overrides) = workspace_settings.project_options_overrides() { + metadata.apply_overrides(overrides); + } + + tracing::debug!("Replacing project settings for {}", root); + db.project().reload(db, metadata); + } + _ => { + tracing::debug!( + "Could not retrieve metadata, skipping project settings update for {}", + root + ); + } + } + } + /// Registers the dynamic capabilities with the client as per the resolved global settings. /// /// ## Diagnostic capability @@ -899,13 +1003,35 @@ impl Session { /// /// This capability is used to enable / disable rename functionality as per the /// `ty.experimental.rename` global setting. + /// + /// ## Did change configuration capability + /// + /// This capability is used to enable or disable didChangeConfiguration requests fn register_capabilities(&mut self, client: &Client) { static DIAGNOSTIC_REGISTRATION_ID: &str = "ty/textDocument/diagnostic"; static FILE_WATCHER_REGISTRATION_ID: &str = "ty/workspace/didChangeWatchedFiles"; + static DID_CHANGE_CONFIGURATION_ID: &str = "ty/workspace/didChangeConfiguration"; let mut registrations = vec![]; let mut unregistrations = vec![]; + if self + .resolved_client_capabilities + .supports_change_conf_notifications() + { + if self.registrations.contains(DidChangeConfiguration::METHOD) { + unregistrations.push(Unregistration { + id: DID_CHANGE_CONFIGURATION_ID.into(), + method: DidChangeConfiguration::METHOD.into(), + }); + } + registrations.push(Registration { + id: DID_CHANGE_CONFIGURATION_ID.into(), + method: DidChangeConfiguration::METHOD.into(), + register_options: Some(serde_json::to_value(()).unwrap()), + }); + } + if self .resolved_client_capabilities .supports_diagnostic_dynamic_registration() diff --git a/crates/ty_server/tests/e2e/configuration.rs b/crates/ty_server/tests/e2e/configuration.rs index 85d6f6f001141e..4f484b7eddd7c5 100644 --- a/crates/ty_server/tests/e2e/configuration.rs +++ b/crates/ty_server/tests/e2e/configuration.rs @@ -423,3 +423,35 @@ unresolved-reference="warn" Ok(()) } + +#[test] +fn configuration_notification() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo() -> str: + return a +"; + + let mut server = TestServerBuilder::new()? + .with_workspace(workspace_root, Some(ClientOptions::default()))? + .with_file(foo, foo_content)? + .enable_pull_diagnostics(true) + .build() + .wait_until_workspaces_are_initialized(); + + server.replace_workspace_configuration( + workspace_root, + ClientOptions { + workspace: WorkspaceOptions { + disable_language_services: Some(false), + ..WorkspaceOptions::default() + }, + ..ClientOptions::default() + }, + )?; + + server.did_change_configuration(); + + Ok(()) +} diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index b43d9cd809cff4..bfbe07b48e9a5d 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -55,8 +55,9 @@ use crossbeam::channel::RecvTimeoutError; use insta::internals::SettingsBindDropGuard; use lsp_server::{Connection, Message, RequestId, Response, ResponseError}; use lsp_types::notification::{ - DidChangeTextDocument, DidChangeWatchedFiles, DidChangeWorkspaceFolders, DidCloseTextDocument, - DidOpenTextDocument, Exit, Initialized, Notification, + DidChangeConfiguration, DidChangeTextDocument, DidChangeWatchedFiles, + DidChangeWorkspaceFolders, DidCloseTextDocument, DidOpenTextDocument, Exit, Initialized, + Notification, }; use lsp_types::request::{ Completion, DocumentDiagnosticRequest, HoverRequest, Initialize, InlayHintRequest, @@ -854,6 +855,33 @@ impl TestServer { self.send_notification::(params); } + pub(crate) fn replace_workspace_configuration( + &mut self, + workspace_path: &SystemPath, + new_configuration: ClientOptions, + ) -> Result<()> { + let workspace_url: Url = Url::from_file_path(self.file_path(workspace_path).as_std_path()) + .map_err(|()| anyhow!("Failed to convert workspace path to URL: {workspace_path}"))?; + + self.workspace_configurations + .insert(workspace_url, new_configuration); + + Ok(()) + } + + pub(crate) fn did_change_configuration(&mut self) { + let params = lsp_types::DidChangeConfigurationParams { + settings: serde_json::Value::Null, + }; + self.send_notification::(params); + + // Handle sending back the configuration + let (request_id, params) = + self.await_request::(); + + self.handle_workspace_configuration_request(request_id, ¶ms); + } + /// Send a `workspace/didChangeWorkspaceFolders` notification with the given added/removed /// workspace folders. The paths provided should be paths to the root of the workspace folder. pub(crate) fn change_workspace_folders>(