[ty] Implement diagnostic caching (#19605)

This commit is contained in:
Micha Reiser 2025-07-30 12:04:34 +02:00 committed by GitHub
parent 4ecf1d205a
commit 2a5ace6e55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 1322 additions and 100 deletions

View file

@ -6,7 +6,7 @@ use ruff_db::system::{OsSystem, SystemPathBuf};
pub use crate::logging::{LogLevel, init_logging};
pub use crate::server::Server;
pub use crate::session::ClientOptions;
pub use crate::session::{ClientOptions, DiagnosticMode};
pub use document::{NotebookDocument, PositionEncoding, TextDocument};
pub(crate) use session::{DocumentQuery, Session};

View file

@ -98,7 +98,10 @@ impl<S> tracing_subscriber::layer::Filter<S> for LogLevelFilter {
meta: &tracing::Metadata<'_>,
_: &tracing_subscriber::layer::Context<'_, S>,
) -> bool {
let filter = if meta.target().starts_with("ty") || meta.target().starts_with("ruff") {
let filter = if meta.target().starts_with("ty")
|| meta.target().starts_with("ruff")
|| meta.target().starts_with("e2e")
{
self.filter.trace_level()
} else {
tracing::Level::WARN

View file

@ -1,3 +1,5 @@
use std::hash::{DefaultHasher, Hash as _, Hasher as _};
use lsp_types::notification::PublishDiagnostics;
use lsp_types::{
CodeDescription, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag,
@ -15,17 +17,77 @@ use crate::document::{DocumentKey, FileRangeExt, ToRangeExt};
use crate::session::DocumentSnapshot;
use crate::session::client::Client;
use crate::system::{AnySystemPath, file_to_url};
use crate::{PositionEncoding, Session};
use crate::{DocumentQuery, PositionEncoding, Session};
pub(super) struct Diagnostics<'a> {
items: Vec<ruff_db::diagnostic::Diagnostic>,
encoding: PositionEncoding,
document: &'a DocumentQuery,
}
impl Diagnostics<'_> {
pub(super) fn result_id_from_hash(diagnostics: &[ruff_db::diagnostic::Diagnostic]) -> String {
// Generate result ID based on raw diagnostic content only
let mut hasher = DefaultHasher::new();
// Hash the length first to ensure different numbers of diagnostics produce different hashes
diagnostics.hash(&mut hasher);
format!("{:x}", hasher.finish())
}
pub(super) fn result_id(&self) -> String {
Self::result_id_from_hash(&self.items)
}
pub(super) fn to_lsp_diagnostics(&self, db: &ProjectDatabase) -> LspDiagnostics {
if let Some(notebook) = self.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
// without diagnostics still get updated.
for cell_url in notebook.cell_urls() {
cell_diagnostics.entry(cell_url.clone()).or_default();
}
for (cell_index, diagnostic) in self.items.iter().map(|diagnostic| {
(
// TODO: Use the cell index instead using `SourceKind`
usize::default(),
to_lsp_diagnostic(db, diagnostic, self.encoding),
)
}) {
let Some(cell_uri) = notebook.cell_uri_by_index(cell_index) else {
tracing::warn!("Unable to find notebook cell at index {cell_index}");
continue;
};
cell_diagnostics
.entry(cell_uri.clone())
.or_default()
.push(diagnostic);
}
LspDiagnostics::NotebookDocument(cell_diagnostics)
} else {
LspDiagnostics::TextDocument(
self.items
.iter()
.map(|diagnostic| to_lsp_diagnostic(db, diagnostic, self.encoding))
.collect(),
)
}
}
}
/// Represents the diagnostics for a text document or a notebook document.
pub(super) enum Diagnostics {
pub(super) enum LspDiagnostics {
TextDocument(Vec<Diagnostic>),
/// A map of cell URLs to the diagnostics for that cell.
NotebookDocument(FxHashMap<Url, Vec<Diagnostic>>),
}
impl Diagnostics {
impl LspDiagnostics {
/// Returns the diagnostics for a text document.
///
/// # Panics
@ -33,8 +95,8 @@ impl Diagnostics {
/// Panics if the diagnostics are for a notebook document.
pub(super) fn expect_text_document(self) -> Vec<Diagnostic> {
match self {
Diagnostics::TextDocument(diagnostics) => diagnostics,
Diagnostics::NotebookDocument(_) => {
LspDiagnostics::TextDocument(diagnostics) => diagnostics,
LspDiagnostics::NotebookDocument(_) => {
panic!("Expected a text document diagnostics, but got notebook diagnostics")
}
}
@ -99,11 +161,11 @@ pub(super) fn publish_diagnostics(session: &Session, key: &DocumentKey, client:
});
};
match diagnostics {
Diagnostics::TextDocument(diagnostics) => {
match diagnostics.to_lsp_diagnostics(db) {
LspDiagnostics::TextDocument(diagnostics) => {
publish_diagnostics_notification(url, diagnostics);
}
Diagnostics::NotebookDocument(cell_diagnostics) => {
LspDiagnostics::NotebookDocument(cell_diagnostics) => {
for (cell_url, diagnostics) in cell_diagnostics {
publish_diagnostics_notification(cell_url, diagnostics);
}
@ -187,10 +249,10 @@ pub(crate) fn publish_settings_diagnostics(
}
}
pub(super) fn compute_diagnostics(
pub(super) fn compute_diagnostics<'a>(
db: &ProjectDatabase,
snapshot: &DocumentSnapshot,
) -> Option<Diagnostics> {
snapshot: &'a DocumentSnapshot,
) -> Option<Diagnostics<'a>> {
let document = match snapshot.document() {
Ok(document) => document,
Err(err) => {
@ -206,41 +268,11 @@ pub(super) fn compute_diagnostics(
let diagnostics = db.check_file(file);
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
// without diagnostics still get updated.
for cell_url in notebook.cell_urls() {
cell_diagnostics.entry(cell_url.clone()).or_default();
}
for (cell_index, diagnostic) in diagnostics.iter().map(|diagnostic| {
(
// TODO: Use the cell index instead using `SourceKind`
usize::default(),
to_lsp_diagnostic(db, diagnostic, snapshot.encoding()),
)
}) {
let Some(cell_uri) = notebook.cell_uri_by_index(cell_index) else {
tracing::warn!("Unable to find notebook cell at index {cell_index}");
continue;
};
cell_diagnostics
.entry(cell_uri.clone())
.or_default()
.push(diagnostic);
}
Some(Diagnostics::NotebookDocument(cell_diagnostics))
} else {
Some(Diagnostics::TextDocument(
diagnostics
.iter()
.map(|diagnostic| to_lsp_diagnostic(db, diagnostic, snapshot.encoding()))
.collect(),
))
}
Some(Diagnostics {
items: diagnostics,
encoding: snapshot.encoding(),
document,
})
}
/// Converts the tool specific [`Diagnostic`][ruff_db::diagnostic::Diagnostic] to an LSP

View file

@ -3,11 +3,12 @@ use std::borrow::Cow;
use lsp_types::request::DocumentDiagnosticRequest;
use lsp_types::{
DocumentDiagnosticParams, DocumentDiagnosticReport, DocumentDiagnosticReportResult,
FullDocumentDiagnosticReport, RelatedFullDocumentDiagnosticReport, Url,
FullDocumentDiagnosticReport, RelatedFullDocumentDiagnosticReport,
RelatedUnchangedDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport, Url,
};
use crate::server::Result;
use crate::server::api::diagnostics::{Diagnostics, compute_diagnostics};
use crate::server::api::diagnostics::compute_diagnostics;
use crate::server::api::traits::{
BackgroundDocumentRequestHandler, RequestHandler, RetriableRequestHandler,
};
@ -30,20 +31,38 @@ impl BackgroundDocumentRequestHandler for DocumentDiagnosticRequestHandler {
db: &ProjectDatabase,
snapshot: DocumentSnapshot,
_client: &Client,
_params: DocumentDiagnosticParams,
params: DocumentDiagnosticParams,
) -> Result<DocumentDiagnosticReportResult> {
Ok(DocumentDiagnosticReportResult::Report(
let diagnostics = compute_diagnostics(db, &snapshot);
let Some(diagnostics) = diagnostics else {
return Ok(DocumentDiagnosticReportResult::Report(
DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport::default()),
));
};
let result_id = diagnostics.result_id();
let report = if params.previous_result_id.as_deref() == Some(&result_id) {
DocumentDiagnosticReport::Unchanged(RelatedUnchangedDocumentDiagnosticReport {
related_documents: None,
unchanged_document_diagnostic_report: UnchangedDocumentDiagnosticReport {
result_id,
},
})
} else {
DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport {
related_documents: None,
full_document_diagnostic_report: FullDocumentDiagnosticReport {
result_id: None,
result_id: Some(result_id),
// SAFETY: Pull diagnostic requests are only called for text documents, not for
// notebook documents.
items: compute_diagnostics(db, &snapshot)
.map_or_else(Vec::new, Diagnostics::expect_text_document),
items: diagnostics.to_lsp_diagnostics(db).expect_text_document(),
},
}),
))
})
};
Ok(DocumentDiagnosticReportResult::Report(report))
}
}

View file

@ -1,19 +1,20 @@
use lsp_types::request::WorkspaceDiagnosticRequest;
use lsp_types::{
FullDocumentDiagnosticReport, Url, WorkspaceDiagnosticParams, WorkspaceDiagnosticReport,
WorkspaceDiagnosticReportResult, WorkspaceDocumentDiagnosticReport,
WorkspaceFullDocumentDiagnosticReport,
};
use rustc_hash::FxHashMap;
use std::collections::BTreeMap;
use crate::server::Result;
use crate::server::api::diagnostics::to_lsp_diagnostic;
use crate::server::api::diagnostics::{Diagnostics, to_lsp_diagnostic};
use crate::server::api::traits::{
BackgroundRequestHandler, RequestHandler, RetriableRequestHandler,
};
use crate::session::SessionSnapshot;
use crate::session::client::Client;
use crate::system::file_to_url;
use lsp_types::request::WorkspaceDiagnosticRequest;
use lsp_types::{
FullDocumentDiagnosticReport, UnchangedDocumentDiagnosticReport, Url,
WorkspaceDiagnosticParams, WorkspaceDiagnosticReport, WorkspaceDiagnosticReportResult,
WorkspaceDocumentDiagnosticReport, WorkspaceFullDocumentDiagnosticReport,
WorkspaceUnchangedDocumentDiagnosticReport,
};
pub(crate) struct WorkspaceDiagnosticRequestHandler;
@ -25,26 +26,31 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler {
fn run(
snapshot: SessionSnapshot,
_client: &Client,
_params: WorkspaceDiagnosticParams,
params: WorkspaceDiagnosticParams,
) -> Result<WorkspaceDiagnosticReportResult> {
let index = snapshot.index();
if !index.global_settings().diagnostic_mode().is_workspace() {
// VS Code sends us the workspace diagnostic request every 2 seconds, so these logs can
// be quite verbose.
tracing::trace!("Workspace diagnostics is disabled; returning empty report");
tracing::debug!("Workspace diagnostics is disabled; returning empty report");
return Ok(WorkspaceDiagnosticReportResult::Report(
WorkspaceDiagnosticReport { items: vec![] },
));
}
// Create a map of previous result IDs for efficient lookup
let mut previous_results: BTreeMap<_, _> = params
.previous_result_ids
.into_iter()
.map(|prev| (prev.uri, prev.value))
.collect();
let mut items = Vec::new();
for db in snapshot.projects() {
let diagnostics = db.check();
// Group diagnostics by URL
let mut diagnostics_by_url: FxHashMap<Url, Vec<_>> = FxHashMap::default();
let mut diagnostics_by_url: BTreeMap<Url, Vec<_>> = BTreeMap::default();
for diagnostic in diagnostics {
if let Some(span) = diagnostic.primary_span() {
@ -66,6 +72,23 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler {
.ok()
.and_then(|key| index.make_document_ref(key).ok())
.map(|doc| i64::from(doc.version()));
let result_id = Diagnostics::result_id_from_hash(&file_diagnostics);
// Check if this file's diagnostics have changed since the previous request
if let Some(previous_result_id) = previous_results.remove(&url) {
if previous_result_id == result_id {
// Diagnostics haven't changed, return unchanged report
items.push(WorkspaceDocumentDiagnosticReport::Unchanged(
WorkspaceUnchangedDocumentDiagnosticReport {
uri: url,
version,
unchanged_document_diagnostic_report:
UnchangedDocumentDiagnosticReport { result_id },
},
));
continue;
}
}
// Convert diagnostics to LSP format
let lsp_diagnostics = file_diagnostics
@ -75,13 +98,13 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler {
})
.collect::<Vec<_>>();
// Diagnostics have changed or this is the first request, return full report
items.push(WorkspaceDocumentDiagnosticReport::Full(
WorkspaceFullDocumentDiagnosticReport {
uri: url,
version,
full_document_diagnostic_report: FullDocumentDiagnosticReport {
// TODO: We don't implement result ID caching yet
result_id: None,
result_id: Some(result_id),
items: lsp_diagnostics,
},
},
@ -89,6 +112,28 @@ impl BackgroundRequestHandler for WorkspaceDiagnosticRequestHandler {
}
}
// Handle files that had diagnostics in previous request but no longer have any
// Any remaining entries in previous_results are files that were fixed
for (previous_url, _previous_result_id) in previous_results {
// This file had diagnostics before but doesn't now, so we need to report it as having no diagnostics
let version = index
.key_from_url(previous_url.clone())
.ok()
.and_then(|key| index.make_document_ref(key).ok())
.map(|doc| i64::from(doc.version()));
items.push(WorkspaceDocumentDiagnosticReport::Full(
WorkspaceFullDocumentDiagnosticReport {
uri: previous_url,
version,
full_document_diagnostic_report: FullDocumentDiagnosticReport {
result_id: None, // No result ID needed for empty diagnostics
items: vec![], // No diagnostics
},
},
));
}
Ok(WorkspaceDiagnosticReportResult::Report(
WorkspaceDiagnosticReport { items },
))

View file

@ -21,8 +21,8 @@ use ty_project::{ChangeResult, Db as _, ProjectDatabase, ProjectMetadata};
pub(crate) use self::capabilities::ResolvedClientCapabilities;
pub(crate) use self::index::DocumentQuery;
pub use self::options::ClientOptions;
pub(crate) use self::options::{AllOptions, DiagnosticMode};
pub(crate) use self::options::AllOptions;
pub use self::options::{ClientOptions, DiagnosticMode};
pub(crate) use self::settings::ClientSettings;
use crate::document::{DocumentKey, DocumentVersion, NotebookDocument};
use crate::server::publish_settings_diagnostics;

View file

@ -65,7 +65,7 @@ pub struct ClientOptions {
#[derive(Clone, Copy, Debug, Default, Serialize, Deserialize)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
pub(crate) enum DiagnosticMode {
pub enum DiagnosticMode {
/// Check only currently open files.
#[default]
OpenFilesOnly,
@ -140,6 +140,13 @@ impl ClientOptions {
overrides,
}
}
/// Create a new `ClientOptions` with the specified diagnostic mode
#[must_use]
pub fn with_diagnostic_mode(mut self, mode: DiagnosticMode) -> Self {
self.diagnostic_mode = Some(mode);
self
}
}
// TODO(dhruvmanila): We need to mirror the "python.*" namespace on the server side but ideally it