Skip to content

Commit

Permalink
[clangd] Send empty diagnostics when a file is closed
Browse files Browse the repository at this point in the history
Summary:
The LSP clients cannot know clangd will not send diagnostic updates
for closed files, so we send them an empty list of diagnostics to
avoid showing stale diagnostics for closed files in the UI, e.g. in the
"Problems" pane of VSCode.

Fixes PR41217.

Reviewers: hokein

Reviewed By: hokein

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59757

llvm-svn: 356880
  • Loading branch information
ilya-biryukov committed Mar 25, 2019
1 parent a5a4bb6 commit 49c1071
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
30 changes: 23 additions & 7 deletions clang-tools-extra/clangd/ClangdLSPServer.cpp
Expand Up @@ -536,6 +536,17 @@ void ClangdLSPServer::onDocumentDidClose(
PathRef File = Params.textDocument.uri.file();
DraftMgr.removeDraft(File);
Server->removeDocument(File);

{
std::lock_guard<std::mutex> Lock(FixItsMutex);
FixItsMap.erase(File);
}
// clangd will not send updates for this file anymore, so we empty out the
// list of diagnostics shown on the client (e.g. in the "Problems" pane of
// VSCode). Note that this cannot race with actual diagnostics responses
// because removeDocument() guarantees no diagnostic callbacks will be
// executed after it returns.
publishDiagnostics(URIForFile::canonicalize(File, /*TUPath=*/File), {});
}

void ClangdLSPServer::onDocumentOnTypeFormatting(
Expand Down Expand Up @@ -836,6 +847,16 @@ void ClangdLSPServer::applyConfiguration(
reparseOpenedFiles();
}

void ClangdLSPServer::publishDiagnostics(
const URIForFile &File, std::vector<clangd::Diagnostic> Diagnostics) {
// Publish diagnostics.
notify("textDocument/publishDiagnostics",
llvm::json::Object{
{"uri", File},
{"diagnostics", std::move(Diagnostics)},
});
}

// FIXME: This function needs to be properly tested.
void ClangdLSPServer::onChangeConfiguration(
const DidChangeConfigurationParams &Params) {
Expand Down Expand Up @@ -978,17 +999,12 @@ void ClangdLSPServer::onDiagnosticsReady(PathRef File,

// Cache FixIts
{
// FIXME(ibiryukov): should be deleted when documents are removed
std::lock_guard<std::mutex> Lock(FixItsMutex);
FixItsMap[File] = LocalFixIts;
}

// Publish diagnostics.
notify("textDocument/publishDiagnostics",
llvm::json::Object{
{"uri", URI},
{"diagnostics", std::move(LSPDiagnostics)},
});
// Send a notification to the LSP client.
publishDiagnostics(URI, std::move(LSPDiagnostics));
}

void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/clangd/ClangdLSPServer.h
Expand Up @@ -113,6 +113,10 @@ class ClangdLSPServer : private DiagnosticsConsumer {
void reparseOpenedFiles();
void applyConfiguration(const ConfigurationSettings &Settings);

/// Sends a "publishDiagnostics" notification to the LSP client.
void publishDiagnostics(const URIForFile &File,
std::vector<clangd::Diagnostic> Diagnostics);

/// Used to indicate that the 'shutdown' request was received from the
/// Language Server client.
bool ShutdownRequestReceived = false;
Expand Down
9 changes: 9 additions & 0 deletions clang-tools-extra/test/clangd/diagnostics.test
Expand Up @@ -23,6 +23,15 @@
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":2,"method":"sync","params":null}
---
{"jsonrpc":"2.0","method":"textDocument/didClose","params":{"textDocument":{"uri":"test:///foo.c"}}}
# CHECK: "method": "textDocument/publishDiagnostics",
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"
# CHECK-NEXT: }
---
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}

0 comments on commit 49c1071

Please sign in to comment.