Skip to content

Commit

Permalink
ruff server: Support publish diagnostics as a fallback when pull di…
Browse files Browse the repository at this point in the history
…agnostics aren't supported (#11092)

## Summary

Fixes #11059 

Several major editors don't support [pull
diagnostics](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_pullDiagnostics),
a method of sending diagnostics to the client that was introduced in
version `0.3.17` of the specification. Until now, `ruff server` has only
used pull diagnostics, which resulted in diagnostics not being available
on Neovim and Helix, which don't support pull diagnostics yet (though
Neovim `10.0` will have support for this).

`ruff server` will now utilize the older method of sending diagnostics,
known as 'publish diagnostics', when pull diagnostics aren't supported
by the client. This involves re-linting a document every time it is
opened or modified, and then sending the diagnostics generated from that
lint to the client via the `textDocument/publishDiagnostics`
notification.

## Test Plan

The easiest way to test that this PR works is to check if diagnostics
show up on Neovim `<=0.9`.
  • Loading branch information
snowsignal committed Apr 23, 2024
1 parent 111bbc6 commit 62478c3
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 13 deletions.
1 change: 1 addition & 0 deletions crates/ruff_server/src/server/api.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{server::schedule::Task, session::Session};
use lsp_server as server;

mod diagnostics;
mod notifications;
mod requests;
mod traits;
Expand Down
34 changes: 34 additions & 0 deletions crates/ruff_server/src/server/api/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use crate::{server::client::Notifier, session::DocumentSnapshot};

use super::LSPResult;

pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> Vec<lsp_types::Diagnostic> {
if snapshot.client_settings().lint() {
crate::lint::check(
snapshot.document(),
&snapshot.settings().linter,
snapshot.encoding(),
)
} else {
vec![]
}
}

pub(super) fn publish_diagnostics_for_document(
snapshot: &DocumentSnapshot,
notifier: &Notifier,
) -> crate::server::Result<()> {
let diagnostics = generate_diagnostics(snapshot);

notifier
.notify::<lsp_types::notification::PublishDiagnostics>(
lsp_types::PublishDiagnosticsParams {
uri: snapshot.url().clone(),
diagnostics,
version: Some(snapshot.document().version()),
},
)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;

Ok(())
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::server::api::diagnostics::publish_diagnostics_for_document;
use crate::server::api::LSPResult;
use crate::server::client::{Notifier, Requester};
use crate::server::Result;
Expand All @@ -15,7 +16,7 @@ impl super::SyncNotificationHandler for DidChange {
#[tracing::instrument(skip_all, fields(file=%uri))]
fn run(
session: &mut Session,
_notifier: Notifier,
notifier: Notifier,
_requester: &mut Requester,
types::DidChangeTextDocumentParams {
text_document:
Expand All @@ -40,6 +41,12 @@ impl super::SyncNotificationHandler for DidChange {
.make_mut()
.apply_changes(content_changes, new_version, encoding);

// Publish diagnostics if the client doesnt support pull diagnostics
if !session.resolved_client_capabilities().pull_diagnostics {
let snapshot = session.take_snapshot(&uri).unwrap();
publish_diagnostics_for_document(&snapshot, &notifier)?;
}

Ok(())
}
}
16 changes: 15 additions & 1 deletion crates/ruff_server/src/server/api/notifications/did_open.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::server::api::diagnostics::publish_diagnostics_for_document;
use crate::server::api::LSPResult;
use crate::server::client::{Notifier, Requester};
use crate::server::Result;
use crate::session::Session;
Expand All @@ -14,7 +16,7 @@ impl super::SyncNotificationHandler for DidOpen {
#[tracing::instrument(skip_all, fields(file=%url))]
fn run(
session: &mut Session,
_notifier: Notifier,
notifier: Notifier,
_requester: &mut Requester,
types::DidOpenTextDocumentParams {
text_document:
Expand All @@ -27,6 +29,18 @@ impl super::SyncNotificationHandler for DidOpen {
}: types::DidOpenTextDocumentParams,
) -> Result<()> {
session.open_document(url, text, version);

// Publish diagnostics if the client doesnt support pull diagnostics
if !session.resolved_client_capabilities().pull_diagnostics {
let snapshot = session
.take_snapshot(url)
.ok_or_else(|| {
anyhow::anyhow!("Unable to take snapshot for document with URL {url}")
})
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
publish_diagnostics_for_document(&snapshot, &notifier)?;
}

Ok(())
}
}
13 changes: 2 additions & 11 deletions crates/ruff_server/src/server/api/requests/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::server::api::diagnostics::generate_diagnostics;
use crate::server::{client::Notifier, Result};
use crate::session::DocumentSnapshot;
use lsp_types::{self as types, request as req};
Expand All @@ -19,23 +20,13 @@ impl super::BackgroundDocumentRequestHandler for DocumentDiagnostic {
_notifier: Notifier,
_params: types::DocumentDiagnosticParams,
) -> Result<DocumentDiagnosticReportResult> {
let diagnostics = if snapshot.client_settings().lint() {
crate::lint::check(
snapshot.document(),
&snapshot.settings().linter,
snapshot.encoding(),
)
} else {
vec![]
};

Ok(DocumentDiagnosticReportResult::Report(
types::DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport {
related_documents: None,
full_document_diagnostic_report: FullDocumentDiagnosticReport {
// TODO(jane): eventually this will be important for caching diagnostic information.
result_id: None,
items: diagnostics,
items: generate_diagnostics(&snapshot),
},
}),
))
Expand Down
8 changes: 8 additions & 0 deletions crates/ruff_server/src/session/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub(crate) struct ResolvedClientCapabilities {
pub(crate) apply_edit: bool,
pub(crate) document_changes: bool,
pub(crate) workspace_refresh: bool,
pub(crate) pull_diagnostics: bool,
}

impl ResolvedClientCapabilities {
Expand Down Expand Up @@ -48,12 +49,19 @@ impl ResolvedClientCapabilities {
.unwrap_or_default();
*/

let pull_diagnostics = client_capabilities
.text_document
.as_ref()
.and_then(|text_document| text_document.diagnostic.as_ref())
.is_some();

Self {
code_action_deferred_edit_resolution: code_action_data_support
&& code_action_edit_resolution,
apply_edit,
document_changes,
workspace_refresh,
pull_diagnostics,
}
}
}

0 comments on commit 62478c3

Please sign in to comment.