From ec034fc359208e725bba7ba14f9d3dd1c3273c02 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 22 Dec 2025 16:46:02 +0100 Subject: [PATCH] [ty] Add new `diagnosticMode: off` (#22073) --- crates/ty_server/src/capabilities.rs | 2 +- .../ty_server/src/server/api/diagnostics.rs | 21 ++++-- .../src/server/api/requests/diagnostic.rs | 6 ++ crates/ty_server/src/session.rs | 48 ++++++++----- crates/ty_server/src/session/options.rs | 12 +++- .../tests/e2e/publish_diagnostics.rs | 72 +++++++++++++++++++ .../ty_server/tests/e2e/pull_diagnostics.rs | 31 +++++++- 7 files changed, 164 insertions(+), 28 deletions(-) diff --git a/crates/ty_server/src/capabilities.rs b/crates/ty_server/src/capabilities.rs index e08ca6ef00..3ec4fb5a2f 100644 --- a/crates/ty_server/src/capabilities.rs +++ b/crates/ty_server/src/capabilities.rs @@ -354,7 +354,7 @@ pub(crate) fn server_capabilities( // capabilities dynamically based on the `ty.diagnosticMode` setting. None } else { - // Otherwise, we always advertise support for workspace diagnostics. + // Otherwise, we always advertise support for workspace and pull diagnostics. Some(DiagnosticServerCapabilities::Options( server_diagnostic_options(true), )) diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index 60420b4651..a46356b90b 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -21,7 +21,7 @@ use crate::document::{FileRangeExt, ToRangeExt}; use crate::session::DocumentHandle; use crate::session::client::Client; use crate::system::{AnySystemPath, file_to_url}; -use crate::{DIAGNOSTIC_NAME, Db}; +use crate::{DIAGNOSTIC_NAME, Db, DiagnosticMode}; use crate::{PositionEncoding, Session}; pub(super) struct Diagnostics { @@ -134,7 +134,7 @@ pub(super) fn clear_diagnostics_if_needed( return; } - clear_diagnostics(document.url(), client); + clear_diagnostics(document.url(), session, client); } /// Clears the diagnostics for the document identified by `uri`. @@ -142,7 +142,11 @@ pub(super) fn clear_diagnostics_if_needed( /// This is done by notifying the client with an empty list of diagnostics for the document. /// For notebook cells, this clears diagnostics for the specific cell. /// For other document types, this clears diagnostics for the main document. -pub(super) fn clear_diagnostics(uri: &lsp_types::Url, client: &Client) { +pub(super) fn clear_diagnostics(uri: &lsp_types::Url, session: &Session, client: &Client) { + if session.global_settings().diagnostic_mode().is_off() { + return; + } + client.send_notification::(PublishDiagnosticsParams { uri: uri.clone(), diagnostics: vec![], @@ -174,6 +178,10 @@ pub(super) fn publish_diagnostics_if_needed( /// Publishes the diagnostics for the given document snapshot using the [publish diagnostics /// notification]. pub(super) fn publish_diagnostics(document: &DocumentHandle, session: &Session, client: &Client) { + if session.global_settings().diagnostic_mode().is_off() { + return; + } + let db = session.project_db(document.notebook_or_file_path()); let Some(diagnostics) = compute_diagnostics(db, document, session.position_encoding()) else { @@ -215,8 +223,11 @@ 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. - if session.global_settings().diagnostic_mode().is_workspace() { - return; + match session.global_settings().diagnostic_mode() { + DiagnosticMode::Workspace | DiagnosticMode::Off => { + return; + } + DiagnosticMode::OpenFilesOnly => {} } let session_encoding = session.position_encoding(); diff --git a/crates/ty_server/src/server/api/requests/diagnostic.rs b/crates/ty_server/src/server/api/requests/diagnostic.rs index 97f0633c8a..47156625b5 100644 --- a/crates/ty_server/src/server/api/requests/diagnostic.rs +++ b/crates/ty_server/src/server/api/requests/diagnostic.rs @@ -33,6 +33,12 @@ impl BackgroundDocumentRequestHandler for DocumentDiagnosticRequestHandler { _client: &Client, params: DocumentDiagnosticParams, ) -> Result { + if snapshot.global_settings().diagnostic_mode().is_off() { + return Ok(DocumentDiagnosticReportResult::Report( + DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport::default()), + )); + } + let diagnostics = compute_diagnostics(db, snapshot.document(), snapshot.encoding()); let Some(diagnostics) = diagnostics else { diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 22f80eed2b..a5139b714a 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -634,24 +634,35 @@ impl Session { let diagnostic_mode = self.global_settings.diagnostic_mode; - tracing::debug!( - "Registering diagnostic capability with {diagnostic_mode:?} diagnostic mode" - ); - registrations.push(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(), - ), - }); + match diagnostic_mode { + DiagnosticMode::Off => { + tracing::debug!( + "Skipping registration of diagnostic capability because diagnostics are turned off" + ); + } + DiagnosticMode::OpenFilesOnly | DiagnosticMode::Workspace => { + tracing::debug!( + "Registering diagnostic capability with {diagnostic_mode:?} diagnostic mode" + ); + registrations.push(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(), + ), + }); + } + } } if let Some(register_options) = self.file_watcher_registration_options() { @@ -1024,7 +1035,6 @@ impl DocumentSnapshot { } /// Returns the client settings for all workspaces. - #[expect(unused)] pub(crate) fn global_settings(&self) -> &GlobalSettings { &self.global_settings } diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index 85385b23f0..e3d1ca5b57 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -334,9 +334,13 @@ impl CompletionOptions { #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum DiagnosticMode { + /// Disable all diagnostics so that ty can be used as an LSP only + Off, + /// Check only currently open files. #[default] OpenFilesOnly, + /// Check all files in the workspace. Workspace, } @@ -351,6 +355,10 @@ impl DiagnosticMode { pub(crate) const fn is_open_files_only(self) -> bool { matches!(self, DiagnosticMode::OpenFilesOnly) } + + pub(crate) const fn is_off(self) -> bool { + matches!(self, DiagnosticMode::Off) + } } impl Combine for DiagnosticMode { @@ -363,8 +371,8 @@ impl Combine for DiagnosticMode { // 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; + if other != DiagnosticMode::default() { + *self = other; } } } diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index aea31db40c..e70f0ad49b 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -6,6 +6,7 @@ use lsp_types::{ notification::{DidOpenTextDocument, PublishDiagnostics}, }; use ruff_db::system::SystemPath; +use ty_server::ClientOptions; use crate::TestServerBuilder; @@ -33,6 +34,37 @@ def foo() -> str: Ok(()) } +#[test] +fn on_did_open_diagnostics_off() -> 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_diagnostic_mode(ty_server::DiagnosticMode::Off)), + )? + .with_file(foo, foo_content)? + .enable_pull_diagnostics(false) + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + let diagnostics = + server.try_await_notification::(Some(Duration::from_millis(100))); + + assert!( + diagnostics.is_err(), + "Server should not send a publish diagnostics notification when diagnostics are off" + ); + + Ok(()) +} + #[test] fn on_did_change() -> Result<()> { let workspace_root = SystemPath::new("src"); @@ -69,6 +101,46 @@ def foo() -> str: Ok(()) } +#[test] +fn on_did_change_diagnostics_off() -> 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_diagnostic_mode(ty_server::DiagnosticMode::Off)), + )? + .with_file(foo, foo_content)? + .enable_pull_diagnostics(false) + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + + let changes = vec![lsp_types::TextDocumentContentChangeEvent { + range: None, + range_length: None, + text: "def foo() -> int: return 42".to_string(), + }]; + + server.change_text_document(foo, changes, 2); + + let diagnostics = + server.try_await_notification::(Some(Duration::from_millis(100))); + + assert!( + diagnostics.is_err(), + "Server should not send a publish diagnostics notification when diagnostics are off" + ); + + Ok(()) +} + #[test] fn message_without_related_information_support() -> Result<()> { let workspace_root = SystemPath::new("src"); diff --git a/crates/ty_server/tests/e2e/pull_diagnostics.rs b/crates/ty_server/tests/e2e/pull_diagnostics.rs index 581541e25b..3187fcad92 100644 --- a/crates/ty_server/tests/e2e/pull_diagnostics.rs +++ b/crates/ty_server/tests/e2e/pull_diagnostics.rs @@ -1,7 +1,7 @@ use std::time::Duration; use anyhow::Result; -use insta::assert_debug_snapshot; +use insta::{assert_compact_json_snapshot, assert_debug_snapshot}; use lsp_server::RequestId; use lsp_types::request::WorkspaceDiagnosticRequest; use lsp_types::{ @@ -39,6 +39,35 @@ def foo() -> str: Ok(()) } +#[test] +fn on_did_open_diagnostics_off() -> Result<()> { + let _filter = filter_result_id(); + + 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_diagnostic_mode(DiagnosticMode::Off)), + )? + .with_file(foo, foo_content)? + .enable_pull_diagnostics(true) + .build() + .wait_until_workspaces_are_initialized(); + + server.open_text_document(foo, foo_content, 1); + let diagnostics = server.document_diagnostic_request(foo, None); + + assert_compact_json_snapshot!(diagnostics, @r#"{"kind": "full", "items": []}"#); + + Ok(()) +} + #[test] fn document_diagnostic_caching_unchanged() -> Result<()> { let _filter = filter_result_id();