[ty] Support publishing diagnostics in the server #18309

dhruvmanila · 2025-05-26T04:26:24Z

Summary

This PR adds support for publishing diagnostics from the ty language server.

It only adds support for it for text documents and not notebook documents because the server doesn't have full notebook support yet.

Closes: astral-sh/ty#79

Test Plan

Testing this out in Helix and Zed since those are the two editors that I know of that doesn't support pull diagnostics:

Helix

Screen.Recording.2025-05-26.at.10.36.21.mov

Zed

Screen.Recording.2025-05-26.at.10.30.22.mov

github-actions · 2025-05-26T04:29:13Z

`mypy_primer` results

No ecosystem changes detected ✅

MichaReiser

Thanks. This overall looks good.

I think there's an opportunity to reduce code duplication a bit and there are some code paths that I don't understand (see inline comments)

crates/ty_server/src/server/api/diagnostics.rs

MichaReiser · 2025-05-26T06:59:44Z

crates/ty_server/src/server/api/diagnostics.rs

+    let lsp_diagnostics = diagnostics.as_slice().iter().map(|diagnostic| {
+        (
+            // TODO: Use the cell index instead using `source_kind`
+            usize::default(),


Is usize::default correct here? What is it used for? I also don't understand the comment beauase it's unclear to me where we use source kind here

We would need to use source_kind (which is currently commented out) to get the NotebookIndex which will be used to get the cell index in which the diagnostics occur. This index will be used to get the URI for that cell to publish the diagnostics for.

crates/ty_server/src/server/api/notifications/did_change.rs

crates/ty_server/src/server/api/notifications/did_open.rs

crates/ty_server/src/server/api/diagnostics.rs

MichaReiser

Oh, we also need to think about how this integrates with didWatchedFilesChange. I don't think it's necessary to call publish diagnostic if a single python file changed because that file isn't open. But we have to re-publish all diagnostics when the configuration changed

dhruvmanila · 2025-05-27T07:34:53Z

Video testing that any changes to configuration files refreshes the diagnostics via publish mechanism in Zed:

Screen.Recording.2025-05-27.at.13.03.29.mov

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

dhruvmanila · 2025-05-28T06:20:48Z

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

+
+            // SAFETY: Only paths that are part of the workspace are registered for file watching.
+            // So, virtual paths and paths that are outside of a workspace does not trigger this
+            // notification.
            let db = session.project_db_for_path_mut(&*root).unwrap();


@MichaReiser can you verify this comment?

I think this is accurate. It may change in the future but we'll then need to change this line :)

dhruvmanila · 2025-05-28T06:21:54Z

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

+            let result = db.apply_changes(changes, None);
+
+            project_changed |= result.project_changed();


I think this should be fine even after we add support for multiple workspaces.

crates/ty_project/src/db/changes.rs

MichaReiser · 2025-05-28T06:32:40Z

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

+
+            // SAFETY: Only paths that are part of the workspace are registered for file watching.
+            // So, virtual paths and paths that are outside of a workspace does not trigger this
+            // notification.
            let db = session.project_db_for_path_mut(&*root).unwrap();


I think this is accurate. It may change in the future but we'll then need to change this line :)

* main: [ty] Support ephemeral uv virtual environments (#18335) Add a `ViolationMetadata::rule` method (#18234) Return `DiagnosticGuard` from `Checker::report_diagnostic` (#18232) [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (#18337) [ty] Support cancellation and retry in the server (#18273) [ty] Synthetic function-like callables (#18242) [ty] Support publishing diagnostics in the server (#18309) Add Autofix for ISC003 (#18256) [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (#18334) [pycodestyle] Make `E712` suggestion not assume a context (#18328)

* main: (246 commits) [ty] Simplify signature types, use them in `CallableType` (astral-sh#18344) [ty] Support ephemeral uv virtual environments (astral-sh#18335) Add a `ViolationMetadata::rule` method (astral-sh#18234) Return `DiagnosticGuard` from `Checker::report_diagnostic` (astral-sh#18232) [flake8_use_pathlib]: Replace os.symlink with Path.symlink_to (PTH211) (astral-sh#18337) [ty] Support cancellation and retry in the server (astral-sh#18273) [ty] Synthetic function-like callables (astral-sh#18242) [ty] Support publishing diagnostics in the server (astral-sh#18309) Add Autofix for ISC003 (astral-sh#18256) [`pyupgrade`]: new rule UP050 (`useless-class-metaclass-type`) (astral-sh#18334) [pycodestyle] Make `E712` suggestion not assume a context (astral-sh#18328) put similar dunder-call tests next to each other (astral-sh#18343) [ty] Derive `PartialOrd, Ord` for `KnownInstanceType` (astral-sh#18340) [ty] Simplify `Type::try_bool()` (astral-sh#18342) [ty] Simplify `Type::normalized` slightly (astral-sh#18339) [ty] Move arviz off the list of selected primer projects (astral-sh#18336) [ty] Add --config-file CLI arg (astral-sh#18083) [ty] Tell the user why we inferred a certain Python version when reporting version-specific syntax errors (astral-sh#18295) [ty] Implement implicit inheritance from `Generic[]` for PEP-695 generic classes (astral-sh#18283) [ty] Add hint if async context manager is used in non-async with statement (astral-sh#18299) ...

dhruvmanila added server Related to the LSP server ty Multi-file analysis & type inference labels May 26, 2025

dhruvmanila force-pushed the dhruv/publish-diagnostics branch from 4ed53ac to 86bde6d Compare May 26, 2025 05:17

dhruvmanila marked this pull request as ready for review May 26, 2025 05:17

dhruvmanila requested review from carljm, MichaReiser, AlexWaygood, sharkdp and dcreager as code owners May 26, 2025 05:17

dhruvmanila removed request for dcreager, carljm, sharkdp and AlexWaygood May 26, 2025 05:18

MichaReiser approved these changes May 26, 2025

View reviewed changes

MichaReiser reviewed May 26, 2025

View reviewed changes

MichaReiser self-requested a review May 26, 2025 07:12

dhruvmanila force-pushed the dhruv/publish-diagnostics branch from d6b28cf to e76575a Compare May 27, 2025 00:20

MichaReiser reviewed May 27, 2025

View reviewed changes

crates/ty_server/src/server/api/notifications/did_change_watched_files.rs Outdated Show resolved Hide resolved

dhruvmanila added 9 commits May 28, 2025 11:45

[ty] Support publishing diagnostics in the server

652e790

Add Diagnostics to make the cell diagnostics explicit

4af00b9

Add Session::project_db_or_default

cc78105

Use existing client_capabilities method

9463125

Inline snapshot.query()

25b166b

Publish diagnostics on DidChangeWatchedFiles

d77cd30

Match against file names

51b0914

Update apply_changes to return ChangeResult

a680ecb

Add common publish_diagnostics function

fc2d725

dhruvmanila force-pushed the dhruv/publish-diagnostics branch from 3794ace to fc2d725 Compare May 28, 2025 06:18

dhruvmanila commented May 28, 2025

View reviewed changes

dhruvmanila requested a review from MichaReiser May 28, 2025 06:22

MichaReiser approved these changes May 28, 2025

View reviewed changes

Address final review comments

6f1591e

dhruvmanila merged commit 48c425c into main May 28, 2025
33 of 34 checks passed

dhruvmanila deleted the dhruv/publish-diagnostics branch May 28, 2025 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ty] Support publishing diagnostics in the server #18309

[ty] Support publishing diagnostics in the server #18309

Uh oh!

dhruvmanila commented May 26, 2025 •

edited

Loading

Uh oh!

github-actions bot commented May 26, 2025 •

edited

Loading

Uh oh!

MichaReiser left a comment

Uh oh!

Uh oh!

MichaReiser May 26, 2025 •

edited

Loading

Uh oh!

dhruvmanila May 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser left a comment

Uh oh!

dhruvmanila commented May 27, 2025

Uh oh!

Uh oh!

dhruvmanila May 28, 2025

Uh oh!

MichaReiser May 28, 2025

Uh oh!

dhruvmanila May 28, 2025

Uh oh!

Uh oh!

Uh oh!

MichaReiser May 28, 2025

Uh oh!

Uh oh!

Uh oh!

		let result = db.apply_changes(changes, None);

		project_changed \|= result.project_changed();

[ty] Support publishing diagnostics in the server #18309

[ty] Support publishing diagnostics in the server #18309

Uh oh!

Conversation

dhruvmanila commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Helix

Zed

Uh oh!

github-actions bot commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MichaReiser May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented May 27, 2025

Uh oh!

Uh oh!

dhruvmanila May 28, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser May 28, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MichaReiser May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dhruvmanila commented May 26, 2025 •

edited

Loading

github-actions bot commented May 26, 2025 •

edited

Loading

`mypy_primer` results

MichaReiser May 26, 2025 •

edited

Loading