Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions crates/ty_server/src/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions crates/ty_server/src/server/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ pub(super) fn notification(notif: server::Notification) -> Task {
notifications::DidChangeWorkspaceFoldersHandler::METHOD => {
sync_notification_task::<notifications::DidChangeWorkspaceFoldersHandler>(notif)
}
notifications::DidChangeConfiguration::METHOD => {
sync_notification_task::<notifications::DidChangeConfiguration>(notif)
}
lsp_types::notification::Cancel::METHOD => {
sync_notification_task::<notifications::CancelNotificationHandler>(notif)
}
Expand Down
2 changes: 2 additions & 0 deletions crates/ty_server/src/server/api/notifications.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
use crate::server::Action;
use crate::server::Result;
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 {
fn run(
session: &mut Session,
client: &Client,
params: types::DidChangeConfigurationParams,
) -> Result<()> {
tracing::debug!("Received workspace/didChangeConfiguration");
// workspace/didChangeConfiguration is a pull based, meaning the request should be empty, and
// the server needs to pull the workspace configuration by requesting it from the
// client.
// See https://github.com/microsoft/vscode-languageserver-node/issues/380#issuecomment-414691493
// See https://github.com/microsoft/language-server-protocol/issues/676
assert!(params.settings.is_null());

let workspace_urls: Vec<Url> = session
.workspaces()
.into_iter()
.map(|(_, workspace)| workspace.url().clone())
.collect();

let items: Vec<types::ConfigurationItem> = 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::<lsp_types::request::WorkspaceConfiguration>(
session,
ConfigurationParams { items },
|client, result: Vec<serde_json::value::Value>| {
// 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));
},
);

Ok(())
}
}
10 changes: 10 additions & 0 deletions crates/ty_server/src/server/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@ impl Server {
// paths into account.
// self.try_register_file_watcher(&client);
}

Action::UpdateWorkspaceConfigs(workspaces_with_options) => {
tracing::debug!("Checking and updating workspace configs");

self.session
.update_workspace_folders(&client, workspaces_with_options);
}
},
}
}
Expand Down Expand Up @@ -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)]
Expand Down
60 changes: 59 additions & 1 deletion crates/ty_server/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -780,6 +780,22 @@ impl Session {
},
);
}
// Updates workspace folders from a workspace/didChangeConfiguration request.
// Reinitializes the workspaces (unregistered), and then hands off to the main
// initialize_workspace_folders function.
pub(crate) fn update_workspace_folders(
&mut self,
client: &Client,
workspace_folders: Vec<(Url, ClientOptions)>,
) {
tracing::debug!("Updating workspace folders...");

for (url, _) in &workspace_folders {
self.reinitialize_workspace(url.clone());
}

self.initialize_workspace_folders(client, workspace_folders);
}

/// Removes a workspace folder at the given URL.
///
Expand Down Expand Up @@ -887,6 +903,30 @@ impl Session {
}
}

// Unregister and reregister a current initialized workspace
fn reinitialize_workspace(&mut self, url: Url) {
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;
}
};

// Refresh the workspace with the new options.
self.workspaces.unregister(&root);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaning all workspaces just because e.g. the selected interpreter is a bit rough :)

Are there any settings that change the known workspaces? Is there a way we could only update the settings or is this much more complicated?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, initially I had considered some kind of diff checking to see what specific workspace settings had changed, but it seemed quite complicated between the pulled ClientOptions and the existing WorkspaceSettings in terms of implementation. If we could find a good solution for that we could:

A. Only update workspaces that we knew had changed
B. Only update the specific settings that had changed

Potentially this is done through manually comparing each setting on the two structs (and maybe returning or modifying the existing WorkspaceSettings)? Please let me know if you have any ideas 😃 .

Copy link
Copy Markdown
Author

@pierrem964 pierrem964 Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also just replace Arc<WorkspaceSettings> with the pulled settings as well, potentially we don't need to reregister and reinitialize unless the configuration file has changed? (This is the only thing I can see relying on settings in initialize_workspace_folder - https://github.com/pharmac1st/ruff/blob/a9ffadb41636a472c5a8a52aedca350ad64a8ac1/crates/ty_server/src/session.rs#L592).

We could also move handling updating global settings in update_workspace_folders, and not rely at all on initialize_workspace_folders.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have to take a closer look, but just overriding the WorkspaceSettings seems fine; we don't need to diff them. But we also need to re-resolve the settings passed to the ProjectDatabase. It's okay if we replace the options if they don't compare equal, but that we should avoid is creating a new ProjectDatabase because that clears all caches

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated - I saw there was reload() which sort of did want we want, I split this function out into a more specific one that updates the settings.

I've also assumed that we want to update the global settings as well, and that should be set from the last workspace received (similar to how it's done in initialize_workspace_folders).

let _ = self.workspaces.register(url);
}

/// Registers the dynamic capabilities with the client as per the resolved global settings.
///
/// ## Diagnostic capability
Expand All @@ -901,10 +941,28 @@ impl Session {
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()
Expand Down
32 changes: 32 additions & 0 deletions crates/ty_server/tests/e2e/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,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(())
}
32 changes: 30 additions & 2 deletions crates/ty_server/tests/e2e/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -854,6 +855,33 @@ impl TestServer {
self.send_notification::<DidChangeWatchedFiles>(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::<DidChangeConfiguration>(params);

// Handle sending back the configuration
let (request_id, params) =
self.await_request::<lsp_types::request::WorkspaceConfiguration>();

self.handle_workspace_configuration_request(request_id, &params);
}

/// 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<P: AsRef<SystemPath>>(
Expand Down