Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions crates/ty_server/src/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ bitflags::bitflags! {
const WORKSPACE_CONFIGURATION = 1 << 15;
const RENAME_DYNAMIC_REGISTRATION = 1 << 16;
const COMPLETION_ITEM_LABEL_DETAILS_SUPPORT = 1 << 17;
const DIAGNOSTIC_RELATED_INFORMATION = 1 << 18;
}
}

Expand Down Expand Up @@ -163,6 +164,11 @@ impl ResolvedClientCapabilities {
self.contains(Self::DIAGNOSTIC_DYNAMIC_REGISTRATION)
}

/// Returns `true` if the client has related information support for diagnostics.
pub(crate) const fn supports_diagnostic_related_information(self) -> bool {
self.contains(Self::DIAGNOSTIC_RELATED_INFORMATION)
}

/// Returns `true` if the client supports dynamic registration for rename capabilities.
pub(crate) const fn supports_rename_dynamic_registration(self) -> bool {
self.contains(Self::RENAME_DYNAMIC_REGISTRATION)
Expand Down Expand Up @@ -211,15 +217,22 @@ impl ResolvedClientCapabilities {
}
}

if text_document.is_some_and(|text_document| text_document.diagnostic.is_some()) {
if let Some(diagnostic) =
text_document.and_then(|text_document| text_document.diagnostic.as_ref())
{
flags |= Self::PULL_DIAGNOSTICS;

if diagnostic.dynamic_registration == Some(true) {
flags |= Self::DIAGNOSTIC_DYNAMIC_REGISTRATION;
}
}

if text_document
.and_then(|text_document| text_document.diagnostic.as_ref()?.dynamic_registration)
.unwrap_or_default()
if let Some(publish_diagnostics) =
text_document.and_then(|text_document| text_document.publish_diagnostics.as_ref())
{
flags |= Self::DIAGNOSTIC_DYNAMIC_REGISTRATION;
if publish_diagnostics.related_information == Some(true) {
flags |= Self::DIAGNOSTIC_RELATED_INFORMATION;
}
}

if text_document
Expand Down
94 changes: 63 additions & 31 deletions crates/ty_server/src/server/api/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use ruff_db::system::SystemPathBuf;
use serde::{Deserialize, Serialize};
use ty_project::{Db as _, ProjectDatabase};

use crate::capabilities::ResolvedClientCapabilities;
use crate::document::{FileRangeExt, ToRangeExt};
use crate::session::DocumentHandle;
use crate::session::client::Client;
Expand Down Expand Up @@ -56,7 +57,11 @@ impl Diagnostics {
Self::result_id_from_hash(&self.items)
}

pub(super) fn to_lsp_diagnostics(&self, db: &ProjectDatabase) -> LspDiagnostics {
pub(super) fn to_lsp_diagnostics(
&self,
db: &ProjectDatabase,
client_capabilities: ResolvedClientCapabilities,
) -> LspDiagnostics {
if let Some(notebook_document) = db.notebook_document(self.file_or_notebook) {
let mut cell_diagnostics: FxHashMap<Url, Vec<Diagnostic>> = FxHashMap::default();

Expand All @@ -67,7 +72,8 @@ impl Diagnostics {
}

for diagnostic in &self.items {
let (url, lsp_diagnostic) = to_lsp_diagnostic(db, diagnostic, self.encoding);
let (url, lsp_diagnostic) =
to_lsp_diagnostic(db, diagnostic, self.encoding, client_capabilities);

let Some(url) = url else {
tracing::warn!("Unable to find notebook cell");
Expand All @@ -85,7 +91,9 @@ impl Diagnostics {
LspDiagnostics::TextDocument(
self.items
.iter()
.map(|diagnostic| to_lsp_diagnostic(db, diagnostic, self.encoding).1)
.map(|diagnostic| {
to_lsp_diagnostic(db, diagnostic, self.encoding, client_capabilities).1
})
.collect(),
)
}
Expand Down Expand Up @@ -181,7 +189,7 @@ pub(super) fn publish_diagnostics(document: &DocumentHandle, session: &Session,
});
};

match diagnostics.to_lsp_diagnostics(db) {
match diagnostics.to_lsp_diagnostics(db, session.client_capabilities()) {
LspDiagnostics::TextDocument(diagnostics) => {
publish_diagnostics_notification(document.url().clone(), diagnostics);
}
Expand Down Expand Up @@ -212,6 +220,7 @@ pub(crate) fn publish_settings_diagnostics(
}

let session_encoding = session.position_encoding();
let client_capabilities = session.client_capabilities();
let state = session.project_state_mut(&AnySystemPath::System(path));
let db = &state.db;
let project = db.project();
Expand Down Expand Up @@ -253,7 +262,9 @@ pub(crate) fn publish_settings_diagnostics(
// Convert diagnostics to LSP format
let lsp_diagnostics = file_diagnostics
.into_iter()
.map(|diagnostic| to_lsp_diagnostic(db, &diagnostic, session_encoding).1)
.map(|diagnostic| {
to_lsp_diagnostic(db, &diagnostic, session_encoding, client_capabilities).1
})
.collect::<Vec<_>>();

client.send_notification::<PublishDiagnostics>(PublishDiagnosticsParams {
Expand Down Expand Up @@ -292,7 +303,11 @@ pub(super) fn to_lsp_diagnostic(
db: &dyn Db,
diagnostic: &ruff_db::diagnostic::Diagnostic,
encoding: PositionEncoding,
client_capabilities: ResolvedClientCapabilities,
) -> (Option<lsp_types::Url>, Diagnostic) {
let supports_related_information =
client_capabilities.supports_diagnostic_related_information();

let location = diagnostic.primary_span().and_then(|span| {
let file = span.expect_ty_file();
span.range()?
Expand Down Expand Up @@ -330,31 +345,35 @@ pub(super) fn to_lsp_diagnostic(
Some(CodeDescription { href })
});

let mut related_information = Vec::new();

related_information.extend(
diagnostic
.secondary_annotations()
.filter_map(|annotation| annotation_to_related_information(db, annotation, encoding)),
);
let related_information =
if supports_related_information {
let mut related_information = Vec::new();
related_information.extend(diagnostic.secondary_annotations().filter_map(
|annotation| annotation_to_related_information(db, annotation, encoding),
));

for sub_diagnostic in diagnostic.sub_diagnostics() {
related_information.extend(sub_diagnostic_to_related_information(
db,
sub_diagnostic,
encoding,
));

related_information.extend(
sub_diagnostic
.annotations()
.iter()
.filter(|annotation| !annotation.is_primary())
.filter_map(|annotation| {
annotation_to_related_information(db, annotation, encoding)
}),
);
}

for sub_diagnostic in diagnostic.sub_diagnostics() {
related_information.extend(sub_diagnostic_to_related_information(
db,
sub_diagnostic,
encoding,
));

related_information.extend(
sub_diagnostic
.annotations()
.iter()
.filter(|annotation| !annotation.is_primary())
.filter_map(|annotation| {
annotation_to_related_information(db, annotation, encoding)
}),
);
}
Some(related_information)
} else {
None
};

let data = DiagnosticData::try_from_diagnostic(db, diagnostic, encoding);

Expand All @@ -367,8 +386,21 @@ pub(super) fn to_lsp_diagnostic(
code: Some(NumberOrString::String(diagnostic.id().to_string())),
code_description,
source: Some(DIAGNOSTIC_NAME.into()),
message: diagnostic.concise_message().to_string(),
related_information: Some(related_information),
message: if supports_related_information {
// Show both the primary and annotation messages if available,
// because we don't create a related information for the primary message.
if let Some(annotation_message) = diagnostic
.primary_annotation()
.and_then(|annotation| annotation.get_message())
{
format!("{}: {annotation_message}", diagnostic.primary_message())
} else {
diagnostic.primary_message().to_string()
}
} else {
diagnostic.concise_message().to_string()
},
related_information,
data: serde_json::to_value(data).ok(),
},
)
Expand Down
4 changes: 3 additions & 1 deletion crates/ty_server/src/server/api/requests/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ impl BackgroundDocumentRequestHandler for DocumentDiagnosticRequestHandler {
result_id: new_id,
// SAFETY: Pull diagnostic requests are only called for text documents, not for
// notebook documents.
items: diagnostics.to_lsp_diagnostics(db).expect_text_document(),
items: diagnostics
.to_lsp_diagnostics(db, snapshot.resolved_client_capabilities())
.expect_text_document(),
},
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use serde::{Deserialize, Serialize};
use ty_project::{ProgressReporter, ProjectDatabase};

use crate::PositionEncoding;
use crate::capabilities::ResolvedClientCapabilities;
use crate::document::DocumentKey;
use crate::server::api::diagnostics::{Diagnostics, to_lsp_diagnostic};
use crate::server::api::traits::{
Expand Down Expand Up @@ -318,6 +319,7 @@ struct ResponseWriter<'a> {
mode: ReportingMode,
index: &'a Index,
position_encoding: PositionEncoding,
client_capabilities: ResolvedClientCapabilities,
// It's important that we use `AnySystemPath` over `Url` here because
// `file_to_url` isn't guaranteed to return the exact same URL as the one provided
// by the client.
Expand Down Expand Up @@ -357,6 +359,7 @@ impl<'a> ResponseWriter<'a> {
mode,
index,
position_encoding,
client_capabilities: snapshot.resolved_client_capabilities(),
previous_result_ids,
}
}
Expand Down Expand Up @@ -406,7 +409,15 @@ impl<'a> ResponseWriter<'a> {
new_id => {
let lsp_diagnostics = diagnostics
.iter()
.map(|diagnostic| to_lsp_diagnostic(db, diagnostic, self.position_encoding).1)
.map(|diagnostic| {
to_lsp_diagnostic(
db,
diagnostic,
self.position_encoding,
self.client_capabilities,
)
.1
})
.collect::<Vec<_>>();

WorkspaceDocumentDiagnosticReport::Full(WorkspaceFullDocumentDiagnosticReport {
Expand Down
10 changes: 10 additions & 0 deletions crates/ty_server/tests/e2e/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,16 @@ impl TestServerBuilder {
self
}

pub(crate) fn enable_diagnostic_related_information(mut self, enabled: bool) -> Self {
self.client_capabilities
.text_document
.get_or_insert_default()
.publish_diagnostics
.get_or_insert_default()
.related_information = Some(enabled);
self
}

/// Set custom client capabilities (overrides any previously set capabilities)
#[expect(dead_code)]
pub(crate) fn with_client_capabilities(mut self, capabilities: ClientCapabilities) -> Self {
Expand Down
4 changes: 2 additions & 2 deletions crates/ty_server/tests/e2e/notebook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ static FILTERS: &[(&str, &str)] = &[(r#""sortText": "[0-9 ]+""#, r#""sortText":
#[test]
fn publish_diagnostics_open() -> anyhow::Result<()> {
let mut server = TestServerBuilder::new()?
.enable_diagnostic_related_information(true)
.build()
.wait_until_workspaces_are_initialized();

Expand Down Expand Up @@ -219,8 +220,7 @@ fn swap_cells() -> anyhow::Result<()> {
"href": "https://ty.dev/rules#unresolved-reference"
},
"source": "ty",
"message": "Name `a` used when not defined",
"relatedInformation": []
"message": "Name `a` used when not defined"
}
],
"vscode-notebook-cell://src/test.ipynb#1": [],
Expand Down
51 changes: 51 additions & 0 deletions crates/ty_server/tests/e2e/publish_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,57 @@ def foo() -> str:
Ok(())
}

#[test]
fn message_without_related_information_support() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = r#"
from typing import assert_type

assert_type("test", list[str])
"#;

let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(foo, foo_content)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();

server.open_text_document(foo, foo_content, 1);
let diagnostics = server.await_notification::<PublishDiagnostics>();

insta::assert_debug_snapshot!(diagnostics);

Ok(())
}

#[test]
fn message_with_related_information_support() -> Result<()> {
let workspace_root = SystemPath::new("src");
let foo = SystemPath::new("src/foo.py");
let foo_content = r#"
from typing import assert_type

assert_type("test", list[str])
"#;

let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(foo, foo_content)?
.enable_diagnostic_related_information(true)
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();

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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ expression: code_actions
},
"source": "ty",
"message": "Unused `ty: ignore` directive",
"relatedInformation": [],
"tags": [
1
]
Expand Down Expand Up @@ -74,7 +73,6 @@ expression: code_actions
},
"source": "ty",
"message": "Unused `ty: ignore` directive",
"relatedInformation": [],
"tags": [
1
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ expression: code_actions
"href": "https://ty.dev/rules#unresolved-reference"
},
"source": "ty",
"message": "Name `typing` used when not defined",
"relatedInformation": []
"message": "Name `typing` used when not defined"
}
],
"edit": {
Expand Down Expand Up @@ -70,8 +69,7 @@ expression: code_actions
"href": "https://ty.dev/rules#unresolved-reference"
},
"source": "ty",
"message": "Name `typing` used when not defined",
"relatedInformation": []
"message": "Name `typing` used when not defined"
}
],
"edit": {
Expand Down
Loading
Loading