[clangd] Extend textDocument/didChange to specify whether diagnostics should be generated.

[ioeric]

Summary

This would allow us to disable diagnostics when didChange is called but
diagnostics are not wanted (e.g. code completion).

[sammccall]

Nice, this will be useful for at least a couple of editor integrations.

clangd/Protocol.h

301

Nit: a little weird to lead with the missing case. Suggest rephrase as:

Forces diagnostics to be generated, or to not be generated. for this version of the file.
If not set, diagnostics are eventually consistent: either they will be provided for this version
or some subsequent one.

test/clangd/want-diagnostics.test

5

I think this test doesn't test anything useful because the check lines are not sequenced with respect to the input.

I'm not sure a lit test is that useful here, the actual logic is already unit tested. If we really want to test the ClangdLSPServer change, a unit test might be easier to get right, but personally I'd be happy enough leaving the logic untested (and maybe throwing a wantDiagnostics into one of the updates in protocol.test)

[ioeric]

Addressed review comments. Removed test.

clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
   Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text,
                      WantDiagnostics::Yes);
 }
 
 void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
   if (Params.contentChanges.size() != 1)
     return replyError(ErrorCode::InvalidParams,
                       "can only apply one change at a time");
+  auto WantDiags = WantDiagnostics::Auto;
+  if (Params.wantDiagnostics.hasValue())
+    WantDiags = Params.wantDiagnostics.getValue() ? WantDiagnostics::Yes
+                                                  : WantDiagnostics::No;
   // We only support full syncing right now.
   Server.addDocument(Params.textDocument.uri.file(),
-                     Params.contentChanges[0].text, WantDiagnostics::Auto);
+                     Params.contentChanges[0].text, WantDiags);
 }
 
 void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
   Server.onFileEvent(Params);
 }
 
 void ClangdLSPServer::onCommand(ExecuteCommandParams &Params) {
   auto ApplyEdit = [](WorkspaceEdit WE) {

clangd/Protocol.h

 struct DidChangeTextDocumentParams {
   /// The document that did change. The version number points
   /// to the version after all provided content changes have
   /// been applied.
   TextDocumentIdentifier textDocument;
 
   /// The actual content changes.
   std::vector<TextDocumentContentChangeEvent> contentChanges;
+
+  /// If this is not set, diagnostics will be generated for the current version
+  /// or a subsequent one.
+  /// If this is set to true, dianostics are guaranteed to be generated for the
+  /// current version.
+  /// If this is set to false, dianostics will not be generated for this
+  /// request.
+  /// This is a clangd extension.
+  llvm::Optional<bool> wantDiagnostics;
 };
 bool fromJSON(const json::Expr &, DidChangeTextDocumentParams &);
 
 enum class FileChangeType {
   /// The file got created.
   Created = 1,
   /// The file got changed.
   Changed = 2,

clangd/Protocol.cpp

 bool fromJSON(const json::Expr &Params, DidCloseTextDocumentParams &R) {
   json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument);
 }
 
 bool fromJSON(const json::Expr &Params, DidChangeTextDocumentParams &R) {
   json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument) &&
-         O.map("contentChanges", R.contentChanges);
+         O.map("contentChanges", R.contentChanges) &&
+         O.map("wantDiagnostics", R.wantDiagnostics);
 }
 
 bool fromJSON(const json::Expr &E, FileChangeType &Out) {
   if (auto T = E.asInteger()) {
     if (*T < static_cast<int>(FileChangeType::Created) ||
         *T > static_cast<int>(FileChangeType::Deleted))
       return false;
     Out = static_cast<FileChangeType>(*T);

test/clangd/want-diagnostics.test

+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void main() {}"}}}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+---
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///main.cpp","version":2},"contentChanges":[{"text":"void main() {}"}]}}
+---
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///main.cpp","version":3},"contentChanges":[{"text":"void main() {}"}],"wantDiagnostics":true}}
+---
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///main.cpp","version":4},"contentChanges":[{"text":"void main() {}"}],"wantDiagnostics":false}}
+---
+#      CHECK-NOT:  "method": "textDocument/publishDiagnostics",
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+#      CHECK:  "method": "textDocument/publishDiagnostics",
+---
+{"jsonrpc":"2.0","method":"exit"}