[ty] Don't send publish diagnostics for clients supporting pull diagnostics

Author: MichaReiser

crates/ruff_db/src/system/path.rs

@@ -667,6 +667,13 @@ impl Deref for SystemPathBuf {
     }
 }
 
+impl AsRef<Path> for SystemPathBuf {
+    #[inline]
+    fn as_ref(&self) -> &Path {
+        self.0.as_std_path()
+    }
+}
+
 impl<P: AsRef<SystemPath>> FromIterator<P> for SystemPathBuf {
     fn from_iter<I: IntoIterator<Item = P>>(iter: I) -> Self {
         let mut buf = SystemPathBuf::new();

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);
             }
         }
 

crates/ty_server/tests/e2e/main.rs

@@ -742,15 +742,18 @@ impl TestServer {
     }
 
     pub(crate) fn file_uri(&self, path: impl AsRef<SystemPath>) -> 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<SystemPath>) -> SystemPathBuf {
+        self.test_context.root().join(path)
     }
 
     /// Send a `textDocument/didOpen` notification
     pub(crate) fn open_text_document(
         &mut self,
         path: impl AsRef<SystemPath>,
-        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<FileEvent>) {
         let params = DidChangeWatchedFilesParams { changes: events };
         self.send_notification::<DidChangeWatchedFiles>(params);

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::<PublishDiagnostics>();
+
+    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::<PublishDiagnostics>();
+
+    // 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::<PublishDiagnostics>(Some(Duration::from_millis(100)));
+
+    assert!(
+        diagnostics.is_err(),
+        "Server should not send a publish diagnostic notification if the client supports pull diagnostics"
+    );
+
+    Ok(())
+}

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://<temp_dir>/src/foo.py",
+  "diagnostics": [],
+  "version": 1
+}