diff --git a/Cargo.lock b/Cargo.lock index da883be1d4..184cb47985 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4208,6 +4208,7 @@ dependencies = [ "tracing", "tracing-flame", "tracing-subscriber", + "ty_combine", "ty_project", "ty_python_semantic", "ty_server", @@ -4215,6 +4216,16 @@ dependencies = [ "wild", ] +[[package]] +name = "ty_combine" +version = "0.0.0" +dependencies = [ + "ordermap", + "ruff_db", + "ruff_python_ast", + "ty_python_semantic", +] + [[package]] name = "ty_ide" version = "0.0.0" @@ -4267,6 +4278,7 @@ dependencies = [ "thiserror 2.0.12", "toml 0.9.4", "tracing", + "ty_combine", "ty_python_semantic", "ty_vendored", ] @@ -4338,6 +4350,7 @@ dependencies = [ "lsp-types", "regex", "ruff_db", + "ruff_macros", "ruff_notebook", "ruff_python_ast", "ruff_source_file", @@ -4351,6 +4364,7 @@ dependencies = [ "thiserror 2.0.12", "tracing", "tracing-subscriber", + "ty_combine", "ty_ide", "ty_project", "ty_python_semantic", diff --git a/Cargo.toml b/Cargo.toml index 8d6726a9a0..dcc022dfc8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ ruff_text_size = { path = "crates/ruff_text_size" } ruff_workspace = { path = "crates/ruff_workspace" } ty = { path = "crates/ty" } +ty_combine = { path = "crates/ty_combine" } ty_ide = { path = "crates/ty_ide" } ty_project = { path = "crates/ty_project", default-features = false } ty_python_semantic = { path = "crates/ty_python_semantic" } diff --git a/crates/ruff_macros/src/combine.rs b/crates/ruff_macros/src/combine.rs index f18ecd60ee..37dff885ba 100644 --- a/crates/ruff_macros/src/combine.rs +++ b/crates/ruff_macros/src/combine.rs @@ -12,14 +12,14 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result crate::combine::Combine::combine_with(&mut self.#member, other.#member) + member.span() => ty_combine::Combine::combine_with(&mut self.#member, other.#member) ) }) .collect(); Ok(quote! { #[automatically_derived] - impl crate::combine::Combine for #ident { + impl ty_combine::Combine for #ident { #[allow(deprecated)] fn combine_with(&mut self, other: Self) { #( diff --git a/crates/ruff_macros/src/lib.rs b/crates/ruff_macros/src/lib.rs index ef72240409..edc0c363df 100644 --- a/crates/ruff_macros/src/lib.rs +++ b/crates/ruff_macros/src/lib.rs @@ -47,8 +47,8 @@ pub fn derive_combine_options(input: TokenStream) -> TokenStream { .into() } -/// Automatically derives a `ty_project::project::Combine` implementation for the attributed type -/// that calls `ty_project::project::Combine::combine` for each field. +/// Automatically derives a `ty_combine::Combine` implementation for the attributed type +/// that calls `ty_combine::Combine::combine` for each field. /// /// The derive macro can only be used on structs. Enums aren't yet supported. #[proc_macro_derive(Combine)] diff --git a/crates/ty/Cargo.toml b/crates/ty/Cargo.toml index 70af4d306b..4189758bb1 100644 --- a/crates/ty/Cargo.toml +++ b/crates/ty/Cargo.toml @@ -16,6 +16,7 @@ license.workspace = true [dependencies] ruff_db = { workspace = true, features = ["os", "cache"] } ruff_python_ast = { workspace = true } +ty_combine = { workspace = true } ty_python_semantic = { workspace = true } ty_project = { workspace = true, features = ["zstd"] } ty_server = { workspace = true } diff --git a/crates/ty/src/args.rs b/crates/ty/src/args.rs index 243f596998..f518e028db 100644 --- a/crates/ty/src/args.rs +++ b/crates/ty/src/args.rs @@ -3,7 +3,7 @@ use crate::python_version::PythonVersion; use clap::error::ErrorKind; use clap::{ArgAction, ArgMatches, Error, Parser}; use ruff_db::system::SystemPathBuf; -use ty_project::combine::Combine; +use ty_combine::Combine; use ty_project::metadata::options::{EnvironmentOptions, Options, SrcOptions, TerminalOptions}; use ty_project::metadata::value::{RangedValue, RelativeGlobPattern, RelativePathBuf, ValueSource}; use ty_python_semantic::lint; diff --git a/crates/ty_combine/Cargo.toml b/crates/ty_combine/Cargo.toml new file mode 100644 index 0000000000..7262099024 --- /dev/null +++ b/crates/ty_combine/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "ty_combine" +version = "0.0.0" +edition.workspace = true +rust-version.workspace = true +homepage.workspace = true +documentation.workspace = true +repository.workspace = true +authors.workspace = true +license.workspace = true + +[dependencies] +ruff_db = { workspace = true } +ruff_python_ast = { workspace = true } +ty_python_semantic = { workspace = true } + +ordermap = { workspace = true } + +[lints] +workspace = true diff --git a/crates/ty_project/src/combine.rs b/crates/ty_combine/src/lib.rs similarity index 99% rename from crates/ty_project/src/combine.rs rename to crates/ty_combine/src/lib.rs index 98222d4ee8..ab8b7a013b 100644 --- a/crates/ty_project/src/combine.rs +++ b/crates/ty_combine/src/lib.rs @@ -161,10 +161,11 @@ impl_noop_combine!(String); #[cfg(test)] mod tests { - use crate::combine::Combine; use ordermap::OrderMap; use std::collections::HashMap; + use super::Combine; + #[test] fn combine_option() { assert_eq!(Some(1).combine(Some(2)), Some(1)); diff --git a/crates/ty_project/Cargo.toml b/crates/ty_project/Cargo.toml index 18684975e9..77b87a18a9 100644 --- a/crates/ty_project/Cargo.toml +++ b/crates/ty_project/Cargo.toml @@ -19,6 +19,7 @@ ruff_options_metadata = { workspace = true } ruff_python_ast = { workspace = true, features = ["serde"] } ruff_python_formatter = { workspace = true, optional = true } ruff_text_size = { workspace = true } +ty_combine = { workspace = true } ty_python_semantic = { workspace = true, features = ["serde"] } ty_vendored = { workspace = true } diff --git a/crates/ty_project/src/db.rs b/crates/ty_project/src/db.rs index 04eb6471bc..2b995c8690 100644 --- a/crates/ty_project/src/db.rs +++ b/crates/ty_project/src/db.rs @@ -107,8 +107,10 @@ impl ProjectDatabase { /// Set the check mode for the project. pub fn set_check_mode(&mut self, mode: CheckMode) { - tracing::debug!("Updating project to check {mode}"); - self.project().set_check_mode(self).to(mode); + if self.project().check_mode(self) != mode { + tracing::debug!("Updating project to check {mode}"); + self.project().set_check_mode(self).to(mode); + } } /// Returns a mutable reference to the system. diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index 5f54274bee..2ea0df54a0 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -28,8 +28,6 @@ use ty_python_semantic::lint::{LintRegistry, LintRegistryBuilder, RuleSelection} use ty_python_semantic::types::check_types; use ty_python_semantic::{add_inferred_python_version_hint_to_diagnostic, register_lints}; -pub mod combine; - mod db; mod files; mod glob; diff --git a/crates/ty_project/src/metadata.rs b/crates/ty_project/src/metadata.rs index 8405f8a61b..4afefec310 100644 --- a/crates/ty_project/src/metadata.rs +++ b/crates/ty_project/src/metadata.rs @@ -4,9 +4,9 @@ use ruff_db::vendored::VendoredFileSystem; use ruff_python_ast::name::Name; use std::sync::Arc; use thiserror::Error; +use ty_combine::Combine; use ty_python_semantic::ProgramSettings; -use crate::combine::Combine; use crate::metadata::options::ProjectOptionsOverrides; use crate::metadata::pyproject::{Project, PyProject, PyProjectError, ResolveRequiresPythonError}; use crate::metadata::value::ValueSource; diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 3d01e6e2a9..47dbbe19f6 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -1,5 +1,4 @@ use crate::Db; -use crate::combine::Combine; use crate::glob::{ExcludeFilter, IncludeExcludeFilter, IncludeFilter, PortableGlobKind}; use crate::metadata::settings::{OverrideSettings, SrcSettings}; @@ -28,6 +27,7 @@ use std::hash::BuildHasherDefault; use std::ops::Deref; use std::sync::Arc; use thiserror::Error; +use ty_combine::Combine; use ty_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection}; use ty_python_semantic::{ ProgramSettings, PythonEnvironment, PythonPlatform, PythonVersionFileSource, diff --git a/crates/ty_project/src/metadata/settings.rs b/crates/ty_project/src/metadata/settings.rs index aa4196fafb..6c88377756 100644 --- a/crates/ty_project/src/metadata/settings.rs +++ b/crates/ty_project/src/metadata/settings.rs @@ -1,10 +1,11 @@ use std::sync::Arc; use ruff_db::files::File; +use ty_combine::Combine; use ty_python_semantic::lint::RuleSelection; use crate::metadata::options::{InnerOverrideOptions, OutputFormat}; -use crate::{Db, combine::Combine, glob::IncludeExcludeFilter}; +use crate::{Db, glob::IncludeExcludeFilter}; /// The resolved [`super::Options`] for the project. /// diff --git a/crates/ty_project/src/metadata/value.rs b/crates/ty_project/src/metadata/value.rs index c07257d412..fdbee185a9 100644 --- a/crates/ty_project/src/metadata/value.rs +++ b/crates/ty_project/src/metadata/value.rs @@ -1,5 +1,4 @@ use crate::Db; -use crate::combine::Combine; use crate::glob::{ AbsolutePortableGlobPattern, PortableGlobError, PortableGlobKind, PortableGlobPattern, }; @@ -15,6 +14,7 @@ use std::hash::{Hash, Hasher}; use std::ops::{Deref, DerefMut}; use std::sync::Arc; use toml::Spanned; +use ty_combine::Combine; #[derive(Clone, Debug)] pub enum ValueSource { diff --git a/crates/ty_server/Cargo.toml b/crates/ty_server/Cargo.toml index f0efbfe1c5..3e7da397f6 100644 --- a/crates/ty_server/Cargo.toml +++ b/crates/ty_server/Cargo.toml @@ -12,11 +12,13 @@ license = { workspace = true } [dependencies] ruff_db = { workspace = true, features = ["os"] } +ruff_macros = { workspace = true } ruff_notebook = { workspace = true } ruff_python_ast = { workspace = true } ruff_source_file = { workspace = true } ruff_text_size = { workspace = true } +ty_combine = { workspace = true } ty_ide = { workspace = true } ty_project = { workspace = true } ty_python_semantic = { workspace = true } diff --git a/crates/ty_server/src/session/capabilities.rs b/crates/ty_server/src/capabilities.rs similarity index 79% rename from crates/ty_server/src/session/capabilities.rs rename to crates/ty_server/src/capabilities.rs index 1c96e633b0..e3bfc054fa 100644 --- a/crates/ty_server/src/session/capabilities.rs +++ b/crates/ty_server/src/capabilities.rs @@ -1,4 +1,4 @@ -use lsp_types::{ClientCapabilities, MarkupKind}; +use lsp_types::{ClientCapabilities, DiagnosticOptions, MarkupKind, WorkDoneProgressOptions}; bitflags::bitflags! { /// Represents the resolved client capabilities for the language server. @@ -18,7 +18,9 @@ bitflags::bitflags! { const SIGNATURE_ACTIVE_PARAMETER_SUPPORT = 1 << 9; const HIERARCHICAL_DOCUMENT_SYMBOL_SUPPORT = 1 << 10; const WORK_DONE_PROGRESS = 1 << 11; - const DID_CHANGE_WATCHED_FILES_DYNAMIC_REGISTRATION= 1 << 12; + const FILE_WATCHER_SUPPORT = 1 << 12; + const DIAGNOSTIC_DYNAMIC_REGISTRATION = 1 << 13; + const WORKSPACE_CONFIGURATION = 1 << 14; } } @@ -28,6 +30,11 @@ impl ResolvedClientCapabilities { self.contains(Self::WORKSPACE_DIAGNOSTIC_REFRESH) } + /// Returns `true` if the client supports workspace configuration. + pub(crate) const fn supports_workspace_configuration(self) -> bool { + self.contains(Self::WORKSPACE_CONFIGURATION) + } + /// Returns `true` if the client supports inlay hint refresh. pub(crate) const fn supports_inlay_hint_refresh(self) -> bool { self.contains(Self::INLAY_HINT_REFRESH) @@ -83,9 +90,14 @@ impl ResolvedClientCapabilities { self.contains(Self::WORK_DONE_PROGRESS) } - /// Returns `true` if the client supports dynamic registration for watched files changes. - pub(crate) const fn supports_did_change_watched_files_dynamic_registration(self) -> bool { - self.contains(Self::DID_CHANGE_WATCHED_FILES_DYNAMIC_REGISTRATION) + /// Returns `true` if the client supports file watcher capabilities. + pub(crate) const fn supports_file_watcher(self) -> bool { + self.contains(Self::FILE_WATCHER_SUPPORT) + } + + /// Returns `true` if the client supports dynamic registration for diagnostic capabilities. + pub(crate) const fn supports_diagnostic_dynamic_registration(self) -> bool { + self.contains(Self::DIAGNOSTIC_DYNAMIC_REGISTRATION) } pub(super) fn new(client_capabilities: &ClientCapabilities) -> Self { @@ -101,6 +113,13 @@ impl ResolvedClientCapabilities { flags |= Self::WORKSPACE_DIAGNOSTIC_REFRESH; } + if workspace + .and_then(|workspace| workspace.configuration) + .unwrap_or_default() + { + flags |= Self::WORKSPACE_CONFIGURATION; + } + if workspace .and_then(|workspace| workspace.inlay_hint.as_ref()?.refresh_support) .unwrap_or_default() @@ -108,10 +127,24 @@ impl ResolvedClientCapabilities { flags |= Self::INLAY_HINT_REFRESH; } + if workspace + .and_then(|workspace| workspace.did_change_watched_files?.dynamic_registration) + .unwrap_or_default() + { + flags |= Self::FILE_WATCHER_SUPPORT; + } + if text_document.is_some_and(|text_document| text_document.diagnostic.is_some()) { flags |= Self::PULL_DIAGNOSTICS; } + if text_document + .and_then(|text_document| text_document.diagnostic.as_ref()?.dynamic_registration) + .unwrap_or_default() + { + flags |= Self::DIAGNOSTIC_DYNAMIC_REGISTRATION; + } + if text_document .and_then(|text_document| text_document.type_definition?.link_support) .unwrap_or_default() @@ -212,15 +245,20 @@ impl ResolvedClientCapabilities { flags |= Self::WORK_DONE_PROGRESS; } - if client_capabilities - .workspace - .as_ref() - .and_then(|workspace| workspace.did_change_watched_files?.dynamic_registration) - .unwrap_or_default() - { - flags |= Self::DID_CHANGE_WATCHED_FILES_DYNAMIC_REGISTRATION; - } - flags } } + +/// Creates the default [`DiagnosticOptions`] for the server. +pub(crate) fn server_diagnostic_options(workspace_diagnostics: bool) -> DiagnosticOptions { + DiagnosticOptions { + identifier: Some(crate::DIAGNOSTIC_NAME.to_string()), + inter_file_dependencies: true, + workspace_diagnostics, + work_done_progress_options: WorkDoneProgressOptions { + // Currently, the server only supports reporting work done progress for "workspace" + // diagnostic mode. + work_done_progress: Some(workspace_diagnostics), + }, + } +} diff --git a/crates/ty_server/src/lib.rs b/crates/ty_server/src/lib.rs index 078a10b05d..a56a95cb38 100644 --- a/crates/ty_server/src/lib.rs +++ b/crates/ty_server/src/lib.rs @@ -10,6 +10,7 @@ pub use crate::session::{ClientOptions, DiagnosticMode}; pub use document::{NotebookDocument, PositionEncoding, TextDocument}; pub(crate) use session::{DocumentQuery, Session}; +mod capabilities; mod document; mod logging; mod server; diff --git a/crates/ty_server/src/server.rs b/crates/ty_server/src/server.rs index c4ddf93d88..9be62c8b40 100644 --- a/crates/ty_server/src/server.rs +++ b/crates/ty_server/src/server.rs @@ -2,10 +2,12 @@ use self::schedule::spawn_main_loop; use crate::PositionEncoding; -use crate::session::{AllOptions, ClientOptions, DiagnosticMode, Session}; +use crate::capabilities::{ResolvedClientCapabilities, server_diagnostic_options}; +use crate::session::{InitializationOptions, Session}; +use anyhow::Context; use lsp_server::Connection; use lsp_types::{ - ClientCapabilities, DeclarationCapability, DiagnosticOptions, DiagnosticServerCapabilities, + ClientCapabilities, DeclarationCapability, DiagnosticServerCapabilities, HoverProviderCapability, InitializeParams, InlayHintOptions, InlayHintServerCapabilities, MessageType, SelectionRangeProviderCapability, SemanticTokensLegend, SemanticTokensOptions, SemanticTokensServerCapabilities, ServerCapabilities, SignatureHelpOptions, @@ -47,23 +49,38 @@ impl Server { in_test: bool, ) -> crate::Result { let (id, init_value) = connection.initialize_start()?; - let init_params: InitializeParams = serde_json::from_value(init_value)?; - let AllOptions { - global: global_options, - workspace: mut workspace_options, - } = AllOptions::from_value( - init_params - .initialization_options - .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())), - ); + let InitializeParams { + initialization_options, + capabilities: client_capabilities, + workspace_folders, + .. + } = serde_json::from_value(init_value) + .context("Failed to deserialize initialization parameters")?; - let client_capabilities = init_params.capabilities; + let (initialization_options, deserialization_error) = + InitializationOptions::from_value(initialization_options); + + if !in_test { + crate::logging::init_logging( + initialization_options.log_level.unwrap_or_default(), + initialization_options.log_file.as_deref(), + ); + } + + if let Some(error) = deserialization_error { + tracing::error!("Failed to deserialize initialization options: {error}"); + } + + tracing::debug!("Initialization options: {initialization_options:?}"); + + let resolved_client_capabilities = ResolvedClientCapabilities::new(&client_capabilities); let position_encoding = Self::find_best_position_encoding(&client_capabilities); let server_capabilities = - Self::server_capabilities(position_encoding, global_options.diagnostic_mode()); + Self::server_capabilities(position_encoding, resolved_client_capabilities); let version = ruff_db::program_version().unwrap_or("Unknown"); + tracing::debug!("Version: {version}"); connection.initialize_finish( id, @@ -81,37 +98,14 @@ impl Server { let (main_loop_sender, main_loop_receiver) = crossbeam::channel::bounded(32); let client = Client::new(main_loop_sender.clone(), connection.sender.clone()); - if !in_test { - crate::logging::init_logging( - global_options.tracing.log_level.unwrap_or_default(), - global_options.tracing.log_file.as_deref(), - ); - } - - tracing::debug!("Version: {version}"); - - let mut workspace_for_url = |url: Url| { - let Some(workspace_settings) = workspace_options.as_mut() else { - return (url, ClientOptions::default()); - }; - let settings = workspace_settings.remove(&url).unwrap_or_else(|| { - tracing::warn!( - "No workspace options found for {}, using default options", - url - ); - ClientOptions::default() - }); - (url, settings) - }; - - let workspaces = init_params - .workspace_folders + // Get workspace URLs without settings - settings will come from workspace/configuration + let workspace_urls = workspace_folders .filter(|folders| !folders.is_empty()) .map(|folders| { folders .into_iter() - .map(|folder| workspace_for_url(folder.uri)) - .collect() + .map(|folder| folder.uri) + .collect::>() }) .or_else(|| { let current_dir = native_system @@ -125,7 +119,7 @@ impl Server { current_dir.display() ); let uri = Url::from_file_path(current_dir).ok()?; - Some(vec![workspace_for_url(uri)]) + Some(vec![uri]) }) .ok_or_else(|| { anyhow::anyhow!( @@ -134,19 +128,19 @@ impl Server { ) })?; - let workspaces = if workspaces.len() > 1 { - let first_workspace = workspaces.into_iter().next().unwrap(); + let workspace_urls = if workspace_urls.len() > 1 { + let first_workspace = workspace_urls.into_iter().next().unwrap(); tracing::warn!( "Multiple workspaces are not yet supported, using the first workspace: {}", - &first_workspace.0 + &first_workspace ); client.show_warning_message(format_args!( "Multiple workspaces are not yet supported, using the first workspace: {}", - &first_workspace.0, + &first_workspace, )); vec![first_workspace] } else { - workspaces + workspace_urls }; Ok(Self { @@ -155,10 +149,10 @@ impl Server { main_loop_receiver, main_loop_sender, session: Session::new( - &client_capabilities, + resolved_client_capabilities, position_encoding, - global_options, - workspaces, + workspace_urls, + initialization_options, native_system, in_test, )?, @@ -190,21 +184,26 @@ impl Server { .unwrap_or_default() } + // TODO: Move this to `capabilities.rs`? fn server_capabilities( position_encoding: PositionEncoding, - diagnostic_mode: DiagnosticMode, + resolved_client_capabilities: ResolvedClientCapabilities, ) -> ServerCapabilities { + let diagnostic_provider = + if resolved_client_capabilities.supports_diagnostic_dynamic_registration() { + // If the client supports dynamic registration, we will register the diagnostic + // capabilities dynamically based on the `ty.diagnosticMode` setting. + None + } else { + // Otherwise, we always advertise support for workspace diagnostics. + Some(DiagnosticServerCapabilities::Options( + server_diagnostic_options(true), + )) + }; + ServerCapabilities { position_encoding: Some(position_encoding.into()), - diagnostic_provider: Some(DiagnosticServerCapabilities::Options(DiagnosticOptions { - identifier: Some(crate::DIAGNOSTIC_NAME.into()), - inter_file_dependencies: true, - // TODO: Dynamically register for workspace diagnostics. - workspace_diagnostics: diagnostic_mode.is_workspace(), - work_done_progress_options: WorkDoneProgressOptions { - work_done_progress: Some(diagnostic_mode.is_workspace()), - }, - })), + diagnostic_provider, text_document_sync: Some(TextDocumentSyncCapability::Options( TextDocumentSyncOptions { open_close: Some(true), diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index 4728431732..3d5c16ca01 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -199,12 +199,7 @@ pub(crate) fn publish_settings_diagnostics( // Note we DO NOT respect the fact that clients support pulls because these are // files they *specifically* won't pull diagnostics from us for, because we don't // claim to be an LSP for them. - let has_workspace_diagnostics = session - .workspaces() - .for_path(&path) - .map(|workspace| workspace.settings().diagnostic_mode().is_workspace()) - .unwrap_or(false); - if has_workspace_diagnostics { + if session.global_settings().diagnostic_mode().is_workspace() { return; } 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 751808fab4..7425b8504e 100644 --- a/crates/ty_server/src/server/api/notifications/did_close.rs +++ b/crates/ty_server/src/server/api/notifications/did_close.rs @@ -53,6 +53,20 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler { // interned in the lookup table (`Files`). tracing::warn!("Salsa file does not exists for {}", system_path); } + + // For non-virtual files, we clear diagnostics if: + // + // 1. The file does not belong to any workspace e.g., opening a random file from + // outside the workspace because closing it acts like the file doesn't exists + // 2. The diagnostic mode is set to open-files only + if session.workspaces().for_path(system_path).is_none() + || session + .global_settings() + .diagnostic_mode() + .is_open_files_only() + { + clear_diagnostics(&key, client); + } } AnySystemPath::SystemVirtual(virtual_path) => { if let Some(virtual_file) = db.files().try_virtual_file(virtual_path) { @@ -61,14 +75,11 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler { } else { tracing::warn!("Salsa virtual file does not exists for {}", virtual_path); } - } - } - if !session.global_settings().diagnostic_mode().is_workspace() { - // The server needs to clear the diagnostics regardless of whether the client supports - // pull diagnostics or not. This is because the client only has the capability to fetch - // the diagnostics but does not automatically clear them when a document is closed. - clear_diagnostics(&key, client); + // Always clear diagnostics for virtual files, as they don't really exist on disk + // which means closing them is like deleting the file. + clear_diagnostics(&key, client); + } } Ok(()) diff --git a/crates/ty_server/src/server/api/requests/completion.rs b/crates/ty_server/src/server/api/requests/completion.rs index f97c2b3f3a..67d987a9b7 100644 --- a/crates/ty_server/src/server/api/requests/completion.rs +++ b/crates/ty_server/src/server/api/requests/completion.rs @@ -34,7 +34,10 @@ impl BackgroundDocumentRequestHandler for CompletionRequestHandler { ) -> crate::server::Result> { let start = Instant::now(); - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/doc_highlights.rs b/crates/ty_server/src/server/api/requests/doc_highlights.rs index 22bea4609e..60de6eaeba 100644 --- a/crates/ty_server/src/server/api/requests/doc_highlights.rs +++ b/crates/ty_server/src/server/api/requests/doc_highlights.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for DocumentHighlightRequestHandler { _client: &Client, params: DocumentHighlightParams, ) -> crate::server::Result>> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/document_symbols.rs b/crates/ty_server/src/server/api/requests/document_symbols.rs index 9e82247531..4b14bffb56 100644 --- a/crates/ty_server/src/server/api/requests/document_symbols.rs +++ b/crates/ty_server/src/server/api/requests/document_symbols.rs @@ -32,7 +32,10 @@ impl BackgroundDocumentRequestHandler for DocumentSymbolRequestHandler { _client: &Client, params: DocumentSymbolParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/goto_declaration.rs b/crates/ty_server/src/server/api/requests/goto_declaration.rs index 5509bc1d93..15b211866d 100644 --- a/crates/ty_server/src/server/api/requests/goto_declaration.rs +++ b/crates/ty_server/src/server/api/requests/goto_declaration.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for GotoDeclarationRequestHandler { _client: &Client, params: GotoDeclarationParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/goto_definition.rs b/crates/ty_server/src/server/api/requests/goto_definition.rs index 76c7ff2fd9..00d89477ac 100644 --- a/crates/ty_server/src/server/api/requests/goto_definition.rs +++ b/crates/ty_server/src/server/api/requests/goto_definition.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for GotoDefinitionRequestHandler { _client: &Client, params: GotoDefinitionParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/goto_references.rs b/crates/ty_server/src/server/api/requests/goto_references.rs index 4d18b0c366..031d11ed29 100644 --- a/crates/ty_server/src/server/api/requests/goto_references.rs +++ b/crates/ty_server/src/server/api/requests/goto_references.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for ReferencesRequestHandler { _client: &Client, params: ReferenceParams, ) -> crate::server::Result>> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/goto_type_definition.rs b/crates/ty_server/src/server/api/requests/goto_type_definition.rs index b514064616..e0641cc264 100644 --- a/crates/ty_server/src/server/api/requests/goto_type_definition.rs +++ b/crates/ty_server/src/server/api/requests/goto_type_definition.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for GotoTypeDefinitionRequestHandler { _client: &Client, params: GotoTypeDefinitionParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/hover.rs b/crates/ty_server/src/server/api/requests/hover.rs index 508c86eb2c..d795f41d8f 100644 --- a/crates/ty_server/src/server/api/requests/hover.rs +++ b/crates/ty_server/src/server/api/requests/hover.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for HoverRequestHandler { _client: &Client, params: HoverParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/inlay_hints.rs b/crates/ty_server/src/server/api/requests/inlay_hints.rs index 2c59c9fcfd..23f1b13c13 100644 --- a/crates/ty_server/src/server/api/requests/inlay_hints.rs +++ b/crates/ty_server/src/server/api/requests/inlay_hints.rs @@ -29,7 +29,10 @@ impl BackgroundDocumentRequestHandler for InlayHintRequestHandler { _client: &Client, params: InlayHintParams, ) -> crate::server::Result>> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/selection_range.rs b/crates/ty_server/src/server/api/requests/selection_range.rs index 6903b055f6..98e1d48b25 100644 --- a/crates/ty_server/src/server/api/requests/selection_range.rs +++ b/crates/ty_server/src/server/api/requests/selection_range.rs @@ -30,7 +30,10 @@ impl BackgroundDocumentRequestHandler for SelectionRangeRequestHandler { _client: &Client, params: SelectionRangeParams, ) -> crate::server::Result>> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/semantic_tokens.rs b/crates/ty_server/src/server/api/requests/semantic_tokens.rs index b2a96a0c91..f9a965fffb 100644 --- a/crates/ty_server/src/server/api/requests/semantic_tokens.rs +++ b/crates/ty_server/src/server/api/requests/semantic_tokens.rs @@ -26,7 +26,10 @@ impl BackgroundDocumentRequestHandler for SemanticTokensRequestHandler { _client: &Client, _params: SemanticTokensParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/semantic_tokens_range.rs b/crates/ty_server/src/server/api/requests/semantic_tokens_range.rs index b3e56eca70..61b61862bd 100644 --- a/crates/ty_server/src/server/api/requests/semantic_tokens_range.rs +++ b/crates/ty_server/src/server/api/requests/semantic_tokens_range.rs @@ -28,7 +28,10 @@ impl BackgroundDocumentRequestHandler for SemanticTokensRangeRequestHandler { _client: &Client, params: SemanticTokensRangeParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/signature_help.rs b/crates/ty_server/src/server/api/requests/signature_help.rs index 4a92d49b55..3cf9b8ca12 100644 --- a/crates/ty_server/src/server/api/requests/signature_help.rs +++ b/crates/ty_server/src/server/api/requests/signature_help.rs @@ -32,7 +32,10 @@ impl BackgroundDocumentRequestHandler for SignatureHelpRequestHandler { _client: &Client, params: SignatureHelpParams, ) -> crate::server::Result> { - if snapshot.client_settings().is_language_services_disabled() { + if snapshot + .workspace_settings() + .is_language_services_disabled() + { return Ok(None); } diff --git a/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs b/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs index e650842898..0b2ef62126 100644 --- a/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs +++ b/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs @@ -103,9 +103,7 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler { client: &Client, params: WorkspaceDiagnosticParams, ) -> Result { - let index = snapshot.index(); - - if !index.global_settings().diagnostic_mode().is_workspace() { + if !snapshot.global_settings().diagnostic_mode().is_workspace() { tracing::debug!("Workspace diagnostics is disabled; returning empty report"); return Ok(WorkspaceDiagnosticReportResult::Report( WorkspaceDiagnosticReport { items: vec![] }, diff --git a/crates/ty_server/src/server/api/requests/workspace_symbols.rs b/crates/ty_server/src/server/api/requests/workspace_symbols.rs index 213d920ce6..e6b170a264 100644 --- a/crates/ty_server/src/server/api/requests/workspace_symbols.rs +++ b/crates/ty_server/src/server/api/requests/workspace_symbols.rs @@ -23,15 +23,6 @@ impl BackgroundRequestHandler for WorkspaceSymbolRequestHandler { _client: &Client, params: WorkspaceSymbolParams, ) -> crate::server::Result> { - // Check if language services are disabled - if snapshot - .index() - .global_settings() - .is_language_services_disabled() - { - return Ok(None); - } - let query = ¶ms.query; let mut all_symbols = Vec::new(); diff --git a/crates/ty_server/src/server/lazy_work_done_progress.rs b/crates/ty_server/src/server/lazy_work_done_progress.rs index 8acab9c901..567ef6cb4a 100644 --- a/crates/ty_server/src/server/lazy_work_done_progress.rs +++ b/crates/ty_server/src/server/lazy_work_done_progress.rs @@ -1,4 +1,4 @@ -use crate::session::ResolvedClientCapabilities; +use crate::capabilities::ResolvedClientCapabilities; use crate::session::client::Client; use lsp_types::request::WorkDoneProgressCreate; use lsp_types::{ diff --git a/crates/ty_server/src/server/main_loop.rs b/crates/ty_server/src/server/main_loop.rs index 4363147a1c..6ae2cf19b3 100644 --- a/crates/ty_server/src/server/main_loop.rs +++ b/crates/ty_server/src/server/main_loop.rs @@ -5,7 +5,7 @@ use crate::session::{ClientOptions, SuspendedWorkspaceDiagnosticRequest}; use anyhow::anyhow; use crossbeam::select; use lsp_server::Message; -use lsp_types::notification::Notification; +use lsp_types::notification::{DidChangeWatchedFiles, Notification}; use lsp_types::{ ConfigurationParams, DidChangeWatchedFilesRegistrationOptions, FileSystemWatcher, Url, }; @@ -194,12 +194,43 @@ impl Server { } fn initialize(&mut self, client: &Client) { + self.request_workspace_configurations(client); + self.try_register_file_watcher(client); + } + + /// Requests workspace configurations from the client for all the workspaces in the session. + /// + /// If the client does not support workspace configuration, it initializes the workspaces + /// using the initialization options provided by the client. + fn request_workspace_configurations(&mut self, client: &Client) { + if !self + .session + .client_capabilities() + .supports_workspace_configuration() + { + tracing::info!( + "Client does not support workspace configuration, initializing workspaces \ + using the initialization options" + ); + self.session.initialize_workspaces( + self.session + .workspaces() + .urls() + .cloned() + .map(|url| (url, self.session.initialization_options().options.clone())) + .collect::>(), + client, + ); + return; + } + let urls = self .session .workspaces() .urls() .cloned() .collect::>(); + let items = urls .iter() .map(|root| lsp_types::ConfigurationItem { @@ -209,95 +240,109 @@ impl Server { .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()); + client.send_request::( + &self.session, + ConfigurationParams { items }, + |client, result: Vec| { + tracing::debug!("Received workspace configurations, initializing workspaces"); - 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."); + // 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(), + urls.len(), + "Mismatch in number of workspace URLs ({}) and configuration results ({})", + urls.len(), + result.len() + ); + + let workspaces_with_options: Vec<_> = 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(); - - - client.queue_action(Action::InitializeWorkspaces(workspaces_with_options)); - }, - ); - - let fs_watcher = self - .session - .client_capabilities() - .supports_did_change_watched_files_dynamic_registration(); - - if fs_watcher { - let registration = lsp_types::Registration { - id: "workspace/didChangeWatchedFiles".to_owned(), - method: "workspace/didChangeWatchedFiles".to_owned(), - register_options: Some( - serde_json::to_value(DidChangeWatchedFilesRegistrationOptions { - watchers: vec![ - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String("**/ty.toml".into()), - kind: None, - }, - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String( - "**/.gitignore".into(), - ), - kind: None, - }, - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String("**/.ignore".into()), - kind: None, - }, - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String( - "**/pyproject.toml".into(), - ), - kind: None, - }, - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String("**/*.py".into()), - kind: None, - }, - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String("**/*.pyi".into()), - kind: None, - }, - FileSystemWatcher { - glob_pattern: lsp_types::GlobPattern::String("**/*.ipynb".into()), - kind: None, - }, - ], + (url, options) }) - .unwrap(), - ), - }; - let response_handler = move |_: &Client, ()| { - tracing::info!("File watcher successfully registered"); - }; + .collect(); - client.send_request::( - &self.session, - lsp_types::RegistrationParams { - registrations: vec![registration], - }, - response_handler, - ); - } else { - tracing::warn!("The client does not support file system watching."); + client.queue_action(Action::InitializeWorkspaces(workspaces_with_options)); + }, + ); + } + + /// Try to register the file watcher provided by the client if the client supports it. + fn try_register_file_watcher(&mut self, client: &Client) { + static FILE_WATCHER_REGISTRATION_ID: &str = "ty/workspace/didChangeWatchedFiles"; + + if !self.session.client_capabilities().supports_file_watcher() { + tracing::warn!("Client does not support file system watching"); + return; } + + let registration = lsp_types::Registration { + id: FILE_WATCHER_REGISTRATION_ID.to_owned(), + method: DidChangeWatchedFiles::METHOD.to_owned(), + register_options: Some( + serde_json::to_value(DidChangeWatchedFilesRegistrationOptions { + watchers: vec![ + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String("**/ty.toml".into()), + kind: None, + }, + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String("**/.gitignore".into()), + kind: None, + }, + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String("**/.ignore".into()), + kind: None, + }, + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String( + "**/pyproject.toml".into(), + ), + kind: None, + }, + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String("**/*.py".into()), + kind: None, + }, + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String("**/*.pyi".into()), + kind: None, + }, + FileSystemWatcher { + glob_pattern: lsp_types::GlobPattern::String("**/*.ipynb".into()), + kind: None, + }, + ], + }) + .unwrap(), + ), + }; + + client.send_request::( + &self.session, + lsp_types::RegistrationParams { + registrations: vec![registration], + }, + |_: &Client, ()| { + tracing::info!("File watcher registration completed successfully"); + }, + ); } } diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 443d71e836..f40ba03f8a 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -4,25 +4,33 @@ use anyhow::{Context, anyhow}; use index::DocumentQueryError; use lsp_server::{Message, RequestId}; use lsp_types::notification::{Exit, Notification}; -use lsp_types::request::{Request, Shutdown, WorkspaceDiagnosticRequest}; -use lsp_types::{ClientCapabilities, TextDocumentContentChangeEvent, Url}; +use lsp_types::request::{ + DocumentDiagnosticRequest, RegisterCapability, Request, Shutdown, UnregisterCapability, + WorkspaceDiagnosticRequest, +}; +use lsp_types::{ + DiagnosticRegistrationOptions, DiagnosticServerCapabilities, Registration, RegistrationParams, + TextDocumentContentChangeEvent, Unregistration, UnregistrationParams, Url, +}; use options::GlobalOptions; use ruff_db::Db; use ruff_db::files::File; use ruff_db::system::{System, SystemPath, SystemPathBuf}; +use settings::GlobalSettings; use std::collections::{BTreeMap, VecDeque}; use std::ops::{Deref, DerefMut}; use std::panic::RefUnwindSafe; use std::sync::Arc; +use ty_combine::Combine; use ty_project::metadata::Options; use ty_project::watch::ChangeEvent; -use ty_project::{ChangeResult, Db as _, ProjectDatabase, ProjectMetadata}; +use ty_project::{ChangeResult, CheckMode, Db as _, ProjectDatabase, ProjectMetadata}; -pub(crate) use self::capabilities::ResolvedClientCapabilities; pub(crate) use self::index::DocumentQuery; -pub(crate) use self::options::AllOptions; +pub(crate) use self::options::InitializationOptions; pub use self::options::{ClientOptions, DiagnosticMode}; -pub(crate) use self::settings::ClientSettings; +pub(crate) use self::settings::WorkspaceSettings; +use crate::capabilities::{ResolvedClientCapabilities, server_diagnostic_options}; use crate::document::{DocumentKey, DocumentVersion, NotebookDocument}; use crate::server::{Action, publish_settings_diagnostics}; use crate::session::client::Client; @@ -31,7 +39,6 @@ use crate::system::{AnySystemPath, LSPSystem}; use crate::{PositionEncoding, TextDocument}; use index::Index; -mod capabilities; pub(crate) mod client; pub(crate) mod index; mod options; @@ -65,6 +72,12 @@ pub(crate) struct Session { /// That's what we use the default project for. default_project: DefaultProject, + /// Initialization options that were provided by the client during server initialization. + initialization_options: InitializationOptions, + + /// Resolved global settings that are shared across all workspaces. + global_settings: Arc, + /// The global position encoding, negotiated during LSP initialization. position_encoding: PositionEncoding, @@ -77,6 +90,9 @@ pub(crate) struct Session { /// Has the client requested the server to shutdown. shutdown_requested: bool, + /// Whether the server has dynamically registered the diagnostic capability with the client. + diagnostic_capability_registered: bool, + /// Is the connected client a `TestServer` instance. in_test: bool, @@ -121,18 +137,20 @@ pub(crate) struct ProjectState { impl Session { pub(crate) fn new( - client_capabilities: &ClientCapabilities, + resolved_client_capabilities: ResolvedClientCapabilities, position_encoding: PositionEncoding, - global_options: GlobalOptions, - workspace_folders: Vec<(Url, ClientOptions)>, + workspace_urls: Vec, + initialization_options: InitializationOptions, native_system: Arc, in_test: bool, ) -> crate::Result { - let index = Arc::new(Index::new(global_options.into_settings())); + let index = Arc::new(Index::new()); let mut workspaces = Workspaces::default(); - for (url, workspace_options) in workspace_folders { - workspaces.register(url, workspace_options.into_settings())?; + // Register workspaces with default settings - they'll be initialized with real settings + // when workspace/configuration response is received + for url in workspace_urls { + workspaces.register(url)?; } Ok(Self { @@ -142,10 +160,13 @@ impl Session { deferred_messages: VecDeque::new(), index: Some(index), default_project: DefaultProject::new(), + initialization_options, + global_settings: Arc::new(GlobalSettings::default()), projects: BTreeMap::new(), - resolved_client_capabilities: ResolvedClientCapabilities::new(client_capabilities), + resolved_client_capabilities, request_queue: RequestQueue::new(), shutdown_requested: false, + diagnostic_capability_registered: false, in_test, suspended_workspace_diagnostics_request: None, revision: 0, @@ -155,10 +176,15 @@ impl Session { pub(crate) fn request_queue(&self) -> &RequestQueue { &self.request_queue } + pub(crate) fn request_queue_mut(&mut self) -> &mut RequestQueue { &mut self.request_queue } + pub(crate) fn initialization_options(&self) -> &InitializationOptions { + &self.initialization_options + } + pub(crate) fn is_shutdown_requested(&self) -> bool { self.shutdown_requested } @@ -414,11 +440,27 @@ impl Session { client: &Client, ) { assert!(!self.workspaces.all_initialized()); + + // These are the options combined from all the global options received by the server for + // each workspace via the workspace configuration request. + let mut combined_global_options: Option = None; + for (url, options) in workspace_settings { tracing::debug!("Initializing workspace `{url}`"); - let settings = options.into_settings(); - let Some((root, workspace)) = self.workspaces.initialize(&url, settings) else { + // Combine the global options specified during initialization with the + // workspace-specific options to create the final workspace options. + let ClientOptions { global, workspace } = self + .initialization_options + .options + .clone() + .combine(options.clone()); + + combined_global_options.combine_with(Some(global)); + + let workspace_settings = workspace.into_settings(); + let Some((root, workspace)) = self.workspaces.initialize(&url, workspace_settings) + else { continue; }; @@ -445,17 +487,16 @@ impl Session { }); let (root, db) = match project { - Ok(mut db) => { - db.set_check_mode(workspace.settings.diagnostic_mode().into_check_mode()); - (root, db) - } + Ok(db) => (root, db), Err(err) => { tracing::error!( - "Failed to create project for `{root}`: {err:#}. Falling back to default settings" + "Failed to create project for `{root}`: {err:#}. \ + Falling back to default settings" ); client.show_error_message(format!( - "Failed to load project rooted at {root}. Please refer to the logs for more details.", + "Failed to load project rooted at {root}. \ + Please refer to the logs for more details.", )); let db_with_default_settings = @@ -488,6 +529,18 @@ impl Session { publish_settings_diagnostics(self, client, root); } + if let Some(global_options) = combined_global_options.take() { + let global_settings = global_options.into_settings(); + if global_settings.diagnostic_mode().is_workspace() { + for project in self.projects.values_mut() { + project.db.set_check_mode(CheckMode::AllFiles); + } + } + self.global_settings = Arc::new(global_settings); + } + + self.register_diagnostic_capability(client); + assert!( self.workspaces.all_initialized(), "All workspaces should be initialized after calling `initialize_workspaces`" @@ -502,17 +555,83 @@ impl Session { } } + /// Sends a registration notification to the client to enable / disable workspace diagnostics + /// as per the `diagnostic_mode`. + /// + /// This method is a no-op if the client doesn't support dynamic registration of diagnostic + /// capabilities. + fn register_diagnostic_capability(&mut self, client: &Client) { + static DIAGNOSTIC_REGISTRATION_ID: &str = "ty/textDocument/diagnostic"; + + if !self + .resolved_client_capabilities + .supports_diagnostic_dynamic_registration() + { + return; + } + + let diagnostic_mode = self.global_settings.diagnostic_mode; + + if self.diagnostic_capability_registered { + client.send_request::( + self, + UnregistrationParams { + unregisterations: vec![Unregistration { + id: DIAGNOSTIC_REGISTRATION_ID.into(), + method: DocumentDiagnosticRequest::METHOD.into(), + }], + }, + |_: &Client, ()| { + tracing::debug!("Unregistered diagnostic capability"); + }, + ); + } + + let registration = Registration { + id: DIAGNOSTIC_REGISTRATION_ID.into(), + method: DocumentDiagnosticRequest::METHOD.into(), + register_options: Some( + serde_json::to_value(DiagnosticServerCapabilities::RegistrationOptions( + DiagnosticRegistrationOptions { + diagnostic_options: server_diagnostic_options( + diagnostic_mode.is_workspace(), + ), + ..Default::default() + }, + )) + .unwrap(), + ), + }; + + client.send_request::( + self, + RegistrationParams { + registrations: vec![registration], + }, + move |_: &Client, ()| { + tracing::debug!( + "Registered diagnostic capability in {diagnostic_mode:?} diagnostic mode" + ); + }, + ); + + self.diagnostic_capability_registered = true; + } + /// Creates a document snapshot with the URL referencing the document to snapshot. pub(crate) fn take_document_snapshot(&self, url: Url) -> DocumentSnapshot { - let index = self.index(); + let key = self + .key_from_url(url) + .map_err(DocumentQueryError::InvalidUrl); DocumentSnapshot { resolved_client_capabilities: self.resolved_client_capabilities, - client_settings: index.global_settings(), + workspace_settings: key + .as_ref() + .ok() + .and_then(|key| self.workspaces.settings_for_path(key.path().as_system()?)) + .unwrap_or_else(|| Arc::new(WorkspaceSettings::default())), position_encoding: self.position_encoding, - document_query_result: self - .key_from_url(url) - .map_err(DocumentQueryError::InvalidUrl) - .and_then(|key| index.make_document_ref(key)), + document_query_result: key.and_then(|key| self.index().make_document_ref(key)), } } @@ -526,6 +645,7 @@ impl Session { .cloned() .collect(), index: self.index.clone().unwrap(), + global_settings: self.global_settings.clone(), position_encoding: self.position_encoding, in_test: self.in_test, resolved_client_capabilities: self.resolved_client_capabilities, @@ -582,6 +702,7 @@ impl Session { /// Calling this multiple times for the same document is a logic error. pub(crate) fn close_document(&mut self, key: &DocumentKey) -> crate::Result<()> { self.index_mut().close_document(key)?; + self.bump_revision(); Ok(()) } @@ -626,8 +747,8 @@ impl Session { self.resolved_client_capabilities } - pub(crate) fn global_settings(&self) -> Arc { - self.index().global_settings() + pub(crate) fn global_settings(&self) -> &GlobalSettings { + &self.global_settings } pub(crate) fn position_encoding(&self) -> PositionEncoding { @@ -678,7 +799,7 @@ impl Drop for MutIndexGuard<'_> { #[derive(Debug)] pub(crate) struct DocumentSnapshot { resolved_client_capabilities: ResolvedClientCapabilities, - client_settings: Arc, + workspace_settings: Arc, position_encoding: PositionEncoding, document_query_result: Result, } @@ -694,9 +815,9 @@ impl DocumentSnapshot { self.position_encoding } - /// Returns the client settings for this document. - pub(crate) fn client_settings(&self) -> &ClientSettings { - &self.client_settings + /// Returns the client settings for the workspace that this document belongs to. + pub(crate) fn workspace_settings(&self) -> &WorkspaceSettings { + &self.workspace_settings } /// Returns the result of the document query for this snapshot. @@ -726,6 +847,7 @@ impl DocumentSnapshot { /// An immutable snapshot of the current state of [`Session`]. pub(crate) struct SessionSnapshot { index: Arc, + global_settings: Arc, position_encoding: PositionEncoding, resolved_client_capabilities: ResolvedClientCapabilities, in_test: bool, @@ -752,6 +874,10 @@ impl SessionSnapshot { &self.index } + pub(crate) fn global_settings(&self) -> &GlobalSettings { + &self.global_settings + } + pub(crate) fn position_encoding(&self) -> PositionEncoding { self.position_encoding } @@ -783,7 +909,9 @@ impl Workspaces { /// the workspace are announced to the server during the `initialize` request, but the /// resolved settings are only available after the client has responded to the `workspace/configuration` /// request. - pub(crate) fn register(&mut self, url: Url, settings: ClientSettings) -> anyhow::Result<()> { + /// + /// [`initialize`]: Workspaces::initialize + pub(crate) fn register(&mut self, url: Url) -> anyhow::Result<()> { let path = url .to_file_path() .map_err(|()| anyhow!("Workspace URL is not a file or directory: {url:?}"))?; @@ -792,8 +920,13 @@ impl Workspaces { let system_path = SystemPathBuf::from_path_buf(path) .map_err(|_| anyhow!("Workspace URL is not valid UTF8"))?; - self.workspaces - .insert(system_path, Workspace { url, settings }); + self.workspaces.insert( + system_path, + Workspace { + url, + settings: Arc::new(WorkspaceSettings::default()), + }, + ); self.uninitialized += 1; @@ -808,7 +941,7 @@ impl Workspaces { pub(crate) fn initialize( &mut self, url: &Url, - settings: ClientSettings, + settings: WorkspaceSettings, ) -> Option<(SystemPathBuf, &mut Workspace)> { let path = url.to_file_path().ok()?; @@ -816,7 +949,7 @@ impl Workspaces { let system_path = SystemPathBuf::from_path_buf(path).ok()?; if let Some(workspace) = self.workspaces.get_mut(&system_path) { - workspace.settings = settings; + workspace.settings = Arc::new(settings); self.uninitialized -= 1; Some((system_path, workspace)) } else { @@ -824,6 +957,8 @@ impl Workspaces { } } + /// Returns a reference to the workspace for the given path, [`None`] if there's no workspace + /// registered for the path. pub(crate) fn for_path(&self, path: impl AsRef) -> Option<&Workspace> { self.workspaces .range(..=path.as_ref().to_path_buf()) @@ -831,10 +966,22 @@ impl Workspaces { .map(|(_, db)| db) } + /// Returns the client settings for the workspace at the given path, [`None`] if there's no + /// workspace registered for the path. + pub(crate) fn settings_for_path( + &self, + path: impl AsRef, + ) -> Option> { + self.for_path(path).map(Workspace::settings_arc) + } + pub(crate) fn urls(&self) -> impl Iterator + '_ { self.workspaces.values().map(Workspace::url) } + /// Returns `true` if all workspaces have been [initialized]. + /// + /// [initialized]: Workspaces::initialize pub(crate) fn all_initialized(&self) -> bool { self.uninitialized == 0 } @@ -853,7 +1000,7 @@ impl<'a> IntoIterator for &'a Workspaces { pub(crate) struct Workspace { /// The workspace root URL as sent by the client during initialization. url: Url, - settings: ClientSettings, + settings: Arc, } impl Workspace { @@ -861,9 +1008,13 @@ impl Workspace { &self.url } - pub(crate) fn settings(&self) -> &ClientSettings { + pub(crate) fn settings(&self) -> &WorkspaceSettings { &self.settings } + + pub(crate) fn settings_arc(&self) -> Arc { + self.settings.clone() + } } /// Thin wrapper around the default project database that ensures it only gets initialized @@ -899,11 +1050,8 @@ impl DefaultProject { ) .unwrap(); - let mut db = ProjectDatabase::new(metadata, system).unwrap(); - db.set_check_mode(index.global_settings().diagnostic_mode().into_check_mode()); - ProjectState { - db, + db: ProjectDatabase::new(metadata, system).unwrap(), untracked_files_with_pushed_diagnostics: Vec::new(), } }) diff --git a/crates/ty_server/src/session/index.rs b/crates/ty_server/src/session/index.rs index dc815292b6..624a075198 100644 --- a/crates/ty_server/src/session/index.rs +++ b/crates/ty_server/src/session/index.rs @@ -5,7 +5,6 @@ use ruff_db::Db; use ruff_db::files::{File, system_path_to_file}; use rustc_hash::FxHashMap; -use crate::session::settings::ClientSettings; use crate::{ PositionEncoding, TextDocument, document::{DocumentKey, DocumentVersion, NotebookDocument}, @@ -20,17 +19,13 @@ pub(crate) struct Index { /// Maps opaque cell URLs to a notebook path (document) notebook_cells: FxHashMap, - - /// Global settings provided by the client. - global_settings: Arc, } impl Index { - pub(super) fn new(global_settings: ClientSettings) -> Self { + pub(super) fn new() -> Self { Self { documents: FxHashMap::default(), notebook_cells: FxHashMap::default(), - global_settings: Arc::new(global_settings), } } @@ -188,10 +183,6 @@ impl Index { Ok(()) } - pub(crate) fn global_settings(&self) -> Arc { - self.global_settings.clone() - } - fn document_controller_for_key( &mut self, key: &DocumentKey, diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index c4c0e496c8..9de441b6f5 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -1,98 +1,145 @@ use lsp_types::Url; use ruff_db::system::SystemPathBuf; +use ruff_macros::Combine; use ruff_python_ast::PythonVersion; -use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; -use ty_project::CheckMode; -use ty_project::metadata::Options; +use serde_json::Value; + +use ty_combine::Combine; +use ty_project::metadata::Options as TyOptions; use ty_project::metadata::options::ProjectOptionsOverrides; use ty_project::metadata::value::{RangedValue, RelativePathBuf}; use crate::logging::LogLevel; -use crate::session::ClientSettings; -pub(crate) type WorkspaceOptionsMap = FxHashMap; +use super::settings::{GlobalSettings, WorkspaceSettings}; -#[derive(Debug, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] +/// Initialization options that are set once at server startup that never change. +/// +/// There are two sets of options that are defined here: +/// 1. Options that are static, set once and are required at server startup. Any changes to these +/// options require a server restart to take effect. +/// 2. Options that are dynamic and can change during the runtime of the server, such as the +/// diagnostic mode. +/// +/// The dynamic options are also accepted during the initialization phase, so that we can support +/// clients that do not support the `workspace/didChangeConfiguration` notification. +/// +/// Note that this structure has a limitation in that there's no way to specify different options +/// for different workspaces in the initialization options which means that the server will not +/// support multiple workspaces for clients that do not implement the `workspace/configuration` +/// endpoint. Most editors support this endpoint, so this is not a significant limitation. +#[derive(Clone, Debug, Deserialize, Default)] #[serde(rename_all = "camelCase")] -pub(crate) struct GlobalOptions { +pub(crate) struct InitializationOptions { + /// The log level for the language server. + pub(crate) log_level: Option, + + /// Path to the log file, defaults to stderr if not set. + /// + /// Tildes (`~`) and environment variables (e.g., `$HOME`) are expanded. + pub(crate) log_file: Option, + + /// The remaining options that are dynamic and can change during the runtime of the server. #[serde(flatten)] - client: ClientOptions, - - // These settings are only needed for tracing, and are only read from the global configuration. - // These will not be in the resolved settings. - #[serde(flatten)] - pub(crate) tracing: TracingOptions, + pub(crate) options: ClientOptions, } -impl GlobalOptions { - pub(crate) fn into_settings(self) -> ClientSettings { - self.client.into_settings() - } - - pub(crate) fn diagnostic_mode(&self) -> DiagnosticMode { - self.client.diagnostic_mode.unwrap_or_default() - } -} - -/// This is a direct representation of the workspace settings schema, which inherits the schema of -/// [`ClientOptions`] and adds extra fields to describe the workspace it applies to. -#[derive(Debug, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct WorkspaceOptions { - #[serde(flatten)] - options: ClientOptions, - workspace: Url, -} - -/// This is a direct representation of the settings schema sent by the client. -#[derive(Clone, Debug, Serialize, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -pub struct ClientOptions { - /// Settings under the `python.*` namespace in VS Code that are useful for the ty language - /// server. - python: Option, - /// Diagnostic mode for the language server. - diagnostic_mode: Option, - - python_extension: Option, -} - -/// Diagnostic mode for the language server. -#[derive(Clone, Copy, Debug, Default, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -pub enum DiagnosticMode { - /// Check only currently open files. - #[default] - OpenFilesOnly, - /// Check all files in the workspace. - Workspace, -} - -impl DiagnosticMode { - pub(crate) fn is_workspace(self) -> bool { - matches!(self, DiagnosticMode::Workspace) - } - - pub(crate) fn into_check_mode(self) -> CheckMode { - match self { - DiagnosticMode::OpenFilesOnly => CheckMode::OpenFiles, - DiagnosticMode::Workspace => CheckMode::AllFiles, +impl InitializationOptions { + /// Create the initialization options from the given JSON value that corresponds to the + /// initialization options sent by the client. + /// + /// It returns a tuple of the initialization options and an optional error if the JSON value + /// could not be deserialized into the initialization options. In case of an error, the default + /// initialization options are returned. + pub(crate) fn from_value( + options: Option, + ) -> (InitializationOptions, Option) { + let Some(options) = options else { + return (InitializationOptions::default(), None); + }; + match serde_json::from_value(options) { + Ok(options) => (options, None), + Err(err) => (InitializationOptions::default(), Some(err)), } } } +/// Options that configure the behavior of the language server. +/// +/// This is the direct representation of the options that the client sends to the server when +/// asking for workspace configuration. These options are dynamic and can change during the runtime +/// of the server via the `workspace/didChangeConfiguration` notification. +/// +/// The representation of the options is split into two parts: +/// 1. Global options contains options that are global to the language server. They are applied to +/// all workspaces managed by the language server. +/// 2. Workspace options contains options that are specific to a workspace. They are applied to the +/// workspace these options are associated with. +#[derive(Clone, Combine, Debug, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub struct ClientOptions { + #[serde(flatten)] + pub(crate) global: GlobalOptions, + + #[serde(flatten)] + pub(crate) workspace: WorkspaceOptions, +} + impl ClientOptions { - /// Returns the client settings that are relevant to the language server. - pub(crate) fn into_settings(self) -> ClientSettings { + #[must_use] + pub fn with_diagnostic_mode(mut self, diagnostic_mode: DiagnosticMode) -> Self { + self.global.diagnostic_mode = Some(diagnostic_mode); + self + } + + #[must_use] + pub fn with_disable_language_services(mut self, disable_language_services: bool) -> Self { + self.workspace.disable_language_services = Some(disable_language_services); + self + } +} + +/// Options that are global to the language server. +/// +/// These are the dynamic options that are applied to all workspaces managed by the language +/// server. +#[derive(Clone, Combine, Debug, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub(crate) struct GlobalOptions { + /// Diagnostic mode for the language server. + diagnostic_mode: Option, +} + +impl GlobalOptions { + pub(crate) fn into_settings(self) -> GlobalSettings { + GlobalSettings { + diagnostic_mode: self.diagnostic_mode.unwrap_or_default(), + } + } +} + +/// Options that are specific to a workspace. +/// +/// These are the dynamic options that are applied to a specific workspace. +#[derive(Clone, Combine, Debug, Serialize, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub(crate) struct WorkspaceOptions { + /// Whether to disable language services like code completions, hover, etc. + disable_language_services: Option, + + /// Information about the currently active Python environment in the VS Code Python extension. + /// + /// This is relevant only for VS Code and is populated by the ty VS Code extension. + python_extension: Option, +} + +impl WorkspaceOptions { + pub(crate) fn into_settings(self) -> WorkspaceSettings { let overrides = self.python_extension.and_then(|extension| { let active_environment = extension.active_environment?; - let mut overrides = ProjectOptionsOverrides::new(None, Options::default()); + let mut overrides = ProjectOptionsOverrides::new(None, TyOptions::default()); overrides.fallback_python = if let Some(environment) = &active_environment.environment { environment.folder_uri.to_file_path().ok().and_then(|path| { @@ -116,59 +163,84 @@ impl ClientOptions { if let Some(python) = &overrides.fallback_python { tracing::debug!( - "Using the Python environment selected in the VS Code Python extension in case the configuration doesn't specify a Python environment: {python}", + "Using the Python environment selected in the VS Code Python extension \ + in case the configuration doesn't specify a Python environment: {python}", python = python.path() ); } if let Some(version) = &overrides.fallback_python_version { tracing::debug!( - "Using the Python version selected in the VS Code Python extension: {version} in case the configuration doesn't specify a Python version", + "Using the Python version selected in the VS Code Python extension: {version} \ + in case the configuration doesn't specify a Python version", ); } Some(overrides) }); - ClientSettings { - disable_language_services: self - .python - .and_then(|python| python.ty) - .and_then(|ty| ty.disable_language_services) - .unwrap_or_default(), - diagnostic_mode: self.diagnostic_mode.unwrap_or_default(), + WorkspaceSettings { + disable_language_services: self.disable_language_services.unwrap_or_default(), overrides, } } +} - /// Create a new `ClientOptions` with the specified diagnostic mode - #[must_use] - pub fn with_diagnostic_mode(mut self, mode: DiagnosticMode) -> Self { - self.diagnostic_mode = Some(mode); - self +/// Diagnostic mode for the language server. +#[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum DiagnosticMode { + /// Check only currently open files. + #[default] + OpenFilesOnly, + /// Check all files in the workspace. + Workspace, +} + +impl DiagnosticMode { + /// Returns `true` if the diagnostic mode is set to check all files in the workspace. + pub(crate) const fn is_workspace(self) -> bool { + matches!(self, DiagnosticMode::Workspace) + } + + /// Returns `true` if the diagnostic mode is set to check only currently open files. + pub(crate) const fn is_open_files_only(self) -> bool { + matches!(self, DiagnosticMode::OpenFilesOnly) } } -// 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.*". - -#[derive(Clone, Debug, Serialize, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct Python { - ty: Option, +impl Combine for DiagnosticMode { + fn combine_with(&mut self, other: Self) { + // Diagnostic mode is a global option but as it can be updated without a server restart, + // it is part of the dynamic option set. But, there's no easy way to enforce the fact that + // this option should not be set for individual workspaces. The ty VS Code extension + // enforces this but we're not in control of other clients. + // + // So, this is a workaround to ensure that if the diagnostic mode is set to `workspace` in + // either an initialization options or one of the workspace options, it is always set to + // `workspace` in the global options. + if other.is_workspace() { + *self = DiagnosticMode::Workspace; + } + } } #[derive(Clone, Debug, Serialize, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] struct PythonExtension { active_environment: Option, } +impl Combine for PythonExtension { + fn combine_with(&mut self, _other: Self) { + panic!( + "`python_extension` is not expected to be combined with the initialization options as \ + it's only set by the ty VS Code extension in the `workspace/configuration` request." + ); + } +} + #[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] pub(crate) struct ActiveEnvironment { pub(crate) executable: PythonExecutable, @@ -177,7 +249,6 @@ pub(crate) struct ActiveEnvironment { } #[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] pub(crate) struct EnvironmentVersion { pub(crate) major: i64, @@ -189,7 +260,6 @@ pub(crate) struct EnvironmentVersion { } #[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] pub(crate) struct PythonEnvironment { pub(crate) folder_uri: Url, @@ -201,100 +271,9 @@ pub(crate) struct PythonEnvironment { } #[derive(Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] #[serde(rename_all = "camelCase")] pub(crate) struct PythonExecutable { #[allow(dead_code)] pub(crate) uri: Url, pub(crate) sys_prefix: SystemPathBuf, } - -#[derive(Clone, Debug, Serialize, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct Ty { - disable_language_services: Option, -} - -/// This is a direct representation of the settings schema sent by the client. -/// Settings needed to initialize tracing. These will only be read from the global configuration. -#[derive(Debug, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -pub(crate) struct TracingOptions { - pub(crate) log_level: Option, - - /// Path to the log file - tildes and environment variables are supported. - pub(crate) log_file: Option, -} - -/// This is the exact schema for initialization options sent in by the client during -/// initialization. -#[derive(Debug, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(untagged)] -enum InitializationOptions { - #[serde(rename_all = "camelCase")] - HasWorkspaces { - #[serde(rename = "globalSettings")] - global: GlobalOptions, - #[serde(rename = "settings")] - workspace: Vec, - }, - GlobalOnly { - #[serde(default)] - settings: GlobalOptions, - }, -} - -impl Default for InitializationOptions { - fn default() -> Self { - Self::GlobalOnly { - settings: GlobalOptions::default(), - } - } -} - -/// Built from the initialization options provided by the client. -#[derive(Debug)] -pub(crate) struct AllOptions { - pub(crate) global: GlobalOptions, - /// If this is `None`, the client only passed in global settings. - pub(crate) workspace: Option, -} - -impl AllOptions { - /// Initializes the controller from the serialized initialization options. This fails if - /// `options` are not valid initialization options. - pub(crate) fn from_value(options: serde_json::Value) -> Self { - Self::from_init_options( - serde_json::from_value(options) - .map_err(|err| { - tracing::error!("Failed to deserialize initialization options: {err}. Falling back to default client settings..."); - }) - .unwrap_or_default(), - ) - } - - fn from_init_options(options: InitializationOptions) -> Self { - let (global_options, workspace_options) = match options { - InitializationOptions::GlobalOnly { settings: options } => (options, None), - InitializationOptions::HasWorkspaces { - global: global_options, - workspace: workspace_options, - } => (global_options, Some(workspace_options)), - }; - - Self { - global: global_options, - workspace: workspace_options.map(|workspace_options| { - workspace_options - .into_iter() - .map(|workspace_options| { - (workspace_options.workspace, workspace_options.options) - }) - .collect() - }), - } - } -} diff --git a/crates/ty_server/src/session/settings.rs b/crates/ty_server/src/session/settings.rs index 3d8899be65..d633d13ec7 100644 --- a/crates/ty_server/src/session/settings.rs +++ b/crates/ty_server/src/session/settings.rs @@ -2,26 +2,33 @@ use super::options::DiagnosticMode; use ty_project::metadata::options::ProjectOptionsOverrides; -/// Resolved client settings for a specific document. These settings are meant to be -/// used directly by the server, and are *not* a 1:1 representation with how the client -/// sends them. -#[derive(Clone, Debug)] -#[cfg_attr(test, derive(PartialEq, Eq))] -pub(crate) struct ClientSettings { - pub(super) disable_language_services: bool, +/// Resolved client settings that are shared across all workspaces. +#[derive(Clone, Default, Debug, PartialEq)] +pub(crate) struct GlobalSettings { pub(super) diagnostic_mode: DiagnosticMode, +} + +impl GlobalSettings { + pub(crate) fn diagnostic_mode(&self) -> DiagnosticMode { + self.diagnostic_mode + } +} + +/// Resolved client settings for a specific workspace. +/// +/// These settings are meant to be used directly by the server, and are *not* a 1:1 representation +/// with how the client sends them. +#[derive(Clone, Default, Debug)] +pub(crate) struct WorkspaceSettings { + pub(super) disable_language_services: bool, pub(super) overrides: Option, } -impl ClientSettings { +impl WorkspaceSettings { pub(crate) fn is_language_services_disabled(&self) -> bool { self.disable_language_services } - pub(crate) fn diagnostic_mode(&self) -> DiagnosticMode { - self.diagnostic_mode - } - pub(crate) fn project_options_overrides(&self) -> Option<&ProjectOptionsOverrides> { self.overrides.as_ref() } diff --git a/crates/ty_server/tests/e2e/initialize.rs b/crates/ty_server/tests/e2e/initialize.rs index f9a695aba0..f5b6453d27 100644 --- a/crates/ty_server/tests/e2e/initialize.rs +++ b/crates/ty_server/tests/e2e/initialize.rs @@ -1,6 +1,7 @@ use anyhow::Result; +use lsp_types::{Position, request::RegisterCapability}; use ruff_db::system::SystemPath; -use ty_server::ClientOptions; +use ty_server::{ClientOptions, DiagnosticMode}; use crate::TestServerBuilder; @@ -21,7 +22,7 @@ fn empty_workspace_folders() -> Result<()> { fn single_workspace_folder() -> Result<()> { let workspace_root = SystemPath::new("foo"); let server = TestServerBuilder::new()? - .with_workspace(workspace_root, ClientOptions::default())? + .with_workspace(workspace_root, None)? .build()? .wait_until_workspaces_are_initialized()?; @@ -31,3 +32,353 @@ fn single_workspace_folder() -> Result<()> { Ok(()) } + +/// Tests that the server sends a registration request for diagnostics if workspace diagnostics +/// are enabled via initialization options and dynamic registration is enabled, even if the +/// workspace configuration is not supported by the client. +#[test] +fn workspace_diagnostic_registration_without_configuration() -> Result<()> { + let workspace_root = SystemPath::new("foo"); + let mut server = TestServerBuilder::new()? + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), + ) + .with_workspace(workspace_root, None)? + .enable_workspace_configuration(false) + .enable_diagnostic_dynamic_registration(true) + .build()?; + + // No need to wait for workspaces to initialize as the client does not support workspace + // configuration. + + let (_, params) = server.await_request::()?; + let [registration] = params.registrations.as_slice() else { + panic!( + "Expected a single registration, got: {:#?}", + params.registrations + ); + }; + + insta::assert_json_snapshot!(registration, @r#" + { + "id": "ty/textDocument/diagnostic", + "method": "textDocument/diagnostic", + "registerOptions": { + "documentSelector": null, + "identifier": "ty", + "interFileDependencies": true, + "workDoneProgress": true, + "workspaceDiagnostics": true + } + } + "#); + + Ok(()) +} + +/// Tests that the server sends a registration request for diagnostics if open files diagnostics +/// are enabled via initialization options and dynamic registration is enabled, even if the +/// workspace configuration is not supported by the client. +#[test] +fn open_files_diagnostic_registration_without_configuration() -> Result<()> { + let workspace_root = SystemPath::new("foo"); + let mut server = TestServerBuilder::new()? + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::OpenFilesOnly), + ) + .with_workspace(workspace_root, None)? + .enable_workspace_configuration(false) + .enable_diagnostic_dynamic_registration(true) + .build()?; + + // No need to wait for workspaces to initialize as the client does not support workspace + // configuration. + + let (_, params) = server.await_request::()?; + let [registration] = params.registrations.as_slice() else { + panic!( + "Expected a single registration, got: {:#?}", + params.registrations + ); + }; + + insta::assert_json_snapshot!(registration, @r#" + { + "id": "ty/textDocument/diagnostic", + "method": "textDocument/diagnostic", + "registerOptions": { + "documentSelector": null, + "identifier": "ty", + "interFileDependencies": true, + "workDoneProgress": false, + "workspaceDiagnostics": false + } + } + "#); + + Ok(()) +} + +/// Tests that the server sends a registration request for diagnostics if workspace diagnostics +/// are enabled via initialization options and dynamic registration is enabled. +#[test] +fn workspace_diagnostic_registration_via_initialization() -> Result<()> { + let workspace_root = SystemPath::new("foo"); + let mut server = TestServerBuilder::new()? + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), + ) + .with_workspace(workspace_root, None)? + .enable_diagnostic_dynamic_registration(true) + .build()? + .wait_until_workspaces_are_initialized()?; + + let (_, params) = server.await_request::()?; + let [registration] = params.registrations.as_slice() else { + panic!( + "Expected a single registration, got: {:#?}", + params.registrations + ); + }; + + insta::assert_json_snapshot!(registration, @r#" + { + "id": "ty/textDocument/diagnostic", + "method": "textDocument/diagnostic", + "registerOptions": { + "documentSelector": null, + "identifier": "ty", + "interFileDependencies": true, + "workDoneProgress": true, + "workspaceDiagnostics": true + } + } + "#); + + Ok(()) +} + +/// Tests that the server sends a registration request for diagnostics if open files diagnostics +/// are enabled via initialization options and dynamic registration is enabled. +#[test] +fn open_files_diagnostic_registration_via_initialization() -> Result<()> { + let workspace_root = SystemPath::new("foo"); + let mut server = TestServerBuilder::new()? + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::OpenFilesOnly), + ) + .with_workspace(workspace_root, None)? + .enable_diagnostic_dynamic_registration(true) + .build()? + .wait_until_workspaces_are_initialized()?; + + let (_, params) = server.await_request::()?; + let [registration] = params.registrations.as_slice() else { + panic!( + "Expected a single registration, got: {:#?}", + params.registrations + ); + }; + + insta::assert_json_snapshot!(registration, @r#" + { + "id": "ty/textDocument/diagnostic", + "method": "textDocument/diagnostic", + "registerOptions": { + "documentSelector": null, + "identifier": "ty", + "interFileDependencies": true, + "workDoneProgress": false, + "workspaceDiagnostics": false + } + } + "#); + + Ok(()) +} + +/// Tests that the server sends a registration request for diagnostics if workspace diagnostics +/// are enabled and dynamic registration is enabled. +#[test] +fn workspace_diagnostic_registration() -> Result<()> { + let workspace_root = SystemPath::new("foo"); + let mut server = TestServerBuilder::new()? + .with_workspace( + workspace_root, + Some(ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace)), + )? + .enable_diagnostic_dynamic_registration(true) + .build()? + .wait_until_workspaces_are_initialized()?; + + let (_, params) = server.await_request::()?; + let [registration] = params.registrations.as_slice() else { + panic!( + "Expected a single registration, got: {:#?}", + params.registrations + ); + }; + + insta::assert_json_snapshot!(registration, @r#" + { + "id": "ty/textDocument/diagnostic", + "method": "textDocument/diagnostic", + "registerOptions": { + "documentSelector": null, + "identifier": "ty", + "interFileDependencies": true, + "workDoneProgress": true, + "workspaceDiagnostics": true + } + } + "#); + + Ok(()) +} + +/// Tests that the server sends a registration request for diagnostics if workspace diagnostics are +/// disabled and dynamic registration is enabled. +#[test] +fn open_files_diagnostic_registration() -> Result<()> { + let workspace_root = SystemPath::new("foo"); + let mut server = TestServerBuilder::new()? + .with_workspace( + workspace_root, + Some(ClientOptions::default().with_diagnostic_mode(DiagnosticMode::OpenFilesOnly)), + )? + .enable_diagnostic_dynamic_registration(true) + .build()? + .wait_until_workspaces_are_initialized()?; + + let (_, params) = server.await_request::()?; + let [registration] = params.registrations.as_slice() else { + panic!( + "Expected a single registration, got: {:#?}", + params.registrations + ); + }; + + insta::assert_json_snapshot!(registration, @r#" + { + "id": "ty/textDocument/diagnostic", + "method": "textDocument/diagnostic", + "registerOptions": { + "documentSelector": null, + "identifier": "ty", + "interFileDependencies": true, + "workDoneProgress": false, + "workspaceDiagnostics": false + } + } + "#); + + Ok(()) +} + +/// Tests that the server can disable language services for a workspace via initialization options. +#[test] +fn disable_language_services_set_on_initialization() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo() -> str: + return 42 +"; + + let mut server = TestServerBuilder::new()? + .with_initialization_options(ClientOptions::default().with_disable_language_services(true)) + .with_workspace(workspace_root, None)? + .enable_pull_diagnostics(true) + .with_file(foo, foo_content)? + .build()? + .wait_until_workspaces_are_initialized()?; + + server.open_text_document(foo, &foo_content, 1); + let hover = server.hover_request(foo, Position::new(0, 5))?; + + assert!( + hover.is_none(), + "Expected no hover information, got: {hover:?}" + ); + + Ok(()) +} + +/// Tests that the server can disable language services for a workspace via workspace configuration +/// request. +#[test] +fn disable_language_services_set_on_workspace() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo() -> str: + return 42 +"; + + let mut server = TestServerBuilder::new()? + .with_workspace( + workspace_root, + Some(ClientOptions::default().with_disable_language_services(true)), + )? + .enable_pull_diagnostics(true) + .with_file(foo, foo_content)? + .build()? + .wait_until_workspaces_are_initialized()?; + + server.open_text_document(foo, &foo_content, 1); + let hover = server.hover_request(foo, Position::new(0, 5))?; + + assert!( + hover.is_none(), + "Expected no hover information, got: {hover:?}" + ); + + Ok(()) +} + +/// Tests that the server can disable language services for one workspace while keeping them +/// enabled for another. +#[test] +#[ignore = "Requires multiple workspace support in the server and test server"] +fn disable_language_services_for_one_workspace() -> Result<()> { + let workspace_a = SystemPath::new("src/a"); + let workspace_b = SystemPath::new("src/b"); + let foo = SystemPath::new("src/a/foo.py"); + let bar = SystemPath::new("src/b/bar.py"); + let foo_content = "\ +def foo() -> str: + return 42 +"; + let bar_content = "\ +def bar() -> str: + return 42 +"; + + let mut server = TestServerBuilder::new()? + .with_workspace( + workspace_a, + Some(ClientOptions::default().with_disable_language_services(true)), + )? + .with_workspace(workspace_b, None)? + .enable_pull_diagnostics(true) + .with_file(foo, foo_content)? + .with_file(bar, bar_content)? + .build()? + .wait_until_workspaces_are_initialized()?; + + server.open_text_document(foo, &foo_content, 1); + let hover_foo = server.hover_request(foo, Position::new(0, 5))?; + assert!( + hover_foo.is_none(), + "Expected no hover information for workspace A, got: {hover_foo:?}" + ); + + server.open_text_document(bar, &bar_content, 1); + let hover_bar = server.hover_request(bar, Position::new(0, 5))?; + assert!( + hover_bar.is_some(), + "Expected hover information for workspace B, got: {hover_bar:?}" + ); + + Ok(()) +} diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index ea82cd726f..dc90f8ae3a 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -48,24 +48,23 @@ use lsp_types::notification::{ Initialized, Notification, }; use lsp_types::request::{ - DocumentDiagnosticRequest, Initialize, Request, Shutdown, WorkspaceConfiguration, + DocumentDiagnosticRequest, HoverRequest, Initialize, Request, Shutdown, WorkspaceConfiguration, WorkspaceDiagnosticRequest, }; use lsp_types::{ ClientCapabilities, ConfigurationParams, DiagnosticClientCapabilities, DidChangeTextDocumentParams, DidChangeWatchedFilesClientCapabilities, DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, - DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, InitializeParams, - InitializeResult, InitializedParams, NumberOrString, PartialResultParams, PreviousResultId, - PublishDiagnosticsClientCapabilities, TextDocumentClientCapabilities, - TextDocumentContentChangeEvent, TextDocumentIdentifier, TextDocumentItem, Url, - VersionedTextDocumentIdentifier, WorkDoneProgressParams, WorkspaceClientCapabilities, - WorkspaceDiagnosticParams, WorkspaceDiagnosticReportResult, WorkspaceFolder, + DocumentDiagnosticParams, DocumentDiagnosticReportResult, FileEvent, Hover, HoverParams, + InitializeParams, InitializeResult, InitializedParams, NumberOrString, PartialResultParams, + Position, PreviousResultId, PublishDiagnosticsClientCapabilities, + TextDocumentClientCapabilities, TextDocumentContentChangeEvent, TextDocumentIdentifier, + TextDocumentItem, TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, + WorkDoneProgressParams, WorkspaceClientCapabilities, WorkspaceDiagnosticParams, + WorkspaceDiagnosticReportResult, WorkspaceFolder, }; use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf, TestSystem}; use rustc_hash::FxHashMap; -use serde::de::DeserializeOwned; -use serde_json::json; use tempfile::TempDir; use ty_server::{ClientOptions, LogLevel, Server, init_logging}; @@ -149,9 +148,6 @@ pub(crate) struct TestServer { /// Workspace configurations for `workspace/configuration` requests workspace_configurations: HashMap, - /// Capabilities registered by the server - registered_capabilities: Vec, - /// Whether a Shutdown request has been sent by the test /// and the exit sequence should be skipped during `Drop` shutdown_requested: bool, @@ -160,7 +156,7 @@ pub(crate) struct TestServer { impl TestServer { /// Create a new test server with the given workspace configurations fn new( - workspaces: Vec<(WorkspaceFolder, ClientOptions)>, + workspaces: Vec<(WorkspaceFolder, Option)>, test_context: TestContext, capabilities: ClientCapabilities, initialization_options: Option, @@ -197,7 +193,7 @@ impl TestServer { let workspace_configurations = workspaces .into_iter() - .map(|(folder, options)| (folder.uri, options)) + .filter_map(|(folder, options)| Some((folder.uri, options?))) .collect::>(); Self { @@ -210,13 +206,16 @@ impl TestServer { requests: VecDeque::new(), initialize_response: None, workspace_configurations, - registered_capabilities: Vec::new(), shutdown_requested: false, } .initialize(workspace_folders, capabilities, initialization_options) } /// Perform LSP initialization handshake + /// + /// # Panics + /// + /// If the `initialization_options` cannot be serialized to JSON fn initialize( mut self, workspace_folders: Vec, @@ -226,15 +225,16 @@ impl TestServer { let init_params = InitializeParams { capabilities, workspace_folders: Some(workspace_folders), - // TODO: This should be configurable by the test server builder. This might not be - // required after client settings are implemented in the server. - initialization_options: initialization_options - .map(|options| json!({ "settings": options})), + initialization_options: initialization_options.map(|options| { + serde_json::to_value(options) + .context("Failed to serialize initialization options to `ClientOptions`") + .unwrap() + }), ..Default::default() }; let init_request_id = self.send_request::(init_params); - self.initialize_response = Some(self.await_response::(&init_request_id)?); + self.initialize_response = Some(self.await_response::(&init_request_id)?); self.send_notification::(InitializedParams {}); Ok(self) @@ -365,7 +365,10 @@ impl TestServer { /// called once per request ID. /// /// [`send_request`]: TestServer::send_request - pub(crate) fn await_response(&mut self, id: &RequestId) -> Result { + pub(crate) fn await_response(&mut self, id: &RequestId) -> Result + where + R: Request, + { loop { if let Some(response) = self.responses.remove(id) { match response { @@ -374,7 +377,7 @@ impl TestServer { result: Some(result), .. } => { - return Ok(serde_json::from_value::(result)?); + return Ok(serde_json::from_value::(result)?); } Response { error: Some(err), @@ -574,19 +577,26 @@ impl TestServer { }; let config_value = if let Some(options) = self.workspace_configurations.get(scope_uri) { // Return the configuration for the specific workspace + // + // As per the spec: + // + // > If the client can't provide a configuration setting for a given scope + // > then null needs to be present in the returned array. match item.section.as_deref() { Some("ty") => serde_json::to_value(options)?, - Some(_) | None => { - // TODO: Handle `python` section once it's implemented in the server - // As per the spec: - // - // > If the client can't provide a configuration setting for a given scope - // > then null needs to be present in the returned array. + Some(section) => { + tracing::debug!("Unrecognized section `{section}` for {scope_uri}"); + serde_json::Value::Null + } + None => { + tracing::debug!( + "No section specified for workspace configuration of {scope_uri}", + ); serde_json::Value::Null } } } else { - tracing::warn!("No workspace configuration found for {scope_uri}"); + tracing::debug!("No workspace configuration provided for {scope_uri}"); serde_json::Value::Null }; results.push(config_value); @@ -677,7 +687,7 @@ impl TestServer { partial_result_params: PartialResultParams::default(), }; let id = self.send_request::(params); - self.await_response::(&id) + self.await_response::(&id) } /// Send a `workspace/diagnostic` request with optional previous result IDs. @@ -694,7 +704,26 @@ impl TestServer { }; let id = self.send_request::(params); - self.await_response::(&id) + self.await_response::(&id) + } + + /// Send a `textDocument/hover` request for the document at the given path and position. + pub(crate) fn hover_request( + &mut self, + path: impl AsRef, + position: Position, + ) -> Result> { + let params = HoverParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { + uri: self.file_uri(path), + }, + position, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + }; + let id = self.send_request::(params); + self.await_response::(&id) } } @@ -708,7 +737,6 @@ impl fmt::Debug for TestServer { .field("server_requests", &self.requests) .field("initialize_response", &self.initialize_response) .field("workspace_configurations", &self.workspace_configurations) - .field("registered_capabilities", &self.registered_capabilities) .finish_non_exhaustive() } } @@ -723,7 +751,7 @@ impl Drop for TestServer { // it dropped the client connection. let shutdown_error = if self.server_thread.is_some() && !self.shutdown_requested { let shutdown_id = self.send_request::(()); - match self.await_response::<()>(&shutdown_id) { + match self.await_response::(&shutdown_id) { Ok(()) => { self.send_notification::(()); None @@ -761,7 +789,7 @@ impl Drop for TestServer { /// Builder for creating test servers with specific configurations pub(crate) struct TestServerBuilder { test_context: TestContext, - workspaces: Vec<(WorkspaceFolder, ClientOptions)>, + workspaces: Vec<(WorkspaceFolder, Option)>, initialization_options: Option, client_capabilities: ClientCapabilities, } @@ -769,10 +797,13 @@ pub(crate) struct TestServerBuilder { impl TestServerBuilder { /// Create a new builder pub(crate) fn new() -> Result { - // Default client capabilities for the test server. These are assumptions made by the real - // server and are common for most clients: + // Default client capabilities for the test server: // + // These are common capabilities that all clients support: // - Supports publishing diagnostics + // + // These are enabled by default for convenience but can be disabled using the builder + // methods: // - Supports pulling workspace configuration let client_capabilities = ClientCapabilities { text_document: Some(TextDocumentClientCapabilities { @@ -794,6 +825,7 @@ impl TestServerBuilder { }) } + /// Set the initial client options for the test server pub(crate) fn with_initialization_options(mut self, options: ClientOptions) -> Self { self.initialization_options = Some(options); self @@ -803,10 +835,13 @@ impl TestServerBuilder { /// /// This option will be used to respond to the `workspace/configuration` request that the /// server will send to the client. + /// + /// If `options` is `None`, the test server will respond with `null` for this workspace + /// when the server sends a `workspace/configuration` request. pub(crate) fn with_workspace( mut self, workspace_root: &SystemPath, - options: ClientOptions, + options: Option, ) -> Result { // TODO: Support multiple workspaces in the test server if self.workspaces.len() == 1 { @@ -830,7 +865,6 @@ impl TestServerBuilder { } /// Enable or disable pull diagnostics capability - #[must_use] pub(crate) fn enable_pull_diagnostics(mut self, enabled: bool) -> Self { self.client_capabilities .text_document @@ -843,8 +877,27 @@ impl TestServerBuilder { self } + /// Enable or disable dynamic registration of diagnostics capability + pub(crate) fn enable_diagnostic_dynamic_registration(mut self, enabled: bool) -> Self { + self.client_capabilities + .text_document + .get_or_insert_with(Default::default) + .diagnostic + .get_or_insert_with(Default::default) + .dynamic_registration = Some(enabled); + self + } + + /// Enable or disable workspace configuration capability + pub(crate) fn enable_workspace_configuration(mut self, enabled: bool) -> Self { + self.client_capabilities + .workspace + .get_or_insert_with(Default::default) + .configuration = Some(enabled); + self + } + /// Enable or disable file watching capability - #[must_use] #[expect(dead_code)] pub(crate) fn enable_did_change_watched_files(mut self, enabled: bool) -> Self { self.client_capabilities @@ -859,7 +912,6 @@ impl TestServerBuilder { } /// Set custom client capabilities (overrides any previously set capabilities) - #[must_use] #[expect(dead_code)] pub(crate) fn with_client_capabilities(mut self, capabilities: ClientCapabilities) -> Self { self.client_capabilities = capabilities; diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index ebb90c98c5..c8a9088f8e 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -1,7 +1,6 @@ use anyhow::Result; use lsp_types::notification::PublishDiagnostics; use ruff_db::system::SystemPath; -use ty_server::ClientOptions; use crate::TestServerBuilder; @@ -15,7 +14,7 @@ def foo() -> str: "; let mut server = TestServerBuilder::new()? - .with_workspace(workspace_root, ClientOptions::default())? + .with_workspace(workspace_root, None)? .with_file(foo, foo_content)? .enable_pull_diagnostics(false) .build()? diff --git a/crates/ty_server/tests/e2e/pull_diagnostics.rs b/crates/ty_server/tests/e2e/pull_diagnostics.rs index 4ac84ccfd4..cb41b88465 100644 --- a/crates/ty_server/tests/e2e/pull_diagnostics.rs +++ b/crates/ty_server/tests/e2e/pull_diagnostics.rs @@ -23,7 +23,7 @@ def foo() -> str: "; let mut server = TestServerBuilder::new()? - .with_workspace(workspace_root, ClientOptions::default())? + .with_workspace(workspace_root, None)? .with_file(foo, foo_content)? .enable_pull_diagnostics(true) .build()? @@ -49,7 +49,7 @@ def foo() -> str: "; let mut server = TestServerBuilder::new()? - .with_workspace(workspace_root, ClientOptions::default())? + .with_workspace(workspace_root, None)? .with_file(foo, foo_content)? .enable_pull_diagnostics(true) .build()? @@ -105,7 +105,7 @@ def foo() -> str: "; let mut server = TestServerBuilder::new()? - .with_workspace(workspace_root, ClientOptions::default())? + .with_workspace(workspace_root, None)? .with_file(foo, foo_content_v1)? .enable_pull_diagnostics(true) .build()? @@ -217,14 +217,11 @@ def foo() -> str: return 42 # Same error: expected str, got int "; - let global_options = ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace); - let mut server = TestServerBuilder::new()? - .with_workspace( - workspace_root, + .with_workspace(workspace_root, None)? + .with_initialization_options( ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), - )? - .with_initialization_options(global_options) + ) .with_file(file_a, file_a_content)? .with_file(file_b, file_b_content_v1)? .with_file(file_c, file_c_content_v1)? @@ -335,12 +332,12 @@ def foo() -> str: return 42 "; - let global_options = ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace); - let mut server = TestServerBuilder::new()? - .with_workspace(workspace_root, global_options.clone())? + .with_workspace(workspace_root, None)? .with_file(foo, foo_content)? - .with_initialization_options(global_options) + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), + ) .enable_pull_diagnostics(true) .build()? .wait_until_workspaces_are_initialized()?; @@ -431,14 +428,11 @@ def foo() -> str: return 42 # Type error: expected str, got int "; - let global_options = ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace); - let mut builder = TestServerBuilder::new()? - .with_workspace( - workspace_root, + .with_workspace(workspace_root, None)? + .with_initialization_options( ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), - )? - .with_initialization_options(global_options); + ); for i in 0..NUM_FILES { let file_path_string = format!("src/file_{i:03}.py"); @@ -467,7 +461,7 @@ def foo() -> str: // First, read the response of the workspace diagnostic request. // Note: This response comes after the progress notifications but it simplifies the test to read it first. - let final_response = server.await_response::(&request_id)?; + let final_response = server.await_response::(&request_id)?; // Process the final report. // This should always be a partial report. However, the type definition in the LSP specification @@ -523,11 +517,11 @@ fn workspace_diagnostic_streaming_with_caching() -> Result<()> { let error_content = "def foo() -> str:\n return 42 # Error"; let changed_content = "def foo() -> str:\n return true # Error"; - let global_options = ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace); - let mut builder = TestServerBuilder::new()? - .with_workspace(workspace_root, global_options.clone())? - .with_initialization_options(global_options); + .with_workspace(workspace_root, None)? + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), + ); for i in 0..NUM_FILES { let file_path_string = format!("src/error_{i}.py"); @@ -596,7 +590,7 @@ fn workspace_diagnostic_streaming_with_caching() -> Result<()> { }, }); - let final_response2 = server.await_response::(&request2_id)?; + let final_response2 = server.await_response::(&request2_id)?; let mut all_items = Vec::new(); @@ -739,8 +733,7 @@ def hello() -> str: ); // The workspace diagnostic request should now complete with the new diagnostic - let workspace_response = - server.await_response::(&request_id)?; + let workspace_response = server.await_response::(&request_id)?; // Verify we got a report with one file containing the new diagnostic assert_debug_snapshot!( @@ -782,7 +775,7 @@ def hello() -> str: server.cancel(&request_id); // The workspace diagnostic request should now respond with a cancellation response (Err). - let result = server.await_response::(&request_id); + let result = server.await_response::(&request_id); assert_debug_snapshot!( "workspace_diagnostic_long_polling_cancellation_result", result @@ -840,7 +833,7 @@ def hello() -> str: ); // First request should complete with diagnostics - let first_response = server.await_response::(&request_id_1)?; + let first_response = server.await_response::(&request_id_1)?; // Extract result IDs from the first response for the second request let previous_result_ids = extract_result_ids_from_response(&first_response); @@ -873,8 +866,7 @@ def hello() -> str: ); // Second request should complete with the fix (no diagnostics) - let second_response = - server.await_response::(&request_id_2)?; + let second_response = server.await_response::(&request_id_2)?; // Snapshot both responses to verify the full cycle assert_debug_snapshot!( @@ -895,12 +887,12 @@ fn create_workspace_server_with_file( file_path: &SystemPath, file_content: &str, ) -> Result { - let global_options = ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace); - TestServerBuilder::new()? - .with_workspace(workspace_root, global_options.clone())? + .with_workspace(workspace_root, None)? .with_file(file_path, file_content)? - .with_initialization_options(global_options) + .with_initialization_options( + ClientOptions::default().with_diagnostic_mode(DiagnosticMode::Workspace), + ) .enable_pull_diagnostics(true) .build()? .wait_until_workspaces_are_initialized() @@ -930,10 +922,10 @@ fn shutdown_and_await_workspace_diagnostic( let shutdown_id = server.send_request::(()); // The workspace diagnostic request should now respond with an empty report - let workspace_response = server.await_response::(request_id); + let workspace_response = server.await_response::(request_id); // Complete shutdown sequence - server.await_response::<()>(&shutdown_id)?; + server.await_response::(&shutdown_id)?; server.send_notification::(()); workspace_response @@ -944,7 +936,7 @@ fn assert_workspace_diagnostics_suspends_for_long_polling( server: &mut TestServer, request_id: &lsp_server::RequestId, ) { - match server.await_response::(request_id) { + match server.await_response::(request_id) { Ok(_) => { panic!("Expected workspace diagnostic request to suspend for long-polling."); } diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization.snap b/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization.snap index 2fdd5481e6..0c00d640c8 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization.snap @@ -64,8 +64,8 @@ expression: initialization_result "diagnosticProvider": { "identifier": "ty", "interFileDependencies": true, - "workspaceDiagnostics": false, - "workDoneProgress": false + "workspaceDiagnostics": true, + "workDoneProgress": true } }, "serverInfo": { diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization_with_workspace.snap b/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization_with_workspace.snap index 2fdd5481e6..0c00d640c8 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization_with_workspace.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__initialize__initialization_with_workspace.snap @@ -64,8 +64,8 @@ expression: initialization_result "diagnosticProvider": { "identifier": "ty", "interFileDependencies": true, - "workspaceDiagnostics": false, - "workDoneProgress": false + "workspaceDiagnostics": true, + "workDoneProgress": true } }, "serverInfo": {