[clangd] Revamp handling of diagnostics.

[ilya-biryukov]

Summary

The new implementation attaches notes to diagnostic message and shows
the original diagnostics in the message of the note.

[ilya-biryukov]

This revision is still missing tests, but I wanted to get feedback for the overall design and the new diagnostic messages. So sending out for review now.

[ilya-biryukov]

clangd/Diagnostics.h

29

This could probably use a better name

[sammccall]

Behavior looks good. I think we can do a bit better with (most) fixits - your call on whether it makes sense to do here.

As discussed i'd simplify the diagnostic containers until we know there's a strong need.

clangd/ClangdLSPServer.cpp

323

nit: while here, can we add a colon after FixIt (or if appropriate in practice, just drop the prefix altogether?). My recollection is this is a bit hard to parse.

clangd/Diagnostics.cpp

86

inside

126

mind pasting a small example as a function comment?
Logic looks good, but I had to reconstruct the message in my head.

148

or just mainMessage (this is another verbs-for-pure-functions case where the style recommendation seems to hurt readability to me, but up to you)

193

PreBuild could be FillBasicFields or something? found this name confusing.

201

(nit: if the goal with the callback function is to avoid allocations, shouldn't we reuse the DiagWithFixIts?)

323

do you need else log here? (and just log the main diag)

clangd/Diagnostics.h

29

I found it a bit confusing that this represents both "top-level" diagnostics and notes. It obscures the nature of the hierarchy a bit: there *are* a fixed number of levels with different "kinds", but the kinds are similar enough to share a type.

I'd actually consider using inheritance here to model the relationships more clearly. (Yes, gross, I know)
Like:

struct DiagBase { Message, File, Range, InMainFile }
struct Diag : DiagBase { FixIts, Notes, Severity }
struct Note : DiagBase {} // or leave this one out until needed

(I think we came to the conclusion that different severity of notes isn't interesting and potentially just confusing)

36

As discussed offline - I think fixits should probably be a different struct hanging off the main diagnostic, with a name. Following clang's example seems... less than clear here.

They could also be modeled as notes with an optional TextEdit - this seems sligthly less clear but more faithful to clang internals*.
Either way, it should be clear that they're only allowed in *one* of these locations - having notes and main diags be distinct types would help.

I think this probably affects how we should expose them through LSP - they should be named code actions attached to the original diagnostic.
Maybe they should also be separate diagnostics, but only if they contribute a unique opportunity to the user e.g. they have a non-empty range that isn't contained within the diagnostic's range.
This patch seems like a reasonable place to do that, but also OK if you want to defer.

36

Nit: can we call these "Fix"es, which is a noun (at least in common usage?)

Clang calls them FixItHint, but I think Fix is no less clear (and shorter)

59

This is cool :) but really looks like premature optimization.
Particularly, if the diagnostics were a self-contained value type, this could just be vector<diag> I think. And if we care about copying strings (I'm not sure we do), being lifetime-scoped to the ASTcontext is probably OK.

(rawDiags() has potential uses but having callers explicitly traverse is going to be clearer as your comment suggests)

114

Can we try removing this struct in favor of two callback params? It seems like a small source of confusion, considering how important the type used to be.

124

I don't think this overload pays for itself - making callers write the outer loop would be a little clearer, I think.

(I'd consider having that fn return a smallvector instead of taking a callback, but don't feel strongly)

129

brief doc.
In particular: what filtering is applied

129

Maybe just StoreDiags?

131

If there's no strong reason otherwise, DiagList take() is slightly easier/clearer for tests. compare

DiagList Diags;
StoreDiagsConsumer Cons(Diags);
...
EXPECT_THAT(Diags, ...)

vs

StoreDiagsConsumer Diags;
...
EXPECT_THAT(Diags.take(), ...)

clangd/SourceCode.cpp

53

isn't this sourceLocToPosition?

test/clangd/diagnostics.test

20–21

I think ideally this wouldn't show up at all, "change return type to int" would be the name of the code action returned for the first diag. Note the range is the same, so we're not giving the user an extra place to apply the fix.

[ilya-biryukov]

This is not final, there are still unadressed comments.

• Significantly simplified the implementation by removing the DiagList, etc.
• Addressed some of the rest of the comments.

clangd/ClangdLSPServer.cpp

323

I added a colon for now, but happy to look into removing the prefix. The use-case that's slightly confusing is:

use of undeclared identifier 'foo'. did you mean 'bar'?

Since the actual fix-it is at the end, it's might be a bit hard to understand.

clangd/Diagnostics.cpp

201

Can we actually avoid allocating some of the strings there?
We're std::moveing the result into the callback, so we don't seem to waste memory too.

clangd/Diagnostics.h

59

Using a straight-forward representation for now.

124

Still using a callback, but happy to return a SmallVector instead. It seems the pattern for using SmallVector in llvm is passing it in as an output parameter (so that the users might decide themselves what's the count of items on the stack), which is slightly ugly.
It might be fine to return SmallVector here though, this function can probably make assumptions about the number of notes in a diagnostic.

[ilya-biryukov]

Addressed review comments.

The only thing that's left on my todo-list is a test for toLSPDiags().
Everything else should be ready.

[sammccall]

Not sure if you're waiting on comments from me: the changes look good.
As discussed I still think Fix should be a separate thing with a name because that's how editors treat it, and this is the right layer to decide how to do that mapping.

clangd/ClangdLSPServer.cpp

323

Yeah. I'd still be inclined to drop the prefix:

• this case isn't ideal, but is also not terrible
• this is a "no separate note for fixit" case, where we can consider synthesizing the message from the text edit rather than reusing the diagnostic. That would work well at least in this case.

323

nit: fixit --> fix in prefix and in lambda
(And drop the caps in the prefix I think, as this is now a regular english noun rather than a pseudo-product-name)

clangd/Diagnostics.h

41

I don't understand what "persistent" means in this context.
This does seem to be an is-a relationship rather than has-a, so I'd find DiagBase/Diag + inheritance clearer.

But with composition, this one seems rike it should be "Diag" and the type of Main could be DiagDetails or so?

[sammccall]

Sorry, last comments were concurrent with the latest snapshot.

Except where noted, the old comments still apply I think.

clangd/ClangdLSPServer.cpp

317

inline this into the for loop?

clangd/ClangdLSPServer.h

76

getFixes?

clangd/Diagnostics.h

36

This is done, and looks good to me.

41

(this happened already, yay!)

48

nit: severity of notes can...

52

Seems a bit confusing to use "diagnostic" to mean something importantly different than "diag", and to define top-level diagnostic in terms of notes.

Maybe just "A top-level diagnostic that may have Notes and Fixes"?

55

I think these deserve some comments:

• fixes are *alternative* fixes for this diagnostic, one should be chosen
• notes elaborate on the problem, usually pointing to a related piece of code

58

I'm not sure what "conventional" means here?

unittests/clangd/ClangdUnitTests.cpp

23

can you make these operator<< and move to the main code? This seems a fine general-purpose debug representation (could be tweaked, but also later).

That avoids the ODR issue, and makes casual logging easier.

38

Ah, and now that you've written this... ;-)

For the specific case of "field X matches matcher M" you can use ::testing::Field (you'd keep WithNote, but make it call Field)

Messages are good as long as you pass the optional field_name param.

69

this "same range and message as the diagnostic itself" is an important property of the code being tested, and it's hidden here in a comment of a helper function.
I'd probably prefer avoiding this helper altogether (spelling the string twice in the test) so the test behavior/change is obvious when changing the code.
Failing that, DiagWithEqualFix or something?

[ilya-biryukov]

• Added unit test for toLSPDiags

[ilya-biryukov]

• Addressed review comments

clangd/Diagnostics.h

36

We now provide a separate Fix class, but we still don't return it to LSP, keeping the behaviour as is (i.e. mapping LSP ranges to code actions on the server side).

The fixes are not reported as separate diagnostics, they are only shown as code actions when users hovers over the diagnostic.

58

Removed conventional from the comment. Other parts should make sense.

unittests/clangd/ClangdUnitTests.cpp

38

Was a useful exercise anyway.

Couldn't find the optional 'field_name' param, though, am I missing something?

69

I agree that, but spelling this in the code is so much noise. Renaming to DiagWithEqualFix makes total sense, did that.

[ilya-biryukov]

clangd/ClangdLSPServer.cpp

323

I still find a prefix useful in some cases, e.g. the unresolved identifier foo. did you mean bar?.
Mostly for cases where a fix note uses the error message of the main diagnostic or when it's badly phrased (haven't really seen those, but I would be surprised if there are none).

[sammccall]

Ship it! (but let's drop the extra operator== if possible?)

clangd/ClangdLSPServer.cpp

323

that's... exactly the case I was replying to :)
I don't think it's common or confusing enough, and there are other fixes when using the main diagnostic. But this isn't a big deal; let's leave it for now.

clangd/Protocol.h

169

The TextEdit/Diagnostic == operators seem odd - what do we need these for?

unittests/clangd/ClangdUnitTests.cpp

38

You're not, sorry - it was added recently to google's copy and hasn't been pushed to upstream gmock yet. So for now we have slightly bad error messages (or have the full MatcherInterface implementation)

[ilya-biryukov]

• Replace equality comparison ops with matchers, they were only used in tests.

clangd/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_library(clangDaemon
   AST.cpp
   ClangdLSPServer.cpp
   ClangdServer.cpp
   ClangdUnit.cpp
   CodeComplete.cpp
   CodeCompletionStrings.cpp
   CompileArgsCache.cpp
   Compiler.cpp
   Context.cpp
+  Diagnostics.cpp
   DraftStore.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp
   JSONExpr.cpp
   JSONRPCDispatcher.cpp
   Logger.cpp
   Protocol.cpp

clangd/ClangdLSPServer.h

public:
   /// each instance of ClangdLSPServer.
   ///
   /// \return Wether we received a 'shutdown' request before an 'exit' request
   bool run(std::istream &In,
            JSONStreamStyle InputStyle = JSONStreamStyle::Standard);
 
 private:
   // Implement DiagnosticsConsumer.
-  virtual void
-  onDiagnosticsReady(PathRef File,
-                     Tagged<std::vector<DiagWithFixIts>> Diagnostics) override;
+  void onDiagnosticsReady(PathRef File,
+                          Tagged<std::vector<Diag>> Diagnostics) override;
 
   // Implement ProtocolCallbacks.
   void onInitialize(InitializeParams &Params) override;
   void onShutdown(ShutdownParams &Params) override;
   void onExit(ExitParams &Params) override;
   void onDocumentDidOpen(DidOpenTextDocumentParams &Params) override;
   void onDocumentDidChange(DidChangeTextDocumentParams &Params) override;
   void onDocumentDidClose(DidCloseTextDocumentParams &Params) override;
private:
   void onSwitchSourceHeader(TextDocumentIdentifier &Params) override;
   void onDocumentHighlight(TextDocumentPositionParams &Params) override;
   void onFileEvent(DidChangeWatchedFilesParams &Params) override;
   void onCommand(ExecuteCommandParams &Params) override;
   void onRename(RenameParams &Parames) override;
   void onHover(TextDocumentPositionParams &Params) override;
   void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
 
-  std::vector<TextEdit> getFixIts(StringRef File, const clangd::Diagnostic &D);
+  std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 
   JSONOutput &Out;
   /// Used to indicate that the 'shutdown' request was received from the
   /// 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<TextEdit>,
-                   LSPDiagnosticCompare>
+  typedef std::map<clangd::Diagnostic, std::vector<Fix>, LSPDiagnosticCompare>
       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;

clangd/ClangdLSPServer.cpp

 //===--- ClangdLSPServer.cpp - LSP server ------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #include "ClangdLSPServer.h"
+#include "Diagnostics.h"
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 
 using namespace clang::clangd;
 using namespace clang;
void ClangdLSPServer::onCodeAction(CodeActionParams &Params) {
   // which has FixIts available.
   auto Code = Server.getDocument(Params.textDocument.uri.file());
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onCodeAction called for non-added file");
 
   json::ary Commands;
   for (Diagnostic &D : Params.context.diagnostics) {
-    auto Edits = getFixIts(Params.textDocument.uri.file(), D);
-    if (!Edits.empty()) {
+    for (auto &F : getFixes(Params.textDocument.uri.file(), D)) {
       WorkspaceEdit WE;
+      std::vector<TextEdit> Edits(F.Edits.begin(), F.Edits.end());
       WE.changes = {{Params.textDocument.uri.uri(), std::move(Edits)}};
       Commands.push_back(json::obj{
-          {"title", llvm::formatv("Apply FixIt {0}", D.message)},
+          {"title", llvm::formatv("Apply fix: {0}", F.Message)},
           {"command", ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND},
           {"arguments", {WE}},
       });
     }
   }
   reply(std::move(Commands));
 }
 
 void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) {
bool ClangdLSPServer::run(std::istream &In, JSONStreamStyle InputStyle) {
 
   // 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<TextEdit> ClangdLSPServer::getFixIts(StringRef File,
-                                                 const clangd::Diagnostic &D) {
+std::vector<Fix> ClangdLSPServer::getFixes(StringRef File,
   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) {
+    PathRef File, Tagged<std::vector<Diag>> Diagnostics) {
   json::ary DiagnosticsJSON;
 
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
-  for (auto &DiagWithFixes : Diagnostics.Value) {
-    auto Diag = DiagWithFixes.Diag;
+  for (auto &Diag : Diagnostics.Value) {
+    toLSPDiags(Diag, [&](clangd::Diagnostic Diag, llvm::ArrayRef<Fix> Fixes) {
       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::copy(Fixes.begin(), Fixes.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;
   }

clangd/ClangdServer.h

Tagged<typename std::decay<T>::type> make_tagged(T &&Value, VFSTag Tag) {
   return Tagged<typename std::decay<T>::type>(std::forward<T>(Value), Tag);
 }
 
 class DiagnosticsConsumer {
 public:
   virtual ~DiagnosticsConsumer() = default;
 
   /// Called by ClangdServer when \p Diagnostics for \p File are ready.
-  virtual void
-  onDiagnosticsReady(PathRef File,
-                     Tagged<std::vector<DiagWithFixIts>> Diagnostics) = 0;
+  virtual void onDiagnosticsReady(PathRef File,
+                                  Tagged<std::vector<Diag>> Diagnostics) = 0;
 };
 
 class FileSystemProvider {
 public:
   virtual ~FileSystemProvider() = default;
   /// Called by ClangdServer to obtain a vfs::FileSystem to be used for parsing.
   /// Name of the file that will be parsed is passed in \p File.
   ///

clangd/ClangdServer.cpp

void ClangdServer::scheduleReparseAndDiags(
     PathRef File, VersionedDraft Contents, WantDiagnostics WantDiags,
     Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
   tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
 
   DocVersion Version = Contents.Version;
   Path FileStr = File.str();
   VFSTag Tag = std::move(TaggedFS.Tag);
 
-  auto Callback = [this, Version, FileStr,
-                   Tag](std::vector<DiagWithFixIts> Diags) {
+  auto Callback = [this, Version, FileStr, Tag](std::vector<Diag> Diags) {
     // We need to serialize access to resulting diagnostics to avoid calling
     // `onDiagnosticsReady` in the wrong order.
     std::lock_guard<std::mutex> DiagsLock(DiagnosticsMutex);
     DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr];
     // 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.

clangd/ClangdUnit.h

 //===--- ClangdUnit.h -------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDUNIT_H
 
+#include "Diagnostics.h"
 #include "Function.h"
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/PrecompiledPreamble.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"

 }
 
 namespace tooling {
 struct CompileCommand;
 }
 
 namespace clangd {
 
-/// A diagnostic with its FixIts.
-struct DiagWithFixIts {
-  clangd::Diagnostic Diag;
-  llvm::SmallVector<TextEdit, 1> FixIts;
-};
-
 using InclusionLocations = std::vector<std::pair<Range, Path>>;
 
 // Stores Preamble and associated data.
 struct PreambleData {
   PreambleData(PrecompiledPreamble Preamble,
                std::vector<serialization::DeclID> TopLevelDeclIDs,
-               std::vector<DiagWithFixIts> Diags,
-               InclusionLocations IncLocations);
+               std::vector<Diag> Diags, InclusionLocations IncLocations);
 
   PrecompiledPreamble Preamble;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
-  std::vector<DiagWithFixIts> Diags;
+  std::vector<Diag> Diags;
   InclusionLocations IncLocations;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.
 struct ParseInputs {
   tooling::CompileCommand CompileCommand;
   IntrusiveRefCntPtr<vfs::FileSystem> FS;
   std::string Contents;
public:
   std::shared_ptr<Preprocessor> getPreprocessorPtr();
   const Preprocessor &getPreprocessor() const;
 
   /// This function returns all top-level decls, including those that come
   /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
   /// this call might be expensive.
   ArrayRef<const Decl *> getTopLevelDecls();
 
-  const std::vector<DiagWithFixIts> &getDiagnostics() const;
+  const std::vector<Diag> &getDiagnostics() const;
 
   /// Returns the esitmated size of the AST and the accessory structures, in
   /// bytes. Does not include the size of the preamble.
   std::size_t getUsedBytes() const;
   const InclusionLocations &getInclusionLocations() const;
 
 private:
   ParsedAST(std::shared_ptr<const PreambleData> Preamble,
             std::unique_ptr<CompilerInstance> Clang,
             std::unique_ptr<FrontendAction> Action,
-            std::vector<const Decl *> TopLevelDecls,
-            std::vector<DiagWithFixIts> Diags, InclusionLocations IncLocations);
+            std::vector<const Decl *> TopLevelDecls, std::vector<Diag> Diags,
+            InclusionLocations IncLocations);
 
 private:
   void ensurePreambleDeclsDeserialized();
 
   // In-memory preambles must outlive the AST, it is important that this member
   // goes before Clang and Action.
   std::shared_ptr<const PreambleData> Preamble;
   // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called
   // on it) and CompilerInstance used to run it. That way we don't have to do
   // complex memory management of all Clang structures on our own. (They are
   // stored in CompilerInstance and cleaned up by
   // FrontendAction.EndSourceFile).
   std::unique_ptr<CompilerInstance> Clang;
   std::unique_ptr<FrontendAction> Action;
 
   // Data, stored after parsing.
-  std::vector<DiagWithFixIts> Diags;
+  std::vector<Diag> Diags;
   std::vector<const Decl *> TopLevelDecls;
   bool PreambleDeclsDeserialized;
   InclusionLocations IncLocations;
 };
 
 using ASTParsedCallback = std::function<void(PathRef Path, ParsedAST *)>;
 
 /// Manages resources, required by clangd. Allows to rebuild file with new
 /// contents, and provides AST and Preamble for it.
 class CppFile {
 public:
   CppFile(PathRef FileName, bool StorePreamblesInMemory,
           std::shared_ptr<PCHContainerOperations> PCHs,
           ASTParsedCallback ASTCallback);
 
   /// Rebuild the AST and the preamble.
   /// Returns a list of diagnostics or llvm::None, if an error occured.
-  llvm::Optional<std::vector<DiagWithFixIts>> rebuild(ParseInputs &&Inputs);
+  llvm::Optional<std::vector<Diag>> rebuild(ParseInputs &&Inputs);
   /// Returns the last built preamble.
   const std::shared_ptr<const PreambleData> &getPreamble() const;
   /// Returns the last built AST.
   ParsedAST *getAST() const;
   /// Returns an estimated size, in bytes, currently occupied by the AST and the
   /// Preamble.
   std::size_t getUsedBytes() const;
 

clangd/ClangdUnit.cpp

 //===--- ClangdUnit.cpp -----------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #include "ClangdUnit.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "Logger.h"
+#include "SourceCode.h"
 #include "Trace.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
                                                  StringRef InFile) override {
     return llvm::make_unique<DeclTrackingASTConsumer>(/*ref*/ TopLevelDecls);
   }
 
 private:
   std::vector<const Decl *> TopLevelDecls;
 };
 
-// Converts a half-open clang source range to an LSP range.
-// Note that clang also uses closed source ranges, which this can't handle!
-Range toRange(CharSourceRange R, const SourceManager &M) {
-  // Clang is 1-based, LSP uses 0-based indexes.
-  Position Begin;
-  Begin.line = static_cast<int>(M.getSpellingLineNumber(R.getBegin())) - 1;
-  Begin.character =
-      static_cast<int>(M.getSpellingColumnNumber(R.getBegin())) - 1;
-
-  Position End;
-  End.line = static_cast<int>(M.getSpellingLineNumber(R.getEnd())) - 1;
-  End.character = static_cast<int>(M.getSpellingColumnNumber(R.getEnd())) - 1;
-
-  return {Begin, End};
-}
-
 class InclusionLocationsCollector : public PPCallbacks {
 public:
   InclusionLocationsCollector(SourceManager &SourceMgr,
                               InclusionLocations &IncLocations)
       : SourceMgr(SourceMgr), IncLocations(IncLocations) {}
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
                           const Module *Imported) override {
     auto SR = FilenameRange.getAsRange();
     if (SR.isInvalid() || !File || File->tryGetRealPathName().empty())
       return;
 
     if (SourceMgr.isInMainFile(SR.getBegin())) {
       // Only inclusion directives in the main file make sense. The user cannot
       // select directives not in the main file.
-      IncLocations.emplace_back(toRange(FilenameRange, SourceMgr),
+      IncLocations.emplace_back(halfOpenToRange(SourceMgr, FilenameRange),
                                 File->tryGetRealPathName());
     }
   }
 
 private:
   SourceManager &SourceMgr;
   InclusionLocations &IncLocations;
 };

 
 private:
   std::vector<Decl *> TopLevelDecls;
   std::vector<serialization::DeclID> TopLevelDeclIDs;
   InclusionLocations IncLocations;
   SourceManager *SourceMgr = nullptr;
 };
 
-/// Convert from clang diagnostic level to LSP severity.
-static int getSeverity(DiagnosticsEngine::Level L) {
-  switch (L) {
-  case DiagnosticsEngine::Remark:
-    return 4;
-  case DiagnosticsEngine::Note:
-    return 3;
-  case DiagnosticsEngine::Warning:
-    return 2;
-  case DiagnosticsEngine::Fatal:
-  case DiagnosticsEngine::Error:
-    return 1;
-  case DiagnosticsEngine::Ignored:
-    return 0;
-  }
-  llvm_unreachable("Unknown diagnostic level!");
-}
-
-// Checks whether a location is within a half-open range.
-// Note that clang also uses closed source ranges, which this can't handle!
-bool locationInRange(SourceLocation L, CharSourceRange R,
-                     const SourceManager &M) {
-  assert(R.isCharRange());
-  if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) ||
-      M.getFileID(R.getBegin()) != M.getFileID(L))
-    return false;
-  return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
-}
-
-// Clang diags have a location (shown as ^) and 0 or more ranges (~~~~).
-// LSP needs a single range.
-Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
-  auto &M = D.getSourceManager();
-  auto Loc = M.getFileLoc(D.getLocation());
-  // Accept the first range that contains the location.
-  for (const auto &CR : D.getRanges()) {
-    auto R = Lexer::makeFileCharRange(CR, M, L);
-    if (locationInRange(Loc, R, M))
-      return toRange(R, M);
-  }
-  // The range may be given as a fixit hint instead.
-  for (const auto &F : D.getFixItHints()) {
-    auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
-    if (locationInRange(Loc, R, M))
-      return toRange(R, M);
-  }
-  // If no suitable range is found, just use the token at the location.
-  auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
-  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
-    R = CharSourceRange::getCharRange(Loc);
-  return toRange(R, M);
-}
-
-TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
-                    const LangOptions &L) {
-  TextEdit Result;
-  Result.range = toRange(Lexer::makeFileCharRange(FixIt.RemoveRange, M, L), M);
-  Result.newText = FixIt.CodeToInsert;
-  return Result;
-}
-
-llvm::Optional<DiagWithFixIts> toClangdDiag(const clang::Diagnostic &D,
-                                            DiagnosticsEngine::Level Level,
-                                            const LangOptions &LangOpts) {
-  if (!D.hasSourceManager() || !D.getLocation().isValid() ||
-      !D.getSourceManager().isInMainFile(D.getLocation())) {
-    IgnoreDiagnostics::log(Level, D);
-    return llvm::None;
-  }
-
-  SmallString<64> Message;
-  D.FormatDiagnostic(Message);
-
-  DiagWithFixIts Result;
-  Result.Diag.range = diagnosticRange(D, LangOpts);
-  Result.Diag.severity = getSeverity(Level);
-  Result.Diag.message = Message.str();
-  for (const FixItHint &Fix : D.getFixItHints())
-    Result.FixIts.push_back(toTextEdit(Fix, D.getSourceManager(), LangOpts));
-  return std::move(Result);
-}
-
-class StoreDiagsConsumer : public DiagnosticConsumer {
-public:
-  StoreDiagsConsumer(std::vector<DiagWithFixIts> &Output) : Output(Output) {}
-
-  // Track language options in case we need to expand token ranges.
-  void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override {
-    LangOpts = Opts;
-  }
-
-  void EndSourceFile() override { LangOpts = llvm::None; }
-
-  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                        const clang::Diagnostic &Info) override {
-    DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
-
-    if (LangOpts)
-      if (auto D = toClangdDiag(Info, DiagLevel, *LangOpts))
-        Output.push_back(std::move(*D));
-  }
-
-private:
-  std::vector<DiagWithFixIts> &Output;
-  llvm::Optional<LangOptions> LangOpts;
-};
-
 } // namespace
 
 void clangd::dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
   AST.getASTContext().getTranslationUnitDecl()->dump(OS, true);
 }
 
 llvm::Optional<ParsedAST>
 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) {
-  std::vector<DiagWithFixIts> ASTDiags;
-  StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ 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);
+
+  StoreDiags ASTDiags;
+  auto Clang =
+      prepareCompilerInstance(std::move(CI), PreamblePCH, std::move(Buffer),
+                              std::move(PCHs), std::move(VFS), ASTDiags);
   if (!Clang)
     return llvm::None;
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance> CICleanup(
       Clang.get());
 
   auto Action = llvm::make_unique<ClangdFrontendAction>();
Clang->getPreprocessor().addPPCallbacks(
                                                      IncLocations));
 
   if (!Action->Execute())
     log("Execute() failed when building AST for " + MainInput.getFile());
 
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
+  // CompilerInstance won't run this callback, do it directly.
+  ASTDiags.EndSourceFile();
 
   std::vector<const Decl *> ParsedDecls = Action->takeTopLevelDecls();
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-                   std::move(ParsedDecls), std::move(ASTDiags),
+                   std::move(ParsedDecls), ASTDiags.take(),
                    std::move(IncLocations));
 }
 
 namespace {
 
 SourceLocation getMacroArgExpandedLocation(const SourceManager &Mgr,
                                            const FileEntry *FE, Position Pos) {
   SourceLocation InputLoc =
const Preprocessor &ParsedAST::getPreprocessor() const {
   return Clang->getPreprocessor();
 }
 
 ArrayRef<const Decl *> ParsedAST::getTopLevelDecls() {
   ensurePreambleDeclsDeserialized();
   return TopLevelDecls;
 }
 
-const std::vector<DiagWithFixIts> &ParsedAST::getDiagnostics() const {
-  return Diags;
-}
+const std::vector<Diag> &ParsedAST::getDiagnostics() const { return Diags; }
 
 std::size_t ParsedAST::getUsedBytes() const {
   auto &AST = getASTContext();
   // FIXME(ibiryukov): we do not account for the dynamically allocated part of
-  // SmallVector<FixIt> inside each Diag.
+  // Message and Fixes inside each diagnostic.
   return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() +
          ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags);
 }
 
 const InclusionLocations &ParsedAST::getInclusionLocations() const {
   return IncLocations;
 }
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
                            std::vector<serialization::DeclID> TopLevelDeclIDs,
-                           std::vector<DiagWithFixIts> Diags,
+                           std::vector<Diag> Diags,
                            InclusionLocations IncLocations)
     : Preamble(std::move(Preamble)),
       TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
       IncLocations(std::move(IncLocations)) {}
 
 ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
                      std::unique_ptr<CompilerInstance> Clang,
                      std::unique_ptr<FrontendAction> Action,
                      std::vector<const Decl *> TopLevelDecls,
-                     std::vector<DiagWithFixIts> Diags,
-                     InclusionLocations IncLocations)
+                     std::vector<Diag> Diags, InclusionLocations IncLocations)
     : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Diags(std::move(Diags)),
       TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false),
       IncLocations(std::move(IncLocations)) {
   assert(this->Clang);
   assert(this->Action);
 }
 
 CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
                  std::shared_ptr<PCHContainerOperations> PCHs,
                  ASTParsedCallback ASTCallback)
     : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
       PCHs(std::move(PCHs)), ASTCallback(std::move(ASTCallback)) {
   log("Created CppFile for " + FileName);
 }
 
-llvm::Optional<std::vector<DiagWithFixIts>>
-CppFile::rebuild(ParseInputs &&Inputs) {
+llvm::Optional<std::vector<Diag>> CppFile::rebuild(ParseInputs &&Inputs) {
   log("Rebuilding file " + FileName + " with command [" +
       Inputs.CompileCommand.Directory + "] " +
       llvm::join(Inputs.CompileCommand.CommandLine, " "));
 
   std::vector<const char *> ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
     ArgStrs.push_back(S.c_str());
 
llvm::Optional<std::vector<Diag>> CppFile::rebuild(ParseInputs &&Inputs) {
   llvm::Optional<ParsedAST> NewAST;
   {
     trace::Span Tracer("Build");
     SPAN_ATTACH(Tracer, "File", FileName);
     NewAST = ParsedAST::Build(std::move(CI), NewPreamble,
                               std::move(ContentsBuffer), PCHs, Inputs.FS);
   }
 
-  std::vector<DiagWithFixIts> Diagnostics;
+  std::vector<Diag> Diagnostics;
   if (NewAST) {
     // Collect diagnostics from both the preamble and the AST.
     if (NewPreamble)
-      Diagnostics.insert(Diagnostics.begin(), NewPreamble->Diags.begin(),
-                         NewPreamble->Diags.end());
+      Diagnostics = NewPreamble->Diags;
     Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(),
                        NewAST->getDiagnostics().end());
   }
   if (ASTCallback && NewAST) {
     trace::Span Tracer("Running ASTCallback");
     ASTCallback(FileName, NewAST.getPointer());
   }
 
if (OldPreamble && compileCommandsAreEqual(this->Command, Command) &&
     log("Reusing preamble for file " + Twine(FileName));
     return OldPreamble;
   }
   log("Preamble for file " + Twine(FileName) +
       " cannot be reused. Attempting to rebuild it.");
 
   trace::Span Tracer("Preamble");
   SPAN_ATTACH(Tracer, "File", FileName);
-  std::vector<DiagWithFixIts> PreambleDiags;
-  StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
+  StoreDiags PreambleDiagnostics;
   IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
       CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(),
-                                          &PreambleDiagnosticsConsumer, false);
+                                          &PreambleDiagnostics, false);
 
   // Skip function bodies when building the preamble to speed up building
   // the preamble and make it smaller.
   assert(!CI.getFrontendOpts().SkipFunctionBodies);
   CI.getFrontendOpts().SkipFunctionBodies = true;
 
   CppFilePreambleCallbacks SerializedDeclsCollector;
   auto BuiltPreamble = PrecompiledPreamble::Build(
       CI, &ContentsBuffer, Bounds, *PreambleDiagsEngine, FS, PCHs,
       /*StoreInMemory=*/StorePreamblesInMemory, SerializedDeclsCollector);
 
   // When building the AST for the main file, we do want the function
   // bodies.
   CI.getFrontendOpts().SkipFunctionBodies = false;
 
   if (BuiltPreamble) {
     log("Built preamble of size " + Twine(BuiltPreamble->getSize()) +
         " for file " + Twine(FileName));
 
     return std::make_shared<PreambleData>(
         std::move(*BuiltPreamble),
         SerializedDeclsCollector.takeTopLevelDeclIDs(),
-        std::move(PreambleDiags),
+        PreambleDiagnostics.take(),
         SerializedDeclsCollector.takeInclusionLocations());
   } else {
     log("Could not build a preamble for file " + Twine(FileName));
     return nullptr;
   }
 }
 
 SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit,

clangd/Diagnostics.h

+//===--- Diagnostics.h ------------------------------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H
+
+#include "Path.h"
+#include "Protocol.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringSet.h"
+#include <cassert>
+#include <string>
+
+namespace clang {
+namespace clangd {
+
+/// 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;
+  // Since File is only descriptive, we store a separate flag to distinguish
+  // diags from the main file.
+  bool InsideMainFile = false;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const DiagBase &D);
+
+/// Represents a single fix-it that editor can apply to fix the error.
+struct Fix {
+  /// Message for the fix-it.
+  std::string Message;
+  /// TextEdits from clang's fix-its. Must be non-empty.
+  llvm::SmallVector<TextEdit, 1> Edits;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Fix &F);
+
+/// Represents a note for the diagnostic. Severity of notes can only be 'note'
+/// or 'remark'.
+struct Note : DiagBase {};
+
+/// A top-level diagnostic that may have Notes and Fixes.
+struct Diag : DiagBase {
+  /// Elaborate on the problem, usually pointing to a related piece of code.
+  std::vector<Note> Notes;
+  /// *Alternative* fixes for this diagnostic, one should be chosen.
+  std::vector<Fix> Fixes;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D);
+
+/// Conversion to LSP diagnostics. Formats the error message of each diagnostic
+/// to include all its notes. Notes inside main file are also provided as
+/// separate diagnostics with their corresponding fixits. Notes outside main
+/// file do not have a corresponding LSP diagnostic, but can still be included
+/// as part of their main diagnostic's message.
+void toLSPDiags(
+    const Diag &D,
+    llvm::function_ref<void(clangd::Diagnostic, llvm::ArrayRef<Fix>)> OutFn);
+
+/// Convert from clang diagnostic level to LSP severity.
+int getSeverity(DiagnosticsEngine::Level L);
+
+/// StoreDiags collects the diagnostics that can later be reported by
+/// clangd. It groups all notes for a diagnostic into a single Diag
+/// and filters out diagnostics that don't mention the main file (i.e. neither
+/// the diag itself nor its notes are in the main file).
+class StoreDiags : public DiagnosticConsumer {
+public:
+  std::vector<Diag> take();
+
+  void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override;
+  void EndSourceFile() override;
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+                        const clang::Diagnostic &Info) override;
+
+private:
+  bool shouldIgnore(DiagnosticsEngine::Level DiagLevel,
+                    const clang::Diagnostic &Info);
+
+  void flushLastDiag();
+
+  std::vector<Diag> Output;
+  llvm::Optional<LangOptions> LangOpts;
+  llvm::Optional<Diag> LastDiag;
+  /// Is any diag or note from LastDiag in the main file?
+  bool LastDiagMentionsMainFile = false;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif

clangd/Diagnostics.cpp

+//===--- Diagnostics.cpp ----------------------------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---------------------------------------------------------------------===//
+
+#include "Diagnostics.h"
+#include "Compiler.h"
+#include "Logger.h"
+#include "SourceCode.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/Support/Capacity.h"
+#include "llvm/Support/Path.h"
+#include <algorithm>
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+bool mentionsMainFile(const Diag &D) {
+  if (D.InsideMainFile)
+    return true;
+  // Fixes are always in the main file.
+  if (!D.Fixes.empty())
+    return true;
+  for (auto &N : D.Notes) {
+    if (N.InsideMainFile)
+      return true;
+  }
+  return false;
+}
+
+// Checks whether a location is within a half-open range.
+// Note that clang also uses closed source ranges, which this can't handle!
+bool locationInRange(SourceLocation L, CharSourceRange R,
+                     const SourceManager &M) {
+  assert(R.isCharRange());
+  if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) ||
+      M.getFileID(R.getBegin()) != M.getFileID(L))
+    return false;
+  return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
+}
+
+// Clang diags have a location (shown as ^) and 0 or more ranges (~~~~).
+// LSP needs a single range.
+Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
+  auto &M = D.getSourceManager();
+  auto Loc = M.getFileLoc(D.getLocation());
+  // Accept the first range that contains the location.
+  for (const auto &CR : D.getRanges()) {
+    auto R = Lexer::makeFileCharRange(CR, M, L);
+    if (locationInRange(Loc, R, M))
+      return halfOpenToRange(M, R);
+  }
+  // The range may be given as a fixit hint instead.
+  for (const auto &F : D.getFixItHints()) {
+    auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
+    if (locationInRange(Loc, R, M))
+      return halfOpenToRange(M, R);
+  }
+  // If no suitable range is found, just use the token at the location.
+  auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
+  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
+    R = CharSourceRange::getCharRange(Loc);
+  return halfOpenToRange(M, R);
+}
+
+TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
+                    const LangOptions &L) {
+  TextEdit Result;
+  Result.range =
+      halfOpenToRange(M, Lexer::makeFileCharRange(FixIt.RemoveRange, M, L));
+  Result.newText = FixIt.CodeToInsert;
+  return Result;
+}
+
+bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
+  return Loc.isValid() && M.isInMainFile(Loc);
+}
+
+bool isInsideMainFile(const clang::Diagnostic &D) {
+  if (!D.hasSourceManager())
+    return false;
+
+  return isInsideMainFile(D.getLocation(), D.getSourceManager());
+}
+
+bool isNote(DiagnosticsEngine::Level L) {
+  return L == DiagnosticsEngine::Note || L == DiagnosticsEngine::Remark;
+}
+
+llvm::StringRef diagLeveltoString(DiagnosticsEngine::Level Lvl) {
+  switch (Lvl) {
+  case DiagnosticsEngine::Ignored:
+    return "ignored";
+  case DiagnosticsEngine::Note:
+    return "note";
+  case DiagnosticsEngine::Remark:
+    return "remark";
+  case DiagnosticsEngine::Warning:
+    return "warning";
+  case DiagnosticsEngine::Error:
+    return "error";
+  case DiagnosticsEngine::Fatal:
+    return "fatal error";
+  }
+  llvm_unreachable("unhandled DiagnosticsEngine::Level");
+}
+
+/// Prints a single diagnostic in a clang-like manner, the output includes
+/// location, severity and error message. An example of the output message is:
+///
+///     main.cpp:12:23: error: undeclared identifier
+///
+/// For main file we only print the basename and for all other files we print
+/// the filename on a separate line to provide a slightly more readable output
+/// in the editors:
+///
+///     dir1/dir2/dir3/../../dir4/header.h:12:23
+///     error: undeclared identifier
+void printDiag(llvm::raw_string_ostream &OS, const DiagBase &D) {
+  if (D.InsideMainFile) {
+    // Paths to main files are often taken from compile_command.json, where they
+    // are typically absolute. To reduce noise we print only basename for them,
+    // it should not be confusing and saves space.
+    OS << llvm::sys::path::filename(D.File) << ":";
+  } else {
+    OS << D.File << ":";
+  }
+  // Note +1 to line and character. clangd::Range is zero-based, but when
+  // printing for users we want one-based indexes.
+  auto Pos = D.Range.start;
+  OS << (Pos.line + 1) << ":" << (Pos.character + 1) << ":";
+  // The non-main-file paths are often too long, putting them on a separate
+  // line improves readability.
+  if (D.InsideMainFile)
+    OS << " ";
+  else
+    OS << "\n";
+  OS << diagLeveltoString(D.Severity) << ": " << D.Message;
+}
+
+/// Returns a message sent to LSP for the main diagnostic in \p D.
+/// The message includes all the notes with their corresponding locations.
+/// However, notes with fix-its are excluded as those usually only contain a
+/// fix-it message and just add noise if included in the message for diagnostic.
+/// Example output:
+///
+///     no matching function for call to 'foo'
+///
+///     main.cpp:3:5: note: candidate function not viable: requires 2 arguments
+///
+///     dir1/dir2/dir3/../../dir4/header.h:12:23
+///     note: candidate function not viable: requires 3 arguments
+std::string mainMessage(const Diag &D) {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  OS << D.Message;
+  for (auto &Note : D.Notes) {
+    OS << "\n\n";
+    printDiag(OS, Note);
+  }
+  OS.flush();
+  return Result;
+}
+
+/// Returns a message sent to LSP for the note of the main diagnostic.
+/// The message includes the main diagnostic to provide the necessary context
+/// for the user to understand the note.
+std::string noteMessage(const Diag &Main, const DiagBase &Note) {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  OS << Note.Message;
+  OS << "\n\n";
+  printDiag(OS, Main);
+  OS.flush();
+  return Result;
+}
+} // namespace
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const DiagBase &D) {
+  if (!D.InsideMainFile)
+    OS << "[in " << D.File << "] ";
+  return OS << D.Message;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Fix &F) {
+  OS << F.Message << " {";
+  const char *Sep = "";
+  for (const auto &Edit : F.Edits) {
+    OS << Sep << Edit;
+    Sep = ", ";
+  }
+  return OS << "}";
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D) {
+  OS << static_cast<const DiagBase &>(D);
+  if (!D.Notes.empty()) {
+    OS << ", notes: {";
+    const char *Sep = "";
+    for (auto &Note : D.Notes) {
+      OS << Sep << Note;
+      Sep = ", ";
+    }
+    OS << "}";
+  }
+  if (!D.Fixes.empty()) {
+    OS << ", fixes: {";
+    const char *Sep = "";
+    for (auto &Fix : D.Fixes) {
+      OS << Sep << Fix;
+      Sep = ", ";
+    }
+  }
+  return OS;
+}
+
+void toLSPDiags(
+    const Diag &D,
+    llvm::function_ref<void(clangd::Diagnostic, llvm::ArrayRef<Fix>)> OutFn) {
+  auto FillBasicFields = [](const DiagBase &D) -> clangd::Diagnostic {
+    clangd::Diagnostic Res;
+    Res.range = D.Range;
+    Res.severity = getSeverity(D.Severity);
+    return Res;
+  };
+
+  {
+    clangd::Diagnostic Main = FillBasicFields(D);
+    Main.message = mainMessage(D);
+    OutFn(std::move(Main), D.Fixes);
+  }
+
+  for (auto &Note : D.Notes) {
+    if (!Note.InsideMainFile)
+      continue;
+    clangd::Diagnostic Res = FillBasicFields(Note);
+    Res.message = noteMessage(D, Note);
+    OutFn(std::move(Res), llvm::ArrayRef<Fix>());
+  }
+}
+
+int getSeverity(DiagnosticsEngine::Level L) {
+  switch (L) {
+  case DiagnosticsEngine::Remark:
+    return 4;
+  case DiagnosticsEngine::Note:
+    return 3;
+  case DiagnosticsEngine::Warning:
+    return 2;
+  case DiagnosticsEngine::Fatal:
+  case DiagnosticsEngine::Error:
+    return 1;
+  case DiagnosticsEngine::Ignored:
+    return 0;
+  }
+  llvm_unreachable("Unknown diagnostic level!");
+}
+
+std::vector<Diag> StoreDiags::take() { return std::move(Output); }
+
+void StoreDiags::BeginSourceFile(const LangOptions &Opts,
+                                 const Preprocessor *) {
+  LangOpts = Opts;
+}
+
+void StoreDiags::EndSourceFile() {
+  flushLastDiag();
+  LangOpts = llvm::None;
+}
+
+void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+                                  const clang::Diagnostic &Info) {
+  DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
+
+  if (!LangOpts || !Info.hasSourceManager()) {
+    IgnoreDiagnostics::log(DiagLevel, Info);
+    return;
+  }
+
+  bool InsideMainFile = isInsideMainFile(Info);
+
+  auto FillDiagBase = [&](DiagBase &D) {
+    D.Range = diagnosticRange(Info, *LangOpts);
+    llvm::SmallString<64> Message;
+    Info.FormatDiagnostic(Message);
+    D.Message = Message.str();
+    D.InsideMainFile = InsideMainFile;
+    D.File = Info.getSourceManager().getFilename(Info.getLocation());
+    D.Severity = DiagLevel;
+    return D;
+  };
+
+  auto AddFix = [&]() -> bool {
+    assert(!Info.getFixItHints().empty() &&
+           "diagnostic does not have attached fix-its");
+    if (!InsideMainFile)
+      return false;
+
+    llvm::SmallVector<TextEdit, 1> Edits;
+    for (auto &FixIt : Info.getFixItHints()) {
+      if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
+                            Info.getSourceManager()))
+        return false;
+      Edits.push_back(toTextEdit(FixIt, Info.getSourceManager(), *LangOpts));
+    }
+
+    llvm::SmallString<64> Message;
+    Info.FormatDiagnostic(Message);
+    LastDiag->Fixes.push_back(Fix{Message.str(), std::move(Edits)});
+    return true;
+  };
+
+  if (!isNote(DiagLevel)) {
+    // Handle the new main diagnostic.
+    flushLastDiag();
+
+    LastDiag = Diag();
+    FillDiagBase(*LastDiag);
+
+    if (!Info.getFixItHints().empty())
+      AddFix();
+  } else {
+    // Handle a note to an existing diagnostic.
+    if (!LastDiag) {
+      assert(false && "Adding a note without main diagnostic");
+      IgnoreDiagnostics::log(DiagLevel, Info);
+      return;
+    }
+
+    if (!Info.getFixItHints().empty()) {
+      // A clang note with fix-it is not a separate diagnostic in clangd. We
+      // attach it as a Fix to the main diagnostic instead.
+      if (!AddFix())
+        IgnoreDiagnostics::log(DiagLevel, Info);
+    } else {
+      // A clang note without fix-its corresponds to clangd::Note.
+      Note N;
+      FillDiagBase(N);
+
+      LastDiag->Notes.push_back(std::move(N));
+    }
+  }
+}
+
+void StoreDiags::flushLastDiag() {
+  if (!LastDiag)
+    return;
+  if (mentionsMainFile(*LastDiag))
+    Output.push_back(std::move(*LastDiag));
+  else
+    log(Twine("Dropped diagnostic outside main file:") + LastDiag->File + ":" +
+        LastDiag->Message);
+  LastDiag.reset();
+}
+
+} // namespace clangd
+} // namespace clang

clangd/Protocol.h

struct Range {
   Position start;
 
   /// The range's end position.
   Position end;
 
   friend bool operator==(const Range &LHS, const Range &RHS) {
     return std::tie(LHS.start, LHS.end) == std::tie(RHS.start, RHS.end);
   }
+  friend bool operator!=(const Range &LHS, const Range &RHS) {
+    return !(LHS == RHS);
+  }
   friend bool operator<(const Range &LHS, const Range &RHS) {
     return std::tie(LHS.start, LHS.end) < std::tie(RHS.start, RHS.end);
   }
 
   bool contains(Position Pos) const { return start <= Pos && Pos < end; }
 };
 bool fromJSON(const json::Expr &, Range &);
 json::Expr toJSON(const Range &);
struct TextEdit {
   /// The range of the text document to be manipulated. To insert
   /// text into a document create a range where start === end.
   Range range;
 
   /// The string to be inserted. For delete operations use an
   /// empty string.
   std::string newText;
 };
 bool fromJSON(const json::Expr &, TextEdit &);
 json::Expr toJSON(const TextEdit &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const TextEdit &);
 
 struct TextDocumentItem {
   /// The text document's URI.
   URIForFile uri;
 
   /// The text document's language identifier.
struct Diagnostic {
   /// A human-readable string describing the source of this
   /// diagnostic, e.g. 'typescript' or 'super lint'.
   /// Note: Not currently used by clangd
   // std::string source;
 
   /// The diagnostic's message.
   std::string message;
 };
+
 /// A LSP-specific comparator used to find diagnostic in a container like
 /// std:map.
 /// We only use the required fields of Diagnostic to do the comparsion to avoid
 /// any regression issues from LSP clients (e.g. VScode), see
 /// https://git.io/vbr29
 struct LSPDiagnosticCompare {
-  bool operator()(const Diagnostic& LHS, const Diagnostic& RHS) const {
+  bool operator()(const Diagnostic &LHS, const Diagnostic &RHS) const {
     return std::tie(LHS.range, LHS.message) < std::tie(RHS.range, RHS.message);
   }
 };
 bool fromJSON(const json::Expr &, Diagnostic &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Diagnostic &);
 
 struct CodeActionContext {
   /// An array of diagnostics.

clangd/SourceCode.h

 size_t positionToOffset(llvm::StringRef Code, Position P);
 
 /// Turn an offset in Code into a [line, column] pair.
 Position offsetToPosition(llvm::StringRef Code, size_t Offset);
 
 /// Turn a SourceLocation into a [line, column] pair.
 Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc);
 
+// Converts a half-open clang source range to an LSP range.
+// Note that clang also uses closed source ranges, which this can't handle!
+Range halfOpenToRange(const SourceManager &SM, CharSourceRange R);
+
 } // namespace clangd
 } // namespace clang
 #endif

clangd/SourceCode.cpp

 
 Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc) {
   Position P;
   P.line = static_cast<int>(SM.getSpellingLineNumber(Loc)) - 1;
   P.character = static_cast<int>(SM.getSpellingColumnNumber(Loc)) - 1;
   return P;
 }
 
+Range halfOpenToRange(const SourceManager &SM, CharSourceRange R) {
+  // Clang is 1-based, LSP uses 0-based indexes.
+  Position Begin = sourceLocToPosition(SM, R.getBegin());
+  Position End = sourceLocToPosition(SM, R.getEnd());
+
+  return {Begin, End};
+}
+
 } // namespace clangd
 } // namespace clang

clangd/TUScheduler.h

public:
   /// Returns estimated memory usage for each of the currently open files.
   /// The order of results is unspecified.
   std::vector<std::pair<Path, std::size_t>> getUsedBytesPerFile() const;
 
   /// Schedule an update for \p File. Adds \p File to a list of tracked files if
   /// \p File was not part of it before.
   /// FIXME(ibiryukov): remove the callback from this function.
   void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD,
-              UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated);
+              UniqueFunction<void(std::vector<Diag>)> OnUpdated);
 
   /// Remove \p File from the list of tracked files and schedule removal of its
   /// resources.
   void remove(PathRef File);
 
   /// Schedule an async read of the AST. \p Action will be called when AST is
   /// ready. The AST passed to \p Action refers to the version of \p File
   /// tracked at the time of the call, even if new updates are received before

clangd/TUScheduler.cpp

public:
   /// synchronously instead. \p Barrier is acquired when processing each
   /// request, it is be used to limit the number of actively running threads.
   static ASTWorkerHandle Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
                                 Semaphore &Barrier, CppFile AST,
                                 steady_clock::duration UpdateDebounce);
   ~ASTWorker();
 
   void update(ParseInputs Inputs, WantDiagnostics,
-              UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated);
+              UniqueFunction<void(std::vector<Diag>)> OnUpdated);
   void runWithAST(llvm::StringRef Name,
                   UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action);
   bool blockUntilIdle(Deadline Timeout) const;
 
   std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
   std::size_t getUsedBytes() const;
 
 private:
private:
   // Inputs, corresponding to the current state of AST.
   ParseInputs FileInputs;
   // Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
   std::shared_ptr<const PreambleData> LastBuiltPreamble; /* GUARDED_BY(Mutex) */
   // Result of getUsedBytes() after the last rebuild or read of AST.
   std::size_t LastASTSize; /* GUARDED_BY(Mutex) */
   // Set to true to signal run() to finish processing.
-  bool Done;                           /* GUARDED_BY(Mutex) */
-  std::deque<Request> Requests;        /* GUARDED_BY(Mutex) */
+  bool Done;                    /* GUARDED_BY(Mutex) */
+  std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
   mutable std::condition_variable RequestsCV;
 };
 
 /// A smart-pointer-like class that points to an active ASTWorker.
 /// In destructor, signals to the underlying ASTWorker that no new requests will
 /// be sent and the processing loop may exit (after running all pending
 /// requests).
 class ASTWorkerHandle {

 ASTWorker::~ASTWorker() {
 #ifndef NDEBUG
   std::lock_guard<std::mutex> Lock(Mutex);
   assert(Done && "handle was not destroyed");
   assert(Requests.empty() && "unprocessed requests when destroying ASTWorker");
 #endif
 }
 
-void ASTWorker::update(
-    ParseInputs Inputs, WantDiagnostics WantDiags,
-    UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated) {
+void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags,
+                       UniqueFunction<void(std::vector<Diag>)> OnUpdated) {
   auto Task = [=](decltype(OnUpdated) OnUpdated) mutable {
     FileInputs = Inputs;
     auto Diags = AST.rebuild(std::move(Inputs));
 
     {
       std::lock_guard<std::mutex> Lock(Mutex);
       if (AST.getPreamble())
         LastBuiltPreamble = AST.getPreamble();
for (auto &File : Files)
     if (!File.getValue()->Worker->blockUntilIdle(D))
       return false;
   if (PreambleTasks)
     if (!PreambleTasks->wait(D))
       return false;
   return true;
 }
 
-void TUScheduler::update(
-    PathRef File, ParseInputs Inputs, WantDiagnostics WantDiags,
-    UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated) {
+void TUScheduler::update(PathRef File, ParseInputs Inputs,
+                         WantDiagnostics WantDiags,
+                         UniqueFunction<void(std::vector<Diag>)> OnUpdated) {
   std::unique_ptr<FileData> &FD = Files[File];
   if (!FD) {
     // Create a new worker to process the AST-related tasks.
     ASTWorkerHandle Worker = ASTWorker::Create(
         File, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
         CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback),
         UpdateDebounce);
     FD = std::unique_ptr<FileData>(new FileData{Inputs, std::move(Worker)});

test/clangd/diagnostics.test

 # CHECK-NEXT:            "character": 4,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # 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": 4,
-# CHECK-NEXT:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:  }
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}

test/clangd/execute-command.test

 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 32,
 # 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:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:  }
 ---
 # No command name
 {"jsonrpc":"2.0","id":3,"method":"workspace/executeCommand","params":{}}
 ---

test/clangd/extra-flags.test

 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # 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:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:  }
 ---
 {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///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:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # 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:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:  }
 ---
 {"jsonrpc":"2.0","id":5,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0":"method":"exit"}
 
 

test/clangd/fixits.test

 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 32,
 # 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:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:            "line": 0
-# CHECK-NEXT:          },
-# CHECK-NEXT:          "start": {
-# 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:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "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":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"}]}}}
 #      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:              }
 # CHECK-NEXT:            ]
 # CHECK-NEXT:          }
 # CHECK-NEXT:        }
 # CHECK-NEXT:      ],
 # CHECK-NEXT:      "command": "clangd.applyFix",
-# CHECK-NEXT:      "title": "Apply FixIt place parentheses around the assignment to silence this warning"
+# CHECK-NEXT:      "title": "Apply fix: place parentheses around the assignment to silence this warning"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
 # CHECK-NEXT:          "changes": {
 # CHECK-NEXT:            "file://{{.*}}/foo.c": [
 # CHECK-NEXT:              {
 # CHECK-NEXT:                "newText": "==",
 # CHECK-NEXT:                "range": {
 # CHECK-NEXT:                  "end": {
 # CHECK-NEXT:                    "character": 35,
 # CHECK-NEXT:                    "line": 0
 # CHECK-NEXT:                  },
 # CHECK-NEXT:                  "start": {
 # CHECK-NEXT:                    "character": 34,
 # CHECK-NEXT:                    "line": 0
 # CHECK-NEXT:                  }
 # CHECK-NEXT:                }
 # CHECK-NEXT:              }
 # CHECK-NEXT:            ]
 # CHECK-NEXT:          }
 # 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:      "title": "Apply fix: use '==' to turn this assignment into an equality comparison"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  ]
 ---
-{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"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": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "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":"test:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"}]}}}
 # 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": {

 # CHECK-NEXT:                  }
 # CHECK-NEXT:                }
 # CHECK-NEXT:              }
 # CHECK-NEXT:            ]
 # CHECK-NEXT:          }
 # CHECK-NEXT:        }
 # CHECK-NEXT:      ],
 # CHECK-NEXT:      "command": "clangd.applyFix",
-# CHECK-NEXT:      "title": "Apply FixIt place parentheses around the assignment to silence this warning"
+# CHECK-NEXT:      "title": "Apply fix: place parentheses around the assignment to silence this warning"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "arguments": [
 # CHECK-NEXT:        {
 # CHECK-NEXT:          "changes": {
 # CHECK-NEXT:            "file://{{.*}}/foo.c": [
 # CHECK-NEXT:              {
 # CHECK-NEXT:                "newText": "==",
 # CHECK-NEXT:                "range": {
 # CHECK-NEXT:                  "end": {
 # CHECK-NEXT:                    "character": 35,
 # CHECK-NEXT:                    "line": 0
 # CHECK-NEXT:                  },
 # CHECK-NEXT:                  "start": {
 # CHECK-NEXT:                    "character": 34,
 # CHECK-NEXT:                    "line": 0
 # CHECK-NEXT:                  }
 # CHECK-NEXT:                }
 # CHECK-NEXT:              }
 # CHECK-NEXT:            ]
 # CHECK-NEXT:          }
 # 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:      "title": "Apply fix: use '==' to turn this assignment into an equality comparison"
 # CHECK-NEXT:    }
 # CHECK-NEXT:  ]
 ---
 {"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyFix","arguments":[{"changes":{"test:///foo.c":[{"range":{"start":{"line":0,"character":32},"end":{"line":0,"character":32}},"newText":"("},{"range":{"start":{"line":0,"character":37},"end":{"line":0,"character":37}},"newText":")"}]}}]}}
 #      CHECK:  "id": 4,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": "Fix applied."
 #

unittests/clangd/ClangdTests.cpp

 using ::testing::Eq;
 using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::Pair;
 using ::testing::UnorderedElementsAre;
 
 namespace {
 
-static bool diagsContainErrors(ArrayRef<DiagWithFixIts> Diagnostics) {
-  for (const auto &DiagAndFixIts : Diagnostics) {
-    // FIXME: severities returned by clangd should have a descriptive
-    // diagnostic severity enum
-    const int ErrorSeverity = 1;
-    if (DiagAndFixIts.Diag.severity == ErrorSeverity)
+static bool diagsContainErrors(const std::vector<Diag> &Diagnostics) {
+  for (auto D : Diagnostics) {
+    if (D.Severity == DiagnosticsEngine::Error ||
+        D.Severity == DiagnosticsEngine::Fatal)
       return true;
   }
   return false;
 }
 
 class ErrorCheckingDiagConsumer : public DiagnosticsConsumer {
 public:
-  void
-  onDiagnosticsReady(PathRef File,
-                     Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+  void onDiagnosticsReady(PathRef File,
+                          Tagged<std::vector<Diag>> Diagnostics) override {
     bool HadError = diagsContainErrors(Diagnostics.Value);
 
     std::lock_guard<std::mutex> Lock(Mutex);
     HadErrorInLastDiags = HadError;
     LastVFSTag = Diagnostics.Tag;
   }
 
   bool hadErrorInLastDiags() {
     std::lock_guard<std::mutex> Lock(Mutex);
     return HadErrorInLastDiags;
   }
 
   VFSTag lastVFSTag() { return LastVFSTag; }
 
 private:
   std::mutex Mutex;
   bool HadErrorInLastDiags = false;
   VFSTag LastVFSTag = VFSTag();
 };
 
 /// For each file, record whether the last published diagnostics contained at
 /// least one error.
 class MultipleErrorCheckingDiagConsumer : public DiagnosticsConsumer {
 public:
-  void
-  onDiagnosticsReady(PathRef File,
-                     Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+  void onDiagnosticsReady(PathRef File,
+                          Tagged<std::vector<Diag>> Diagnostics) override {
     bool HadError = diagsContainErrors(Diagnostics.Value);
 
     std::lock_guard<std::mutex> Lock(Mutex);
     LastDiagsHadError[File] = HadError;
   }
 
   /// Exposes all files consumed by onDiagnosticsReady in an unspecified order.
   /// For each file, a bool value indicates whether the last diagnostics
struct FileStat {
     unsigned HitsWithErrors = 0;
     bool HadErrorsInLastDiags = false;
   };
 
   class TestDiagConsumer : public DiagnosticsConsumer {
   public:
     TestDiagConsumer() : Stats(FilesCount, FileStat()) {}
 
-    void onDiagnosticsReady(
-        PathRef File,
-        Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+    void onDiagnosticsReady(PathRef File,
+                            Tagged<std::vector<Diag>> Diagnostics) override {
       StringRef FileIndexStr = llvm::sys::path::stem(File);
       ASSERT_TRUE(FileIndexStr.consume_front("Foo"));
 
       unsigned long FileIndex = std::stoul(FileIndexStr.str());
 
       bool HadError = diagsContainErrors(Diagnostics.Value);
 
       std::lock_guard<std::mutex> Lock(Mutex);

 TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
   class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
   public:
     std::atomic<int> Count = {0};
 
     NoConcurrentAccessDiagConsumer(std::promise<void> StartSecondReparse)
         : StartSecondReparse(std::move(StartSecondReparse)) {}
 
-    void onDiagnosticsReady(PathRef,
-                            Tagged<std::vector<DiagWithFixIts>>) override {
+    void onDiagnosticsReady(PathRef, Tagged<std::vector<Diag>>) override {
       ++Count;
       std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock_t());
       ASSERT_TRUE(Lock.owns_lock())
           << "Detected concurrent onDiagnosticsReady calls for the same file.";
 
       // If we started the second parse immediately, it might cancel the first.
       // So we don't allow it to start until the first has delivered diags...
       if (FirstRequest) {
TEST_F(ClangdVFSTest, InsertIncludes) {
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
   std::string SearchDirArg = (llvm::Twine("-I") + testRoot()).str();
   CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(), {SearchDirArg.c_str()});
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto FooCpp = testPath("foo.cpp");
   const auto Code = R"cpp(
-#include "z.h"
 #include "x.h"
+#include "z.h"
 
 void f() {}
 )cpp";
   FS.Files[FooCpp] = Code;
   runAddDocument(Server, FooCpp, Code);
 
   auto ChangedCode = [&](llvm::StringRef Original, llvm::StringRef Preferred) {
     auto Replaces = Server.insertInclude(
         FooCpp, Code, Original, Preferred.empty() ? Original : Preferred);
     EXPECT_TRUE(static_cast<bool>(Replaces));
     auto Changed = tooling::applyAllReplacements(Code, *Replaces);
     EXPECT_TRUE(static_cast<bool>(Changed));
     return *Changed;
   };
   auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred,
                       llvm::StringRef Expected) {
     return llvm::StringRef(ChangedCode(Original, Preferred))
         .contains((llvm::Twine("#include ") + Expected + "").str());
   };
 
-  EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"","\"y.h\""));
+  EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"", "\"y.h\""));
   EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"\"Y.h\"", "\"Y.h\""));
   EXPECT_TRUE(Inserted("<string>", /*Preferred=*/"", "<string>"));
   EXPECT_TRUE(Inserted("<string>", /*Preferred=*/"", "<string>"));
 
   std::string OriginalHeader = URI::createFile(testPath("y.h")).toString();
   std::string PreferredHeader = URI::createFile(testPath("Y.h")).toString();
   EXPECT_TRUE(Inserted(OriginalHeader,
                        /*Preferred=*/"", "\"y.h\""));

 TEST_F(ClangdVFSTest, FormatCode) {
   MockFSProvider FS;
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
   auto Path = testPath("foo.cpp");
   std::string Code = R"cpp(
-#include "y.h"
 #include "x.h"
+#include "y.h"
 
 void f(  )  {}
 )cpp";
   std::string Expected = R"cpp(
 #include "x.h"
 #include "y.h"
 
 void f() {}

unittests/clangd/ClangdUnitTests.cpp

 //===-- ClangdUnitTests.cpp - ClangdUnit tests ------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#include "ClangdUnit.h"
 #include "Annotations.h"
+#include "ClangdUnit.h"
 #include "TestFS.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Frontend/Utils.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 using namespace llvm;
-void PrintTo(const DiagWithFixIts &D, std::ostream *O) {
-  llvm::raw_os_ostream OS(*O);
-  OS << D.Diag;
-  if (!D.FixIts.empty()) {
-    OS << " {";
-    const char *Sep = "";
-    for (const auto &F : D.FixIts) {
-      OS << Sep << F;
-      Sep = ", ";
-    }
-    OS << "}";
-  }
-}
 
 namespace {
 using testing::ElementsAre;
+using testing::Field;
+using testing::IsEmpty;
+using testing::Pair;
+
+testing::Matcher<const Diag &> WithFix(testing::Matcher<Fix> FixMatcher) {
+  return Field(&Diag::Fixes, ElementsAre(FixMatcher));
+}
+
+testing::Matcher<const Diag &> WithNote(testing::Matcher<Note> NoteMatcher) {
+  return Field(&Diag::Notes, ElementsAre(NoteMatcher));
+}
 
 // FIXME: this is duplicated with FileIndexTests. Share it.
-ParsedAST build(StringRef Code, std::vector<const char*> Flags = {}) {
-  std::vector<const char*> Cmd = {"clang", "main.cpp"};
+ParsedAST build(StringRef Code, std::vector<const char *> Flags = {}) {
+  std::vector<const char *> Cmd = {"clang", "main.cpp"};
   Cmd.insert(Cmd.begin() + 1, Flags.begin(), Flags.end());
   auto CI = createInvocationFromCommandLine(Cmd);
   auto Buf = MemoryBuffer::getMemBuffer(Code);
   auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),
                               std::make_shared<PCHContainerOperations>(),
                               vfs::getRealFileSystem());
   assert(AST.hasValue());
   return std::move(*AST);
 }
 
+std::vector<Diag> buildDiags(llvm::StringRef Code,
+                             std::vector<const char *> Flags = {}) {
+  return build(Code, std::move(Flags)).getDiagnostics();
+}
+
 MATCHER_P2(Diag, Range, Message,
-           "Diagnostic at " + llvm::to_string(Range) + " = [" + Message + "]") {
-  return arg.Diag.range == Range && arg.Diag.message == Message &&
-         arg.FixIts.empty();
+           "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
 }
 
 MATCHER_P3(Fix, Range, Replacement, Message,
            "Fix " + llvm::to_string(Range) + " => " +
                testing::PrintToString(Replacement) + " = [" + Message + "]") {
-  return arg.Diag.range == Range && arg.Diag.message == Message &&
-         arg.FixIts.size() == 1 && arg.FixIts[0].range == Range &&
-         arg.FixIts[0].newText == Replacement;
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+         arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
+MATCHER_P(EqualToLSPDiag, LSPDiag,
+          "LSP diagnostic " + llvm::to_string(LSPDiag)) {
+  return std::tie(arg.range, arg.severity, arg.message) ==
+         std::tie(LSPDiag.range, LSPDiag.severity, LSPDiag.message);
+}
+
+MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) {
+  if (arg.Message != Fix.Message)
+    return false;
+  if (arg.Edits.size() != Fix.Edits.size())
+    return false;
+  for (std::size_t I = 0; I < arg.Edits.size(); ++I) {
+    if (arg.Edits[I].range != Fix.Edits[I].range ||
+        arg.Edits[I].newText != Fix.Edits[I].newText)
+      return false;
+  }
+  return true;
+}
+
+// Helper function to make tests shorter.
+Position pos(int line, int character) {
+  Position Res;
+  Res.line = line;
+  Res.character = character;
+  return Res;
+};
+
+/// Matches diagnostic that has exactly one fix with the same range and message
+/// as the diagnostic itself.
+testing::Matcher<const clangd::Diag &> DiagWithEqualFix(clangd::Range Range,
+                                                        std::string Replacement,
+                                                        std::string Message) {
+  return AllOf(Diag(Range, Message), WithFix(Fix(Range, Replacement, Message)));
 }
 
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
     void $decl[[foo]]();
     int main() {
       $typo[[go\
 o]]();
       foo()$semicolon[[]]
       $unk[[unknown]]();
     }
   )cpp");
-      llvm::errs() << Test.code();
-      EXPECT_THAT(
-          build(Test.code()).getDiagnostics(),
-          ElementsAre(
-              // This range spans lines.
-              Fix(Test.range("typo"), "foo",
+      buildDiags(Test.code()),
+          AllOf(DiagWithEqualFix(
+                    Test.range("typo"), "foo",
                     "use of undeclared identifier 'goo'; did you mean 'foo'?"),
                 // This is a pretty normal range.
-              Diag(Test.range("decl"), "'foo' declared here"),
-              // This range is zero-width, and at the end of a line.
-              Fix(Test.range("semicolon"), ";",
+                WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
+          DiagWithEqualFix(Test.range("semicolon"), ";",
                            "expected ';' after expression"),
-              // This range isn't provided by clang, we expand to the token.
-              Diag(Test.range("unk"),
-                   "use of undeclared identifier 'unknown'")));
+          Diag(Test.range("unk"), "use of undeclared identifier 'unknown'")));
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
   Annotations Test("[[void]] main() {}");
-  EXPECT_THAT(
-      build(Test.code()).getDiagnostics(),
-      ElementsAre(Fix(Test.range(), "int", "'main' must return 'int'")));
+  EXPECT_THAT(buildDiags(Test.code()),
+              ElementsAre(DiagWithEqualFix(Test.range(), "int",
+                                           "'main' must return 'int'")));
   // Same code built as C gets different diagnostics.
   EXPECT_THAT(
-      build(Test.code(), {"-x", "c"}).getDiagnostics(),
-      ElementsAre(
-          // FIXME: ideally this would be one diagnostic with a named FixIt.
+      buildDiags(Test.code(), {"-x", "c"}),
+      ElementsAre(AllOf(
           Diag(Test.range(), "return type of 'main' is not 'int'"),
-          Fix(Test.range(), "int", "change return type to 'int'")));
+          WithFix(Fix(Test.range(), "int", "change return type to 'int'")))));
 }
 
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
   //  - the #else doesn't generate diagnostics (we had this bug)
   //  - we get diagnostics from the taken branch
   //  - we get no diagnostics from the not taken branch
   Annotations Test(R"cpp(
     #ifndef FOO
     #define FOO
       int a = [[b]];
     #else
       int x = y;
     #endif
     )cpp");
   EXPECT_THAT(
-      build(Test.code()).getDiagnostics(),
+      buildDiags(Test.code()),
       ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
 }
 
+TEST(DiagnosticsTest, ToLSP) {
+  clangd::Diag D;
+  D.Message = "something terrible happened";
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.InsideMainFile = true;
+  D.Severity = DiagnosticsEngine::Error;
+  D.File = "foo/bar/main.cpp";
+
+  clangd::Note NoteInMain;
+  NoteInMain.Message = "declared somewhere in the main file";
+  NoteInMain.Range = {pos(5, 6), pos(7, 8)};
+  NoteInMain.Severity = DiagnosticsEngine::Remark;
+  NoteInMain.File = "../foo/bar/main.cpp";
+  NoteInMain.InsideMainFile = true;
+  D.Notes.push_back(NoteInMain);
+
+  clangd::Note NoteInHeader;
+  NoteInHeader.Message = "declared somewhere in the header file";
+  NoteInHeader.Range = {pos(9, 10), pos(11, 12)};
+  NoteInHeader.Severity = DiagnosticsEngine::Note;
+  NoteInHeader.File = "../foo/baz/header.h";
+  NoteInHeader.InsideMainFile = false;
+  D.Notes.push_back(NoteInHeader);
+
+  clangd::Fix F;
+  F.Message = "do something";
+  D.Fixes.push_back(F);
+
+  auto MatchingLSP = [](const DiagBase &D, llvm::StringRef Message) {
+    clangd::Diagnostic Res;
+    Res.range = D.Range;
+    Res.severity = getSeverity(D.Severity);
+    Res.message = Message;
+    return Res;
+  };
+
+  // Diagnostics should turn into these:
+  clangd::Diagnostic MainLSP = MatchingLSP(D, R"(something terrible happened
+
+main.cpp:6:7: remark: declared somewhere in the main file
+
+../foo/baz/header.h:10:11:
+note: declared somewhere in the header file)");
+
+  clangd::Diagnostic NoteInMainLSP =
+      MatchingLSP(NoteInMain, R"(declared somewhere in the main file
+
+main.cpp:2:3: error: something terrible happened)");
+
+  // Transform dianostics and check the results.
+  std::vector<std::pair<clangd::Diagnostic, std::vector<clangd::Fix>>> LSPDiags;
+  toLSPDiags(D, [&](clangd::Diagnostic LSPDiag,
+                    llvm::ArrayRef<clangd::Fix> Fixes) {
+    LSPDiags.push_back({std::move(LSPDiag),
+                        std::vector<clangd::Fix>(Fixes.begin(), Fixes.end())});
+  });
+
+  EXPECT_THAT(
+      LSPDiags,
+      ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F))),
+                  Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty())));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/CodeCompleteTests.cpp

 }
 
 namespace {
 using namespace llvm;
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::Each;
 using ::testing::ElementsAre;
+using ::testing::Field;
 using ::testing::Not;
 using ::testing::UnorderedElementsAre;
-using ::testing::Field;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
-  void onDiagnosticsReady(
-      PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {}
+  void onDiagnosticsReady(PathRef File,
+                          Tagged<std::vector<Diag>> Diagnostics) override {}
 };
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.insertText == Name; }
 MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
 MATCHER_P(Kind, K, "") { return arg.kind == K; }
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
 MATCHER_P(Doc, D, "") { return arg.documentation == D; }

 
 TEST(CompletionTest, CompletionOptions) {
   auto Test = [&](const clangd::CodeCompleteOptions &Opts) {
     TestAfterDotCompletion(Opts);
     TestGlobalScopeCompletion(Opts);
   };
   // We used to test every combination of options, but that got too slow (2^N).
   auto Flags = {
       &clangd::CodeCompleteOptions::IncludeMacros,
       &clangd::CodeCompleteOptions::IncludeBriefComments,
       &clangd::CodeCompleteOptions::EnableSnippets,
       &clangd::CodeCompleteOptions::IncludeCodePatterns,
       &clangd::CodeCompleteOptions::IncludeIneligibleResults,
   };
   // Test default options.
   Test({});
   // Test with one flag flipped.
   for (auto &F : Flags) {
     clangd::CodeCompleteOptions O;
     O.*F ^= true;
     Test(O);
TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   auto ClassFromPreamble =
       runCodeComplete(Server, File, Test.point("2"), Opts).Value;
   EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
 
   Opts.Index = nullptr;
   auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
   EXPECT_THAT(WithoutIndex.items,
               UnorderedElementsAre(Named("local"), Named("preamble")));
-
 }
 
 TEST(CompletionTest, DynamicIndexMultiFile) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   auto Opts = ClangdServer::optsForTest();
   Opts.BuildDynamicSymbolIndex = true;

unittests/clangd/TUSchedulerTests.cpp

 #include <utility>
 
 namespace clang {
 namespace clangd {
 
 using ::testing::Pair;
 using ::testing::Pointee;
 
-void ignoreUpdate(llvm::Optional<std::vector<DiagWithFixIts>>) {}
+void ignoreUpdate(llvm::Optional<std::vector<Diag>>) {}
 void ignoreError(llvm::Error Err) {
   handleAllErrors(std::move(Err), [](const llvm::ErrorInfoBase &) {});
 }
 
 class TUSchedulerTests : public ::testing::Test {
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
     return ParseInputs{*CDB.getCompileCommand(File), buildTestFS(Files),
std::atomic<int> CallbackCount(0);
     Notification Ready;
     TUScheduler S(
         getDefaultAsyncThreadsCount(),
         /*StorePreamblesInMemory=*/true,
         /*ASTParsedCallback=*/nullptr,
         /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
     auto Path = testPath("foo.cpp");
     S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
-             [&](std::vector<DiagWithFixIts>) { Ready.wait(); });
+             [&](std::vector<Diag>) { Ready.wait(); });
 
     S.update(Path, getInputs(Path, "request diags"), WantDiagnostics::Yes,
-             [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+             [&](std::vector<Diag> Diags) { ++CallbackCount; });
     S.update(Path, getInputs(Path, "auto (clobbered)"), WantDiagnostics::Auto,
-             [&](std::vector<DiagWithFixIts> Diags) {
+             [&](std::vector<Diag> Diags) {
                ADD_FAILURE() << "auto should have been cancelled by auto";
              });
     S.update(Path, getInputs(Path, "request no diags"), WantDiagnostics::No,
-             [&](std::vector<DiagWithFixIts> Diags) {
+             [&](std::vector<Diag> Diags) {
                ADD_FAILURE() << "no diags should not be called back";
              });
     S.update(Path, getInputs(Path, "auto (produces)"), WantDiagnostics::Auto,
-             [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+             [&](std::vector<Diag> Diags) { ++CallbackCount; });
     Ready.notify();
   }
   EXPECT_EQ(2, CallbackCount);
 }
 
 TEST_F(TUSchedulerTests, Debounce) {
   std::atomic<int> CallbackCount(0);
   {
     TUScheduler S(getDefaultAsyncThreadsCount(),
                   /*StorePreamblesInMemory=*/true,
                   /*ASTParsedCallback=*/nullptr,
                   /*UpdateDebounce=*/std::chrono::seconds(1));
     // FIXME: we could probably use timeouts lower than 1 second here.
     auto Path = testPath("foo.cpp");
     S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
-             [&](std::vector<DiagWithFixIts> Diags) {
+             [&](std::vector<Diag> Diags) {
                ADD_FAILURE() << "auto should have been debounced and canceled";
              });
     std::this_thread::sleep_for(std::chrono::milliseconds(200));
     S.update(Path, getInputs(Path, "auto (timed out)"), WantDiagnostics::Auto,
-             [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+             [&](std::vector<Diag> Diags) { ++CallbackCount; });
     std::this_thread::sleep_for(std::chrono::seconds(2));
     S.update(Path, getInputs(Path, "auto (shut down)"), WantDiagnostics::Auto,
-             [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+             [&](std::vector<Diag> Diags) { ++CallbackCount; });
   }
   EXPECT_EQ(2, CallbackCount);
 }
 
 TEST_F(TUSchedulerTests, ManyUpdates) {
   const int FilesCount = 3;
   const int UpdatesPerFile = 10;
 
for (int FileI = 0; FileI < FilesCount; ++FileI) {
         auto Contents = AllContents[(FileI + UpdateI) % AllContentsSize];
 
         auto File = Files[FileI];
         auto Inputs = getInputs(File, Contents.str());
 
         {
           WithContextValue WithNonce(NonceKey, ++Nonce);
           S.update(File, Inputs, WantDiagnostics::Auto,
-                   [Nonce, &Mut, &TotalUpdates](
-                       llvm::Optional<std::vector<DiagWithFixIts>> Diags) {
+                   [Nonce, &Mut,
+                    &TotalUpdates](llvm::Optional<std::vector<Diag>> Diags) {
                      EXPECT_THAT(Context::current().get(NonceKey),
                                  Pointee(Nonce));
 
                      std::lock_guard<std::mutex> Lock(Mut);
                      ++TotalUpdates;
                    });
         }
 

unittests/clangd/XRefsTests.cpp

 namespace {
 using testing::ElementsAre;
 using testing::Field;
 using testing::IsEmpty;
 using testing::Matcher;
 using testing::UnorderedElementsAreArray;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
-  void onDiagnosticsReady(
-      PathRef File, Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {}
+  void onDiagnosticsReady(PathRef File,
+                          Tagged<std::vector<Diag>> Diagnostics) override {}
 };
 
 // FIXME: this is duplicated with FileIndexTests. Share it.
 ParsedAST build(StringRef Code) {
   auto CI = createInvocationFromCommandLine(
       {"clang", "-xc++", testPath("Foo.cpp").c_str()});
   auto Buf = MemoryBuffer::getMemBuffer(Code);
   auto AST = ParsedAST::Build(std::move(CI), nullptr, std::move(Buf),