[clangd] various fixes

[rwols]

Summary

• Look up in parent directories for a .clang-format file. Use "LLVM" as fallback style.
• Move cached fixits to ClangdServer, remove fixits once the client closes the document.
• Fix translations between LSP rows/cols (0-based) and clang row/cols (1-based).

[rwols]

Does anyone have an idea how I would write a test for this?

Also, I should have commit access after approval now. So that's less work for you guys ;)

[krasimir]

For testing you could do something similar to how clang-format does it.

[sammccall]

Thanks for this!

clangd/ClangdLSPServer.cpp

130

This function is pretty hard to understand from a high level - it's a bit of an odd split.

Is the duplication really so bad? The straight-line code could be easy to read:

auto File = Params.textDocument.uri.file;
auto Edits = Server.formatRange(File, Params.range);
if (Edits)
  C.reply(replacementsToEdits(Server.getDocument(File), Edits.get()));
else
  C.replyError(Edits.takeError());

(The "[" won't be needed after D39435, and we should add a replyError() overload that takes an Error)

134

It doesn't seem reasonable to assume the error is always .clang-format. Can we just include the message in the error object?

clangd/ClangdServer.cpp

69–70

This needs to use the VFS I think. (I'm not familiar with the details of the VFS usage I'm afraid)

[rwols]

clangd/ClangdLSPServer.cpp

130

This is a very nice idea! I'll probably update the diff tomorrow.

[rwols]

Use llvm::Error for beautiful handling of errors.

It's perhaps an idea to write a replyExpected(llvm::Expected<T>), that will
transparently handle llvm::Errors.

I'll write a unit test for these changes next.

It's probably best to wait for https://reviews.llvm.org/D39435 to land, and then rebase.

[rwols]

clangd/ClangdServer.cpp

69–70

Haven't tried implementing this yet, but from some simple experiments it doesn't seem necessary.

[rwols]

• Rebase to latest SVN revision.
• Use void reply(llvm:Expected<Twine> Result) for transparent error handling.
• Use FSProvider from ClangdServer to provide a filesystem for format::getStyle.
• Organize things around llvm::Expected, I've become a fan of that class :-)

[sammccall]

Thanks! Mostly just comments around the IO (which I'm still learning about myself).
Otherwise this LG.

I'm afraid you're going to run into some merge conflicts with rL317486 - I think/hope not too bad, though.

clangd/ClangdServer.cpp

69–70

Thanks for doing this. The reason it's needed is we want to run clangd on systems where the source tree is not a real filesystem. Looking on the real filesystem for .clang-format files would be incorrect.

352–354

As I understand, we tag the return values to ensure we get consistent reads, and describe the consistency of those reads, on systems that support that (e.g. snapshot filesystems).

I don't think we need to do that here, because:

• formatting only depends on the formatted file, and .clang-format
• the formatted file's contents are passed directly
• we should aggressively cache .clang-format reads, I think (see below)

534

getStyle is going to stat several files, in the usual case.
This seems less than ideal, particularly when we're doing format-on-type. Filesystems are not always fast/cheap.

We should really cache this, and I think "when the file is opened" is a reasonable tradeoff between simplicity and producing sensible user-facing behavior.

Unfortunately I'm not sure there's a sensible place to do things at present: addDocument() is fast and doing blocking IO there might be bad. Doing it asynchronously also seems bad (what happens if style's not ready?)

Next best thing would be "first format request when the file is opened". If you feel like doing that here (only computing the style if it's not already known for this file), then feel free, but a FIXME is fine too.

(FWIW, Google has a slow FS that's important to us, but we disable .clang-format scanning on that FS in other ways because it only holds code in one style. Other users might care about this, though)

clangd/ClangdServer.h

320

I don't understand this comment - I think we require and use Code.

If you mean we can just read Code from disk, that won't work: we want to format the dirty buffer in memory, not the last saved version.

[rwols]

• Merge with upstream, fix merge conflicts
• Add a FIXME for a caching strategy for .clang-format files

[rwols]

Is this ready to merge? I'd like to implement tests in another differential, I'm having trouble referencing a temp dir from lit in a JSON request to clangd.

clangd/ClangdServer.cpp

534

but a FIXME is fine too.

I've added a FIXME note. I think implementing a caching strategy is worthy of its own differential revision.

[ilya-biryukov]

clangd/ClangdLSPServer.cpp

146

NIT: remove braces from single-statement branches

156

NIT: remove braces from single-statement branches

clangd/ClangdServer.h

289

Why do we accept Code as a parameter here instead of getting it internally?

Maybe we should consider moving this method out of ClangdServer? Its signature looks pretty self-contained now.

[rwols]

Removed braces around single statement if-else.

[rwols]

clangd/ClangdServer.h

289

There are a couple intertwined problems:

1. replacementsToEdits wants llvm::StringRef Code
2. ClangdServer::formatCode wants llvm::StringRef Code
3. ClangdServer::getDocument returns an std::string

So yes, in principle you can call getDocument when you need it for replacementsToEdits, and you can let formatCode itself call getDocument for clang::format::getStyle. But then we create two copies of the document contents for one LSP request.

If getDocument returned an llvm::StringRef, I'd probably vote for removing the Code argument everywhere and call getDocument as needed.

[ilya-biryukov]

clangd/ClangdServer.h

289

Oh, I see. Calling getDocument twice does not really make sense.
Maybe we could move a call to replacementsToEdits into formatOnFile and make it return vector<TextEdit>? Seems to be solving both problems.

We could've made getDocument return StringRef, but we'd have to be more careful to ensure it's actually copied when we're scheduling async operations, worth a separate review.

[rwols]

clangd/ClangdServer.h

289

Maybe we could move a call to replacementsToEdits into formatOnFile and make it return vector<TextEdit>?

I disagree, this will bring LSP-specific protocols into ClangdServer. The translation from tooling::Replacement to clangd::TextEdit should remain in ClangdLSPServer.

[rwols]

clangd/ClangdServer.h

289

I tried this anyway, but ClangdLSPServer::getFixIts also uses replacementsToEdits. And that maintains a map with tooling::Replacements. So, if we want to move replacementsToEdits into ClangdServer, we would have to move that map into ClangdServer too I think.

[ilya-biryukov]

clangd/ClangdServer.h

289

I disagree, this will bring LSP-specific protocols into ClangdServer. The translation from tooling::Replacement to clangd::TextEdit should remain in ClangdLSPServer.

+1 to this, I'd also not make ClangdServer LSP-specific. However we already use some LSP structs from Protocol.h there to avoid code duplication (i.e., CompletionItem, though LSP-specific, suits ClangdServer pretty well). In principle, Protocol.h should not be a dependency, though, but we try to manage what we take from there instead. clangd::TextEdit seems to be in line with other things we take from there now (that is, Ranges, Locations, etc)

I tried this anyway, but ClangdLSPServer::getFixIts also uses replacementsToEdits. And that maintains a map with tooling::Replacements. So, if we want to move replacementsToEdits into ClangdServer, we would have to move that map into ClangdServer too I think.

Could we change the map to store clangd::TextEdit instead? It's a bit unfortunate that we'll be making unnecessary tooling::Replacement -> clangd::TextEdit conversions and those aren't free. But honestly, I don't think this should bite us performance-wise.
Given that we already use LSP's Location and Range encoding in other APIs, we'll get a more consistent interface.

[rwols]

• Rebase to latest upstream revision.
• Go all-in with TextEdit, even down to ClangdUnit.cpp.
• Move FixItsMap to ClangdServer. ClangdLSPServer is much cleaner now.
• Remove the cached FixIts when the client closes the document.
• Fix (or introduce bug?) wrong conversion between clang line/columns and LSP line/columns. From what I understand, clang line/cols are 1-based, and LSP line/cols are 0-based. The line numbers were already converted correctly (subtracting 1 when going from clang line numbers to LSP line numbers), but the column numbers were not converted correctly. Consequently, a bunch of tests had to be adjusted for the correct (or wrong?) column numbers.

[rwols]

clangd/ClangdUnit.cpp

168

Why subtract 1 from the line here, but not from the column? When subtracting 1 from both, diagnostics appear correct in my client. I have moved this logic into SourceLocToPosition.

[sammccall]

@ilya-biryukov We should get this landed.
I'm happy (my comments were addressed a few rounds ago) - any concerns?

[rwols]

• Merge with upstream revision 319613
• Fix FixItHints not getting applied correctly in all cases. The problem is that clang::CharSourceRange can either be a TokenRange or a CharRange. In the case of a TokenRange, we have to determine the end of the token ourselves (this would occur, for example, in things like if (i=1) { ... }; the fixit hint to replace = with == would be a TokenRange for some reason).
  
  Consequently, a tiny change had to be made to the fixits.test file, where the fixit hint to replace = with == would previously have an empty replacement range, whereas now it has a replacement range of length 1, exactly removing the = and inserting == in its place.

After playing around with these changes more I'm fairly certain I'm on the right
track for converting clang ranges/positions to clangd ranges/positions.

[malaperle]

• Look up in parent directories for a .clang-format file. Use "LLVM" as fallback style.
• Move cached fixits to ClangdServer, remove fixits once the client closes the document.
• Fix translations between LSP rows/cols (0-based) and clang row/cols (1-based).

I'd rather keep one commit per fix or feature. I would think the clang-format part should be one commit/review on its own. But I'm not very familiar with the practices here so I'll let others comment.

[ilya-biryukov]

In D39430#943387, @malaperle wrote:

I'd rather keep one commit per fix or feature. I would think the clang-format part should be one commit/review on its own. But I'm not very familiar with the practices here so I'll let others comment.

+1, please create a separate review per fix or feature.

clangd/ClangdServer.h

288

There are two ways to return fixits from ClangdServer now, one is DiagnosticsConsumer, the other is the new getFixIts method.

I'd also argue the fact that we can't attach fixits to diagnostics is LSP-specific and therefore it should be handled by ClangdLSPServer.
If the problem is that the cache is never cleaned up, we could simply remove the fixits when handling textDocument/didClose.

I think we should not put this method into ClangdServer.

[rwols]

I will split this diff up into separate parts.

[rwols]

This differential is now split into https://reviews.llvm.org/D41031 and https://reviews.llvm.org/D40860.

clangd/ClangdLSPServer.h

private:
   /// Language Server client.
   bool ShutdownRequestReceived = false;
 
   /// Used to indicate that the 'exit' notification was received from the
   /// Language Server client.
   /// It's used to break out of the LSP parsing loop.
   bool IsDone = false;
 
-  std::mutex FixItsMutex;
-  typedef std::map<clangd::Diagnostic, std::vector<clang::tooling::Replacement>>
-      DiagnosticToReplacementMap;
-  /// Caches FixIts per file and diagnostics
-  llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
-
   // Various ClangdServer parameters go here. It's important they're created
   // before ClangdServer.
   DirectoryBasedGlobalCompilationDatabase CDB;
   RealFileSystemProvider FSProvider;
 
   // 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

 #include "ClangdLSPServer.h"
 #include "JSONRPCDispatcher.h"
 
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace clang::clangd;
 using namespace clang;
 
-namespace {
-
-std::vector<TextEdit>
-replacementsToEdits(StringRef Code,
-                    const std::vector<tooling::Replacement> &Replacements) {
-  // Turn the replacements into the format specified by the Language Server
-  // Protocol. Fuse them into one big JSON array.
-  std::vector<TextEdit> Edits;
-  for (auto &R : Replacements) {
-    Range ReplacementRange = {
-        offsetToPosition(Code, R.getOffset()),
-        offsetToPosition(Code, R.getOffset() + R.getLength())};
-    Edits.push_back({ReplacementRange, R.getReplacementText()});
-  }
-  return Edits;
-}
-
-} // namespace
-
 void ClangdLSPServer::onInitialize(Ctx C, InitializeParams &Params) {
   C.reply(json::obj{
       {{"capabilities",
         json::obj{
             {"textDocumentSync", 1},
             {"documentFormattingProvider", true},
             {"documentRangeFormattingProvider", true},
             {"documentOnTypeFormattingProvider",
if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
     C.replyError(
         ErrorCode::InvalidParams,
         llvm::formatv("Unsupported command \"{0}\".", Params.command).str());
   }
 }
 
 void ClangdLSPServer::onRename(Ctx C, RenameParams &Params) {
   auto File = Params.textDocument.uri.file;
-  auto Replacements = Server.rename(File, Params.position, Params.newName);
-  if (!Replacements) {
-    C.replyError(ErrorCode::InternalError,
-                 llvm::toString(Replacements.takeError()));
-    return;
-  }
-  std::string Code = Server.getDocument(File);
-  std::vector<TextEdit> Edits = replacementsToEdits(Code, *Replacements);
+  auto EditsOrError = Server.rename(File, Params.position, Params.newName);
+  if (EditsOrError) {
     WorkspaceEdit WE;
-  WE.changes = {{Params.textDocument.uri.uri, Edits}};
+    WE.changes = {{Params.textDocument.uri.uri, *EditsOrError}};
     C.reply(WE);
+  } else {
+    C.replyError(ErrorCode::InternalError,
+                 llvm::toString(EditsOrError.takeError()));
+  }
 }
 
 void ClangdLSPServer::onDocumentDidClose(Ctx C,
                                          DidCloseTextDocumentParams &Params) {
   Server.removeDocument(Params.textDocument.uri.file);
 }
 
 void ClangdLSPServer::onDocumentOnTypeFormatting(
     Ctx C, DocumentOnTypeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  C.reply(json::ary(
-      replacementsToEdits(Code, Server.formatOnType(File, Params.position))));
+  const PathRef File = Params.textDocument.uri.file;
+  auto EditsOrError = Server.formatOnType(File, Params.position);
+  if (EditsOrError)
+    C.reply(json::ary{EditsOrError.get()});
+  else
+    C.replyError(EditsOrError.takeError());
 }
 
 void ClangdLSPServer::onDocumentRangeFormatting(
     Ctx C, DocumentRangeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  C.reply(json::ary(
-      replacementsToEdits(Code, Server.formatRange(File, Params.range))));
+  const PathRef File = Params.textDocument.uri.file;
+  auto EditsOrError = Server.formatRange(File, Params.range);
+  if (EditsOrError)
+    C.reply(json::ary{EditsOrError.get()});
+  else
+    C.replyError(EditsOrError.takeError());
 }
 
 void ClangdLSPServer::onDocumentFormatting(Ctx C,
                                            DocumentFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
-  std::string Code = Server.getDocument(File);
-  C.reply(json::ary(replacementsToEdits(Code, Server.formatFile(File))));
+  const PathRef File = Params.textDocument.uri.file;
+  auto EditsOrError = Server.formatFile(File);
+  if (EditsOrError)
+    C.reply(json::ary{EditsOrError.get()});
+  else
+    C.replyError(EditsOrError.takeError());
 }
 
 void ClangdLSPServer::onCodeAction(Ctx C, CodeActionParams &Params) {
   // We provide a code action for each diagnostic at the requested location
   // which has FixIts available.
-  std::string Code = Server.getDocument(Params.textDocument.uri.file);
   json::ary Commands;
-  for (Diagnostic &D : Params.context.diagnostics) {
-    std::vector<clang::tooling::Replacement> Fixes =
-        getFixIts(Params.textDocument.uri.file, D);
-    auto Edits = replacementsToEdits(Code, Fixes);
-    if (!Edits.empty()) {
+  for (const auto &Diag : Params.context.diagnostics) {
+    const auto ServerEdits =
+        Server.getFixIts(Params.textDocument.uri.file, Diag);
+    if (!ServerEdits.empty()) {
       WorkspaceEdit WE;
-      WE.changes = {{Params.textDocument.uri.uri, std::move(Edits)}};
+      WE.changes = {
+          {Params.textDocument.uri.uri,
+           std::vector<TextEdit>{ServerEdits.begin(), ServerEdits.end()}}};
       Commands.push_back(json::obj{
-          {"title", llvm::formatv("Apply FixIt {0}", D.message)},
+          {"title", llvm::formatv("Apply FixIt {0}", Diag.message)},
           {"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND},
           {"arguments", {WE}},
       });
     }
   }
   C.reply(std::move(Commands));
 }
 
bool ClangdLSPServer::run(std::istream &In) {
 
   // Make sure IsDone is set to true after this method exits to ensure assertion
   // at the start of the method fires if it's ever executed again.
   IsDone = true;
 
   return ShutdownRequestReceived;
 }
 
-std::vector<clang::tooling::Replacement>
-ClangdLSPServer::getFixIts(StringRef File, const clangd::Diagnostic &D) {
-  std::lock_guard<std::mutex> Lock(FixItsMutex);
-  auto DiagToFixItsIter = FixItsMap.find(File);
-  if (DiagToFixItsIter == FixItsMap.end())
-    return {};
-
-  const auto &DiagToFixItsMap = DiagToFixItsIter->second;
-  auto FixItsIter = DiagToFixItsMap.find(D);
-  if (FixItsIter == DiagToFixItsMap.end())
-    return {};
-
-  return FixItsIter->second;
-}
-
 void ClangdLSPServer::onDiagnosticsReady(
     PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) {
   json::ary DiagnosticsJSON;
 
-  DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto &DiagWithFixes : Diagnostics.Value) {
     auto Diag = DiagWithFixes.Diag;
     DiagnosticsJSON.push_back(json::obj{
         {"range", Diag.range},
         {"severity", Diag.severity},
         {"message", Diag.message},
     });
-    // We convert to Replacements to become independent of the SourceManager.
-    auto &FixItsForDiagnostic = LocalFixIts[Diag];
-    std::copy(DiagWithFixes.FixIts.begin(), DiagWithFixes.FixIts.end(),
-              std::back_inserter(FixItsForDiagnostic));
-  }
-
-  // Cache FixIts
-  {
-    // FIXME(ibiryukov): should be deleted when documents are removed
-    std::lock_guard<std::mutex> Lock(FixItsMutex);
-    FixItsMap[File] = LocalFixIts;
-  }
-
-  // Publish diagnostics.
-  Out.writeMessage(json::obj{
-      {"jsonrpc", "2.0"},
-      {"method", "textDocument/publishDiagnostics"},
-      {"params",
-       json::obj{
-           {"uri", URI::fromFile(File)},
-           {"diagnostics", std::move(DiagnosticsJSON)},
-       }},
-  });
+  }
+
+  // Publish diagnostics by sending an LSP notification to the client.
+  Out.notify("textDocument/publishDiagnostics",
+             json::obj{{"uri", URI::fromFile(File)},
+                       {"diagnostics", std::move(DiagnosticsJSON)}});
 }

clangd/ClangdServer.h

public:
   /// Get definition of symbol at a specified \p Line and \p Column in \p File.
   llvm::Expected<Tagged<std::vector<Location>>> findDefinitions(PathRef File,
                                                                 Position Pos);
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
   llvm::Optional<Path> switchSourceHeader(PathRef Path);
 
-  /// Run formatting for \p Rng inside \p File.
-  std::vector<tooling::Replacement> formatRange(PathRef File, Range Rng);
-  /// Run formatting for the whole \p File.
-  std::vector<tooling::Replacement> formatFile(PathRef File);
-  /// Run formatting after a character was typed at \p Pos in \p File.
-  std::vector<tooling::Replacement> formatOnType(PathRef File, Position Pos);
+  /// Get "FixIt" replacements for the given diagnostic.
+  llvm::SmallVector<TextEdit, 1> getFixIts(StringRef File,
+                                           const clangd::Diagnostic &D);
+
+  /// Run formatting for \p Rng inside \p File with content \p Code.
+  llvm::Expected<std::vector<TextEdit>> formatRange(PathRef File, Range Rng);
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
-  Expected<std::vector<tooling::Replacement>> rename(PathRef File, Position Pos,
-                                                     llvm::StringRef NewName);
+  Expected<std::vector<TextEdit>> rename(PathRef File, Position Pos,
 
+  /// Run formatting for the whole \p File with content \p Code.
+  llvm::Expected<std::vector<TextEdit>> formatFile(PathRef File);
+
+  /// Run formatting after a character was typed at \p Pos in \p File with
+  /// content \p Code.
+  llvm::Expected<std::vector<TextEdit>> formatOnType(PathRef File,
+                                                     Position Pos);
+
   /// Gets current document contents for \p File. \p File must point to a
   /// currently tracked file.
   /// FIXME(ibiryukov): This function is here to allow offset-to-Position
   /// conversions in outside code, maybe there's a way to get rid of it.
   std::string getDocument(PathRef File);
 
   /// Only for testing purposes.
   /// Waits until all requests to worker thread are finished and dumps AST for
   /// \p File. \p File must be in the list of added documents.
   std::string dumpAST(PathRef File);
   /// Called when an event occurs for a watched file in the workspace.
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
 private:
+  /// FIXME: This stats several files to find a .clang-format file. I/O can be
+  /// slow. Think of a way to cache this.
+  llvm::Expected<std::vector<TextEdit>>
+  formatCode(llvm::StringRef Code, PathRef File,
+             ArrayRef<tooling::Range> Ranges);
+
   std::future<void>
   scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
                           std::shared_ptr<CppFile> Resources,
                           Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
 
   std::future<void> scheduleCancelRebuild(std::shared_ptr<CppFile> Resources);
 
   clangd::Logger &Logger;
private:
   bool StorePreamblesInMemory;
   clangd::CodeCompleteOptions CodeCompleteOpts;
   /// Used to serialize diagnostic callbacks.
   /// FIXME(ibiryukov): get rid of an extra map and put all version counters
   /// into CppFile.
   std::mutex DiagnosticsMutex;
   /// Maps from a filename to the latest version of reported diagnostics.
   llvm::StringMap<DocVersion> ReportedDiagnosticVersions;
+  std::mutex FixItsMutex;
+  typedef std::map<clangd::Diagnostic, llvm::SmallVector<TextEdit, 1>>
+      DiagnosticToTextEditMap;
+  /// Caches FixIts per file and diagnostics
+  llvm::StringMap<DiagnosticToTextEditMap> FixItsMap;
   // WorkScheduler has to be the last member, because its destructor has to be
   // called before all other members to stop the worker thread that references
   // ClangdServer
   ClangdScheduler WorkScheduler;
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/ClangdServer.cpp

 #include "llvm/Support/raw_ostream.h"
 #include <future>
 
 using namespace clang;
 using namespace clang::clangd;
 
 namespace {
 
+TextEdit replacementToEdit(StringRef Code,
+                           const tooling::Replacement &Replacement) {
+  TextEdit Edit;
+  Edit.range = {offsetToPosition(Code, Replacement.getOffset()),
+                offsetToPosition(Code, Replacement.getOffset() +
+                                           Replacement.getLength())};
+  Edit.newText = Replacement.getReplacementText();
+  return Edit;
+}
+
+template <class ForwardIterator>
+std::vector<TextEdit> replacementsToEdits(StringRef Code, ForwardIterator Begin,
+                                          ForwardIterator End) {
+  // Turn the replacements into the format specified by the Language Server
+  // Protocol. Fuse them into one big JSON array.
+  std::vector<TextEdit> Edits;
+  Edits.reserve(std::distance(Begin, End));
+  while (Begin != End) {
+    Edits.emplace_back(replacementToEdit(Code, *Begin));
+    ++Begin;
+  }
+  return Edits;
+}
+
+std::vector<TextEdit> replacementsToEdits(StringRef Code,
+                                          tooling::Replacements Replacements) {
+  return replacementsToEdits(Code, Replacements.begin(), Replacements.end());
+}
+
 class FulfillPromiseGuard {
 public:
   FulfillPromiseGuard(std::promise<void> &Promise) : Promise(Promise) {}
 
   ~FulfillPromiseGuard() { Promise.set_value(); }
 
 private:
   std::promise<void> &Promise;
 };
 
-std::vector<tooling::Replacement> formatCode(StringRef Code, StringRef Filename,
-                                             ArrayRef<tooling::Range> Ranges) {
-  // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  auto Result = format::reformat(Style, Code, Ranges, Filename);
-
-  return std::vector<tooling::Replacement>(Result.begin(), Result.end());
-}
-
 std::string getStandardResourceDir() {
   static int Dummy; // Just an address in this process.
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
 }
 
 class RefactoringResultCollector final
     : public tooling::RefactoringResultConsumer {
 public:
   void handleError(llvm::Error Err) override {
std::shared_ptr<CppFile> Resources = Units.getOrCreateFile(
       File, ResourceDir, CDB, StorePreamblesInMemory, PCHs, Logger);
   return scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
                                  std::move(Resources), std::move(TaggedFS));
 }
 
 std::future<void> ClangdServer::removeDocument(PathRef File) {
   DraftMgr.removeDraft(File);
   std::shared_ptr<CppFile> Resources = Units.removeIfPresent(File);
+  FixItsMap.erase(File);
   return scheduleCancelRebuild(std::move(Resources));
 }
 
 std::future<void> ClangdServer::forceReparse(PathRef File) {
   auto FileContents = DraftMgr.getDraft(File);
   assert(FileContents.Draft &&
          "forceReparse() was called for non-added document");
 
ClangdServer::signatureHelp(PathRef File, Position Pos,
   auto Preamble = Resources->getPossiblyStalePreamble();
   auto Result = clangd::signatureHelp(File, Resources->getCompileCommand(),
                                       Preamble ? &Preamble->Preamble : nullptr,
                                       *OverridenContents, Pos, TaggedFS.Value,
                                       PCHs, Logger);
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
-std::vector<tooling::Replacement> ClangdServer::formatRange(PathRef File,
+llvm::Expected<std::vector<TextEdit>> ClangdServer::formatRange(PathRef File,
                                                                 Range Rng) {
-  std::string Code = getDocument(File);
-
+  const auto Code = getDocument(File);
   size_t Begin = positionToOffset(Code, Rng.start);
   size_t Len = positionToOffset(Code, Rng.end) - Begin;
   return formatCode(Code, File, {tooling::Range(Begin, Len)});
 }
 
-std::vector<tooling::Replacement> ClangdServer::formatFile(PathRef File) {
+llvm::Expected<std::vector<TextEdit>> ClangdServer::formatFile(PathRef File) {
   // Format everything.
-  std::string Code = getDocument(File);
+  const auto Code = getDocument(File);
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-std::vector<tooling::Replacement> ClangdServer::formatOnType(PathRef File,
+llvm::Expected<std::vector<TextEdit>> ClangdServer::formatOnType(PathRef File,
                                                                  Position Pos) {
+  const auto Code = getDocument(File);
   // Look for the previous opening brace from the character position and
   // format starting from there.
-  std::string Code = getDocument(File);
   size_t CursorPos = positionToOffset(Code, Pos);
-  size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos);
+  size_t PreviousLBracePos = Code.find_last_of('{', CursorPos);
   if (PreviousLBracePos == StringRef::npos)
     PreviousLBracePos = CursorPos;
   size_t Len = 1 + CursorPos - PreviousLBracePos;
 
   return formatCode(Code, File, {tooling::Range(PreviousLBracePos, Len)});
 }
 
-Expected<std::vector<tooling::Replacement>>
-ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName) {
+Expected<std::vector<TextEdit>> ClangdServer::rename(PathRef File, Position Pos,
+                                                     llvm::StringRef NewName) {
   std::string Code = getDocument(File);
   std::shared_ptr<CppFile> Resources = Units.getFile(File);
   RefactoringResultCollector ResultCollector;
   Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) {
     const SourceManager &SourceMgr = AST->getASTContext().getSourceManager();
     const FileEntry *FE =
         SourceMgr.getFileEntryForID(SourceMgr.getMainFileID());
     if (!FE)
if (!Rename) {
       return;
     }
     Rename->invoke(ResultCollector, Context);
   });
   assert(ResultCollector.Result.hasValue());
   if (!ResultCollector.Result.getValue())
     return ResultCollector.Result->takeError();
 
-  std::vector<tooling::Replacement> Replacements;
+  std::vector<TextEdit> Edits;
   for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
     tooling::Replacements ChangeReps = Change.getReplacements();
     for (const auto &Rep : ChangeReps) {
       // FIXME: Right now we only support renaming the main file, so we drop
       // replacements not for the main file. In the future, we might consider to
       // support:
       //   * rename in any included header
       //   * rename only in the "main" header
       //   * provide an error if there are symbols we won't rename (e.g.
       //     std::vector)
       //   * rename globally in project
       //   * rename in open files
       if (Rep.getFilePath() == File)
-        Replacements.push_back(Rep);
+        Edits.push_back(replacementToEdit(Code, Rep));
     }
   }
-  return Replacements;
+  return Edits;
 }
 
 std::string ClangdServer::getDocument(PathRef File) {
   auto draft = DraftMgr.getDraft(File);
   assert(draft.Draft && "File is not tracked, cannot get contents");
   return *draft.Draft;
 }
 
for (StringRef NewExt : NewExts) {
     llvm::sys::path::replace_extension(NewPath, NewExt.upper());
     if (FS->exists(NewPath))
       return NewPath.str().str();
   }
 
   return llvm::None;
 }
 
+llvm::SmallVector<TextEdit, 1>
+ClangdServer::getFixIts(StringRef File, const clangd::Diagnostic &D) {
+  std::lock_guard<std::mutex> Lock(FixItsMutex);
+  auto DiagToFixItsIter = FixItsMap.find(File);
+  if (DiagToFixItsIter == FixItsMap.end())
+    return {};
+
+  const auto &DiagToFixItsMap = DiagToFixItsIter->second;
+  auto FixItsIter = DiagToFixItsMap.find(D);
+  if (FixItsIter == DiagToFixItsMap.end())
+    return {};
+
+  return FixItsIter->second;
+}
+
+llvm::Expected<std::vector<TextEdit>>
+ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
+                         ArrayRef<tooling::Range> Ranges) {
+  // Call clang-format.
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  auto StyleOrError =
+      format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());
+  if (!StyleOrError) {
+    return StyleOrError.takeError();
+  } else {
+    return replacementsToEdits(
+        Code, format::reformat(StyleOrError.get(), Code, Ranges, File));
+  }
+}
+
 std::future<void> ClangdServer::scheduleReparseAndDiags(
     PathRef File, VersionedDraft Contents, std::shared_ptr<CppFile> Resources,
     Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
 
   assert(Contents.Draft && "Draft must have contents");
   UniqueFunction<llvm::Optional<std::vector<DiagWithFixIts>>()>
       DeferredRebuild =
           Resources->deferRebuild(*Contents.Draft, TaggedFS.Value);
auto ReparseAndPublishDiags =
     // FIXME(ibiryukov): get rid of '<' comparison here. In the current
     // implementation diagnostics will not be reported after version counters'
     // overflow. This should not happen in practice, since DocVersion is a
     // 64-bit unsigned integer.
     if (Version < LastReportedDiagsVersion)
       return;
     LastReportedDiagsVersion = Version;
 
+    // Cache FixIts for when the client requests textDocument/codeAction.
+    DiagnosticToTextEditMap M;
+    for (const auto& Diag : *Diags) M.emplace(Diag.Diag, Diag.FixIts);
+    {
+      std::unique_lock<std::mutex> Guard(FixItsMutex);
+      FixItsMap[FileStr] = M;
+    }
+
     DiagConsumer.onDiagnosticsReady(FileStr,
                                     make_tagged(std::move(*Diags), Tag));
   };
 
   WorkScheduler.addToFront(std::move(ReparseAndPublishDiags),
                            std::move(DeferredRebuild), std::move(DonePromise));
   return DoneFuture;
 }

clangd/ClangdUnit.h

 
 namespace clangd {
 
 class Logger;
 
 /// A diagnostic with its FixIts.
 struct DiagWithFixIts {
   clangd::Diagnostic Diag;
-  llvm::SmallVector<tooling::Replacement, 1> FixIts;
+  llvm::SmallVector<TextEdit, 1> FixIts;
 };
 
 // Stores Preamble and associated data.
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble,
                std::vector<serialization::DeclID> TopLevelDeclIDs,
                std::vector<DiagWithFixIts> Diags);
 

clangd/ClangdUnit.cpp

for (const auto &Chunk : CCS) {
     default:
       Result += Chunk.Text;
       break;
     }
   }
   return Result;
 }
 
-llvm::Optional<DiagWithFixIts> toClangdDiag(const StoredDiagnostic &D) {
-  auto Location = D.getLocation();
-  if (!Location.isValid() || !Location.getManager().isInMainFile(Location))
+/// Convert a clang::SourceLocation to a clangd::Position
+Position SourceLocToClangdPosition(const SourceManager &SrcMgr,
+                                   SourceLocation Location) {
+  Position P;
+  // LSP rows and columns are 0-based, clang rows and columns are 1-based.
+  P.line = static_cast<int>(SrcMgr.getSpellingLineNumber(Location)) - 1;
+  P.character = static_cast<int>(SrcMgr.getSpellingColumnNumber(Location)) - 1;
+  assert(P.line >= 0 && "Unexpected negative value for P.line");
+  assert(P.character >= 0 && "Unexpected negative value for P.character");
+  return P;
+}
+
+/// Convert a clang::FullSourceLoc to a clangd::Position
+Position SourceLocToClangdPosition(const FullSourceLoc &Location) {
+  return SourceLocToClangdPosition(Location.getManager(), Location);
+}
+
+/// Convert a clang::SourceRange to a clangd::Range
+Range SourceRangeToClangdRange(const SourceManager &SrcMgr,
+                               const SourceRange &Input) {
+  Range Output;
+  Output.start = SourceLocToClangdPosition(SrcMgr, Input.getBegin());
+  Output.end = SourceLocToClangdPosition(SrcMgr, Input.getEnd());
+  return Output;
+}
+
+/// Convert a clang::CharSourceRange to a clangd::Range
+Range CharSourceRangeToClangdRange(const SourceManager &SrcMgr,
+                                   const LangOptions &Options,
+                                   const CharSourceRange &Input) {
+  if (Input.isTokenRange()) {
+    const auto EndLoc =
+        clang::Lexer::getLocForEndOfToken(Input.getEnd(), 0, SrcMgr, Options);
+    clang::SourceRange R(Input.getBegin(), EndLoc);
+    return SourceRangeToClangdRange(SrcMgr, R);
+  } else if (Input.isCharRange()) {
+    return SourceRangeToClangdRange(SrcMgr, Input.getAsRange());
+  } else {
+    llvm_unreachable(
+        "Expected Input to be either a token range or a char range.");
+  }
+}
+
+/// Convert a clang::FixitHint to a clangd::TextEdit
+TextEdit FixitHintToClangdTextEdit(const SourceManager &SrcMgr,
+                                   const LangOptions &Options,
+                                   const FixItHint &Input) {
+  TextEdit Output;
+  Output.range =
+      CharSourceRangeToClangdRange(SrcMgr, Options, Input.RemoveRange);
+  Output.newText = Input.CodeToInsert;
+  return Output;
+}
+
+llvm::Optional<DiagWithFixIts> toClangdDiag(const LangOptions &Options,
+                                            const StoredDiagnostic &D) {
+  const auto &Location = D.getLocation();
+  const auto &SrcMgr = Location.getManager();
+  if (!Location.isValid() || !SrcMgr.isInMainFile(Location))
     return llvm::None;
 
-  Position P;
-  P.line = Location.getSpellingLineNumber() - 1;
-  P.character = Location.getSpellingColumnNumber();
+  const auto P = SourceLocToClangdPosition(Location);
   Range R = {P, P};
   clangd::Diagnostic Diag = {R, getSeverity(D.getLevel()), D.getMessage()};
 
-  llvm::SmallVector<tooling::Replacement, 1> FixItsForDiagnostic;
+  llvm::SmallVector<TextEdit, 1> FixItsForDiagnostic;
   for (const FixItHint &Fix : D.getFixIts()) {
-    FixItsForDiagnostic.push_back(clang::tooling::Replacement(
-        Location.getManager(), Fix.RemoveRange, Fix.CodeToInsert));
+    FixItsForDiagnostic.emplace_back(
+        FixitHintToClangdTextEdit(SrcMgr, Options, Fix));
   }
   return DiagWithFixIts{Diag, std::move(FixItsForDiagnostic)};
 }
 
 class StoreDiagsConsumer : public DiagnosticConsumer {
 public:
-  StoreDiagsConsumer(std::vector<DiagWithFixIts> &Output) : Output(Output) {}
+  StoreDiagsConsumer(const LangOptions &Options,
+                     std::vector<DiagWithFixIts> &Output)
+      : Options(Options), Output(Output) {}
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const clang::Diagnostic &Info) override {
     DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
 
-    if (auto convertedDiag = toClangdDiag(StoredDiagnostic(DiagLevel, Info)))
+    if (auto convertedDiag =
+            toClangdDiag(Options, StoredDiagnostic(DiagLevel, Info)))
       Output.push_back(std::move(*convertedDiag));
   }
 
 private:
+  const LangOptions &Options;
   std::vector<DiagWithFixIts> &Output;
 };
 
 class EmptyDiagsConsumer : public DiagnosticConsumer {
 public:
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const clang::Diagnostic &Info) override {}
 };

 ParsedAST::Build(std::unique_ptr<clang::CompilerInvocation> CI,
                  std::shared_ptr<const PreambleData> Preamble,
                  std::unique_ptr<llvm::MemoryBuffer> Buffer,
                  std::shared_ptr<PCHContainerOperations> PCHs,
                  IntrusiveRefCntPtr<vfs::FileSystem> VFS,
                  clangd::Logger &Logger) {
 
   std::vector<DiagWithFixIts> ASTDiags;
-  StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
+  StoreDiagsConsumer UnitDiagsConsumer(*CI->getLangOpts(), ASTDiags);
 
   const PrecompiledPreamble *PreamblePCH =
       Preamble ? &Preamble->Preamble : nullptr;
   auto Clang = prepareCompilerInstance(
       std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs),
       std::move(VFS), /*ref*/ UnitDiagsConsumer);
 
   // Recover resources if we crash before exiting this method.
auto DoRebuildPreamble =
       }
       // We won't need the OldPreamble anymore, release it so it can be deleted
       // (if there are no other references to it).
       OldPreamble.reset();
 
       trace::Span Tracer("Preamble");
       SPAN_ATTACH(Tracer, "File", That->FileName);
       std::vector<DiagWithFixIts> PreambleDiags;
-      StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
+      StoreDiagsConsumer PreambleDiagnosticsConsumer(*CI->getLangOpts(),
+                                                     PreambleDiags);
       IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
           CompilerInstance::createDiagnostics(
               &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
       CppFilePreambleCallbacks SerializedDeclsCollector;
       auto BuiltPreamble = PrecompiledPreamble::Build(
           *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs,
           /*StoreInMemory=*/That->StorePreamblesInMemory,
           SerializedDeclsCollector);

clangd/JSONRPCDispatcher.h

 
 #include "JSONExpr.h"
 #include "Logger.h"
 #include "Protocol.h"
 #include "Trace.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/YAMLParser.h"
 #include <iosfwd>
 #include <mutex>
 
 namespace clang {
 namespace clangd {
 
 /// Encapsulates output and logs streams and provides thread-safe access to
 /// them.
public:
   /// Write a line to the logging stream.
   void log(const Twine &Message) override;
 
   /// Mirror \p Message into InputMirror stream. Does nothing if InputMirror is
   /// null.
   /// Unlike other methods of JSONOutput, mirrorInput is not thread-safe.
   void mirrorInput(const Twine &Message);
 
+  /// Send a notification to the client.
+  void notify(llvm::StringRef Method, json::Expr &&Params);
+
   // Whether output should be pretty-printed.
   const bool Pretty;
 
 private:
   llvm::raw_ostream &Outs;
   llvm::raw_ostream &Logs;
   llvm::raw_ostream *InputMirror;
 
   std::mutex StreamMutex;
 };
 
 /// Context object passed to handlers to allow replies.
 class RequestContext {
 public:
   RequestContext(JSONOutput &Out, StringRef Method,
                  llvm::Optional<json::Expr> ID)
       : Out(Out), ID(std::move(ID)),
         Tracer(llvm::make_unique<trace::Span>(Method)) {
     if (this->ID)
       SPAN_ATTACH(tracer(), "ID", *this->ID);
   }
 
-  /// Sends a successful reply.
+  /// Send a response to a request from the client.
   void reply(json::Expr &&Result);
-  /// Sends an error response to the client, and logs it.
+  /// Send an error response to the client, and logs it.
   void replyError(ErrorCode code, const llvm::StringRef &Message);
-  /// Sends a request to the client.
+  /// Send all error messages contained in Err to the client, and log them.
+  void replyError(llvm::Error Err);
+  /// Send a request to the client.
   void call(llvm::StringRef Method, json::Expr &&Params);
 
   trace::Span &tracer() { return *Tracer; }
 
 private:
   JSONOutput &Out;
   llvm::Optional<json::Expr> ID;
   std::unique_ptr<trace::Span> Tracer;

clangd/JSONRPCDispatcher.cpp

void JSONOutput::writeMessage(const json::Expr &Message) {
   Logs << "--> " << S << '\n';
   Logs.flush();
 
   // Emit message with header.
   Outs << "Content-Length: " << S.size() << "\r\n\r\n" << S;
   Outs.flush();
 }
 
+void JSONOutput::notify(StringRef Method, json::Expr &&Params) {
+  writeMessage(json::obj{
+    {"jsonrpc", "2.0"},
+    {"method", Method},
+    {"params", std::move(Params)},
+  });
+}
+
 void JSONOutput::log(const Twine &Message) {
   trace::log(Message);
   std::lock_guard<std::mutex> Guard(StreamMutex);
   Logs << Message << '\n';
   Logs.flush();
 }
 
 void JSONOutput::mirrorInput(const Twine &Message) {
Out.writeMessage(json::obj{
         {"jsonrpc", "2.0"},
         {"id", *ID},
         {"error",
          json::obj{{"code", static_cast<int>(code)}, {"message", Message}}},
     });
   }
 }
 
+void RequestContext::replyError(llvm::Error Err) {
+  const auto Message = llvm::toString(std::move(Err));
+  replyError(ErrorCode::UnknownErrorCode, Message);
+}
+
 void RequestContext::call(StringRef Method, json::Expr &&Params) {
   // FIXME: Generate/Increment IDs for every request so that we can get proper
   // replies once we need to.
   SPAN_ATTACH(tracer(), "Call",
               (json::obj{{"method", Method.str()}, {"params", Params}}));
   Out.writeMessage(json::obj{
       {"jsonrpc", "2.0"},
       {"id", 1},

test/clangd/diagnostics.test

 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"void main() {}"}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "return type of 'main' is not 'int'",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 1,
+# CHECK-NEXT:            "character": 0,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 1,
+# CHECK-NEXT:            "character": 0,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "change return type to 'int'",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 1,
+# CHECK-NEXT:            "character": 0,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 1,
+# CHECK-NEXT:            "character": 0,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file:///foo.c"
 # CHECK-NEXT:  }
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 Content-Length: 33
 
 {"jsonrpc":"2.0","method":"exit"}

test/clangd/execute-command.test

 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "place parentheses around the assignment to silence this warning",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "use '==' to turn this assignment into an equality comparison",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file:///foo.c"
 # CHECK-NEXT:  }

test/clangd/extra-flags.test

 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"extraFlags":["-Wall"]}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "variable 'i' is uninitialized when used here",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 28,
+# CHECK-NEXT:            "character": 27,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 28,
+# CHECK-NEXT:            "character": 27,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "initialize the variable 'i' to silence this warning",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 19,
+# CHECK-NEXT:            "character": 18,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 19,
+# CHECK-NEXT:            "character": 18,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file:///foo.c"
 # CHECK-NEXT:  }
 Content-Length: 175
 
 {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i; }"}]}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "variable 'i' is uninitialized when used here",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 28,
+# CHECK-NEXT:            "character": 27,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 28,
+# CHECK-NEXT:            "character": 27,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "initialize the variable 'i' to silence this warning",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 19,
+# CHECK-NEXT:            "character": 18,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 19,
+# CHECK-NEXT:            "character": 18,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file:///foo.c"
 # CHECK-NEXT:  }
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 Content-Length: 33
 
 {"jsonrpc":"2.0":"method":"exit"}
 
 

test/clangd/fixits.test

 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
 #      CHECK:    "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "place parentheses around the assignment to silence this warning",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      },
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "message": "use '==' to turn this assignment into an equality comparison",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
-# CHECK-NEXT:            "character": 35,
+# CHECK-NEXT:            "character": 34,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 3
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file:///foo.c"
 # CHECK-NEXT:  }
 Content-Length: 746
 
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":34}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
 #      CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
 # CHECK-NEXT:          "changes": {
 # CHECK-NEXT:            "file:///foo.c": [

 # CHECK-NEXT:        }
 # CHECK-NEXT:      ],
 # CHECK-NEXT:      "command": "clangd.applyFix",
 # CHECK-NEXT:      "title": "Apply FixIt use '==' to turn this assignment into an equality comparison"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  ]
 Content-Length: 771
 
-{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":34}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
 # Make sure unused "code" and "source" fields ignored gracefully
 #      CHECK:  "id": 3,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
 # CHECK-NEXT:          "changes": {