From 4b026c2a553caff2a25641c14f9cdc8153ee3a63 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Fri, 31 Oct 2025 02:16:43 +0100 Subject: [PATCH] Fix missing diagnostics for notebooks (#21156) --- .../ruff_server/src/server/api/diagnostics.rs | 23 ++----------------- .../server/api/notifications/did_change.rs | 6 ++++- .../api/notifications/did_change_notebook.rs | 5 +++- .../notifications/did_change_watched_files.rs | 10 ++++++-- .../src/server/api/notifications/did_close.rs | 2 +- .../src/server/api/notifications/did_open.rs | 12 +++++++++- .../api/notifications/did_open_notebook.rs | 5 +++- 7 files changed, 35 insertions(+), 28 deletions(-) diff --git a/crates/ruff_server/src/server/api/diagnostics.rs b/crates/ruff_server/src/server/api/diagnostics.rs index 2c8faab6db..6f8efe47e8 100644 --- a/crates/ruff_server/src/server/api/diagnostics.rs +++ b/crates/ruff_server/src/server/api/diagnostics.rs @@ -1,7 +1,4 @@ -use lsp_types::Url; - use crate::{ - Session, lint::DiagnosticsMap, session::{Client, DocumentQuery, DocumentSnapshot}, }; @@ -22,21 +19,10 @@ pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> DiagnosticsMa } pub(super) fn publish_diagnostics_for_document( - session: &Session, - url: &Url, + snapshot: &DocumentSnapshot, client: &Client, ) -> crate::server::Result<()> { - // Publish diagnostics if the client doesn't support pull diagnostics - if session.resolved_client_capabilities().pull_diagnostics { - return Ok(()); - } - - let snapshot = session - .take_snapshot(url.clone()) - .ok_or_else(|| anyhow::anyhow!("Unable to take snapshot for document with URL {url}")) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; - - for (uri, diagnostics) in generate_diagnostics(&snapshot) { + for (uri, diagnostics) in generate_diagnostics(snapshot) { client .send_notification::( lsp_types::PublishDiagnosticsParams { @@ -52,14 +38,9 @@ pub(super) fn publish_diagnostics_for_document( } pub(super) fn clear_diagnostics_for_document( - session: &Session, query: &DocumentQuery, client: &Client, ) -> crate::server::Result<()> { - if session.resolved_client_capabilities().pull_diagnostics { - return Ok(()); - } - client .send_notification::( lsp_types::PublishDiagnosticsParams { diff --git a/crates/ruff_server/src/server/api/notifications/did_change.rs b/crates/ruff_server/src/server/api/notifications/did_change.rs index 5ac7a1f606..8e77cb593f 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change.rs @@ -31,7 +31,11 @@ impl super::SyncNotificationHandler for DidChange { .update_text_document(&key, content_changes, new_version) .with_failure_code(ErrorCode::InternalError)?; - publish_diagnostics_for_document(session, &key.into_url(), client)?; + // Publish diagnostics if the client doesn't support pull diagnostics + if !session.resolved_client_capabilities().pull_diagnostics { + let snapshot = session.take_snapshot(key.into_url()).unwrap(); + publish_diagnostics_for_document(&snapshot, client)?; + } Ok(()) } diff --git a/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs b/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs index da11755d71..d092ccacb8 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs @@ -27,7 +27,10 @@ impl super::SyncNotificationHandler for DidChangeNotebook { .with_failure_code(ErrorCode::InternalError)?; // publish new diagnostics - publish_diagnostics_for_document(session, &key.into_url(), client)?; + let snapshot = session + .take_snapshot(key.into_url()) + .expect("snapshot should be available"); + publish_diagnostics_for_document(&snapshot, client)?; Ok(()) } diff --git a/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs b/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs index cb157d81f9..bc97231411 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs @@ -31,13 +31,19 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles { } else { // publish diagnostics for text documents for url in session.text_document_urls() { - publish_diagnostics_for_document(session, url, client)?; + let snapshot = session + .take_snapshot(url.clone()) + .expect("snapshot should be available"); + publish_diagnostics_for_document(&snapshot, client)?; } } // always publish diagnostics for notebook files (since they don't use pull diagnostics) for url in session.notebook_document_urls() { - publish_diagnostics_for_document(session, url, client)?; + let snapshot = session + .take_snapshot(url.clone()) + .expect("snapshot should be available"); + publish_diagnostics_for_document(&snapshot, client)?; } } diff --git a/crates/ruff_server/src/server/api/notifications/did_close.rs b/crates/ruff_server/src/server/api/notifications/did_close.rs index 5a482c4fcc..a3075a4846 100644 --- a/crates/ruff_server/src/server/api/notifications/did_close.rs +++ b/crates/ruff_server/src/server/api/notifications/did_close.rs @@ -27,7 +27,7 @@ impl super::SyncNotificationHandler for DidClose { ); return Ok(()); }; - clear_diagnostics_for_document(session, snapshot.query(), client)?; + clear_diagnostics_for_document(snapshot.query(), client)?; session .close_document(&key) diff --git a/crates/ruff_server/src/server/api/notifications/did_open.rs b/crates/ruff_server/src/server/api/notifications/did_open.rs index fa5f6b92df..41a6fb6cf8 100644 --- a/crates/ruff_server/src/server/api/notifications/did_open.rs +++ b/crates/ruff_server/src/server/api/notifications/did_open.rs @@ -1,5 +1,6 @@ use crate::TextDocument; use crate::server::Result; +use crate::server::api::LSPResult; use crate::server::api::diagnostics::publish_diagnostics_for_document; use crate::session::{Client, Session}; use lsp_types as types; @@ -29,7 +30,16 @@ impl super::SyncNotificationHandler for DidOpen { session.open_text_document(uri.clone(), document); - publish_diagnostics_for_document(session, &uri, client)?; + // Publish diagnostics if the client doesn't support pull diagnostics + if !session.resolved_client_capabilities().pull_diagnostics { + let snapshot = session + .take_snapshot(uri.clone()) + .ok_or_else(|| { + anyhow::anyhow!("Unable to take snapshot for document with URL {uri}") + }) + .with_failure_code(lsp_server::ErrorCode::InternalError)?; + publish_diagnostics_for_document(&snapshot, client)?; + } Ok(()) } diff --git a/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs b/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs index 3ce27168e4..a75e88ecc5 100644 --- a/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs +++ b/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs @@ -40,7 +40,10 @@ impl super::SyncNotificationHandler for DidOpenNotebook { session.open_notebook_document(uri.clone(), notebook); // publish diagnostics - publish_diagnostics_for_document(session, &uri, client)?; + let snapshot = session + .take_snapshot(uri) + .expect("snapshot should be available"); + publish_diagnostics_for_document(&snapshot, client)?; Ok(()) }