[ty] Make sure to always respond to client requests (#19277)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run

## Summary

This PR fixes a bug that didn't return a response to the client if the
document snapshotting failed.

This is resolved by making sure that the server always creates the
document snapshot and embed the any failures inside the snapshot.

Closes: astral-sh/ty#798

## Test Plan

Using the test case as described in the linked issue:



https://github.com/user-attachments/assets/f32833f8-03e5-4641-8c7f-2a536fe2e270
This commit is contained in:
Dhruv Manilawala 2025-07-11 19:57:27 +05:30 committed by GitHub
parent 39c6364545
commit fd69533fe5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 243 additions and 144 deletions

View file

@ -10,11 +10,10 @@ use ruff_db::files::FileRange;
use ruff_db::source::{line_index, source_text};
use ty_project::{Db, ProjectDatabase};
use super::LSPResult;
use crate::document::{DocumentKey, FileRangeExt, ToRangeExt};
use crate::server::Result;
use crate::session::DocumentSnapshot;
use crate::session::client::Client;
use crate::{DocumentSnapshot, PositionEncoding, Session};
use crate::{PositionEncoding, Session};
/// Represents the diagnostics for a text document or a notebook document.
pub(super) enum Diagnostics {
@ -64,30 +63,29 @@ pub(super) fn clear_diagnostics(key: &DocumentKey, client: &Client) {
/// This function is a no-op if the client supports pull diagnostics.
///
/// [publish diagnostics notification]: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_publishDiagnostics
pub(super) fn publish_diagnostics(
session: &Session,
key: &DocumentKey,
client: &Client,
) -> Result<()> {
pub(super) fn publish_diagnostics(session: &Session, key: &DocumentKey, client: &Client) {
if session.client_capabilities().pull_diagnostics {
return Ok(());
return;
}
let Some(url) = key.to_url() else {
return Ok(());
return;
};
let path = key.path();
let snapshot = session.take_document_snapshot(url.clone());
let snapshot = session
.take_document_snapshot(url.clone())
.ok_or_else(|| anyhow::anyhow!("Unable to take snapshot for document with URL {url}"))
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
let document = match snapshot.document() {
Ok(document) => document,
Err(err) => {
tracing::debug!("Failed to resolve document for URL `{}`: {}", url, err);
return;
}
};
let db = session.project_db_or_default(path);
let db = session.project_db_or_default(key.path());
let Some(diagnostics) = compute_diagnostics(db, &snapshot) else {
return Ok(());
return;
};
// Sends a notification to the client with the diagnostics for the document.
@ -95,7 +93,7 @@ pub(super) fn publish_diagnostics(
client.send_notification::<PublishDiagnostics>(PublishDiagnosticsParams {
uri,
diagnostics,
version: Some(snapshot.query().version()),
version: Some(document.version()),
});
};
@ -109,25 +107,28 @@ pub(super) fn publish_diagnostics(
}
}
}
Ok(())
}
pub(super) fn compute_diagnostics(
db: &ProjectDatabase,
snapshot: &DocumentSnapshot,
) -> Option<Diagnostics> {
let Some(file) = snapshot.file(db) else {
tracing::info!(
"No file found for snapshot for `{}`",
snapshot.query().file_url()
);
let document = match snapshot.document() {
Ok(document) => document,
Err(err) => {
tracing::info!("Failed to resolve document for snapshot: {}", err);
return None;
}
};
let Some(file) = document.file(db) else {
tracing::info!("No file found for snapshot for `{}`", document.file_path());
return None;
};
let diagnostics = db.check_file(file);
if let Some(notebook) = snapshot.query().as_notebook() {
if let Some(notebook) = document.as_notebook() {
let mut cell_diagnostics: FxHashMap<Url, Vec<Diagnostic>> = FxHashMap::default();
// Populates all relevant URLs with an empty diagnostic list. This ensures that documents