[clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

[arphaman]

Summary

This change extends the 'textDocument/publishDiagnostics' notification sent from Clangd to the client. The extension can be enabled using the '-fixit-usage=embed-in-diagnostic' argument. When it's enabled, Clangd sends out the fixits associated with the appropriate diagnostic in the body of the 'publicDiagnostics' notification.

[sammccall]

Couple of thoughts. (Technically I'm out on leave so will let Jan/Ilya
review implementation and happy with whatever you decide)

Enabling

• negotiating LSP extensions is probably better done in the "capabilities"

message exchange than as a command-line flag. Generally, we want this
extension on if the *client* is aware of it. Roughly, the client
capabilities are owned by the client, and the flags are owned by the *user*.

• for simplicity, we could always enable this, unless we really think the

message size is a problem, or are worried about conflicts with future LSP
versions

Naming

• elsewhere in clangd we settled on calling a "Fix" what clang calls a

"FixItHint". The latter is long/awkward/jargon, and often gets shortened to
"FixIt" which isn't obviously a noun. The former mostly has its plain
English meaning. I'd prefer "fix" in the protocol/flags, for the same
reasons.

• obviously feel free to give these any name you prefer in your UI!

[ilya-biryukov]

+1 Sam's suggestion of configuring this during initial LSP handshake, rather than as a command-line flag.
We could put it into ClientCapabilities or into initializationOptions. The latter has type 'any' in LSP, so it seems to be most in-line with the protocol to put clangd-specific config options there, but not opposed to having this in other structs as an extension either.

[arphaman]

• Client now can request the fixes using a 'clientCapabilities/textDocument/publishDiagnostics' extension.
• Use 'Fixes' instead of 'Fixits' in code.

[ilya-biryukov]

clangd/ClangdLSPServer.cpp

441

Maybe remove this parameter now that we configure these via LSP?
We don't set it to anything other than the default anyway.

[arphaman]

remove parameter.

[ilya-biryukov]

LGTM with a small NIT.

clangd/ClangdLSPServer.cpp

507

NIT: maybe avoid . and use something like clangd_fixes? having dots in the object keys might be awkward if someone tries to use those from javascript. (not that we have these use-cases now, but still)

clangd/ClangdLSPServer.h

private:
 
   // Various ClangdServer parameters go here. It's important they're created
   // before ClangdServer.
   CompilationDB CDB;
 
   RealFileSystemProvider FSProvider;
   /// Options used for code completion
   clangd::CodeCompleteOptions CCOpts;
+  /// Options used for diagnostics.
+  ClangdDiagnosticOptions DiagOpts;
   /// The supported kinds of the client.
   SymbolKindBitset SupportedSymbolKinds;
 
   // Store of the current versions of the open documents.
   DraftStore DraftMgr;
 
   // Server must be the last member of the class to allow its destructor to exit
   // the worker thread that may otherwise run an async callback on partially
   // destructed instance of ClangdLSPServer.
   ClangdServer Server;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/ClangdLSPServer.cpp

void ClangdLSPServer::onInitialize(InitializeParams &Params) {
 
   if (Params.rootUri && *Params.rootUri)
     Server.setRootPath(Params.rootUri->file());
   else if (Params.rootPath && !Params.rootPath->empty())
     Server.setRootPath(*Params.rootPath);
 
   CCOpts.EnableSnippets =
       Params.capabilities.textDocument.completion.completionItem.snippetSupport;
+  DiagOpts.EmbedFixesInDiagnostics =
+      Params.capabilities.textDocument.publishDiagnostics.clangdFixSupport;
 
   if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
       Params.capabilities.workspace->symbol->symbolKind) {
     for (SymbolKind Kind :
          *Params.capabilities.workspace->symbol->symbolKind->valueSet) {
       SupportedSymbolKinds.set(static_cast<size_t>(Kind));
     }
   }

 // FIXME: This function needs to be properly tested.
 void ClangdLSPServer::onChangeConfiguration(
     DidChangeConfigurationParams &Params) {
   applyConfiguration(Params.settings);
 }
 
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
                                  const clangd::CodeCompleteOptions &CCOpts,
                                  llvm::Optional<Path> CompileCommandsDir,
                                  bool ShouldUseInMemoryCDB,
                                  const ClangdServer::Options &Opts)
     : Out(Out), CDB(ShouldUseInMemoryCDB ? CompilationDB::makeInMemory()
                                          : CompilationDB::makeDirectoryBased(
                                                std::move(CompileCommandsDir))),
       CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
       Server(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this, Opts) {}
 

 
 void ClangdLSPServer::onDiagnosticsReady(PathRef File,
                                          std::vector<Diag> Diagnostics) {
   json::Array DiagnosticsJSON;
 
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto &Diag : Diagnostics) {
     toLSPDiags(Diag, [&](clangd::Diagnostic Diag, llvm::ArrayRef<Fix> Fixes) {
-      DiagnosticsJSON.push_back(json::Object{
+      json::Object LSPDiag({
           {"range", Diag.range},
           {"severity", Diag.severity},
           {"message", Diag.message},
       });
+      // LSP extension: embed the fixes in the diagnostic.
+      if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
+        json::Array ClangdFixes;
+        for (const auto &Fix : Fixes) {
+          WorkspaceEdit WE;
+          URIForFile URI{File};
+          WE.changes = {{URI.uri(), std::vector<TextEdit>(Fix.Edits.begin(),
+                                                          Fix.Edits.end())}};
+          ClangdFixes.push_back(
+              json::Object{{"edit", toJSON(WE)}, {"title", Fix.Message}});
+        }
+        LSPDiag["clangd_fixes"] = std::move(ClangdFixes);
+      }
+      DiagnosticsJSON.push_back(std::move(LSPDiag));
 
       auto &FixItsForDiagnostic = LocalFixIts[Diag];
       std::copy(Fixes.begin(), Fixes.end(),
                 std::back_inserter(FixItsForDiagnostic));
     });
   }
 
   // Cache FixIts

clangd/Diagnostics.h

 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include <cassert>
 #include <string>
 
 namespace clang {
 namespace clangd {
 
+struct ClangdDiagnosticOptions {
+  /// If true, Clangd uses an LSP extension to embed the fixes with the
+  /// diagnostics that are sent to the client.
+  bool EmbedFixesInDiagnostics = false;
+};
+
 /// Contains basic information about a diagnostic.
 struct DiagBase {
   std::string Message;
   // Intended to be used only in error messages.
   // May be relative, absolute or even artifically constructed.
   std::string File;
   clangd::Range Range;
   DiagnosticsEngine::Level Severity = DiagnosticsEngine::Note;

clangd/Protocol.h

struct CompletionClientCapabilities {
   // llvm::Optional<CompletionItemKindCapabilities> completionItemKind;
 
   /// The client supports to send additional context information for a
   /// `textDocument/completion` request.
   bool contextSupport = false;
 };
 bool fromJSON(const llvm::json::Value &, CompletionClientCapabilities &);
 
+struct PublishDiagnosticsClientCapabilities {
+  // Whether the client accepts diagnostics with related information.
+  // NOTE: not used by clangd at the moment.
+  // bool relatedInformation;
+
+  /// Whether the client accepts diagnostics with fixes attached using the
+  /// "clangd_fixes" extension.
+  bool clangdFixSupport = false;
+};
+bool fromJSON(const llvm::json::Value &,
+              PublishDiagnosticsClientCapabilities &);
+
 /// A symbol kind.
 enum class SymbolKind {
   File = 1,
   Module = 2,
   Namespace = 3,
   Package = 4,
   Class = 5,
   Method = 6,

 };
 bool fromJSON(const llvm::json::Value &, WorkspaceClientCapabilities &);
 
 // FIXME: most of the capabilities are missing from this struct. Only the ones
 // used by clangd are currently there.
 struct TextDocumentClientCapabilities {
   /// Capabilities specific to the `textDocument/completion`
   CompletionClientCapabilities completion;
+
+  /// Capabilities specific to the 'textDocument/publishDiagnostics'
+  PublishDiagnosticsClientCapabilities publishDiagnostics;
 };
 bool fromJSON(const llvm::json::Value &, TextDocumentClientCapabilities &);
 
 struct ClientCapabilities {
   // Workspace specific client capabilities.
   llvm::Optional<WorkspaceClientCapabilities> workspace;
 
   // Text document specific client capabilities.

clangd/Protocol.cpp

bool fromJSON(const json::Value &Params, CompletionClientCapabilities &R) {
   if (!O)
     return false;
   O.map("dynamicRegistration", R.dynamicRegistration);
   O.map("completionItem", R.completionItem);
   O.map("contextSupport", R.contextSupport);
   return true;
 }
 
+bool fromJSON(const llvm::json::Value &Params,
+              PublishDiagnosticsClientCapabilities &R) {
+  json::ObjectMapper O(Params);
+  if (!O)
+    return false;
+  O.map("clangdFixSupport", R.clangdFixSupport);
+  return true;
+}
+
 bool fromJSON(const json::Value &E, SymbolKind &Out) {
   if (auto T = E.getAsInteger()) {
     if (*T < static_cast<int>(SymbolKind::File) ||
         *T > static_cast<int>(SymbolKind::TypeParameter))
       return false;
     Out = static_cast<SymbolKind>(*T);
     return true;
   }
bool fromJSON(const json::Value &Params, WorkspaceClientCapabilities &R) {
   return O && O.map("symbol", R.symbol);
 }
 
 bool fromJSON(const json::Value &Params, TextDocumentClientCapabilities &R) {
   json::ObjectMapper O(Params);
   if (!O)
     return false;
   O.map("completion", R.completion);
+  O.map("publishDiagnostics", R.publishDiagnostics);
   return true;
 }
 
 bool fromJSON(const json::Value &Params, ClientCapabilities &R) {
   json::ObjectMapper O(Params);
   if (!O)
     return false;
   O.map("textDocument", R.textDocument);