Skip to content

Commit a9f2bb4

Browse files
MichaReiserMichaReiser
authored
[ty] Don't send publish diagnostics for clients supporting pull diagnostics (#21772)
1 parent e2b72fb commit a9f2bb4

File tree

10 files changed

+138
-37
lines changed

10 files changed

+138
-37
lines changed

‎crates/ruff_db/src/system/path.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,13 @@ impl Deref for SystemPathBuf {
667667
}
668668
}
669669

670+
impl AsRef<Path> for SystemPathBuf {
671+
#[inline]
672+
fn as_ref(&self) -> &Path {
673+
self.0.as_std_path()
674+
}
675+
}
676+
670677
impl<P: AsRef<SystemPath>> FromIterator<P> for SystemPathBuf {
671678
fn from_iter<I: IntoIterator<Item = P>>(iter: I) -> Self {
672679
let mut buf = SystemPathBuf::new();

‎crates/ty_python_semantic/tests/corpus.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ fn run_corpus_tests(pattern: &str) -> anyhow::Result<()> {
7979
let root = SystemPathBuf::from("/src");
8080

8181
let mut db = CorpusDb::new();
82-
db.memory_file_system()
83-
.create_directory_all(root.as_ref())?;
82+
db.memory_file_system().create_directory_all(&root)?;
8483

8584
let workspace_root = get_cargo_workspace_root()?;
8685
let workspace_root = workspace_root.to_string();

‎crates/ty_server/src/server/api/notifications/did_change_watched_files.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use crate::document::DocumentKey;
22
use crate::server::Result;
3-
use crate::server::api::diagnostics::{publish_diagnostics, publish_settings_diagnostics};
3+
use crate::server::api::diagnostics::{
4+
publish_diagnostics_if_needed, publish_settings_diagnostics,
5+
};
46
use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
57
use crate::session::Session;
68
use crate::session::client::Client;
@@ -92,7 +94,7 @@ impl SyncNotificationHandler for DidChangeWatchedFiles {
9294
);
9395
} else {
9496
for key in session.text_document_handles() {
95-
publish_diagnostics(&key, session, client);
97+
publish_diagnostics_if_needed(&key, session, client);
9698
}
9799
}
98100

‎crates/ty_server/tests/e2e/code_actions.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ unused-ignore-comment = \"warn\"
6565
.build()
6666
.wait_until_workspaces_are_initialized();
6767

68-
server.open_text_document(foo, &foo_content, 1);
68+
server.open_text_document(foo, foo_content, 1);
6969

7070
// Wait for diagnostics to be computed.
7171
let diagnostics = server.document_diagnostic_request(foo, None);
@@ -103,7 +103,7 @@ unused-ignore-comment = \"warn\"
103103
.build()
104104
.wait_until_workspaces_are_initialized();
105105

106-
server.open_text_document(foo, &foo_content, 1);
106+
server.open_text_document(foo, foo_content, 1);
107107

108108
// Wait for diagnostics to be computed.
109109
let diagnostics = server.document_diagnostic_request(foo, None);
@@ -145,7 +145,7 @@ unused-ignore-comment = \"warn\"
145145
.build()
146146
.wait_until_workspaces_are_initialized();
147147

148-
server.open_text_document(foo, &foo_content, 1);
148+
server.open_text_document(foo, foo_content, 1);
149149

150150
// Wait for diagnostics to be computed.
151151
let diagnostics = server.document_diagnostic_request(foo, None);
@@ -182,7 +182,7 @@ def my_func(): ...
182182
.build()
183183
.wait_until_workspaces_are_initialized();
184184

185-
server.open_text_document(foo, &foo_content, 1);
185+
server.open_text_document(foo, foo_content, 1);
186186

187187
// Wait for diagnostics to be computed.
188188
let diagnostics = server.document_diagnostic_request(foo, None);
@@ -221,7 +221,7 @@ def my_func(): ...
221221
.build()
222222
.wait_until_workspaces_are_initialized();
223223

224-
server.open_text_document(foo, &foo_content, 1);
224+
server.open_text_document(foo, foo_content, 1);
225225

226226
// Wait for diagnostics to be computed.
227227
let diagnostics = server.document_diagnostic_request(foo, None);
@@ -257,7 +257,7 @@ x: typing.Literal[1] = 1
257257
.build()
258258
.wait_until_workspaces_are_initialized();
259259

260-
server.open_text_document(foo, &foo_content, 1);
260+
server.open_text_document(foo, foo_content, 1);
261261

262262
// Wait for diagnostics to be computed.
263263
let diagnostics = server.document_diagnostic_request(foo, None);
@@ -294,7 +294,7 @@ html.parser
294294
.build()
295295
.wait_until_workspaces_are_initialized();
296296

297-
server.open_text_document(foo, &foo_content, 1);
297+
server.open_text_document(foo, foo_content, 1);
298298

299299
// Wait for diagnostics to be computed.
300300
let diagnostics = server.document_diagnostic_request(foo, None);

‎crates/ty_server/tests/e2e/initialize.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ def foo() -> str:
294294
.build()
295295
.wait_until_workspaces_are_initialized();
296296

297-
server.open_text_document(foo, &foo_content, 1);
297+
server.open_text_document(foo, foo_content, 1);
298298
let hover = server.hover_request(foo, Position::new(0, 5));
299299

300300
assert!(
@@ -326,7 +326,7 @@ def foo() -> str:
326326
.build()
327327
.wait_until_workspaces_are_initialized();
328328

329-
server.open_text_document(foo, &foo_content, 1);
329+
server.open_text_document(foo, foo_content, 1);
330330
let hover = server.hover_request(foo, Position::new(0, 5));
331331

332332
assert!(
@@ -367,14 +367,14 @@ def bar() -> str:
367367
.build()
368368
.wait_until_workspaces_are_initialized();
369369

370-
server.open_text_document(foo, &foo_content, 1);
370+
server.open_text_document(foo, foo_content, 1);
371371
let hover_foo = server.hover_request(foo, Position::new(0, 5));
372372
assert!(
373373
hover_foo.is_none(),
374374
"Expected no hover information for workspace A, got: {hover_foo:?}"
375375
);
376376

377-
server.open_text_document(bar, &bar_content, 1);
377+
server.open_text_document(bar, bar_content, 1);
378378
let hover_bar = server.hover_request(bar, Position::new(0, 5));
379379
assert!(
380380
hover_bar.is_some(),

‎crates/ty_server/tests/e2e/inlay_hints.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ y = foo(1)
2828
.build()
2929
.wait_until_workspaces_are_initialized();
3030

31-
server.open_text_document(foo, &foo_content, 1);
31+
server.open_text_document(foo, foo_content, 1);
3232
let _ = server.await_notification::<PublishDiagnostics>();
3333

3434
let hints = server
@@ -132,7 +132,7 @@ fn variable_inlay_hints_disabled() -> Result<()> {
132132
.build()
133133
.wait_until_workspaces_are_initialized();
134134

135-
server.open_text_document(foo, &foo_content, 1);
135+
server.open_text_document(foo, foo_content, 1);
136136
let _ = server.await_notification::<PublishDiagnostics>();
137137

138138
let hints = server

‎crates/ty_server/tests/e2e/main.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -742,23 +742,26 @@ impl TestServer {
742742
}
743743

744744
pub(crate) fn file_uri(&self, path: impl AsRef<SystemPath>) -> Url {
745-
Url::from_file_path(self.test_context.root().join(path.as_ref()).as_std_path())
746-
.expect("Path must be a valid URL")
745+
Url::from_file_path(self.file_path(path).as_std_path()).expect("Path must be a valid URL")
746+
}
747+
748+
pub(crate) fn file_path(&self, path: impl AsRef<SystemPath>) -> SystemPathBuf {
749+
self.test_context.root().join(path)
747750
}
748751

749752
/// Send a `textDocument/didOpen` notification
750753
pub(crate) fn open_text_document(
751754
&mut self,
752755
path: impl AsRef<SystemPath>,
753-
content: &impl ToString,
756+
content: impl AsRef<str>,
754757
version: i32,
755758
) {
756759
let params = DidOpenTextDocumentParams {
757760
text_document: TextDocumentItem {
758761
uri: self.file_uri(path),
759762
language_id: "python".to_string(),
760763
version,
761-
text: content.to_string(),
764+
text: content.as_ref().to_string(),
762765
},
763766
};
764767
self.send_notification::<DidOpenTextDocument>(params);
@@ -793,7 +796,6 @@ impl TestServer {
793796
}
794797

795798
/// Send a `workspace/didChangeWatchedFiles` notification with the given file events
796-
#[expect(dead_code)]
797799
pub(crate) fn did_change_watched_files(&mut self, events: Vec<FileEvent>) {
798800
let params = DidChangeWatchedFilesParams { changes: events };
799801
self.send_notification::<DidChangeWatchedFiles>(params);

‎crates/ty_server/tests/e2e/publish_diagnostics.rs‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
use std::time::Duration;
2+
13
use anyhow::Result;
2-
use lsp_types::notification::PublishDiagnostics;
4+
use lsp_types::{FileChangeType, FileEvent, notification::PublishDiagnostics};
35
use ruff_db::system::SystemPath;
46

57
use crate::TestServerBuilder;
@@ -20,10 +22,90 @@ def foo() -> str:
2022
.build()
2123
.wait_until_workspaces_are_initialized();
2224

23-
server.open_text_document(foo, &foo_content, 1);
25+
server.open_text_document(foo, foo_content, 1);
2426
let diagnostics = server.await_notification::<PublishDiagnostics>();
2527

2628
insta::assert_debug_snapshot!(diagnostics);
2729

2830
Ok(())
2931
}
32+
33+
#[test]
34+
fn on_did_change_watched_files() -> Result<()> {
35+
let workspace_root = SystemPath::new("src");
36+
let foo = SystemPath::new("src/foo.py");
37+
let foo_content = "\
38+
def foo() -> str:
39+
print(a)
40+
";
41+
42+
let mut server = TestServerBuilder::new()?
43+
.with_workspace(workspace_root, None)?
44+
.with_file(foo, "")?
45+
.enable_pull_diagnostics(false)
46+
.build()
47+
.wait_until_workspaces_are_initialized();
48+
49+
let foo = server.file_path(foo);
50+
51+
server.open_text_document(&foo, "", 1);
52+
53+
let _open_diagnostics = server.await_notification::<PublishDiagnostics>();
54+
55+
std::fs::write(&foo, foo_content)?;
56+
57+
server.did_change_watched_files(vec![FileEvent {
58+
uri: server.file_uri(foo),
59+
typ: FileChangeType::CHANGED,
60+
}]);
61+
62+
let diagnostics = server.await_notification::<PublishDiagnostics>();
63+
64+
// Note how ty reports no diagnostics here. This is because
65+
// the contents received by didOpen/didChange take precedence over the file
66+
// content on disk. Or, more specifically, because the revision
67+
// of the file is not bumped, because it still uses the version
68+
// from the `didOpen` notification but we don't have any notification
69+
// that we can use here.
70+
insta::assert_json_snapshot!(diagnostics);
71+
72+
Ok(())
73+
}
74+
75+
#[test]
76+
fn on_did_change_watched_files_pull_diagnostics() -> Result<()> {
77+
let workspace_root = SystemPath::new("src");
78+
let foo = SystemPath::new("src/foo.py");
79+
let foo_content = "\
80+
def foo() -> str:
81+
print(a)
82+
";
83+
84+
let mut server = TestServerBuilder::new()?
85+
.with_workspace(workspace_root, None)?
86+
.with_file(foo, "")?
87+
.enable_pull_diagnostics(true)
88+
.build()
89+
.wait_until_workspaces_are_initialized();
90+
91+
let foo = server.file_path(foo);
92+
93+
server.open_text_document(&foo, "", 1);
94+
95+
std::fs::write(&foo, foo_content)?;
96+
97+
server.did_change_watched_files(vec![FileEvent {
98+
uri: server.file_uri(foo),
99+
typ: FileChangeType::CHANGED,
100+
}]);
101+
102+
let diagnostics =
103+
server.try_await_notification::<PublishDiagnostics>(Some(Duration::from_millis(100)));
104+
105+
assert!(
106+
diagnostics.is_err(),
107+
"Server should not send a publish diagnostic notification if the client supports pull diagnostics"
108+
);
109+
110+
Ok(())
111+
}

‎crates/ty_server/tests/e2e/pull_diagnostics.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def foo() -> str:
3131
.build()
3232
.wait_until_workspaces_are_initialized();
3333

34-
server.open_text_document(foo, &foo_content, 1);
34+
server.open_text_document(foo, foo_content, 1);
3535
let diagnostics = server.document_diagnostic_request(foo, None);
3636

3737
assert_debug_snapshot!(diagnostics);
@@ -57,7 +57,7 @@ def foo() -> str:
5757
.build()
5858
.wait_until_workspaces_are_initialized();
5959

60-
server.open_text_document(foo, &foo_content, 1);
60+
server.open_text_document(foo, foo_content, 1);
6161

6262
// First request with no previous result ID
6363
let first_response = server.document_diagnostic_request(foo, None);
@@ -113,7 +113,7 @@ def foo() -> str:
113113
.build()
114114
.wait_until_workspaces_are_initialized();
115115

116-
server.open_text_document(foo, &foo_content_v1, 1);
116+
server.open_text_document(foo, foo_content_v1, 1);
117117

118118
// First request with no previous result ID
119119
let first_response = server.document_diagnostic_request(foo, None);
@@ -233,7 +233,7 @@ def foo() -> str:
233233
.build()
234234
.wait_until_workspaces_are_initialized();
235235

236-
server.open_text_document(file_a, &file_a_content, 1);
236+
server.open_text_document(file_a, file_a_content, 1);
237237

238238
// First request with no previous result IDs
239239
let mut first_response = server
@@ -250,10 +250,10 @@ def foo() -> str:
250250

251251
// Make changes to files B, C, D, and E (leave A unchanged)
252252
// Need to open files before changing them
253-
server.open_text_document(file_b, &file_b_content_v1, 1);
254-
server.open_text_document(file_c, &file_c_content_v1, 1);
255-
server.open_text_document(file_d, &file_d_content_v1, 1);
256-
server.open_text_document(file_e, &file_e_content_v1, 1);
253+
server.open_text_document(file_b, file_b_content_v1, 1);
254+
server.open_text_document(file_c, file_c_content_v1, 1);
255+
server.open_text_document(file_d, file_d_content_v1, 1);
256+
server.open_text_document(file_e, file_e_content_v1, 1);
257257

258258
// File B: Add a new error
259259
server.change_text_document(
@@ -536,9 +536,9 @@ fn workspace_diagnostic_streaming_with_caching() -> Result<()> {
536536
.build()
537537
.wait_until_workspaces_are_initialized();
538538

539-
server.open_text_document(SystemPath::new("src/error_0.py"), &error_content, 1);
540-
server.open_text_document(SystemPath::new("src/error_1.py"), &error_content, 1);
541-
server.open_text_document(SystemPath::new("src/error_2.py"), &error_content, 1);
539+
server.open_text_document(SystemPath::new("src/error_0.py"), error_content, 1);
540+
server.open_text_document(SystemPath::new("src/error_1.py"), error_content, 1);
541+
server.open_text_document(SystemPath::new("src/error_2.py"), error_content, 1);
542542

543543
// First request to get result IDs (non-streaming for simplicity)
544544
let first_response = server.workspace_diagnostic_request(None, None);
@@ -716,7 +716,7 @@ def hello() -> str:
716716
create_workspace_server_with_file(workspace_root, file_path, file_content_no_error)?;
717717

718718
// Open the file first
719-
server.open_text_document(file_path, &file_content_no_error, 1);
719+
server.open_text_document(file_path, file_content_no_error, 1);
720720

721721
// Make a workspace diagnostic request to a project with one file but no diagnostics
722722
// This should trigger long-polling since the project has no diagnostics
@@ -819,7 +819,7 @@ def hello() -> str:
819819
create_workspace_server_with_file(workspace_root, file_path, file_content_no_error)?;
820820

821821
// Open the file first
822-
server.open_text_document(file_path, &file_content_no_error, 1);
822+
server.open_text_document(file_path, file_content_no_error, 1);
823823

824824
// PHASE 1: Initial suspend (no diagnostics)
825825
let request_id_1 = send_workspace_diagnostic_request(&mut server);

‎crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_watched_files.snap‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
3+
expression: diagnostics
4+
---
5+
{
6+
"uri": "file://<temp_dir>/src/foo.py",
7+
"diagnostics": [],
8+
"version": 1
9+
}

0 commit comments

Comments
 (0)