[ty] Don't send publish diagnostics for clients supporting pull diagnostics (#21772)

This commit is contained in:
Micha Reiser 2025-12-04 08:12:04 +01:00 committed by GitHub
parent e2b72fbf99
commit a9f2bb41bd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 138 additions and 37 deletions

View File

@ -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();

View File

@ -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.as_ref())?;
db.memory_file_system().create_directory_all(&root)?;
let workspace_root = get_cargo_workspace_root()?;
let workspace_root = workspace_root.to_string();

View File

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

View File

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

View File

@ -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(),

View File

@ -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::<PublishDiagnostics>();
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::<PublishDiagnostics>();
let hints = server

View File

@ -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 AsRef<str>,
version: i32,
) {
let params = DidOpenTextDocumentParams {
@ -758,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::<DidOpenTextDocument>(params);
@ -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);

View File

@ -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;
@ -20,10 +22,90 @@ 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::<PublishDiagnostics>();
insta::assert_debug_snapshot!(diagnostics);
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(())
}

View File

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

View File

@ -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
}