Publish empty diagnostics on close (#4954)

Without this, diagnostics will linger after closing a file.

This refactors towards a pattern of putting outgoing calls as methods on
`Context`. I'm mixed on this, mainly thinking it's an improvement on
using `outgoing` directly (because it shares the name and structure),
might want to move it to a side-class later that is _only_ LSP wrappers.

I could also make inheritance private on OutgoingMessages and these
kinds of methods public there, but I'm hesitant to adopt that approach
versus a type separation.
This commit is contained in:
Jon Ross-Perkins
2025-02-15 00:09:20 +00:00
committed by GitHub
parent f038aead4c
commit d4f15ab26e
5 changed files with 33 additions and 7 deletions
+4 -3
View File
@@ -61,7 +61,9 @@ class PublishDiagnosticConsumer : public DiagnosticConsumer {
}
// Returns the constructed request.
auto params() -> llvm::json::Value { return params_; }
auto params() -> const clang::clangd::PublishDiagnosticsParams& {
return params_;
}
private:
// Returns the LSP range for a diagnostic. Note that Carbon uses 1-based
@@ -151,8 +153,7 @@ auto Context::File::SetText(Context& context, std::optional<int64_t> version,
// Note we need to publish diagnostics even when empty.
// TODO: Consider caching previously published diagnostics and only publishing
// when they change.
context.outgoing().notify("textDocument/publishDiagnostics",
consumer.params());
context.PublishDiagnostics(consumer.params());
}
auto Context::LookupFile(llvm::StringRef filename) -> File* {
+6 -3
View File
@@ -61,12 +61,15 @@ class Context {
// null.
auto LookupFile(llvm::StringRef filename) -> File*;
// Wrapper for LSP notification.
auto PublishDiagnostics(clang::clangd::PublishDiagnosticsParams params)
-> void {
outgoing_->notify("textDocument/publishDiagnostics", params);
}
auto vlog_stream() -> llvm::raw_ostream* { return vlog_stream_; }
auto file_emitter() -> FileDiagnosticEmitter& { return file_emitter_; }
auto no_loc_emitter() -> NoLocDiagnosticEmitter& { return no_loc_emitter_; }
auto outgoing() -> clang::clangd::LSPBinder::RawOutgoing& {
return *outgoing_;
}
auto files() -> Map<std::string, File>& { return files_; }
@@ -58,7 +58,11 @@ auto HandleDidCloseTextDocument(
return;
}
if (!context.files().Erase(filename)) {
if (context.files().Erase(filename)) {
// Clear diagnostics when the document closes. Otherwise, any diagnostics
// will linger.
context.PublishDiagnostics({.uri = params.textDocument.uri});
} else {
CARBON_DIAGNOSTIC(LanguageServerCloseUnknownFile, Warning,
"tried closing unknown file; ignoring request");
context.file_emitter().Emit(filename, LanguageServerCloseUnknownFile);
@@ -119,6 +119,15 @@
// CHECK:STDOUT: "uri": "file:///test.carbon",
// CHECK:STDOUT: "version": 2
// CHECK:STDOUT: }
// CHECK:STDOUT: }Content-Length: 144{{\r}}
// CHECK:STDOUT: {{\r}}
// CHECK:STDOUT: {
// CHECK:STDOUT: "jsonrpc": "2.0",
// CHECK:STDOUT: "method": "textDocument/publishDiagnostics",
// CHECK:STDOUT: "params": {
// CHECK:STDOUT: "diagnostics": [],
// CHECK:STDOUT: "uri": "file:///test.carbon"
// CHECK:STDOUT: }
// CHECK:STDOUT: }Content-Length: 51{{\r}}
// CHECK:STDOUT: {{\r}}
// CHECK:STDOUT: {
@@ -55,6 +55,15 @@
// CHECK:STDOUT: "diagnostics": [],
// CHECK:STDOUT: "uri": "file:///test.carbon"
// CHECK:STDOUT: }
// CHECK:STDOUT: }Content-Length: 144{{\r}}
// CHECK:STDOUT: {{\r}}
// CHECK:STDOUT: {
// CHECK:STDOUT: "jsonrpc": "2.0",
// CHECK:STDOUT: "method": "textDocument/publishDiagnostics",
// CHECK:STDOUT: "params": {
// CHECK:STDOUT: "diagnostics": [],
// CHECK:STDOUT: "uri": "file:///test.carbon"
// CHECK:STDOUT: }
// CHECK:STDOUT: }Content-Length: 51{{\r}}
// CHECK:STDOUT: {{\r}}
// CHECK:STDOUT: {