From 1e71d29c5b39bc47bd62918ce8a10530b74f208b Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 3 Dec 2025 08:41:44 +0100 Subject: [PATCH 1/3] [ty] Don't send publish diagnostics for clients supporting pull diagnostics --- crates/ruff_db/src/system/path.rs | 7 ++ crates/ty_python_semantic/tests/corpus.rs | 2 +- .../notifications/did_change_watched_files.rs | 6 +- crates/ty_server/tests/e2e/main.rs | 10 ++- .../tests/e2e/publish_diagnostics.rs | 84 ++++++++++++++++++- ...gnostics__on_did_change_watched_files.snap | 9 ++ 6 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_watched_files.snap diff --git a/crates/ruff_db/src/system/path.rs b/crates/ruff_db/src/system/path.rs index a387ae54f62ed..9c525f5acab92 100644 --- a/crates/ruff_db/src/system/path.rs +++ b/crates/ruff_db/src/system/path.rs @@ -667,6 +667,13 @@ impl Deref for SystemPathBuf { } } +impl AsRef for SystemPathBuf { + #[inline] + fn as_ref(&self) -> &Path { + self.0.as_std_path() + } +} + impl> FromIterator

for SystemPathBuf { fn from_iter>(iter: I) -> Self { let mut buf = SystemPathBuf::new(); diff --git a/crates/ty_python_semantic/tests/corpus.rs b/crates/ty_python_semantic/tests/corpus.rs index c09929bcbd17f..ebe6719e2f7bb 100644 --- a/crates/ty_python_semantic/tests/corpus.rs +++ b/crates/ty_python_semantic/tests/corpus.rs @@ -80,7 +80,7 @@ fn run_corpus_tests(pattern: &str) -> anyhow::Result<()> { let mut db = CorpusDb::new(); db.memory_file_system() - .create_directory_all(root.as_ref())?; + .create_directory_all(&root)?; let workspace_root = get_cargo_workspace_root()?; let workspace_root = workspace_root.to_string(); diff --git a/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs b/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs index c67bbd8e70e53..fef69d9e66946 100644 --- a/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs +++ b/crates/ty_server/src/server/api/notifications/did_change_watched_files.rs @@ -1,6 +1,8 @@ use crate::document::DocumentKey; use crate::server::Result; -use crate::server::api::diagnostics::{publish_diagnostics, publish_settings_diagnostics}; +use crate::server::api::diagnostics::{ + publish_diagnostics_if_needed, publish_settings_diagnostics, +}; use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::session::Session; use crate::session::client::Client; @@ -92,7 +94,7 @@ impl SyncNotificationHandler for DidChangeWatchedFiles { ); } else { for key in session.text_document_handles() { - publish_diagnostics(&key, session, client); + publish_diagnostics_if_needed(&key, session, client); } } diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index 8e416d11a3d9d..b45fca7f471d2 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -742,15 +742,18 @@ impl TestServer { } pub(crate) fn file_uri(&self, path: impl AsRef) -> Url { - Url::from_file_path(self.test_context.root().join(path.as_ref()).as_std_path()) - .expect("Path must be a valid URL") + Url::from_file_path(self.file_path(path).as_std_path()).expect("Path must be a valid URL") + } + + pub(crate) fn file_path(&self, path: impl AsRef) -> SystemPathBuf { + self.test_context.root().join(path) } /// Send a `textDocument/didOpen` notification pub(crate) fn open_text_document( &mut self, path: impl AsRef, - content: &impl ToString, + content: impl ToString, version: i32, ) { let params = DidOpenTextDocumentParams { @@ -793,7 +796,6 @@ impl TestServer { } /// Send a `workspace/didChangeWatchedFiles` notification with the given file events - #[expect(dead_code)] pub(crate) fn did_change_watched_files(&mut self, events: Vec) { let params = DidChangeWatchedFilesParams { changes: events }; self.send_notification::(params); diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index ae138506a6861..ed1772fef2c67 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -1,5 +1,7 @@ +use std::time::Duration; + use anyhow::Result; -use lsp_types::notification::PublishDiagnostics; +use lsp_types::{FileChangeType, FileEvent, notification::PublishDiagnostics}; use ruff_db::system::SystemPath; use crate::TestServerBuilder; @@ -27,3 +29,83 @@ def foo() -> str: Ok(()) } + +#[test] +fn on_did_change_watched_files() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo() -> str: + print(a) +"; + + let mut server = TestServerBuilder::new()? + .with_workspace(workspace_root, None)? + .with_file(foo, "")? + .enable_pull_diagnostics(false) + .build() + .wait_until_workspaces_are_initialized(); + + let foo = server.file_path(foo); + + server.open_text_document(&foo, "", 1); + + let _open_diagnostics = server.await_notification::(); + + std::fs::write(&foo, foo_content)?; + + server.did_change_watched_files(vec![FileEvent { + uri: server.file_uri(foo), + typ: FileChangeType::CHANGED, + }]); + + let diagnostics = server.await_notification::(); + + // Note how ty reports no diagnostics here. This is because + // the contents received by didOpen/didChange take precedence over the file + // content on disk. Or, more specifically, because the revision + // of the file is not bumped, because it still uses the version + // from the `didOpen` notification but we don't have any notification + // that we can use here. + insta::assert_json_snapshot!(diagnostics); + + Ok(()) +} + +#[test] +fn on_did_change_watched_files_pull_diagnostics() -> Result<()> { + let workspace_root = SystemPath::new("src"); + let foo = SystemPath::new("src/foo.py"); + let foo_content = "\ +def foo() -> str: + print(a) +"; + + let mut server = TestServerBuilder::new()? + .with_workspace(workspace_root, None)? + .with_file(foo, "")? + .enable_pull_diagnostics(true) + .build() + .wait_until_workspaces_are_initialized(); + + let foo = server.file_path(foo); + + server.open_text_document(&foo, "", 1); + + std::fs::write(&foo, foo_content)?; + + server.did_change_watched_files(vec![FileEvent { + uri: server.file_uri(foo), + typ: FileChangeType::CHANGED, + }]); + + let diagnostics = + server.try_await_notification::(Some(Duration::from_millis(100))); + + assert!( + diagnostics.is_err(), + "Server should not send a publish diagnostic notification if the client supports pull diagnostics" + ); + + Ok(()) +} diff --git a/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_watched_files.snap b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_watched_files.snap new file mode 100644 index 0000000000000..52ce909cad417 --- /dev/null +++ b/crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_watched_files.snap @@ -0,0 +1,9 @@ +--- +source: crates/ty_server/tests/e2e/publish_diagnostics.rs +expression: diagnostics +--- +{ + "uri": "file:///src/foo.py", + "diagnostics": [], + "version": 1 +} From a1f5e7f7cd876192343c993321f93321d862f110 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 3 Dec 2025 10:13:42 +0100 Subject: [PATCH 2/3] Fix clippy warnings --- crates/ty_server/tests/e2e/code_actions.rs | 14 +++++----- crates/ty_server/tests/e2e/initialize.rs | 8 +++--- crates/ty_server/tests/e2e/inlay_hints.rs | 4 +-- crates/ty_server/tests/e2e/main.rs | 4 +-- .../tests/e2e/publish_diagnostics.rs | 2 +- .../ty_server/tests/e2e/pull_diagnostics.rs | 26 +++++++++---------- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/ty_server/tests/e2e/code_actions.rs b/crates/ty_server/tests/e2e/code_actions.rs index 77f4d42fcb95b..0e5eafcd13823 100644 --- a/crates/ty_server/tests/e2e/code_actions.rs +++ b/crates/ty_server/tests/e2e/code_actions.rs @@ -65,7 +65,7 @@ unused-ignore-comment = \"warn\" .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); @@ -103,7 +103,7 @@ unused-ignore-comment = \"warn\" .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); @@ -145,7 +145,7 @@ unused-ignore-comment = \"warn\" .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); @@ -182,7 +182,7 @@ def my_func(): ... .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); @@ -221,7 +221,7 @@ def my_func(): ... .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); @@ -257,7 +257,7 @@ x: typing.Literal[1] = 1 .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); @@ -294,7 +294,7 @@ html.parser .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // Wait for diagnostics to be computed. let diagnostics = server.document_diagnostic_request(foo, None); diff --git a/crates/ty_server/tests/e2e/initialize.rs b/crates/ty_server/tests/e2e/initialize.rs index 87910373b4e50..1526e78022224 100644 --- a/crates/ty_server/tests/e2e/initialize.rs +++ b/crates/ty_server/tests/e2e/initialize.rs @@ -294,7 +294,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); let hover = server.hover_request(foo, Position::new(0, 5)); assert!( @@ -326,7 +326,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); let hover = server.hover_request(foo, Position::new(0, 5)); assert!( @@ -367,14 +367,14 @@ def bar() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + 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); + server.open_text_document(bar, bar_content, 1); let hover_bar = server.hover_request(bar, Position::new(0, 5)); assert!( hover_bar.is_some(), diff --git a/crates/ty_server/tests/e2e/inlay_hints.rs b/crates/ty_server/tests/e2e/inlay_hints.rs index 974f97c3de2b1..2f2848d9b8e6a 100644 --- a/crates/ty_server/tests/e2e/inlay_hints.rs +++ b/crates/ty_server/tests/e2e/inlay_hints.rs @@ -28,7 +28,7 @@ y = foo(1) .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); let _ = server.await_notification::(); let hints = server @@ -132,7 +132,7 @@ fn variable_inlay_hints_disabled() -> Result<()> { .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); let _ = server.await_notification::(); let hints = server diff --git a/crates/ty_server/tests/e2e/main.rs b/crates/ty_server/tests/e2e/main.rs index b45fca7f471d2..7e380562fa971 100644 --- a/crates/ty_server/tests/e2e/main.rs +++ b/crates/ty_server/tests/e2e/main.rs @@ -753,7 +753,7 @@ impl TestServer { pub(crate) fn open_text_document( &mut self, path: impl AsRef, - content: impl ToString, + content: impl AsRef, version: i32, ) { let params = DidOpenTextDocumentParams { @@ -761,7 +761,7 @@ impl TestServer { uri: self.file_uri(path), language_id: "python".to_string(), version, - text: content.to_string(), + text: content.as_ref().to_string(), }, }; self.send_notification::(params); diff --git a/crates/ty_server/tests/e2e/publish_diagnostics.rs b/crates/ty_server/tests/e2e/publish_diagnostics.rs index ed1772fef2c67..b7f1eaf2d9ef5 100644 --- a/crates/ty_server/tests/e2e/publish_diagnostics.rs +++ b/crates/ty_server/tests/e2e/publish_diagnostics.rs @@ -22,7 +22,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); let diagnostics = server.await_notification::(); insta::assert_debug_snapshot!(diagnostics); diff --git a/crates/ty_server/tests/e2e/pull_diagnostics.rs b/crates/ty_server/tests/e2e/pull_diagnostics.rs index ba67404d15b12..3256023905171 100644 --- a/crates/ty_server/tests/e2e/pull_diagnostics.rs +++ b/crates/ty_server/tests/e2e/pull_diagnostics.rs @@ -31,7 +31,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); let diagnostics = server.document_diagnostic_request(foo, None); assert_debug_snapshot!(diagnostics); @@ -57,7 +57,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content, 1); + server.open_text_document(foo, foo_content, 1); // First request with no previous result ID let first_response = server.document_diagnostic_request(foo, None); @@ -113,7 +113,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(foo, &foo_content_v1, 1); + server.open_text_document(foo, foo_content_v1, 1); // First request with no previous result ID let first_response = server.document_diagnostic_request(foo, None); @@ -233,7 +233,7 @@ def foo() -> str: .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(file_a, &file_a_content, 1); + server.open_text_document(file_a, file_a_content, 1); // First request with no previous result IDs let mut first_response = server @@ -250,10 +250,10 @@ def foo() -> str: // Make changes to files B, C, D, and E (leave A unchanged) // Need to open files before changing them - server.open_text_document(file_b, &file_b_content_v1, 1); - server.open_text_document(file_c, &file_c_content_v1, 1); - server.open_text_document(file_d, &file_d_content_v1, 1); - server.open_text_document(file_e, &file_e_content_v1, 1); + server.open_text_document(file_b, file_b_content_v1, 1); + server.open_text_document(file_c, file_c_content_v1, 1); + server.open_text_document(file_d, file_d_content_v1, 1); + server.open_text_document(file_e, file_e_content_v1, 1); // File B: Add a new error server.change_text_document( @@ -536,9 +536,9 @@ fn workspace_diagnostic_streaming_with_caching() -> Result<()> { .build() .wait_until_workspaces_are_initialized(); - server.open_text_document(SystemPath::new("src/error_0.py"), &error_content, 1); - server.open_text_document(SystemPath::new("src/error_1.py"), &error_content, 1); - server.open_text_document(SystemPath::new("src/error_2.py"), &error_content, 1); + server.open_text_document(SystemPath::new("src/error_0.py"), error_content, 1); + server.open_text_document(SystemPath::new("src/error_1.py"), error_content, 1); + server.open_text_document(SystemPath::new("src/error_2.py"), error_content, 1); // First request to get result IDs (non-streaming for simplicity) let first_response = server.workspace_diagnostic_request(None, None); @@ -716,7 +716,7 @@ def hello() -> str: create_workspace_server_with_file(workspace_root, file_path, file_content_no_error)?; // Open the file first - server.open_text_document(file_path, &file_content_no_error, 1); + server.open_text_document(file_path, file_content_no_error, 1); // Make a workspace diagnostic request to a project with one file but no diagnostics // This should trigger long-polling since the project has no diagnostics @@ -819,7 +819,7 @@ def hello() -> str: create_workspace_server_with_file(workspace_root, file_path, file_content_no_error)?; // Open the file first - server.open_text_document(file_path, &file_content_no_error, 1); + server.open_text_document(file_path, file_content_no_error, 1); // PHASE 1: Initial suspend (no diagnostics) let request_id_1 = send_workspace_diagnostic_request(&mut server); From e735dbb8bc982aa4fdd8a956b11e4f7088f12bf4 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 3 Dec 2025 10:14:35 +0100 Subject: [PATCH 3/3] now fmt --- crates/ty_python_semantic/tests/corpus.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ty_python_semantic/tests/corpus.rs b/crates/ty_python_semantic/tests/corpus.rs index ebe6719e2f7bb..505be61383960 100644 --- a/crates/ty_python_semantic/tests/corpus.rs +++ b/crates/ty_python_semantic/tests/corpus.rs @@ -79,8 +79,7 @@ fn run_corpus_tests(pattern: &str) -> anyhow::Result<()> { let root = SystemPathBuf::from("/src"); let mut db = CorpusDb::new(); - db.memory_file_system() - .create_directory_all(&root)?; + db.memory_file_system().create_directory_all(&root)?; let workspace_root = get_cargo_workspace_root()?; let workspace_root = workspace_root.to_string();