Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support textDocument/diagnostic specification (Pull diagnostics) #11315

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ pub enum LanguageServerFeature {
WorkspaceSymbols,
// Symbols, use bitflags, see above?
Diagnostics,
PullDiagnostics,
RenameSymbol,
InlayHints,
}
Expand All @@ -355,6 +356,7 @@ impl Display for LanguageServerFeature {
DocumentSymbols => "document-symbols",
WorkspaceSymbols => "workspace-symbols",
Diagnostics => "diagnostics",
PullDiagnostics => "pull-diagnostics",
RenameSymbol => "rename-symbol",
InlayHints => "inlay-hints",
};
Expand Down
34 changes: 34 additions & 0 deletions helix-lsp/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ impl Client {
Some(OneOf::Left(true) | OneOf::Right(_))
),
LanguageServerFeature::Diagnostics => true, // there's no extra server capability
LanguageServerFeature::PullDiagnostics => capabilities.diagnostic_provider.is_some(),
LanguageServerFeature::RenameSymbol => matches!(
capabilities.rename_provider,
Some(OneOf::Left(true)) | Some(OneOf::Right(_))
Expand Down Expand Up @@ -570,6 +571,9 @@ impl Client {
did_rename: Some(true),
..Default::default()
}),
diagnostic: Some(lsp::DiagnosticWorkspaceClientCapabilities {
refresh_support: Some(true),
}),
..Default::default()
}),
text_document: Some(lsp::TextDocumentClientCapabilities {
Expand Down Expand Up @@ -647,6 +651,10 @@ impl Client {
}),
..Default::default()
}),
diagnostic: Some(lsp::DiagnosticClientCapabilities {
dynamic_registration: Some(false),
related_document_support: Some(true),
}),
publish_diagnostics: Some(lsp::PublishDiagnosticsClientCapabilities {
version_support: Some(true),
tag_support: Some(lsp::TagSupport {
Expand Down Expand Up @@ -1223,6 +1231,32 @@ impl Client {
})
}

pub fn text_document_diagnostic(
&self,
text_document: lsp::TextDocumentIdentifier,
previous_result_id: Option<String>,
) -> Option<impl Future<Output = Result<Value>>> {
let capabilities = self.capabilities();

// Return early if the server does not support pull diagnostic.
let identifier = match capabilities.diagnostic_provider.as_ref()? {
lsp::DiagnosticServerCapabilities::Options(cap) => cap.identifier.clone(),
lsp::DiagnosticServerCapabilities::RegistrationOptions(cap) => {
cap.diagnostic_options.identifier.clone()
}
};

let params = lsp::DocumentDiagnosticParams {
text_document,
identifier,
previous_result_id,
work_done_progress_params: lsp::WorkDoneProgressParams::default(),
partial_result_params: lsp::PartialResultParams::default(),
};

Some(self.call::<lsp::request::DocumentDiagnosticRequest>(params))
}

pub fn text_document_document_highlight(
&self,
text_document: lsp::TextDocumentIdentifier,
Expand Down
2 changes: 2 additions & 0 deletions helix-lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ pub enum MethodCall {
RegisterCapability(lsp::RegistrationParams),
UnregisterCapability(lsp::UnregistrationParams),
ShowDocument(lsp::ShowDocumentParams),
WorkspaceDiagnosticRefresh,
}

impl MethodCall {
Expand Down Expand Up @@ -494,6 +495,7 @@ impl MethodCall {
let params: lsp::ShowDocumentParams = params.parse()?;
Self::ShowDocument(params)
}
lsp::request::WorkspaceDiagnosticRefresh::METHOD => Self::WorkspaceDiagnosticRefresh,
_ => {
return Err(Error::Unhandled);
}
Expand Down
124 changes: 29 additions & 95 deletions helix-term/src/application.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use helix_view::{
align_view,
document::{DocumentOpenError, DocumentSavedEventResult},
editor::{ConfigEvent, EditorEvent},
events::DiagnosticsDidChange,
graphics::Rect,
theme,
tree::Layout,
Expand All @@ -33,7 +32,7 @@ use crate::{
use log::{debug, error, info, warn};
#[cfg(not(feature = "integration"))]
use std::io::stdout;
use std::{collections::btree_map::Entry, io::stdin, path::Path, sync::Arc};
use std::{io::stdin, path::Path, sync::Arc};

#[cfg(not(windows))]
use anyhow::Context;
Expand Down Expand Up @@ -746,9 +745,14 @@ impl Application {
doc.text(),
language_id,
));

handlers::diagnostics::pull_diagnostics_for_document(
doc,
language_server,
);
}
}
Notification::PublishDiagnostics(mut params) => {
Notification::PublishDiagnostics(params) => {
let uri = match helix_core::Uri::try_from(params.uri) {
Ok(uri) => uri,
Err(err) => {
Expand All @@ -761,100 +765,20 @@ impl Application {
log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
return;
}
// have to inline the function because of borrow checking...
let doc = self.editor.documents.values_mut()
.find(|doc| doc.uri().is_some_and(|u| u == uri))
.filter(|doc| {
if let Some(version) = params.version {
if version != doc.version() {
log::info!("Version ({version}) is out of date for {uri:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
return false;
}
}
true
});

let mut unchanged_diag_sources = Vec::new();
if let Some(doc) = &doc {
let lang_conf = doc.language.clone();

if let Some(lang_conf) = &lang_conf {
if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) {
if !lang_conf.persistent_diagnostic_sources.is_empty() {
// Sort diagnostics first by severity and then by line numbers.
// Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
params
.diagnostics
.sort_by_key(|d| (d.severity, d.range.start));
}
for source in &lang_conf.persistent_diagnostic_sources {
let new_diagnostics = params
.diagnostics
.iter()
.filter(|d| d.source.as_ref() == Some(source));
let old_diagnostics = old_diagnostics
.iter()
.filter(|(d, d_server)| {
*d_server == server_id
&& d.source.as_ref() == Some(source)
})
.map(|(d, _)| d);
if new_diagnostics.eq(old_diagnostics) {
unchanged_diag_sources.push(source.clone())
}
}
}
}
}

let diagnostics = params.diagnostics.into_iter().map(|d| (d, server_id));

// Insert the original lsp::Diagnostics here because we may have no open document
// for diagnosic message and so we can't calculate the exact position.
// When using them later in the diagnostics picker, we calculate them on-demand.
let diagnostics = match self.editor.diagnostics.entry(uri) {
Entry::Occupied(o) => {
let current_diagnostics = o.into_mut();
// there may entries of other language servers, which is why we can't overwrite the whole entry
current_diagnostics.retain(|(_, lsp_id)| *lsp_id != server_id);
current_diagnostics.extend(diagnostics);
current_diagnostics
// Sort diagnostics first by severity and then by line numbers.
}
Entry::Vacant(v) => v.insert(diagnostics.collect()),
};

// Sort diagnostics first by severity and then by line numbers.
// Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
diagnostics
.sort_by_key(|(d, server_id)| (d.severity, d.range.start, *server_id));

if let Some(doc) = doc {
let diagnostic_of_language_server_and_not_in_unchanged_sources =
|diagnostic: &lsp::Diagnostic, ls_id| {
ls_id == server_id
&& diagnostic.source.as_ref().map_or(true, |source| {
!unchanged_diag_sources.contains(source)
})
};
let diagnostics = Editor::doc_diagnostics_with_filter(
&self.editor.language_servers,
&self.editor.diagnostics,
doc,
diagnostic_of_language_server_and_not_in_unchanged_sources,
);
doc.replace_diagnostics(
diagnostics,
&unchanged_diag_sources,
Some(server_id),
);
let diagnostics: Vec<(lsp::Diagnostic, LanguageServerId)> = params
.diagnostics
.into_iter()
.map(|d| (d, server_id))
.collect();

let doc = doc.id();
helix_event::dispatch(DiagnosticsDidChange {
editor: &mut self.editor,
doc,
});
}
self.editor.add_diagnostics(
diagnostics,
server_id,
uri,
params.version,
None,
);
}
Notification::ShowMessage(params) => {
if self.config.load().editor.lsp.display_messages {
Expand Down Expand Up @@ -1129,6 +1053,16 @@ impl Application {
let result = self.handle_show_document(params, offset_encoding);
Ok(json!(result))
}
Ok(MethodCall::WorkspaceDiagnosticRefresh) => {
for document in self.editor.documents() {
let language_server = language_server!();
handlers::diagnostics::pull_diagnostics_for_document(
document,
language_server,
);
}
Ok(serde_json::Value::Null)
}
};

tokio::spawn(language_server!().reply(id, reply));
Expand Down
4 changes: 4 additions & 0 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,10 @@ fn goto_buffer(editor: &mut Editor, direction: Direction, count: usize) {

let id = *id;

if let Some(doc) = editor.document(id) {
helix_event::dispatch(helix_view::events::DocumentDidOpen { doc });
};

editor.switch(id, Action::Replace);
}

Expand Down
3 changes: 2 additions & 1 deletion helix-term/src/events.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use helix_event::{events, register_event};
use helix_view::document::Mode;
use helix_view::events::{
DiagnosticsDidChange, DocumentDidChange, DocumentFocusLost, SelectionDidChange,
DiagnosticsDidChange, DocumentDidChange, DocumentDidOpen, DocumentFocusLost, SelectionDidChange,
};

use crate::commands;
Expand All @@ -18,6 +18,7 @@ pub fn register() {
register_event::<PostInsertChar>();
register_event::<PostCommand>();
register_event::<DocumentDidChange>();
register_event::<DocumentDidOpen>();
register_event::<DocumentFocusLost>();
register_event::<SelectionDidChange>();
register_event::<DiagnosticsDidChange>();
Expand Down
5 changes: 4 additions & 1 deletion helix-term/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ use crate::config::Config;
use crate::events;
use crate::handlers::auto_save::AutoSaveHandler;
use crate::handlers::completion::CompletionHandler;
use crate::handlers::diagnostics::PullDiagnosticsHandler;
use crate::handlers::signature_help::SignatureHelpHandler;

pub use completion::trigger_auto_completion;
pub use helix_view::handlers::Handlers;

mod auto_save;
pub mod completion;
mod diagnostics;
pub mod diagnostics;
mod signature_help;
mod snippet;

Expand All @@ -24,11 +25,13 @@ pub fn setup(config: Arc<ArcSwap<Config>>) -> Handlers {
let completions = CompletionHandler::new(config).spawn();
let signature_hints = SignatureHelpHandler::new().spawn();
let auto_save = AutoSaveHandler::new().spawn();
let pull_diagnostics = PullDiagnosticsHandler::new().spawn();

let handlers = Handlers {
completions,
signature_hints,
auto_save,
pull_diagnostics,
};

completion::register_hooks(&handlers);
Expand Down
Loading