Index: clang-tools-extra/trunk/clangd/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clangd/CMakeLists.txt +++ clang-tools-extra/trunk/clangd/CMakeLists.txt @@ -40,6 +40,7 @@ FuzzyMatch.cpp GlobalCompilationDatabase.cpp Headers.cpp + IncludeFixer.cpp JSONTransport.cpp Logger.cpp Protocol.cpp Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -114,6 +114,8 @@ /// Time to wait after a new file version before computing diagnostics. std::chrono::steady_clock::duration UpdateDebounce = std::chrono::milliseconds(500); + + bool SuggestMissingIncludes = false; }; // Sensible default options for use in tests. // Features like indexing must be enabled if desired. @@ -268,6 +270,10 @@ // The provider used to provide a clang-tidy option for a specific file. tidy::ClangTidyOptionsProvider *ClangTidyOptProvider = nullptr; + // If this is true, suggest include insertion fixes for diagnostic errors that + // can be caused by missing includes (e.g. member access in incomplete type). + bool SuggestMissingIncludes = false; + // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex) llvm::StringMap> CachedCompletionFuzzyFindRequestByFile; Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -7,6 +7,7 @@ //===-------------------------------------------------------------------===// #include "ClangdServer.h" +#include "ClangdUnit.h" #include "CodeComplete.h" #include "FindSymbols.h" #include "Headers.h" @@ -106,6 +107,7 @@ ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex) : nullptr), ClangTidyOptProvider(Opts.ClangTidyOptProvider), + SuggestMissingIncludes(Opts.SuggestMissingIncludes), WorkspaceRoot(Opts.WorkspaceRoot), PCHs(std::make_shared()), // Pass a callback into `WorkScheduler` to extract symbols from a newly @@ -141,17 +143,22 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, WantDiagnostics WantDiags) { - tidy::ClangTidyOptions Options = tidy::ClangTidyOptions::getDefaults(); + ParseOptions Opts; + Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults(); if (ClangTidyOptProvider) - Options = ClangTidyOptProvider->getOptions(File); + Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File); + Opts.SuggestMissingIncludes = SuggestMissingIncludes; // FIXME: some build systems like Bazel will take time to preparing // environment to build the file, it would be nice if we could emit a // "PreparingBuild" status to inform users, it is non-trivial given the // current implementation. - WorkScheduler.update(File, ParseInputs{getCompileCommand(File), - FSProvider.getFileSystem(), - Contents.str(), Options}, - WantDiags); + ParseInputs Inputs; + Inputs.CompileCommand = getCompileCommand(File); + Inputs.FS = FSProvider.getFileSystem(); + Inputs.Contents = Contents; + Inputs.Opts = std::move(Opts); + Inputs.Index = Index; + WorkScheduler.update(File, Inputs, WantDiags); } void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); } Index: clang-tools-extra/trunk/clangd/ClangdUnit.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h +++ clang-tools-extra/trunk/clangd/ClangdUnit.h @@ -16,6 +16,7 @@ #include "Headers.h" #include "Path.h" #include "Protocol.h" +#include "index/Index.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/Preprocessor.h" @@ -70,8 +71,8 @@ std::shared_ptr Preamble, std::unique_ptr Buffer, std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS, - const tidy::ClangTidyOptions &ClangTidyOpts); + IntrusiveRefCntPtr VFS, const SymbolIndex *Index, + const ParseOptions &Opts); ParsedAST(ParsedAST &&Other); ParsedAST &operator=(ParsedAST &&Other); Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -11,9 +11,12 @@ #include "../clang-tidy/ClangTidyModuleRegistry.h" #include "Compiler.h" #include "Diagnostics.h" +#include "Headers.h" +#include "IncludeFixer.h" #include "Logger.h" #include "SourceCode.h" #include "Trace.h" +#include "index/Index.h" #include "clang/AST/ASTContext.h" #include "clang/Basic/LangOptions.h" #include "clang/Frontend/CompilerInstance.h" @@ -30,10 +33,12 @@ #include "clang/Serialization/ASTWriter.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/raw_ostream.h" #include +#include namespace clang { namespace clangd { @@ -231,7 +236,7 @@ std::unique_ptr Buffer, std::shared_ptr PCHs, llvm::IntrusiveRefCntPtr VFS, - const tidy::ClangTidyOptions &ClangTidyOpts) { + const SymbolIndex *Index, const ParseOptions &Opts) { assert(CI); // Command-line parsing sets DisableFree to true by default, but we don't want // to leak memory in clangd. @@ -240,9 +245,11 @@ Preamble ? &Preamble->Preamble : nullptr; StoreDiags ASTDiags; + std::string Content = Buffer->getBuffer(); + auto Clang = prepareCompilerInstance(std::move(CI), PreamblePCH, std::move(Buffer), - std::move(PCHs), std::move(VFS), ASTDiags); + std::move(PCHs), VFS, ASTDiags); if (!Clang) return None; @@ -266,12 +273,12 @@ { trace::Span Tracer("ClangTidyInit"); vlog("ClangTidy configuration for file {0}: {1}", MainInput.getFile(), - tidy::configurationAsText(ClangTidyOpts)); + tidy::configurationAsText(Opts.ClangTidyOpts)); tidy::ClangTidyCheckFactories CTFactories; for (const auto &E : tidy::ClangTidyModuleRegistry::entries()) E.instantiate()->addCheckFactories(CTFactories); CTContext.emplace(llvm::make_unique( - tidy::ClangTidyGlobalOptions(), ClangTidyOpts)); + tidy::ClangTidyGlobalOptions(), Opts.ClangTidyOpts)); CTContext->setDiagnosticsEngine(&Clang->getDiagnostics()); CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(MainInput.getFile()); @@ -284,6 +291,27 @@ } } + // Add IncludeFixer which can recorver diagnostics caused by missing includes + // (e.g. incomplete type) and attach include insertion fixes to diagnostics. + llvm::Optional FixIncludes; + auto BuildDir = VFS->getCurrentWorkingDirectory(); + if (Opts.SuggestMissingIncludes && Index && !BuildDir.getError()) { + auto Style = getFormatStyleForFile(MainInput.getFile(), Content, VFS.get()); + auto Inserter = std::make_shared( + MainInput.getFile(), Content, Style, BuildDir.get(), + Clang->getPreprocessor().getHeaderSearchInfo()); + if (Preamble) { + for (const auto &Inc : Preamble->Includes.MainFileIncludes) + Inserter->addExisting(Inc); + } + FixIncludes.emplace(MainInput.getFile(), Inserter, *Index, + /*IndexRequestLimit=*/5); + ASTDiags.contributeFixes([&FixIncludes](DiagnosticsEngine::Level DiagLevl, + const clang::Diagnostic &Info) { + return FixIncludes->fix(DiagLevl, Info); + }); + } + // Copy over the includes from the preamble, then combine with the // non-preamble includes below. auto Includes = Preamble ? Preamble->Includes : IncludeStructure{}; @@ -506,10 +534,10 @@ // dirs. } - return ParsedAST::build(llvm::make_unique(*Invocation), - Preamble, - llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), - PCHs, std::move(VFS), Inputs.ClangTidyOpts); + return ParsedAST::build( + llvm::make_unique(*Invocation), Preamble, + llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents), PCHs, + std::move(VFS), Inputs.Index ? Inputs.Index : nullptr, Inputs.Opts); } SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos, Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -177,28 +177,6 @@ return Result; } -/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal -/// include. -static llvm::Expected toHeaderFile(llvm::StringRef Header, - llvm::StringRef HintPath) { - if (isLiteralInclude(Header)) - return HeaderFile{Header.str(), /*Verbatim=*/true}; - auto U = URI::parse(Header); - if (!U) - return U.takeError(); - - auto IncludePath = URI::includeSpelling(*U); - if (!IncludePath) - return IncludePath.takeError(); - if (!IncludePath->empty()) - return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true}; - - auto Resolved = URI::resolve(*U, HintPath); - if (!Resolved) - return Resolved.takeError(); - return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; -} - /// A code completion result, in clang-native form. /// It may be promoted to a CompletionItem if it's among the top-ranked results. struct CompletionCandidate { @@ -1019,11 +997,12 @@ llvm::IntrusiveRefCntPtr VFS = Input.VFS; if (Input.Preamble && Input.Preamble->StatCache) VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS)); - ParseInputs PInput; - PInput.CompileCommand = Input.Command; - PInput.FS = VFS; - PInput.Contents = Input.Contents; - auto CI = buildCompilerInvocation(PInput); + ParseInputs ParseInput; + ParseInput.CompileCommand = Input.Command; + ParseInput.FS = VFS; + ParseInput.Contents = Input.Contents; + ParseInput.Opts = ParseOptions(); + auto CI = buildCompilerInvocation(ParseInput); if (!CI) { elog("Couldn't create CompilerInvocation"); return false; @@ -1143,24 +1122,6 @@ return CachedReq; } -// Returns the most popular include header for \p Sym. If two headers are -// equally popular, prefer the shorter one. Returns empty string if \p Sym has -// no include header. -llvm::SmallVector getRankedIncludes(const Symbol &Sym) { - auto Includes = Sym.IncludeHeaders; - // Sort in descending order by reference count and header length. - llvm::sort(Includes, [](const Symbol::IncludeHeaderWithReferences &LHS, - const Symbol::IncludeHeaderWithReferences &RHS) { - if (LHS.References == RHS.References) - return LHS.IncludeHeader.size() < RHS.IncludeHeader.size(); - return LHS.References > RHS.References; - }); - llvm::SmallVector Headers; - for (const auto &Include : Includes) - Headers.push_back(Include.IncludeHeader); - return Headers; -} - // Runs Sema-based (AST) and Index-based completion, returns merged results. // // There are a few tricky considerations: @@ -1241,19 +1202,12 @@ CodeCompleteResult Output; auto RecorderOwner = llvm::make_unique(Opts, [&]() { assert(Recorder && "Recorder is not set"); - auto Style = - format::getStyle(format::DefaultFormatStyle, SemaCCInput.FileName, - format::DefaultFallbackStyle, SemaCCInput.Contents, - SemaCCInput.VFS.get()); - if (!Style) { - log("getStyle() failed for file {0}: {1}. Fallback is LLVM style.", - SemaCCInput.FileName, Style.takeError()); - Style = format::getLLVMStyle(); - } + auto Style = getFormatStyleForFile( + SemaCCInput.FileName, SemaCCInput.Contents, SemaCCInput.VFS.get()); // If preprocessor was run, inclusions from preprocessor callback should // already be added to Includes. Inserter.emplace( - SemaCCInput.FileName, SemaCCInput.Contents, *Style, + SemaCCInput.FileName, SemaCCInput.Contents, Style, SemaCCInput.Command.Directory, Recorder->CCSema->getPreprocessor().getHeaderSearchInfo()); for (const auto &Inc : Includes.MainFileIncludes) Index: clang-tools-extra/trunk/clangd/Compiler.h =================================================================== --- clang-tools-extra/trunk/clangd/Compiler.h +++ clang-tools-extra/trunk/clangd/Compiler.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H #include "../clang-tidy/ClangTidyOptions.h" +#include "index/Index.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PrecompiledPreamble.h" @@ -33,12 +34,20 @@ const clang::Diagnostic &Info) override; }; +// Options to run clang e.g. when parsing AST. +struct ParseOptions { + tidy::ClangTidyOptions ClangTidyOpts; + bool SuggestMissingIncludes = false; +}; + /// Information required to run clang, e.g. to parse AST or do code completion. struct ParseInputs { tooling::CompileCommand CompileCommand; IntrusiveRefCntPtr FS; std::string Contents; - tidy::ClangTidyOptions ClangTidyOpts; + // Used to recover from diagnostics (e.g. find missing includes for symbol). + const SymbolIndex *Index = nullptr; + ParseOptions Opts; }; /// Builds compiler invocation that could be used to build AST or preamble. Index: clang-tools-extra/trunk/clangd/Diagnostics.h =================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.h +++ clang-tools-extra/trunk/clangd/Diagnostics.h @@ -99,9 +99,15 @@ void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) override; + using DiagFixer = std::function(DiagnosticsEngine::Level, + const clang::Diagnostic &)>; + /// If set, possibly adds fixes for diagnostics using \p Fixer. + void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; } + private: void flushLastDiag(); + DiagFixer Fixer = nullptr; std::vector Output; llvm::Optional LangOpts; llvm::Optional LastDiag; Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp @@ -374,6 +374,11 @@ if (!Info.getFixItHints().empty()) AddFix(true /* try to invent a message instead of repeating the diag */); + if (Fixer) { + auto ExtraFixes = Fixer(DiagLevel, Info); + LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(), + ExtraFixes.end()); + } } else { // Handle a note to an existing diagnostic. if (!LastDiag) { Index: clang-tools-extra/trunk/clangd/Headers.h =================================================================== --- clang-tools-extra/trunk/clangd/Headers.h +++ clang-tools-extra/trunk/clangd/Headers.h @@ -12,10 +12,12 @@ #include "Path.h" #include "Protocol.h" #include "SourceCode.h" +#include "index/Index.h" #include "clang/Format/Format.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Tooling/Inclusions/HeaderIncludes.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" @@ -37,6 +39,15 @@ bool valid() const; }; +/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal +/// include. +llvm::Expected toHeaderFile(llvm::StringRef Header, + llvm::StringRef HintPath); + +// Returns include headers for \p Sym sorted by popularity. If two headers are +// equally popular, prefer the shorter one. +llvm::SmallVector getRankedIncludes(const Symbol &Sym); + // An #include directive that we found in the main file. struct Inclusion { Range R; // Inclusion range. Index: clang-tools-extra/trunk/clangd/Headers.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Headers.cpp +++ clang-tools-extra/trunk/clangd/Headers.cpp @@ -73,6 +73,41 @@ (!Verbatim && llvm::sys::path::is_absolute(File)); } +llvm::Expected toHeaderFile(llvm::StringRef Header, + llvm::StringRef HintPath) { + if (isLiteralInclude(Header)) + return HeaderFile{Header.str(), /*Verbatim=*/true}; + auto U = URI::parse(Header); + if (!U) + return U.takeError(); + + auto IncludePath = URI::includeSpelling(*U); + if (!IncludePath) + return IncludePath.takeError(); + if (!IncludePath->empty()) + return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true}; + + auto Resolved = URI::resolve(*U, HintPath); + if (!Resolved) + return Resolved.takeError(); + return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; +} + +llvm::SmallVector getRankedIncludes(const Symbol &Sym) { + auto Includes = Sym.IncludeHeaders; + // Sort in descending order by reference count and header length. + llvm::sort(Includes, [](const Symbol::IncludeHeaderWithReferences &LHS, + const Symbol::IncludeHeaderWithReferences &RHS) { + if (LHS.References == RHS.References) + return LHS.IncludeHeader.size() < RHS.IncludeHeader.size(); + return LHS.References > RHS.References; + }); + llvm::SmallVector Headers; + for (const auto &Include : Includes) + Headers.push_back(Include.IncludeHeader); + return Headers; +} + std::unique_ptr collectIncludeStructureCallback(const SourceManager &SM, IncludeStructure *Out) { Index: clang-tools-extra/trunk/clangd/IncludeFixer.h =================================================================== --- clang-tools-extra/trunk/clangd/IncludeFixer.h +++ clang-tools-extra/trunk/clangd/IncludeFixer.h @@ -0,0 +1,54 @@ +//===--- IncludeFixer.h ------------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDE_FIXER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDE_FIXER_H + +#include "Diagnostics.h" +#include "Headers.h" +#include "index/Index.h" +#include "clang/AST/Type.h" +#include "clang/Basic/Diagnostic.h" +#include "llvm/ADT/StringRef.h" +#include + +namespace clang { +namespace clangd { + +/// Attempts to recover from error diagnostics by suggesting include insertion +/// fixes. For example, member access into incomplete type can be fixes by +/// include headers with the definition. +class IncludeFixer { +public: + IncludeFixer(llvm::StringRef File, std::shared_ptr Inserter, + const SymbolIndex &Index, unsigned IndexRequestLimit) + : File(File), Inserter(std::move(Inserter)), Index(Index), + IndexRequestLimit(IndexRequestLimit) {} + + /// Returns include insertions that can potentially recover the diagnostic. + std::vector fix(DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info) const; + +private: + /// Attempts to recover diagnostic caused by an incomplete type \p T. + std::vector fixIncompleteType(const Type &T) const; + + /// Generates header insertion fixes for \p Sym. + std::vector fixesForSymbol(const Symbol &Sym) const; + + std::string File; + std::shared_ptr Inserter; + const SymbolIndex &Index; + const unsigned IndexRequestLimit; // Make at most 5 index requests. + mutable unsigned IndexRequestCount = 0; +}; + +} // namespace clangd +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INCLUDE_FIXER_H Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/IncludeFixer.cpp +++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp @@ -0,0 +1,113 @@ +//===--- IncludeFixer.cpp ----------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "IncludeFixer.h" +#include "AST.h" +#include "Diagnostics.h" +#include "Logger.h" +#include "SourceCode.h" +#include "Trace.h" +#include "index/Index.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Type.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticSema.h" +#include "llvm/ADT/None.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/FormatVariadic.h" + +namespace clang { +namespace clangd { + +std::vector IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info) const { + if (IndexRequestCount >= IndexRequestLimit) + return {}; // Avoid querying index too many times in a single parse. + switch (Info.getID()) { + case diag::err_incomplete_type: + case diag::err_incomplete_member_access: + case diag::err_incomplete_base_class: + // Incomplete type diagnostics should have a QualType argument for the + // incomplete type. + for (unsigned Idx = 0; Idx < Info.getNumArgs(); ++Idx) { + if (Info.getArgKind(Idx) == DiagnosticsEngine::ak_qualtype) { + auto QT = QualType::getFromOpaquePtr((void *)Info.getRawArg(Idx)); + if (const Type *T = QT.getTypePtrOrNull()) + if (T->isIncompleteType()) + return fixIncompleteType(*T); + } + } + } + return {}; +} + +std::vector IncludeFixer::fixIncompleteType(const Type &T) const { + // Only handle incomplete TagDecl type. + const TagDecl *TD = T.getAsTagDecl(); + if (!TD) + return {}; + std::string TypeName = printQualifiedName(*TD); + trace::Span Tracer("Fix include for incomplete type"); + SPAN_ATTACH(Tracer, "type", TypeName); + vlog("Trying to fix include for incomplete type {0}", TypeName); + + auto ID = getSymbolID(TD); + if (!ID) + return {}; + ++IndexRequestCount; + // FIXME: consider batching the requests for all diagnostics. + // FIXME: consider caching the lookup results. + LookupRequest Req; + Req.IDs.insert(*ID); + llvm::Optional Matched; + Index.lookup(Req, [&](const Symbol &Sym) { + if (Matched) + return; + Matched = Sym; + }); + + if (!Matched || Matched->IncludeHeaders.empty() || !Matched->Definition || + Matched->CanonicalDeclaration.FileURI != Matched->Definition.FileURI) + return {}; + return fixesForSymbol(*Matched); +} + +std::vector IncludeFixer::fixesForSymbol(const Symbol &Sym) const { + auto Inserted = [&](llvm::StringRef Header) + -> llvm::Expected> { + auto ResolvedDeclaring = + toHeaderFile(Sym.CanonicalDeclaration.FileURI, File); + if (!ResolvedDeclaring) + return ResolvedDeclaring.takeError(); + auto ResolvedInserted = toHeaderFile(Header, File); + if (!ResolvedInserted) + return ResolvedInserted.takeError(); + return std::make_pair( + Inserter->calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted), + Inserter->shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted)); + }; + + std::vector Fixes; + for (const auto &Inc : getRankedIncludes(Sym)) { + if (auto ToInclude = Inserted(Inc)) { + if (ToInclude->second) + if (auto Edit = Inserter->insert(ToInclude->first)) + Fixes.push_back( + Fix{llvm::formatv("Add include {0} for symbol {1}{2}", + ToInclude->first, Sym.Scope, Sym.Name), + {std::move(*Edit)}}); + } else { + vlog("Failed to calculate include insertion for {0} into {1}: {2}", File, + Inc, ToInclude.takeError()); + } + } + return Fixes; +} + +} // namespace clangd +} // namespace clang Index: clang-tools-extra/trunk/clangd/SourceCode.h =================================================================== --- clang-tools-extra/trunk/clangd/SourceCode.h +++ clang-tools-extra/trunk/clangd/SourceCode.h @@ -16,7 +16,9 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/SHA1.h" namespace clang { @@ -91,6 +93,11 @@ const SourceManager &SourceMgr); bool isRangeConsecutive(const Range &Left, const Range &Right); + +format::FormatStyle getFormatStyleForFile(llvm::StringRef File, + llvm::StringRef Content, + llvm::vfs::FileSystem *FS); + } // namespace clangd } // namespace clang #endif Index: clang-tools-extra/trunk/clangd/SourceCode.cpp =================================================================== --- clang-tools-extra/trunk/clangd/SourceCode.cpp +++ clang-tools-extra/trunk/clangd/SourceCode.cpp @@ -248,5 +248,18 @@ return digest(Content); } +format::FormatStyle getFormatStyleForFile(llvm::StringRef File, + llvm::StringRef Content, + llvm::vfs::FileSystem *FS) { + auto Style = format::getStyle(format::DefaultFormatStyle, File, + format::DefaultFallbackStyle, Content, FS); + if (!Style) { + log("getStyle() failed for file {0}: {1}. Fallback is LLVM style.", File, + Style.takeError()); + Style = format::getLLVMStyle(); + } + return *Style; +} + } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp =================================================================== --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp @@ -207,6 +207,12 @@ ".clang-tidy files)"), llvm::cl::init(""), llvm::cl::Hidden); +static llvm::cl::opt SuggestMissingIncludes( + "suggest-missing-includes", + llvm::cl::desc("Attempts to fix diagnostic errors caused by missing " + "includes using index."), + llvm::cl::init(false)); + namespace { /// \brief Supports a test URI scheme with relaxed constraints for lit tests. @@ -442,6 +448,7 @@ /* Default */ tidy::ClangTidyOptions::getDefaults(), /* Override */ OverrideClangTidyOptions, FSProvider.getFileSystem()); Opts.ClangTidyOptProvider = &ClangTidyOptProvider; + Opts.SuggestMissingIncludes = SuggestMissingIncludes; ClangdLSPServer LSPServer( *TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath, /*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs, Opts); Index: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt +++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt @@ -18,6 +18,7 @@ CodeCompletionStringsTests.cpp ContextTests.cpp DexTests.cpp + DiagnosticsTests.cpp DraftStoreTests.cpp ExpectedTypeTest.cpp FileDistanceTests.cpp Index: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp @@ -19,263 +19,6 @@ namespace { using testing::ElementsAre; -using testing::Field; -using testing::IsEmpty; -using testing::Pair; -using testing::UnorderedElementsAre; - -testing::Matcher WithFix(testing::Matcher FixMatcher) { - return Field(&Diag::Fixes, ElementsAre(FixMatcher)); -} - -testing::Matcher WithNote(testing::Matcher NoteMatcher) { - return Field(&Diag::Notes, ElementsAre(NoteMatcher)); -} - -MATCHER_P2(Diag, Range, Message, - "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.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; -} - -TEST(DiagnosticsTest, DiagnosticRanges) { - // Check we report correct ranges, including various edge-cases. - Annotations Test(R"cpp( - namespace test{}; - void $decl[[foo]](); - int main() { - $typo[[go\ -o]](); - foo()$semicolon[[]]//with comments - $unk[[unknown]](); - double $type[[bar]] = "foo"; - struct Foo { int x; }; Foo a; - a.$nomember[[y]]; - test::$nomembernamespace[[test]]; - } - )cpp"); - EXPECT_THAT( - TestTU::withCode(Test.code()).build().getDiagnostics(), - ElementsAre( - // This range spans lines. - AllOf(Diag(Test.range("typo"), - "use of undeclared identifier 'goo'; did you mean 'foo'?"), - WithFix( - Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")), - // This is a pretty normal range. - WithNote(Diag(Test.range("decl"), "'foo' declared here"))), - // This range is zero-width and insertion. Therefore make sure we are - // not expanding it into other tokens. Since we are not going to - // replace those. - AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"), - WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))), - // This range isn't provided by clang, we expand to the token. - Diag(Test.range("unk"), "use of undeclared identifier 'unknown'"), - Diag(Test.range("type"), - "cannot initialize a variable of type 'double' with an lvalue " - "of type 'const char [4]'"), - Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"), - Diag(Test.range("nomembernamespace"), - "no member named 'test' in namespace 'test'"))); -} - -TEST(DiagnosticsTest, FlagsMatter) { - Annotations Test("[[void]] main() {}"); - auto TU = TestTU::withCode(Test.code()); - EXPECT_THAT(TU.build().getDiagnostics(), - ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"), - WithFix(Fix(Test.range(), "int", - "change 'void' to 'int'"))))); - // Same code built as C gets different diagnostics. - TU.Filename = "Plain.c"; - EXPECT_THAT( - TU.build().getDiagnostics(), - ElementsAre(AllOf( - Diag(Test.range(), "return type of 'main' is not 'int'"), - WithFix(Fix(Test.range(), "int", "change return type to 'int'"))))); -} - -TEST(DiagnosticsTest, ClangTidy) { - Annotations Test(R"cpp( - #include $deprecated[["assert.h"]] - - #define $macrodef[[SQUARE]](X) (X)*(X) - int main() { - return $doubled[[sizeof]](sizeof(int)); - int y = 4; - return SQUARE($macroarg[[++]]y); - } - )cpp"); - auto TU = TestTU::withCode(Test.code()); - TU.HeaderFilename = "assert.h"; // Suppress "not found" error. - TU.ClangTidyChecks = - "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, " - "modernize-deprecated-headers"; - EXPECT_THAT( - TU.build().getDiagnostics(), - UnorderedElementsAre( - AllOf(Diag(Test.range("deprecated"), - "inclusion of deprecated C++ header 'assert.h'; consider " - "using 'cassert' instead [modernize-deprecated-headers]"), - WithFix(Fix(Test.range("deprecated"), "", - "change '\"assert.h\"' to ''"))), - Diag(Test.range("doubled"), - "suspicious usage of 'sizeof(sizeof(...))' " - "[bugprone-sizeof-expression]"), - AllOf( - Diag(Test.range("macroarg"), - "side effects in the 1st macro argument 'X' are repeated in " - "macro expansion [bugprone-macro-repeated-side-effects]"), - WithNote(Diag(Test.range("macrodef"), - "macro 'SQUARE' defined here " - "[bugprone-macro-repeated-side-effects]"))), - Diag(Test.range("macroarg"), - "multiple unsequenced modifications to 'y'"))); -} - -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( - TestTU::withCode(Test.code()).build().getDiagnostics(), - ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'"))); -} - -TEST(DiagnosticsTest, InsideMacros) { - Annotations Test(R"cpp( - #define TEN 10 - #define RET(x) return x + 10 - - int* foo() { - RET($foo[[0]]); - } - int* bar() { - return $bar[[TEN]]; - } - )cpp"); - EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(), - ElementsAre(Diag(Test.range("foo"), - "cannot initialize return object of type " - "'int *' with an rvalue of type 'int'"), - Diag(Test.range("bar"), - "cannot initialize return object of type " - "'int *' with an rvalue of type 'int'"))); -} - -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, 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>> LSPDiags; - toLSPDiags(D, -#ifdef _WIN32 - URIForFile::canonicalize("c:\\path\\to\\foo\\bar\\main.cpp", - /*TUPath=*/""), -#else - URIForFile::canonicalize("/path/to/foo/bar/main.cpp", /*TUPath=*/""), -#endif - ClangdDiagnosticOptions(), - [&](clangd::Diagnostic LSPDiag, ArrayRef Fixes) { - LSPDiags.push_back( - {std::move(LSPDiag), - std::vector(Fixes.begin(), Fixes.end())}); - }); - - EXPECT_THAT( - LSPDiags, - ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F))), - Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty()))); -} TEST(ClangdUnitTest, GetBeginningOfIdentifier) { std::string Preamble = R"cpp( Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -16,6 +16,7 @@ #include "SourceCode.h" #include "SyncAPI.h" #include "TestFS.h" +#include "TestIndex.h" #include "index/MemIndex.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "llvm/Support/Error.h" @@ -137,53 +138,6 @@ FilePath); } -std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle, - llvm::StringRef Repl) { - std::string Result; - llvm::raw_string_ostream OS(Result); - std::pair Split; - for (Split = Haystack.split(Needle); !Split.second.empty(); - Split = Split.first.split(Needle)) - OS << Split.first << Repl; - Result += Split.first; - OS.flush(); - return Result; -} - -// Helpers to produce fake index symbols for memIndex() or completions(). -// USRFormat is a regex replacement string for the unqualified part of the USR. -Symbol sym(llvm::StringRef QName, index::SymbolKind Kind, - llvm::StringRef USRFormat) { - Symbol Sym; - std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand! - size_t Pos = QName.rfind("::"); - if (Pos == llvm::StringRef::npos) { - Sym.Name = QName; - Sym.Scope = ""; - } else { - Sym.Name = QName.substr(Pos + 2); - Sym.Scope = QName.substr(0, Pos + 2); - USR += "@N@" + replace(QName.substr(0, Pos), "::", "@N@"); // ns:: -> @N@ns - } - USR += llvm::Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func# - Sym.ID = SymbolID(USR); - Sym.SymInfo.Kind = Kind; - Sym.Flags |= Symbol::IndexedForCodeCompletion; - Sym.Origin = SymbolOrigin::Static; - return Sym; -} -Symbol func(llvm::StringRef Name) { // Assumes the function has no args. - return sym(Name, index::SymbolKind::Function, "@F@\\0#"); // no args -} -Symbol cls(llvm::StringRef Name) { - return sym(Name, index::SymbolKind::Class, "@S@\\0"); -} -Symbol var(llvm::StringRef Name) { - return sym(Name, index::SymbolKind::Variable, "@\\0"); -} -Symbol ns(llvm::StringRef Name) { - return sym(Name, index::SymbolKind::Namespace, "@N@\\0"); -} Symbol withReferences(int N, Symbol S) { S.References = N; return S; Index: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp @@ -0,0 +1,351 @@ +//===--- DiagnosticsTests.cpp ------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Annotations.h" +#include "ClangdUnit.h" +#include "SourceCode.h" +#include "TestIndex.h" +#include "TestTU.h" +#include "index/MemIndex.h" +#include "llvm/Support/ScopedPrinter.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace { + +using testing::ElementsAre; +using testing::Field; +using testing::IsEmpty; +using testing::Pair; +using testing::UnorderedElementsAre; + +testing::Matcher WithFix(testing::Matcher FixMatcher) { + return Field(&Diag::Fixes, ElementsAre(FixMatcher)); +} + +testing::Matcher WithNote(testing::Matcher NoteMatcher) { + return Field(&Diag::Notes, ElementsAre(NoteMatcher)); +} + +MATCHER_P2(Diag, Range, Message, + "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.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; +} + +TEST(DiagnosticsTest, DiagnosticRanges) { + // Check we report correct ranges, including various edge-cases. + Annotations Test(R"cpp( + namespace test{}; + void $decl[[foo]](); + int main() { + $typo[[go\ +o]](); + foo()$semicolon[[]]//with comments + $unk[[unknown]](); + double $type[[bar]] = "foo"; + struct Foo { int x; }; Foo a; + a.$nomember[[y]]; + test::$nomembernamespace[[test]]; + } + )cpp"); + EXPECT_THAT( + TestTU::withCode(Test.code()).build().getDiagnostics(), + ElementsAre( + // This range spans lines. + AllOf(Diag(Test.range("typo"), + "use of undeclared identifier 'goo'; did you mean 'foo'?"), + WithFix( + Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")), + // This is a pretty normal range. + WithNote(Diag(Test.range("decl"), "'foo' declared here"))), + // This range is zero-width and insertion. Therefore make sure we are + // not expanding it into other tokens. Since we are not going to + // replace those. + AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"), + WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))), + // This range isn't provided by clang, we expand to the token. + Diag(Test.range("unk"), "use of undeclared identifier 'unknown'"), + Diag(Test.range("type"), + "cannot initialize a variable of type 'double' with an lvalue " + "of type 'const char [4]'"), + Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"), + Diag(Test.range("nomembernamespace"), + "no member named 'test' in namespace 'test'"))); +} + +TEST(DiagnosticsTest, FlagsMatter) { + Annotations Test("[[void]] main() {}"); + auto TU = TestTU::withCode(Test.code()); + EXPECT_THAT(TU.build().getDiagnostics(), + ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"), + WithFix(Fix(Test.range(), "int", + "change 'void' to 'int'"))))); + // Same code built as C gets different diagnostics. + TU.Filename = "Plain.c"; + EXPECT_THAT( + TU.build().getDiagnostics(), + ElementsAre(AllOf( + Diag(Test.range(), "return type of 'main' is not 'int'"), + WithFix(Fix(Test.range(), "int", "change return type to 'int'"))))); +} + +TEST(DiagnosticsTest, ClangTidy) { + Annotations Test(R"cpp( + #include $deprecated[["assert.h"]] + + #define $macrodef[[SQUARE]](X) (X)*(X) + int main() { + return $doubled[[sizeof]](sizeof(int)); + int y = 4; + return SQUARE($macroarg[[++]]y); + } + )cpp"); + auto TU = TestTU::withCode(Test.code()); + TU.HeaderFilename = "assert.h"; // Suppress "not found" error. + TU.ClangTidyChecks = + "-*, bugprone-sizeof-expression, bugprone-macro-repeated-side-effects, " + "modernize-deprecated-headers"; + EXPECT_THAT( + TU.build().getDiagnostics(), + UnorderedElementsAre( + AllOf(Diag(Test.range("deprecated"), + "inclusion of deprecated C++ header 'assert.h'; consider " + "using 'cassert' instead [modernize-deprecated-headers]"), + WithFix(Fix(Test.range("deprecated"), "", + "change '\"assert.h\"' to ''"))), + Diag(Test.range("doubled"), + "suspicious usage of 'sizeof(sizeof(...))' " + "[bugprone-sizeof-expression]"), + AllOf( + Diag(Test.range("macroarg"), + "side effects in the 1st macro argument 'X' are repeated in " + "macro expansion [bugprone-macro-repeated-side-effects]"), + WithNote(Diag(Test.range("macrodef"), + "macro 'SQUARE' defined here " + "[bugprone-macro-repeated-side-effects]"))), + Diag(Test.range("macroarg"), + "multiple unsequenced modifications to 'y'"))); +} + +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( + TestTU::withCode(Test.code()).build().getDiagnostics(), + ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'"))); +} + +TEST(DiagnosticsTest, InsideMacros) { + Annotations Test(R"cpp( + #define TEN 10 + #define RET(x) return x + 10 + + int* foo() { + RET($foo[[0]]); + } + int* bar() { + return $bar[[TEN]]; + } + )cpp"); + EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(), + ElementsAre(Diag(Test.range("foo"), + "cannot initialize return object of type " + "'int *' with an rvalue of type 'int'"), + Diag(Test.range("bar"), + "cannot initialize return object of type " + "'int *' with an rvalue of type 'int'"))); +} + +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, 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>> LSPDiags; + toLSPDiags(D, +#ifdef _WIN32 + URIForFile::canonicalize("c:\\path\\to\\foo\\bar\\main.cpp", + /*TUPath=*/""), +#else + URIForFile::canonicalize("/path/to/foo/bar/main.cpp", /*TUPath=*/""), +#endif + ClangdDiagnosticOptions(), + [&](clangd::Diagnostic LSPDiag, ArrayRef Fixes) { + LSPDiags.push_back( + {std::move(LSPDiag), + std::vector(Fixes.begin(), Fixes.end())}); + }); + + EXPECT_THAT( + LSPDiags, + ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F))), + Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty()))); +} + +TEST(IncludeFixerTest, IncompleteType) { + Annotations Test(R"cpp( +$insert[[]]namespace ns { + class X; +} +class Y : $base[[public ns::X]] {}; +int main() { + ns::X *x; + x$access[[->]]f(); +} + )cpp"); + auto TU = TestTU::withCode(Test.code()); + Symbol Sym = cls("ns::X"); + Sym.Flags |= Symbol::IndexedForCodeCompletion; + Sym.CanonicalDeclaration.FileURI = "unittest:///x.h"; + Sym.Definition.FileURI = "unittest:///x.h"; + Sym.IncludeHeaders.emplace_back("\"x.h\"", 1); + + SymbolSlab::Builder Slab; + Slab.insert(Sym); + auto Index = MemIndex::build(std::move(Slab).build(), RefSlab()); + TU.ExternalIndex = Index.get(); + + EXPECT_THAT( + TU.build().getDiagnostics(), + UnorderedElementsAre( + AllOf(Diag(Test.range("base"), "base class has incomplete type"), + WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", + "Add include \"x.h\" for symbol ns::X"))), + AllOf(Diag(Test.range("access"), + "member access into incomplete type 'ns::X'"), + WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n", + "Add include \"x.h\" for symbol ns::X"))))); +} + +TEST(IncludeFixerTest, NoSuggestIncludeWhenNoDefinitionInHeader) { + Annotations Test(R"cpp( +$insert[[]]namespace ns { + class X; +} +class Y : $base[[public ns::X]] {}; +int main() { + ns::X *x; + x$access[[->]]f(); +} + )cpp"); + auto TU = TestTU::withCode(Test.code()); + Symbol Sym = cls("ns::X"); + Sym.Flags |= Symbol::IndexedForCodeCompletion; + Sym.CanonicalDeclaration.FileURI = "unittest:///x.h"; + Sym.Definition.FileURI = "unittest:///x.cc"; + Sym.IncludeHeaders.emplace_back("\"x.h\"", 1); + + SymbolSlab::Builder Slab; + Slab.insert(Sym); + auto Index = MemIndex::build(std::move(Slab).build(), RefSlab()); + TU.ExternalIndex = Index.get(); + + EXPECT_THAT(TU.build().getDiagnostics(), + UnorderedElementsAre( + Diag(Test.range("base"), "base class has incomplete type"), + Diag(Test.range("access"), + "member access into incomplete type 'ns::X'"))); +} + +} // namespace +} // namespace clangd +} // namespace clang + Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp @@ -364,7 +364,7 @@ ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData, llvm::MemoryBuffer::getMemBufferCopy(Main.code()), std::make_shared(), PI.FS, - tidy::ClangTidyOptions::getDefaults()); + /*Index=*/nullptr, ParseOptions()); ASSERT_TRUE(AST); FileIndex Index; Index.updateMain(MainFile, *AST); Index: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp @@ -26,7 +26,6 @@ using ::testing::AnyOf; using ::testing::Each; using ::testing::ElementsAre; -using ::testing::Pair; using ::testing::Pointee; using ::testing::UnorderedElementsAre; @@ -37,9 +36,12 @@ class TUSchedulerTests : public ::testing::Test { protected: ParseInputs getInputs(PathRef File, std::string Contents) { - return ParseInputs{*CDB.getCompileCommand(File), - buildTestFS(Files, Timestamps), std::move(Contents), - tidy::ClangTidyOptions::getDefaults()}; + ParseInputs Inputs; + Inputs.CompileCommand = *CDB.getCompileCommand(File); + Inputs.FS = buildTestFS(Files, Timestamps); + Inputs.Contents = std::move(Contents); + Inputs.Opts = ParseOptions(); + return Inputs; } void updateWithCallback(TUScheduler &S, PathRef File, Index: clang-tools-extra/trunk/unittests/clangd/TestIndex.h =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestIndex.h +++ clang-tools-extra/trunk/unittests/clangd/TestIndex.h @@ -18,6 +18,19 @@ // Creates Symbol instance and sets SymbolID to given QualifiedName. Symbol symbol(llvm::StringRef QName); +// Helpers to produce fake index symbols with proper SymbolID. +// USRFormat is a regex replacement string for the unqualified part of the USR. +Symbol sym(llvm::StringRef QName, index::SymbolKind Kind, + llvm::StringRef USRFormat); +// Creats a function symbol assuming no function arg. +Symbol func(llvm::StringRef Name); +// Creates a class symbol. +Symbol cls(llvm::StringRef Name); +// Creates a variable symbol. +Symbol var(llvm::StringRef Name); +// Creates a namespace symbol. +Symbol ns(llvm::StringRef Name); + // Create a slab of symbols with the given qualified names as IDs and names. SymbolSlab generateSymbols(std::vector QualifiedNames); Index: clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp +++ clang-tools-extra/trunk/unittests/clangd/TestIndex.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "TestIndex.h" +#include "clang/Index/IndexSymbol.h" +#include "llvm/Support/Regex.h" namespace clang { namespace clangd { @@ -25,6 +27,58 @@ return Sym; } +static std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle, + llvm::StringRef Repl) { + std::string Result; + llvm::raw_string_ostream OS(Result); + std::pair Split; + for (Split = Haystack.split(Needle); !Split.second.empty(); + Split = Split.first.split(Needle)) + OS << Split.first << Repl; + Result += Split.first; + OS.flush(); + return Result; +} + +// Helpers to produce fake index symbols for memIndex() or completions(). +// USRFormat is a regex replacement string for the unqualified part of the USR. +Symbol sym(llvm::StringRef QName, index::SymbolKind Kind, + llvm::StringRef USRFormat) { + Symbol Sym; + std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand! + size_t Pos = QName.rfind("::"); + if (Pos == llvm::StringRef::npos) { + Sym.Name = QName; + Sym.Scope = ""; + } else { + Sym.Name = QName.substr(Pos + 2); + Sym.Scope = QName.substr(0, Pos + 2); + USR += "@N@" + replace(QName.substr(0, Pos), "::", "@N@"); // ns:: -> @N@ns + } + USR += llvm::Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func# + Sym.ID = SymbolID(USR); + Sym.SymInfo.Kind = Kind; + Sym.Flags |= Symbol::IndexedForCodeCompletion; + Sym.Origin = SymbolOrigin::Static; + return Sym; +} + +Symbol func(llvm::StringRef Name) { // Assumes the function has no args. + return sym(Name, index::SymbolKind::Function, "@F@\\0#"); // no args +} + +Symbol cls(llvm::StringRef Name) { + return sym(Name, index::SymbolKind::Class, "@S@\\0"); +} + +Symbol var(llvm::StringRef Name) { + return sym(Name, index::SymbolKind::Variable, "@\\0"); +} + +Symbol ns(llvm::StringRef Name) { + return sym(Name, index::SymbolKind::Namespace, "@N@\\0"); +} + SymbolSlab generateSymbols(std::vector QualifiedNames) { SymbolSlab::Builder Slab; for (llvm::StringRef QName : QualifiedNames) Index: clang-tools-extra/trunk/unittests/clangd/TestTU.h =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestTU.h +++ clang-tools-extra/trunk/unittests/clangd/TestTU.h @@ -49,6 +49,8 @@ std::vector ExtraArgs; llvm::Optional ClangTidyChecks; + // Index to use when building AST. + const SymbolIndex *ExternalIndex = nullptr; ParsedAST build() const; SymbolSlab headerSymbols() const; Index: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp +++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp @@ -35,8 +35,11 @@ Inputs.CompileCommand.Directory = testRoot(); Inputs.Contents = Code; Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}}); - Inputs.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults(); - Inputs.ClangTidyOpts.Checks = ClangTidyChecks; + Inputs.Opts = ParseOptions(); + Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks; + Inputs.Index = ExternalIndex; + if (Inputs.Index) + Inputs.Opts.SuggestMissingIncludes = true; auto PCHs = std::make_shared(); auto CI = buildCompilerInvocation(Inputs); assert(CI && "Failed to build compilation invocation.");