Skip to content

Commit

Permalink
ruff server: In 'publish diagnostics' mode, document diagnostics ar…
Browse files Browse the repository at this point in the history
…e cleared properly when a file is closed (#11137)

## Summary

Fixes #11114. 

As part of the `onClose` handler, we publish an empty array of
diagnostics for the document being closed, similar to
[`ruff-lsp`](https://github.com/astral-sh/ruff-lsp/blob/187d7790be0783b9ac41ce025a724cf389bf575c/ruff_lsp/server.py#L459-L464).
This prevent phantom diagnostics from lingering after a document is
closed. We'll only do this if the client doesn't support pull
diagnostics, because otherwise clearing diagnostics is their
responsibility.

## Test Plan

Diagnostics should no longer appear for a document in the Problems tab
after the document is closed.
  • Loading branch information
snowsignal authored Apr 25, 2024
1 parent 19baabb commit 4690890
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
17 changes: 17 additions & 0 deletions crates/ruff_server/src/server/api/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,20 @@ pub(super) fn publish_diagnostics_for_document(

Ok(())
}

pub(super) fn clear_diagnostics_for_document(
snapshot: &DocumentSnapshot,
notifier: &Notifier,
) -> crate::server::Result<()> {
notifier
.notify::<lsp_types::notification::PublishDiagnostics>(
lsp_types::PublishDiagnosticsParams {
uri: snapshot.url().clone(),
diagnostics: vec![],
version: Some(snapshot.document().version()),
},
)
.with_failure_code(lsp_server::ErrorCode::InternalError)?;

Ok(())
}
15 changes: 14 additions & 1 deletion crates/ruff_server/src/server/api/notifications/did_close.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::server::api::diagnostics::clear_diagnostics_for_document;
use crate::server::api::LSPResult;
use crate::server::client::{Notifier, Requester};
use crate::server::Result;
Expand All @@ -15,12 +16,24 @@ impl super::SyncNotificationHandler for DidClose {
#[tracing::instrument(skip_all, fields(file=%uri))]
fn run(
session: &mut Session,
_notifier: Notifier,
notifier: Notifier,
_requester: &mut Requester,
types::DidCloseTextDocumentParams {
text_document: types::TextDocumentIdentifier { uri },
}: types::DidCloseTextDocumentParams,
) -> Result<()> {
// Publish an empty diagnostic report for the document if the client does not support pull diagnostics.
// This will de-register any existing diagnostics.
if !session.resolved_client_capabilities().pull_diagnostics {
let snapshot = session
.take_snapshot(&uri)
.ok_or_else(|| {
anyhow::anyhow!("Unable to take snapshot for document with URL {uri}")
})
.with_failure_code(lsp_server::ErrorCode::InternalError)?;
clear_diagnostics_for_document(&snapshot, &notifier)?;
}

session
.close_document(&uri)
.with_failure_code(lsp_server::ErrorCode::InternalError)
Expand Down

0 comments on commit 4690890

Please sign in to comment.