Skip to content
25 changes: 18 additions & 7 deletions crates/ty_project/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,13 +237,12 @@ impl Project {
)
}

pub fn reload(self, db: &mut dyn Db, metadata: ProjectMetadata) {
tracing::debug!("Reloading project");

self.reload_files(db);

// Returns boolean representing if the settings were changed
//
// Will not replace the settings if the passed [ProjectMetadata] matches the current.
pub fn replace_settings(self, db: &mut dyn Db, metadata: ProjectMetadata) -> bool {
Comment thread
pierrem964 marked this conversation as resolved.
Outdated
if &metadata == self.metadata(db) {
return;
return false;
}

match metadata
Expand All @@ -269,7 +268,19 @@ impl Project {
}

self.set_metadata(db).to(Box::new(metadata));
self.try_add_file_root(db);
true
}

pub fn reload(self, db: &mut dyn Db, metadata: ProjectMetadata) {
tracing::debug!("Reloading project");

self.reload_files(db);

let did_change_settings = self.replace_settings(db, metadata);

if did_change_settings {
self.try_add_file_root(db);
}
}

/// Checks the project and its dependencies according to the project's check mode.
Expand Down
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,87 @@
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 {
// 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,
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.

The params do contain the changed setting. Do you know if it includes all settings or do clients indeed only send the changed settings?

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.

On neovim, the invocation of this notification is manual, so you need to pass the settings as an argument through client:notify.

Had a go trying to see what vscode send with the didChangeConfiguration event, it seems like nothing is sent at all.

It seems this varies from client to client. Let me know if I've misunderstood this comment :)

) -> Result<()> {
tracing::debug!("Received workspace/didChangeConfiguration");

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(())
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.

We should call publish_diagnostics_if_needed here for clients using the push diagnostics model.

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.

Do we need to updates tests to cover this?

}
}
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!("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
128 changes: 127 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 @@ -771,6 +771,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<GlobalOptions> = 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.
///
Expand Down Expand Up @@ -878,6 +910,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().replace_settings(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
Expand All @@ -889,13 +993,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()
Expand Down
Loading