Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -84,6 +84,9 @@ Params.capabilities.textDocument.completion.completionItem.snippetSupport; DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.textDocument.publishDiagnostics.clangdFixSupport; + DiagOpts.AllowNotesWithFixes = + Params.capabilities.textDocument.publishDiagnostics + .clangdNotesWithFixSupport; if (Params.capabilities.workspace && Params.capabilities.workspace->symbol && Params.capabilities.workspace->symbol->symbolKind) { @@ -445,7 +448,8 @@ : CompilationDB::makeDirectoryBased( std::move(CompileCommandsDir))), CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()), - Server(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this, Opts) {} + Server(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this, Opts, DiagOpts) { +} bool ClangdLSPServer::run(std::FILE *In, JSONStreamStyle InputStyle) { assert(!IsDone && "Run was called before"); Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -98,7 +98,8 @@ /// worker thread. Therefore, instances of \p DiagConsumer must properly /// synchronize access to shared state. ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider, - DiagnosticsConsumer &DiagConsumer, const Options &Opts); + DiagnosticsConsumer &DiagConsumer, const Options &Opts, + const ClangdDiagnosticOptions &DiagOpts); /// Set the root path of the workspace. void setRootPath(PathRef RootPath); Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -79,7 +79,8 @@ ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB, FileSystemProvider &FSProvider, DiagnosticsConsumer &DiagConsumer, - const Options &Opts) + const Options &Opts, + const ClangdDiagnosticOptions &DiagOpts) : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str() : getStandardResourceDir()), @@ -98,7 +99,7 @@ std::shared_ptr PP) { FileIdx->update(Path, &AST, std::move(PP)); } : PreambleParsedCallback(), - Opts.UpdateDebounce, Opts.RetentionPolicy) { + Opts.UpdateDebounce, Opts.RetentionPolicy, DiagOpts) { if (FileIdx && Opts.StaticIndex) { MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex); Index = MergedIndex.get(); Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -72,7 +72,8 @@ std::shared_ptr Preamble, std::unique_ptr Buffer, std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS); + IntrusiveRefCntPtr VFS, + const ClangdDiagnosticOptions &DiagOpts = ClangdDiagnosticOptions()); ParsedAST(ParsedAST &&Other); ParsedAST &operator=(ParsedAST &&Other); @@ -140,13 +141,13 @@ /// If \p PreambleCallback is set, it will be run on top of the AST while /// building the preamble. Note that if the old preamble was reused, no AST is /// built and, therefore, the callback will not be executed. -std::shared_ptr -buildPreamble(PathRef FileName, CompilerInvocation &CI, - std::shared_ptr OldPreamble, - const tooling::CompileCommand &OldCompileCommand, - const ParseInputs &Inputs, - std::shared_ptr PCHs, bool StoreInMemory, - PreambleParsedCallback PreambleCallback); +std::shared_ptr buildPreamble( + PathRef FileName, CompilerInvocation &CI, + std::shared_ptr OldPreamble, + const tooling::CompileCommand &OldCompileCommand, const ParseInputs &Inputs, + std::shared_ptr PCHs, bool StoreInMemory, + PreambleParsedCallback PreambleCallback, + const ClangdDiagnosticOptions &DiagOpts = ClangdDiagnosticOptions()); /// Build an AST from provided user inputs. This function does not check if /// preamble can be reused, as this function expects that \p Preamble is the @@ -155,7 +156,8 @@ buildAST(PathRef FileName, std::unique_ptr Invocation, const ParseInputs &Inputs, std::shared_ptr Preamble, - std::shared_ptr PCHs); + std::shared_ptr PCHs, + const ClangdDiagnosticOptions &DiagOpts = ClangdDiagnosticOptions()); /// Get the beginning SourceLocation at a specified \p Pos. /// May be invalid if Pos is, or if there's no identifier. Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -125,7 +125,8 @@ std::shared_ptr Preamble, std::unique_ptr Buffer, std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS) { + IntrusiveRefCntPtr VFS, + const ClangdDiagnosticOptions &DiagOpts) { assert(CI); // Command-line parsing sets DisableFree to true by default, but we don't want // to leak memory in clangd. @@ -133,7 +134,7 @@ const PrecompiledPreamble *PreamblePCH = Preamble ? &Preamble->Preamble : nullptr; - StoreDiags ASTDiags; + StoreDiags ASTDiags(DiagOpts); auto Clang = prepareCompilerInstance(std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs), std::move(VFS), ASTDiags); @@ -296,7 +297,8 @@ std::shared_ptr OldPreamble, const tooling::CompileCommand &OldCompileCommand, const ParseInputs &Inputs, std::shared_ptr PCHs, bool StoreInMemory, - PreambleParsedCallback PreambleCallback) { + PreambleParsedCallback PreambleCallback, + const ClangdDiagnosticOptions &DiagOpts) { // Note that we don't need to copy the input contents, preamble can live // without those. auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Inputs.Contents); @@ -315,7 +317,7 @@ trace::Span Tracer("BuildPreamble"); SPAN_ATTACH(Tracer, "File", FileName); - StoreDiags PreambleDiagnostics; + StoreDiags PreambleDiagnostics(DiagOpts); IntrusiveRefCntPtr PreambleDiagsEngine = CompilerInstance::createDiagnostics(&CI.getDiagnosticOpts(), &PreambleDiagnostics, false); @@ -357,7 +359,8 @@ llvm::Optional clangd::buildAST( PathRef FileName, std::unique_ptr Invocation, const ParseInputs &Inputs, std::shared_ptr Preamble, - std::shared_ptr PCHs) { + std::shared_ptr PCHs, + const ClangdDiagnosticOptions &DiagOpts) { trace::Span Tracer("BuildAST"); SPAN_ATTACH(Tracer, "File", FileName); @@ -367,9 +370,10 @@ // dirs. } - return ParsedAST::build( - llvm::make_unique(*Invocation), Preamble, - llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs, Inputs.FS); + return ParsedAST::build(llvm::make_unique(*Invocation), + Preamble, + llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), + PCHs, Inputs.FS, DiagOpts); } SourceLocation clangd::getBeginningOfIdentifier(ParsedAST &Unit, Index: clangd/Diagnostics.h =================================================================== --- clangd/Diagnostics.h +++ clangd/Diagnostics.h @@ -27,7 +27,20 @@ /// If true, Clangd uses an LSP extension to embed the fixes with the /// diagnostics that are sent to the client. bool EmbedFixesInDiagnostics = false; + + /// If true, Clangd sends the fixes attached to notes with the notes + /// themselves instead of adding them to the main diagnostic. + bool AllowNotesWithFixes = false; +}; + +/// 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 Edits; }; +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Fix &F); /// Contains basic information about a diagnostic. struct DiagBase { @@ -41,18 +54,11 @@ // Since File is only descriptive, we store a separate flag to distinguish // diags from the main file. bool InsideMainFile = false; + /// *Alternative* fixes for this diagnostic, one should be chosen. + std::vector Fixes; }; 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 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 {}; @@ -61,8 +67,6 @@ struct Diag : DiagBase { /// Elaborate on the problem, usually pointing to a related piece of code. std::vector Notes; - /// *Alternative* fixes for this diagnostic, one should be chosen. - std::vector Fixes; }; llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D); @@ -84,6 +88,7 @@ /// the diag itself nor its notes are in the main file). class StoreDiags : public DiagnosticConsumer { public: + StoreDiags(ClangdDiagnosticOptions DiagOpts) : DiagOpts(DiagOpts) {} std::vector take(); void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override; @@ -97,6 +102,7 @@ std::vector Output; llvm::Optional LangOpts; llvm::Optional LastDiag; + ClangdDiagnosticOptions DiagOpts; }; } // namespace clangd Index: clangd/Diagnostics.cpp =================================================================== --- clangd/Diagnostics.cpp +++ clangd/Diagnostics.cpp @@ -184,7 +184,16 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const DiagBase &D) { if (!D.InsideMainFile) OS << "[in " << D.File << "] "; - return OS << D.Message; + OS << D.Message; + if (!D.Fixes.empty()) { + OS << ", fixes: {"; + const char *Sep = ""; + for (auto &Fix : D.Fixes) { + OS << Sep << Fix; + Sep = ", "; + } + } + return OS; } llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Fix &F) { @@ -208,14 +217,6 @@ } OS << "}"; } - if (!D.Fixes.empty()) { - OS << ", fixes: {"; - const char *Sep = ""; - for (auto &Fix : D.Fixes) { - OS << Sep << Fix; - Sep = ", "; - } - } return OS; } @@ -241,7 +242,7 @@ continue; clangd::Diagnostic Res = FillBasicFields(Note); Res.message = noteMessage(D, Note); - OutFn(std::move(Res), llvm::ArrayRef()); + OutFn(std::move(Res), Note.Fixes); } } @@ -299,7 +300,7 @@ return D; }; - auto AddFix = [&](bool SyntheticMessage) -> bool { + auto AddFix = [&](DiagBase &LastDiag, bool SyntheticMessage) -> bool { assert(!Info.getFixItHints().empty() && "diagnostic does not have attached fix-its"); if (!InsideMainFile) @@ -335,7 +336,7 @@ } if (Message.empty()) // either !SytheticMessage, or we failed to make one. Info.FormatDiagnostic(Message); - LastDiag->Fixes.push_back(Fix{Message.str(), std::move(Edits)}); + LastDiag.Fixes.push_back(Fix{Message.str(), std::move(Edits)}); return true; }; @@ -347,7 +348,8 @@ FillDiagBase(*LastDiag); if (!Info.getFixItHints().empty()) - AddFix(true /* try to invent a message instead of repeating the diag */); + AddFix(*LastDiag, + true /* try to invent a message instead of repeating the diag */); } else { // Handle a note to an existing diagnostic. if (!LastDiag) { @@ -356,16 +358,17 @@ return; } - if (!Info.getFixItHints().empty()) { + if (!Info.getFixItHints().empty() && !DiagOpts.AllowNotesWithFixes) { // 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(false /* use the note as the message */)) + if (!AddFix(*LastDiag, false /* use the note as the message */)) IgnoreDiagnostics::log(DiagLevel, Info); } else { // A clang note without fix-its corresponds to clangd::Note. Note N; FillDiagBase(N); - + if (DiagOpts.AllowNotesWithFixes && !Info.getFixItHints().empty()) + AddFix(N, false /* use the note as the message */); LastDiag->Notes.push_back(std::move(N)); } } Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -255,6 +255,11 @@ /// Whether the client accepts diagnostics with fixes attached using the /// "clangd_fixes" extension. bool clangdFixSupport = false; + + /// Whether the client prefers note diagnostics with fixes attached to + /// them with the "clangd_fixes" extension instead of relying on a fix-it + /// attached to the main diagnostic. + bool clangdNotesWithFixSupport = false; }; bool fromJSON(const llvm::json::Value &, PublishDiagnosticsClientCapabilities &); Index: clangd/Protocol.cpp =================================================================== --- clangd/Protocol.cpp +++ clangd/Protocol.cpp @@ -184,6 +184,7 @@ if (!O) return false; O.map("clangdFixSupport", R.clangdFixSupport); + O.map("clangdNotesWithFixSupport", R.clangdNotesWithFixSupport); return true; } Index: clangd/TUScheduler.h =================================================================== --- clangd/TUScheduler.h +++ clangd/TUScheduler.h @@ -63,7 +63,8 @@ TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, PreambleParsedCallback PreambleCallback, std::chrono::steady_clock::duration UpdateDebounce, - ASTRetentionPolicy RetentionPolicy); + ASTRetentionPolicy RetentionPolicy, + const ClangdDiagnosticOptions &DiagOpts); ~TUScheduler(); /// Returns estimated memory usage for each of the currently open files. @@ -141,6 +142,7 @@ llvm::Optional PreambleTasks; llvm::Optional WorkerThreads; std::chrono::steady_clock::duration UpdateDebounce; + const ClangdDiagnosticOptions &DiagOpts; }; } // namespace clangd Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -177,9 +177,10 @@ ~ASTWorker(); void update(ParseInputs Inputs, WantDiagnostics, + const ClangdDiagnosticOptions &DiagOpts, llvm::unique_function)> OnUpdated); void - runWithAST(llvm::StringRef Name, + runWithAST(llvm::StringRef Name, const ClangdDiagnosticOptions &DiagOpts, llvm::unique_function)> Action); bool blockUntilIdle(Deadline Timeout) const; @@ -333,6 +334,7 @@ void ASTWorker::update( ParseInputs Inputs, WantDiagnostics WantDiags, + const ClangdDiagnosticOptions &DiagOpts, llvm::unique_function)> OnUpdated) { auto Task = [=](decltype(OnUpdated) OnUpdated) mutable { // Will be used to check if we can avoid rebuilding the AST. @@ -365,7 +367,7 @@ getPossiblyStalePreamble(); std::shared_ptr NewPreamble = buildPreamble(FileName, *Invocation, OldPreamble, OldCommand, Inputs, - PCHs, StorePreambleInMemory, PreambleCallback); + PCHs, StorePreambleInMemory, PreambleCallback, DiagOpts); bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble); { @@ -406,8 +408,8 @@ // Get the AST for diagnostics. llvm::Optional> AST = IdleASTs.take(this); if (!AST) { - llvm::Optional NewAST = - buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs); + llvm::Optional NewAST = buildAST( + FileName, std::move(Invocation), Inputs, NewPreamble, PCHs, DiagOpts); AST = NewAST ? llvm::make_unique(std::move(*NewAST)) : nullptr; } // We want to report the diagnostics even if this update was cancelled. @@ -426,7 +428,7 @@ } void ASTWorker::runWithAST( - llvm::StringRef Name, + llvm::StringRef Name, const ClangdDiagnosticOptions &DiagOpts, llvm::unique_function)> Action) { auto Task = [=](decltype(Action) Action) { llvm::Optional> AST = IdleASTs.take(this); @@ -438,7 +440,7 @@ Invocation ? buildAST(FileName, llvm::make_unique(*Invocation), - FileInputs, getPossiblyStalePreamble(), PCHs) + FileInputs, getPossiblyStalePreamble(), PCHs, DiagOpts) : llvm::None; AST = NewAST ? llvm::make_unique(std::move(*NewAST)) : nullptr; } @@ -630,12 +632,13 @@ bool StorePreamblesInMemory, PreambleParsedCallback PreambleCallback, std::chrono::steady_clock::duration UpdateDebounce, - ASTRetentionPolicy RetentionPolicy) + ASTRetentionPolicy RetentionPolicy, + const ClangdDiagnosticOptions &DiagOpts) : StorePreamblesInMemory(StorePreamblesInMemory), PCHOps(std::make_shared()), PreambleCallback(std::move(PreambleCallback)), Barrier(AsyncThreadsCount), IdleASTs(llvm::make_unique(RetentionPolicy.MaxRetainedASTs)), - UpdateDebounce(UpdateDebounce) { + UpdateDebounce(UpdateDebounce), DiagOpts(DiagOpts) { if (0 < AsyncThreadsCount) { PreambleTasks.emplace(); WorkerThreads.emplace(); @@ -679,7 +682,8 @@ FD->Contents = Inputs.Contents; FD->Command = Inputs.CompileCommand; } - FD->Worker->update(std::move(Inputs), WantDiags, std::move(OnUpdated)); + FD->Worker->update(std::move(Inputs), WantDiags, DiagOpts, + std::move(OnUpdated)); } void TUScheduler::remove(PathRef File) { @@ -700,7 +704,7 @@ return; } - It->second->Worker->runWithAST(Name, std::move(Action)); + It->second->Worker->runWithAST(Name, DiagOpts, std::move(Action)); } void TUScheduler::runWithPreamble( Index: test/clangd/fixits-embed-in-diagnostic.test =================================================================== --- test/clangd/fixits-embed-in-diagnostic.test +++ test/clangd/fixits-embed-in-diagnostic.test @@ -1,7 +1,7 @@ # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true}}},"trace":"off"}} +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true,"clangdNotesWithFixSupport":true}}},"trace":"off"}} --- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}} +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p; int main(int i, char **a) { if (i = 2) {}}"}}} # CHECK: "method": "textDocument/publishDiagnostics", # CHECK-NEXT: "params": { # CHECK-NEXT: "diagnostics": [ @@ -57,6 +57,112 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: "severity": 3 +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "category": "Semantic Issue", +# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses\n\nfoo.c:1:67: note: place parentheses around the assignment to silence this warning\n\nfoo.c:1:67: note: use '==' to turn this assignment into an equality comparison", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 69, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 64, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 2 +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "category": "Semantic Issue", +# CHECK-NEXT: "clangd_fixes": [ +# CHECK-NEXT: { +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "changes": { +# CHECK-NEXT: "file:///clangd-test/foo.c": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "(", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 64, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 64, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "newText": ")", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 69, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 69, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "title": "place parentheses around the assignment to silence this warning" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "message": "Place parentheses around the assignment to silence this warning\n\nfoo.c:1:65: warning: using the result of an assignment as a condition without parentheses", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 67, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 66, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 3 +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "category": "Semantic Issue", +# CHECK-NEXT: "clangd_fixes": [ +# CHECK-NEXT: { +# CHECK-NEXT: "edit": { +# CHECK-NEXT: "changes": { +# CHECK-NEXT: "file:///clangd-test/foo.c": [ +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "==", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 67, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 66, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: ] +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "title": "use '==' to turn this assignment into an equality comparison" +# CHECK-NEXT: } +# CHECK-NEXT: ], +# CHECK-NEXT: "message": "Use '==' to turn this assignment into an equality comparison\n\nfoo.c:1:65: warning: using the result of an assignment as a condition without parentheses", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 67, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 66, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: "severity": 3 # CHECK-NEXT: } # CHECK-NEXT: ], # CHECK-NEXT: "uri": "file://{{.*}}/foo.c" Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -140,7 +140,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); for (const auto &FileWithContents : ExtraFiles) FS.Files[testPath(FileWithContents.first)] = FileWithContents.second; @@ -192,7 +194,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); const auto SourceContents = R"cpp( #include "foo.h" @@ -227,7 +231,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); const auto SourceContents = R"cpp( #include "foo.h" @@ -280,7 +286,9 @@ MockCompilationDatabase CDB; // Verify that the context is plumbed to the FS provider and diagnostics. - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); { WithContextValue Entrypoint(Secret, 42); Server.addDocument(testPath("foo.cpp"), "void main(){}"); @@ -301,7 +309,9 @@ {"-xc++", "-target", "x86_64-linux-unknown", "-m64", "--gcc-toolchain=/randomusr", "-stdlib=libstdc++"}); - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); // Just a random gcc version string SmallString<8> Version("4.9.3"); @@ -346,7 +356,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto FooCpp = testPath("foo.cpp"); const auto SourceContents1 = R"cpp( @@ -382,7 +394,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto FooCpp = testPath("foo.cpp"); const auto SourceContents = R"cpp( @@ -434,7 +448,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; MultipleErrorCheckingDiagConsumer DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto FooCpp = testPath("foo.cpp"); auto BarCpp = testPath("bar.cpp"); @@ -480,7 +496,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); Path FooCpp = testPath("foo.cpp"); const auto SourceContents = R"cpp( @@ -515,8 +533,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto FooCpp = testPath("foo.cpp"); // clang cannot create CompilerInvocation if we pass two files in the @@ -633,7 +652,9 @@ TestDiagConsumer DiagConsumer; { MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); // Prepare some random distributions for the test. std::random_device RandGen; @@ -766,7 +787,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto SourceContents = R"cpp( #include "foo.h" @@ -891,7 +914,9 @@ NoConcurrentAccessDiagConsumer DiagConsumer(std::move(StartSecondPromise)); MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); Server.addDocument(FooCpp, SourceContentsWithErrors); StartSecond.wait(); Server.addDocument(FooCpp, SourceContentsWithoutErrors); @@ -903,7 +928,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto Path = testPath("foo.cpp"); std::string Code = R"cpp( @@ -932,7 +959,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto SourcePath = testPath("source/foo.cpp"); auto HeaderPath = testPath("headers/foo.h"); Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -122,7 +122,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); return completions(Server, Text, std::move(IndexSymbols), std::move(Opts)); } @@ -563,7 +565,9 @@ FS.Files[BarHeader] = ""; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); Symbol::Details Scratch; auto BarURI = URI::createFile(BarHeader).toString(); Symbol Sym = cls("ns::X"); @@ -594,7 +598,9 @@ MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); Symbol::Details Scratch; Symbol SymX = cls("ns::X"); Symbol SymY = cls("ns::Y"); @@ -624,7 +630,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); FS.Files[testPath("bar.h")] = R"cpp(namespace ns { struct preamble { int member; }; })cpp"; @@ -660,7 +668,8 @@ IgnoreDiagnostics DiagConsumer; auto Opts = ClangdServer::optsForTest(); Opts.BuildDynamicSymbolIndex = true; - ClangdServer Server(CDB, FS, DiagConsumer, Opts); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, Opts, DiagOpts); FS.Files[testPath("foo.h")] = R"cpp( namespace ns { class XYZ {}; void foo(int x) {} } @@ -830,7 +839,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto File = testPath("foo.cpp"); Annotations Test(Text); runAddDocument(Server, File, Test.code()); @@ -1146,7 +1157,9 @@ MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); Annotations Source(R"cpp( #include "foo.h" @@ -1181,7 +1194,9 @@ MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); Annotations Source(R"cpp( // We ignore namespace comments, for rationale see CodeCompletionStrings.h. @@ -1249,7 +1264,9 @@ MockFSProvider FS; FS.Files[FooCpp] = "// empty file"; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); // Run completion outside the file range. Position Pos; Pos.line = 100; @@ -1363,7 +1380,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); CodeCompleteOptions Opts; Opts.IncludeFixIts = true; @@ -1403,7 +1422,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); CodeCompleteOptions Opts; Opts.IncludeFixIts = true; @@ -1483,7 +1504,9 @@ MockFSProvider FS; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); constexpr const char *TestCodes[] = { R"cpp( Index: unittests/clangd/FindSymbolsTests.cpp =================================================================== --- unittests/clangd/FindSymbolsTests.cpp +++ unittests/clangd/FindSymbolsTests.cpp @@ -50,7 +50,7 @@ class WorkspaceSymbolsTest : public ::testing::Test { public: WorkspaceSymbolsTest() - : Server(CDB, FSProvider, DiagConsumer, optsForTests()) { + : Server(CDB, FSProvider, DiagConsumer, optsForTests(), DiagOpts) { // Make sure the test root directory is created. FSProvider.Files[testPath("unused")] = ""; Server.setRootPath(testRoot()); @@ -60,6 +60,7 @@ MockFSProvider FSProvider; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; + ClangdDiagnosticOptions DiagOpts; ClangdServer Server; int Limit = 0; @@ -293,12 +294,13 @@ class DocumentSymbolsTest : public ::testing::Test { public: DocumentSymbolsTest() - : Server(CDB, FSProvider, DiagConsumer, optsForTests()) {} + : Server(CDB, FSProvider, DiagConsumer, optsForTests(), DiagOpts) {} protected: MockFSProvider FSProvider; MockCompilationDatabase CDB; IgnoreDiagnostics DiagConsumer; + ClangdDiagnosticOptions DiagOpts; ClangdServer Server; std::vector getSymbols(PathRef File) { Index: unittests/clangd/TUSchedulerTests.cpp =================================================================== --- unittests/clangd/TUSchedulerTests.cpp +++ unittests/clangd/TUSchedulerTests.cpp @@ -43,11 +43,12 @@ }; TEST_F(TUSchedulerTests, MissingFiles) { + ClangdDiagnosticOptions DiagOpts; TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); auto Added = testPath("added.cpp"); Files[Added] = ""; @@ -99,12 +100,13 @@ // To avoid a racy test, don't allow tasks to actualy run on the worker // thread until we've scheduled them all. Notification Ready; + ClangdDiagnosticOptions DiagOpts; TUScheduler S( getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); auto Path = testPath("foo.cpp"); S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes, [&](std::vector) { Ready.wait(); }); @@ -129,11 +131,12 @@ TEST_F(TUSchedulerTests, Debounce) { std::atomic CallbackCount(0); { + ClangdDiagnosticOptions DiagOpts; TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, /*UpdateDebounce=*/std::chrono::seconds(1), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); // 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, @@ -161,11 +164,12 @@ // Run TUScheduler and collect some stats. { + ClangdDiagnosticOptions DiagOpts; TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*PreambleParsedCallback=*/nullptr, /*UpdateDebounce=*/std::chrono::milliseconds(50), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); std::vector Files; for (int I = 0; I < FilesCount; ++I) { @@ -259,10 +263,12 @@ std::atomic BuiltASTCounter(0); ASTRetentionPolicy Policy; Policy.MaxRetainedASTs = 2; + ClangdDiagnosticOptions DiagOpts; TUScheduler S( /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true, PreambleParsedCallback(), - /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy); + /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy, + DiagOpts); llvm::StringLiteral SourceContents = R"cpp( int* a; @@ -311,11 +317,12 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) { // Testing strategy: we update the file and schedule a few preamble reads at // the same time. All reads should get the same non-null preamble. + ClangdDiagnosticOptions DiagOpts; TUScheduler S( /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true, PreambleParsedCallback(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); auto Foo = testPath("foo.cpp"); auto NonEmptyPreamble = R"cpp( #define FOO 1 @@ -344,11 +351,12 @@ } TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { + ClangdDiagnosticOptions DiagOpts; TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), /*StorePreambleInMemory=*/true, PreambleParsedCallback(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); auto Source = testPath("foo.cpp"); auto Header = testPath("foo.h"); @@ -397,11 +405,12 @@ } TEST_F(TUSchedulerTests, NoChangeDiags) { + ClangdDiagnosticOptions DiagOpts; TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), /*StorePreambleInMemory=*/true, PreambleParsedCallback(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), - ASTRetentionPolicy()); + ASTRetentionPolicy(), DiagOpts); auto FooCpp = testPath("foo.cpp"); auto Contents = "int a; int b;"; Index: unittests/clangd/XRefsTests.cpp =================================================================== --- unittests/clangd/XRefsTests.cpp +++ unittests/clangd/XRefsTests.cpp @@ -339,7 +339,9 @@ IgnoreDiagnostics DiagConsumer; MockFSProvider FS; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); // Fill the filesystem. auto FooCpp = testPath("src/foo.cpp"); @@ -958,7 +960,9 @@ MockFSProvider FS; IgnoreDiagnostics DiagConsumer; MockCompilationDatabase CDB; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + ClangdDiagnosticOptions DiagOpts; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest(), + DiagOpts); auto FooCpp = testPath("foo.cpp"); const char *SourceContents = R"cpp(