From dea48ecef06803502a265488e4a16b3e35778a57 Mon Sep 17 00:00:00 2001 From: Matthew Mckee Date: Sat, 27 Dec 2025 20:24:38 +0000 Subject: [PATCH] [ty] Add option to disable syntax errors in the language server (#22217) Co-authored-by: Micha Reiser --- .../ty_server/src/server/api/diagnostics.rs | 122 ++++++++++++------ .../src/server/api/requests/diagnostic.rs | 6 +- .../api/requests/workspace_diagnostic.rs | 21 +-- crates/ty_server/src/session/options.rs | 13 ++ crates/ty_server/src/session/settings.rs | 5 + crates/ty_server/tests/e2e/notebook.rs | 38 ++++++ .../tests/e2e/publish_diagnostics.rs | 27 ++++ .../ty_server/tests/e2e/pull_diagnostics.rs | 48 +++++++ .../e2e__commands__debug_command.snap | 1 + ...id_syntax_with_syntax_errors_disabled.snap | 21 +++ 10 files changed, 256 insertions(+), 46 deletions(-) create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__invalid_syntax_with_syntax_errors_disabled.snap diff --git a/crates/ty_server/src/server/api/diagnostics.rs b/crates/ty_server/src/server/api/diagnostics.rs index a46356b90b..c86752a560 100644 --- a/crates/ty_server/src/server/api/diagnostics.rs +++ b/crates/ty_server/src/server/api/diagnostics.rs @@ -18,8 +18,8 @@ use ty_project::{Db as _, ProjectDatabase}; use crate::capabilities::ResolvedClientCapabilities; use crate::document::{FileRangeExt, ToRangeExt}; -use crate::session::DocumentHandle; use crate::session::client::Client; +use crate::session::{DocumentHandle, GlobalSettings}; use crate::system::{AnySystemPath, file_to_url}; use crate::{DIAGNOSTIC_NAME, Db, DiagnosticMode}; use crate::{PositionEncoding, Session}; @@ -61,6 +61,7 @@ impl Diagnostics { &self, db: &ProjectDatabase, client_capabilities: ResolvedClientCapabilities, + global_settings: &GlobalSettings, ) -> LspDiagnostics { if let Some(notebook_document) = db.notebook_document(self.file_or_notebook) { let mut cell_diagnostics: FxHashMap> = FxHashMap::default(); @@ -72,8 +73,15 @@ impl Diagnostics { } for diagnostic in &self.items { - let (url, lsp_diagnostic) = - to_lsp_diagnostic(db, diagnostic, self.encoding, client_capabilities); + let Some((url, lsp_diagnostic)) = to_lsp_diagnostic( + db, + diagnostic, + self.encoding, + client_capabilities, + global_settings, + ) else { + continue; + }; let Some(url) = url else { tracing::warn!("Unable to find notebook cell"); @@ -91,8 +99,17 @@ impl Diagnostics { LspDiagnostics::TextDocument( self.items .iter() - .map(|diagnostic| { - to_lsp_diagnostic(db, diagnostic, self.encoding, client_capabilities).1 + .filter_map(|diagnostic| { + Some( + to_lsp_diagnostic( + db, + diagnostic, + self.encoding, + client_capabilities, + global_settings, + )? + .1, + ) }) .collect(), ) @@ -197,7 +214,11 @@ pub(super) fn publish_diagnostics(document: &DocumentHandle, session: &Session, }); }; - match diagnostics.to_lsp_diagnostics(db, session.client_capabilities()) { + match diagnostics.to_lsp_diagnostics( + db, + session.client_capabilities(), + session.global_settings(), + ) { LspDiagnostics::TextDocument(diagnostics) => { publish_diagnostics_notification(document.url().clone(), diagnostics); } @@ -232,49 +253,71 @@ pub(crate) fn publish_settings_diagnostics( let session_encoding = session.position_encoding(); let client_capabilities = session.client_capabilities(); - let state = session.project_state_mut(&AnySystemPath::System(path)); - let db = &state.db; - let project = db.project(); - let settings_diagnostics = project.check_settings(db); - // We need to send diagnostics if we have non-empty ones, or we have ones to clear. - // These will both almost always be empty so this function will almost always be a no-op. - if settings_diagnostics.is_empty() && state.untracked_files_with_pushed_diagnostics.is_empty() { - return; - } + let project_path = AnySystemPath::System(path); - // Group diagnostics by URL - let mut diagnostics_by_url: FxHashMap> = FxHashMap::default(); - for diagnostic in settings_diagnostics { - if let Some(span) = diagnostic.primary_span() { - let file = span.expect_ty_file(); - let Some(url) = file_to_url(db, file) else { - tracing::debug!("Failed to convert file to URL at {}", file.path(db)); - continue; - }; - diagnostics_by_url.entry(url).or_default().push(diagnostic); + let (mut diagnostics_by_url, old_untracked) = { + let state = session.project_state_mut(&project_path); + let db = &state.db; + let project = db.project(); + let settings_diagnostics = project.check_settings(db); + + // We need to send diagnostics if we have non-empty ones, or we have ones to clear. + // These will both almost always be empty so this function will almost always be a no-op. + if settings_diagnostics.is_empty() + && state.untracked_files_with_pushed_diagnostics.is_empty() + { + return; } - } - // Record the URLs we're sending non-empty diagnostics for, so we know to clear them - // the next time we publish settings diagnostics! - let old_untracked = std::mem::replace( - &mut state.untracked_files_with_pushed_diagnostics, - diagnostics_by_url.keys().cloned().collect(), - ); + // Group diagnostics by URL + let mut diagnostics_by_url: FxHashMap> = FxHashMap::default(); + for diagnostic in settings_diagnostics { + if let Some(span) = diagnostic.primary_span() { + let file = span.expect_ty_file(); + let Some(url) = file_to_url(db, file) else { + tracing::debug!("Failed to convert file to URL at {}", file.path(db)); + continue; + }; + diagnostics_by_url.entry(url).or_default().push(diagnostic); + } + } + + // Record the URLs we're sending non-empty diagnostics for, so we know to clear them + // the next time we publish settings diagnostics! + let old_untracked = std::mem::replace( + &mut state.untracked_files_with_pushed_diagnostics, + diagnostics_by_url.keys().cloned().collect(), + ); + + (diagnostics_by_url, old_untracked) + }; // Add empty diagnostics for any files that had diagnostics before but don't now. // This will clear them (either the file is no longer relevant to us or fixed!) for url in old_untracked { diagnostics_by_url.entry(url).or_default(); } + + let db = session.project_db(&project_path); + let global_settings = session.global_settings(); + // Send the settings diagnostics! for (url, file_diagnostics) in diagnostics_by_url { // Convert diagnostics to LSP format let lsp_diagnostics = file_diagnostics .into_iter() - .map(|diagnostic| { - to_lsp_diagnostic(db, &diagnostic, session_encoding, client_capabilities).1 + .filter_map(|diagnostic| { + Some( + to_lsp_diagnostic( + db, + &diagnostic, + session_encoding, + client_capabilities, + global_settings, + )? + .1, + ) }) .collect::>(); @@ -315,7 +358,12 @@ pub(super) fn to_lsp_diagnostic( diagnostic: &ruff_db::diagnostic::Diagnostic, encoding: PositionEncoding, client_capabilities: ResolvedClientCapabilities, -) -> (Option, Diagnostic) { + global_settings: &GlobalSettings, +) -> Option<(Option, Diagnostic)> { + if diagnostic.is_invalid_syntax() && !global_settings.show_syntax_errors() { + return None; + } + let supports_related_information = client_capabilities.supports_diagnostic_related_information(); @@ -388,7 +436,7 @@ pub(super) fn to_lsp_diagnostic( let data = DiagnosticData::try_from_diagnostic(db, diagnostic, encoding); - ( + Some(( url, Diagnostic { range, @@ -414,7 +462,7 @@ pub(super) fn to_lsp_diagnostic( related_information, data: serde_json::to_value(data).ok(), }, - ) + )) } /// Converts an [`Annotation`] to a [`DiagnosticRelatedInformation`]. diff --git a/crates/ty_server/src/server/api/requests/diagnostic.rs b/crates/ty_server/src/server/api/requests/diagnostic.rs index 47156625b5..3ccc6864e9 100644 --- a/crates/ty_server/src/server/api/requests/diagnostic.rs +++ b/crates/ty_server/src/server/api/requests/diagnostic.rs @@ -66,7 +66,11 @@ impl BackgroundDocumentRequestHandler for DocumentDiagnosticRequestHandler { // SAFETY: Pull diagnostic requests are only called for text documents, not for // notebook documents. items: diagnostics - .to_lsp_diagnostics(db, snapshot.resolved_client_capabilities()) + .to_lsp_diagnostics( + db, + snapshot.resolved_client_capabilities(), + snapshot.global_settings(), + ) .expect_text_document(), }, }) 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 4935735ced..54d323a3bd 100644 --- a/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs +++ b/crates/ty_server/src/server/api/requests/workspace_diagnostic.rs @@ -29,7 +29,7 @@ use crate::server::lazy_work_done_progress::LazyWorkDoneProgress; use crate::server::{Action, Result}; use crate::session::client::Client; use crate::session::index::Index; -use crate::session::{SessionSnapshot, SuspendedWorkspaceDiagnosticRequest}; +use crate::session::{GlobalSettings, SessionSnapshot, SuspendedWorkspaceDiagnosticRequest}; use crate::system::file_to_url; /// Handler for [Workspace diagnostics](workspace-diagnostics) @@ -324,6 +324,7 @@ struct ResponseWriter<'a> { // `file_to_url` isn't guaranteed to return the exact same URL as the one provided // by the client. previous_result_ids: FxHashMap, + global_settings: &'a GlobalSettings, } impl<'a> ResponseWriter<'a> { @@ -361,6 +362,7 @@ impl<'a> ResponseWriter<'a> { position_encoding, client_capabilities: snapshot.resolved_client_capabilities(), previous_result_ids, + global_settings: snapshot.global_settings(), } } @@ -409,14 +411,17 @@ impl<'a> ResponseWriter<'a> { new_id => { let lsp_diagnostics = diagnostics .iter() - .map(|diagnostic| { - to_lsp_diagnostic( - db, - diagnostic, - self.position_encoding, - self.client_capabilities, + .filter_map(|diagnostic| { + Some( + to_lsp_diagnostic( + db, + diagnostic, + self.position_encoding, + self.client_capabilities, + self.global_settings, + )? + .1, ) - .1 }) .collect::>(); diff --git a/crates/ty_server/src/session/options.rs b/crates/ty_server/src/session/options.rs index e3d1ca5b57..a068dabfcc 100644 --- a/crates/ty_server/src/session/options.rs +++ b/crates/ty_server/src/session/options.rs @@ -124,6 +124,12 @@ impl ClientOptions { self } + #[must_use] + pub fn with_show_syntax_errors(mut self, show_syntax_errors: bool) -> Self { + self.global.show_syntax_errors = Some(show_syntax_errors); + self + } + #[must_use] pub fn with_unknown(mut self, unknown: HashMap) -> Self { self.unknown = unknown; @@ -143,6 +149,12 @@ pub struct GlobalOptions { /// Experimental features that the server provides on an opt-in basis. pub(crate) experimental: Option, + + /// If `true` or [`None`], show syntax errors as diagnostics. + /// + /// This is useful when using ty with other language servers, allowing the user to refer + /// to syntax errors from only one source. + pub(crate) show_syntax_errors: Option, } impl GlobalOptions { @@ -155,6 +167,7 @@ impl GlobalOptions { GlobalSettings { diagnostic_mode: self.diagnostic_mode.unwrap_or_default(), experimental, + show_syntax_errors: self.show_syntax_errors.unwrap_or(true), } } } diff --git a/crates/ty_server/src/session/settings.rs b/crates/ty_server/src/session/settings.rs index ec5132eb89..eada1c20e4 100644 --- a/crates/ty_server/src/session/settings.rs +++ b/crates/ty_server/src/session/settings.rs @@ -7,12 +7,17 @@ use ty_project::metadata::options::ProjectOptionsOverrides; pub(crate) struct GlobalSettings { pub(super) diagnostic_mode: DiagnosticMode, pub(super) experimental: ExperimentalSettings, + pub(super) show_syntax_errors: bool, } impl GlobalSettings { pub(crate) fn diagnostic_mode(&self) -> DiagnosticMode { self.diagnostic_mode } + + pub(crate) fn show_syntax_errors(&self) -> bool { + self.show_syntax_errors + } } #[derive(Clone, Default, Debug, PartialEq)] diff --git a/crates/ty_server/tests/e2e/notebook.rs b/crates/ty_server/tests/e2e/notebook.rs index c8d5c738db..71b91f9ce7 100644 --- a/crates/ty_server/tests/e2e/notebook.rs +++ b/crates/ty_server/tests/e2e/notebook.rs @@ -432,6 +432,44 @@ b: Litera Ok(()) } +#[test] +fn invalid_syntax_with_syntax_errors_disabled() -> anyhow::Result<()> { + let mut server = TestServerBuilder::new()? + .with_workspace( + SystemPath::new("src"), + Some(ClientOptions::default().with_show_syntax_errors(false)), + )? + .build() + .wait_until_workspaces_are_initialized(); + + server.initialization_result().unwrap(); + + let mut builder = NotebookBuilder::virtual_file("src/test.ipynb"); + + builder.add_python_cell( + r#"def foo( +"#, + ); + + builder.add_python_cell( + r#"x = 1 + +"#, + ); + + builder.open(&mut server); + + let diagnostics = server.collect_publish_diagnostic_notifications(2); + + assert_json_snapshot!(diagnostics, @r###" + { + "vscode-notebook-cell://src/test.ipynb#0": [], + "vscode-notebook-cell://src/test.ipynb#1": [] + } + "###); + + Ok(()) +} + fn semantic_tokens_full_for_cell( server: &mut TestServer, cell_uri: &lsp_types::Url, diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index e70f0ad49b..6c4be0c25b 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -334,3 +334,30 @@ def foo() -> str: Ok(()) } + +#[test] +fn invalid_syntax_with_syntax_errors_disabled() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo( +"; + + let mut server = TestServerBuilder::new()? + .with_workspace( + workspace_root, + Some(ClientOptions::default().with_show_syntax_errors(false)), + )? + .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.await_notification::(); + + insta::assert_debug_snapshot!(diagnostics); + + Ok(()) +} diff --git a/crates/ty_server/tests/e2e/pull_diagnostics.rs b/crates/ty_server/tests/e2e/pull_diagnostics.rs index 3187fcad92..949a5caeaf 100644 --- a/crates/ty_server/tests/e2e/pull_diagnostics.rs +++ b/crates/ty_server/tests/e2e/pull_diagnostics.rs @@ -68,6 +68,54 @@ def foo() -> str: Ok(()) } +#[test] +fn invalid_syntax_with_syntax_errors_disabled() -> Result<()> { + let _filter = filter_result_id(); + + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo( +"; + + let mut server = TestServerBuilder::new()? + .with_workspace( + workspace_root, + Some(ClientOptions::default().with_show_syntax_errors(false)), + )? + .with_file(foo, foo_content)? + .with_initialization_options( + ClientOptions::default() + .with_show_syntax_errors(false) + .with_diagnostic_mode(DiagnosticMode::Workspace), + ) + .enable_pull_diagnostics(true) + .build() + .wait_until_workspaces_are_initialized(); + + let workspace_diagnostics = server.workspace_diagnostic_request(None, None); + assert_compact_json_snapshot!(workspace_diagnostics, @r#" + { + "items": [ + { + "kind": "full", + "uri": "file:///src/foo.py", + "version": null, + "resultId": "[RESULT_ID]", + "items": [] + } + ] + } + "#); + + server.open_text_document(foo, foo_content, 1); + let diagnostics = server.document_diagnostic_request(foo, None); + + assert_compact_json_snapshot!(diagnostics, @r#"{"kind": "full", "resultId": "[RESULT_ID]", "items": []}"#); + + Ok(()) +} + #[test] fn document_diagnostic_caching_unchanged() -> Result<()> { let _filter = filter_result_id(); diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap index dc31621799..1d2ad41fe3 100644 --- a/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap +++ b/crates/ty_server/tests/e2e/snapshots/e2e__commands__debug_command.snap @@ -9,6 +9,7 @@ Position encoding: UTF16 Global settings: GlobalSettings { diagnostic_mode: OpenFilesOnly, experimental: ExperimentalSettings, + show_syntax_errors: true, } Open text documents: 0 diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__invalid_syntax_with_syntax_errors_disabled.snap b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__invalid_syntax_with_syntax_errors_disabled.snap new file mode 100644 index 0000000000..1bea2c0877 --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__invalid_syntax_with_syntax_errors_disabled.snap @@ -0,0 +1,21 @@ +--- +source: crates/ty_server/tests/e2e/publish_diagnostics.rs +expression: diagnostics +--- +PublishDiagnosticsParams { + uri: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/src/foo.py", + query: None, + fragment: None, + }, + diagnostics: [], + version: Some( + 1, + ), +}