ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D59757
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/test/clangd/diagnostics.test
Index: clang-tools-extra/test/clangd/diagnostics.test
===================================================================
--- clang-tools-extra/test/clangd/diagnostics.test
+++ clang-tools-extra/test/clangd/diagnostics.test
@@ -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"}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -113,6 +113,10 @@
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;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -536,6 +536,14 @@
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).
+ publishDiagnostics(URIForFile::canonicalize(File, /*TUPath=*/File), {});
}
void ClangdLSPServer::onDocumentOnTypeFormatting(
@@ -836,6 +844,16 @@
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) {
@@ -978,17 +996,12 @@
// 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) {
Index: clang-tools-extra/test/clangd/diagnostics.test
===================================================================
--- clang-tools-extra/test/clangd/diagnostics.test
+++ clang-tools-extra/test/clangd/diagnostics.test
@@ -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"}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -113,6 +113,10 @@
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;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -536,6 +536,14 @@
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).
+ publishDiagnostics(URIForFile::canonicalize(File, /*TUPath=*/File), {});
}
void ClangdLSPServer::onDocumentOnTypeFormatting(
@@ -836,6 +844,16 @@
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) {
@@ -978,17 +996,12 @@
// 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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
