arphaman created this revision.
arphaman added reviewers: jkorous, sammccall, ilya-biryukov.
Herald added subscribers: dexonsmith, MaskRay, ioeric.

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.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50415

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Diagnostics.h
  clangd/fuzzer/ClangdFuzzer.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/fixits-embed-in-diagnostic.test

Index: test/clangd/fixits-embed-in-diagnostic.test
===================================================================
--- /dev/null
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -0,0 +1,66 @@
+# RUN: clangd -fixit-usage=embed-in-diagnostic -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:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
+#      CHECK:    "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:    "params": {
+# CHECK-NEXT:     "diagnostics": [
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "clangd.fixes": [
+# CHECK-NEXT:          {
+# CHECK-NEXT:            "edit": {
+# CHECK-NEXT:              "changes": {
+# CHECK-NEXT:                "file://{{.*}}/foo.c": [
+# CHECK-NEXT:                  {
+# CHECK-NEXT:                    "newText": "struct",
+# CHECK-NEXT:                    "range": {
+# CHECK-NEXT:                      "end": {
+# CHECK-NEXT:                        "character": 22,
+# CHECK-NEXT:                        "line": 0
+# CHECK-NEXT:                      },
+# CHECK-NEXT:                      "start": {
+# CHECK-NEXT:                        "character": 17,
+# CHECK-NEXT:                        "line": 0
+# CHECK-NEXT:                      }
+# CHECK-NEXT:                    }
+# CHECK-NEXT:                  }
+# CHECK-NEXT:                ]
+# CHECK-NEXT:              }
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "title": "change 'union' to 'struct'"
+# CHECK-NEXT:          }
+# CHECK-NEXT:        ],
+# CHECK-NEXT:        "message": "Use of 'Point' with tag type that does not match previous declaration\n\nfoo.c:1:8: note: previous use is here",
+# CHECK-NEXT:        "range": {
+# CHECK-NEXT:          "end": {
+# CHECK-NEXT:            "character": 22,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "start": {
+# CHECK-NEXT:            "character": 17,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          }
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "severity": 1
+# CHECK-NEXT:      },
+# CHECK-NEXT:      {
+# CHECK-NEXT:        "message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
+# CHECK-NEXT:        "range": {
+# CHECK-NEXT:          "end": {
+# CHECK-NEXT:            "character": 12,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "start": {
+# CHECK-NEXT:            "character": 7,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          }
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "severity": 3
+# CHECK-NEXT:      }
+# CHECK-NEXT:    ],
+# CHECK-NEXT:    "uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -185,6 +185,21 @@
                                 "'compile_commands.json' files")),
     llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt<ClangdDiagnosticOptions::FixitUsageOption> FixitUsage(
+    "fixit-usage",
+    llvm::cl::desc("Controls how the diagnostic fixits are used by the client"),
+    llvm::cl::values(
+        clEnumValN(ClangdDiagnosticOptions::FixitUsageOption::CodeAction,
+                   "code-action",
+                   "The fixits can be discovered by the client using the "
+                   "'textDocument/codeAction' request"),
+        clEnumValN(ClangdDiagnosticOptions::FixitUsageOption::EmbedInDiagnostic,
+                   "embed-in-diagnostic",
+                   "The fixits are sent to the client together with the "
+                   "diagnostics using an LSP extension")),
+    llvm::cl::init(ClangdDiagnosticOptions::FixitUsageOption::CodeAction),
+    llvm::cl::Hidden);
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
@@ -300,9 +315,12 @@
     CCOpts.IncludeIndicator.NoInsert.clear();
   }
 
+  ClangdDiagnosticOptions DiagOpts;
+  DiagOpts.FixitUsage = FixitUsage;
+
   // Initialize and run ClangdLSPServer.
   ClangdLSPServer LSPServer(
-      Out, CCOpts, CompileCommandsDirPath,
+      Out, CCOpts, DiagOpts, CompileCommandsDirPath,
       /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
Index: clangd/fuzzer/ClangdFuzzer.cpp
===================================================================
--- clangd/fuzzer/ClangdFuzzer.cpp
+++ clangd/fuzzer/ClangdFuzzer.cpp
@@ -27,11 +27,12 @@
                                 clang::clangd::Logger::Error, nullptr);
   clang::clangd::CodeCompleteOptions CCOpts;
   CCOpts.EnableSnippets = false;
+  ClangdDiagnosticOptions DiagOpts;
   clang::clangd::ClangdServer::Options Opts;
 
   // Initialize and run ClangdLSPServer.
-  clang::clangd::ClangdLSPServer LSPServer(Out, CCOpts, llvm::None, false,
-                                           Opts);
+  clang::clangd::ClangdLSPServer LSPServer(Out, CCOpts, DiagOpts, llvm::None,
+                                           false, Opts);
   // fmemopen isn't portable, but I think we only run the fuzzer on Linux.
   LSPServer.run(fmemopen(data, size, "r"));
   return 0;
Index: clangd/Diagnostics.h
===================================================================
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -23,6 +23,22 @@
 namespace clang {
 namespace clangd {
 
+struct ClangdDiagnosticOptions {
+  /// Represents the different ways that the client of Clangd can consume the
+  /// fixits.
+  enum class FixitUsageOption {
+    /// Clangd does not send fixits with the diagnostics.
+    /// Instead, the client can discover then using the
+    /// 'textDocument/codeAction' command at the location of the diagnostic.
+    CodeAction,
+    /// Clangd uses an LSP extension to embed the fixits with the diagnostics
+    /// that are sent to the client.
+    EmbedInDiagnostic
+  };
+
+  FixitUsageOption FixitUsage = FixitUsageOption::CodeAction;
+};
+
 /// Contains basic information about a diagnostic.
 struct DiagBase {
   std::string Message;
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -34,6 +34,7 @@
   /// loaded only from \p CompileCommandsDir. Otherwise, clangd will look
   /// for compile_commands.json in all parent directories of each file.
   ClangdLSPServer(JSONOutput &Out, const clangd::CodeCompleteOptions &CCOpts,
+                  const ClangdDiagnosticOptions &DiagOpts,
                   llvm::Optional<Path> CompileCommandsDir,
                   bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts);
 
@@ -155,6 +156,8 @@
   RealFileSystemProvider FSProvider;
   /// Options used for code completion
   clangd::CodeCompleteOptions CCOpts;
+  /// Options used for diagnostics.
+  ClangdDiagnosticOptions DiagOpts;
   /// The supported kinds of the client.
   SymbolKindBitset SupportedSymbolKinds;
 
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -436,13 +436,15 @@
 
 ClangdLSPServer::ClangdLSPServer(JSONOutput &Out,
                                  const clangd::CodeCompleteOptions &CCOpts,
+                                 const ClangdDiagnosticOptions &DiagOpts,
                                  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()),
+      CCOpts(CCOpts), DiagOpts(DiagOpts),
+      SupportedSymbolKinds(defaultSymbolKinds()),
       Server(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this, Opts) {}
 
 bool ClangdLSPServer::run(std::FILE *In, JSONStreamStyle InputStyle) {
@@ -486,11 +488,27 @@
   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 fix-its in the diagnostic.
+      if (DiagOpts.FixitUsage ==
+              ClangdDiagnosticOptions::FixitUsageOption::EmbedInDiagnostic &&
+          !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(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to