Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -161,6 +161,7 @@ // both the old and the new version in case only one of them matches. CompletionList Result = clangd::codeComplete( File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr, + PreambleData ? PreambleData->Inclusions : std::vector(), IP->Contents, Pos, FS, PCHs, CodeCompleteOpts); CB(std::move(Result)); }; Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -15,6 +15,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H +#include "Headers.h" #include "Logger.h" #include "Path.h" #include "Protocol.h" @@ -69,6 +70,7 @@ CompletionList codeComplete(PathRef FileName, const tooling::CompileCommand &Command, PrecompiledPreamble const *Preamble, + const std::vector &PreambleInclusions, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -18,9 +18,11 @@ #include "CodeCompletionStrings.h" #include "Compiler.h" #include "FuzzyMatch.h" +#include "Headers.h" #include "Logger.h" #include "SourceCode.h" #include "Trace.h" +#include "URI.h" #include "index/Index.h" #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" @@ -219,6 +221,28 @@ return S; } +/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal +/// include. +static llvm::Expected toHeaderFile(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 { @@ -255,10 +279,10 @@ } // Builds an LSP completion item. - CompletionItem build(llvm::StringRef FileName, - const CompletionItemScores &Scores, + CompletionItem build(StringRef FileName, const CompletionItemScores &Scores, const CodeCompleteOptions &Opts, - CodeCompletionString *SemaCCS) const { + CodeCompletionString *SemaCCS, + const IncludeInserter *Includes) const { assert(bool(SemaResult) == bool(SemaCCS)); CompletionItem I; if (SemaResult) { @@ -289,6 +313,31 @@ I.documentation = D->Documentation; if (I.detail.empty()) I.detail = D->CompletionDetail; + if (Includes && !D->IncludeHeader.empty()) { + // Fallback to canonical header if declaration location is invalid. + auto Edit = [&]() -> Expected> { + auto ResolvedDeclaring = toHeaderFile( + IndexResult->CanonicalDeclaration.FileURI, FileName); + if (!ResolvedDeclaring) + return ResolvedDeclaring.takeError(); + auto ResolvedInserted = toHeaderFile(D->IncludeHeader, FileName); + if (!ResolvedInserted) + return ResolvedInserted.takeError(); + return Includes->insert(*ResolvedDeclaring, *ResolvedInserted); + }(); + if (!Edit) { + std::string ErrMsg = + ("Failed to generate include insertion edits for adding header " + "(FileURI=\"" + + IndexResult->CanonicalDeclaration.FileURI + + "\", IncludeHeader=\"" + D->IncludeHeader + "\") into " + + FileName) + .str(); + log(ErrMsg + ":" + llvm::toString(Edit.takeError())); + } else if (*Edit) { + I.additionalTextEdits = {std::move(**Edit)}; + } + } } } I.scoreInfo = Scores; @@ -649,6 +698,7 @@ PathRef FileName; const tooling::CompileCommand &Command; PrecompiledPreamble const *Preamble; + const std::vector &PreambleInclusions; StringRef Contents; Position Pos; IntrusiveRefCntPtr VFS; @@ -656,9 +706,12 @@ }; // Invokes Sema code completion on a file. +// If \p Includes is set, it will be initialized after a compiler instance has +// been set up. bool semaCodeComplete(std::unique_ptr Consumer, const clang::CodeCompleteOptions &Options, - const SemaCompleteInput &Input) { + const SemaCompleteInput &Input, + std::unique_ptr *Includes = nullptr) { trace::Span Tracer("Sema completion"); std::vector ArgStrs; for (const auto &S : Input.Command.CommandLine) @@ -729,6 +782,28 @@ Input.FileName); return false; } + if (Includes) { + // Initialize Includes if provided. + + // FIXME(ioeric): needs more consistent style support in clangd server. + auto Style = format::getStyle("file", Input.FileName, "LLVM", + Input.Contents, Input.VFS.get()); + if (!Style) { + log("Failed to get FormaStyle for file" + Input.FileName + + ". Fall back to use LLVM style. Error: " + + llvm::toString(Style.takeError())); + Style = format::getLLVMStyle(); + } + *Includes = llvm::make_unique( + Input.FileName, Input.Contents, *Style, Input.Command.Directory, + Clang->getPreprocessor().getHeaderSearchInfo()); + for (const auto &Inc : Input.PreambleInclusions) + Includes->get()->addExisting(Inc); + Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback( + Clang->getSourceManager(), [&Includes](Inclusion Inc) { + Includes->get()->addExisting(std::move(Inc)); + })); + } if (!Action.Execute()) { log("Execute() failed when running codeComplete for " + Input.FileName); return false; @@ -863,7 +938,8 @@ CompletionRecorder *Recorder = nullptr; int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging. bool Incomplete = false; // Would more be available with a higher limit? - llvm::Optional Filter; // Initialized once Sema runs. + llvm::Optional Filter; // Initialized once Sema runs. + std::unique_ptr Includes; // Initialized once compiler runs. public: // A CodeCompleteFlow object is only useful for calling run() exactly once. @@ -872,20 +948,25 @@ CompletionList run(const SemaCompleteInput &SemaCCInput) && { trace::Span Tracer("CodeCompleteFlow"); + // We run Sema code completion first. It builds an AST and calculates: // - completion results based on the AST. // - partial identifier and context. We need these for the index query. CompletionList Output; auto RecorderOwner = llvm::make_unique(Opts, [&]() { assert(Recorder && "Recorder is not set"); + assert(Includes && "Includes is not set"); + // If preprocessor was run, inclusions from preprocessor callback should + // already be added to Inclusions. Output = runWithSema(); + Includes.release(); // Make sure this doesn't out-live Clang. SPAN_ATTACH(Tracer, "sema_completion_kind", getCompletionKindString(Recorder->CCContext.getKind())); }); Recorder = RecorderOwner.get(); semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(), - SemaCCInput); + SemaCCInput, &Includes); SPAN_ATTACH(Tracer, "sema_results", NSema); SPAN_ATTACH(Tracer, "index_results", NIndex); @@ -1011,19 +1092,21 @@ CodeCompletionString *SemaCCS = nullptr; if (auto *SR = Candidate.SemaResult) SemaCCS = Recorder->codeCompletionString(*SR, Opts.IncludeBriefComments); - return Candidate.build(FileName, Scores, Opts, SemaCCS); + return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get()); } }; CompletionList codeComplete(PathRef FileName, const tooling::CompileCommand &Command, PrecompiledPreamble const *Preamble, + const std::vector &PreambleInclusions, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, CodeCompleteOptions Opts) { return CodeCompleteFlow(FileName, Opts) - .run({FileName, Command, Preamble, Contents, Pos, VFS, PCHs}); + .run({FileName, Command, Preamble, PreambleInclusions, Contents, Pos, VFS, + PCHs}); } SignatureHelp signatureHelp(PathRef FileName, @@ -1040,7 +1123,8 @@ Options.IncludeBriefComments = true; semaCodeComplete(llvm::make_unique(Options, Result), Options, - {FileName, Command, Preamble, Contents, Pos, std::move(VFS), + {FileName, Command, Preamble, + /*PreambleInclusions=*/{}, Contents, Pos, std::move(VFS), std::move(PCHs)}); return Result; } Index: clangd/Headers.h =================================================================== --- clangd/Headers.h +++ clangd/Headers.h @@ -12,10 +12,14 @@ #include "Path.h" #include "Protocol.h" +#include "SourceCode.h" #include "clang/Basic/VirtualFileSystem.h" +#include "clang/Format/Format.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Lex/PPCallbacks.h" -#include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Core/HeaderIncludes.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" namespace clang { @@ -51,20 +55,51 @@ /// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'. /// /// \param File is an absolute file path. +/// \param Inclusions Existing inclusions in the main file. /// \param DeclaringHeader is the original header corresponding to \p /// InsertedHeader e.g. the header that declares a symbol. /// \param InsertedHeader The preferred header to be inserted. This could be the /// same as DeclaringHeader but must be provided. // \return A quoted "path" or . This returns an empty string if: /// - Either \p DeclaringHeader or \p InsertedHeader is already (directly) -/// included in the file (including those included via different paths). +/// in \p Inclusions (including those included via different paths). /// - \p DeclaringHeader or \p InsertedHeader is the same as \p File. -llvm::Expected -calculateIncludePath(PathRef File, llvm::StringRef Code, - const HeaderFile &DeclaringHeader, - const HeaderFile &InsertedHeader, - const tooling::CompileCommand &CompileCommand, - IntrusiveRefCntPtr FS); +llvm::Expected calculateIncludePath( + PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo, + const std::vector &Inclusions, const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader); + +// Calculates insertion edit for including a new header in a file. +class IncludeInserter { +public: + IncludeInserter(StringRef FileName, StringRef Code, + const format::FormatStyle &Style, StringRef BuildDir, + HeaderSearch &HeaderSearchInfo) + : FileName(FileName), Code(Code), BuildDir(BuildDir), + HeaderSearchInfo(HeaderSearchInfo), + Inserter(FileName, Code, Style.IncludeStyle) {} + + void addExisting(Inclusion Inc) { Inclusions.push_back(std::move(Inc)); } + + /// Returns a TextEdit that inserts a new header; if the header is not + /// inserted e.g. it's an existing header, this returns None. If any header is + /// invalid, this returns error. + /// + /// \param DeclaringHeader is the original header corresponding to \p + /// InsertedHeader e.g. the header that declares a symbol. + /// \param InsertedHeader The preferred header to be inserted. This could be + /// the same as DeclaringHeader but must be provided. + Expected> insert(const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader) const; + +private: + StringRef FileName; + StringRef Code; + StringRef BuildDir; + HeaderSearch &HeaderSearchInfo; + std::vector Inclusions; + tooling::HeaderIncludes Inserter; // Computers insertion replacement. +}; } // namespace clangd } // namespace clang Index: clangd/Headers.cpp =================================================================== --- clangd/Headers.cpp +++ clangd/Headers.cpp @@ -15,8 +15,6 @@ #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Lex/HeaderSearch.h" -#include "clang/Lex/PreprocessorOptions.h" -#include "clang/Tooling/CompilationDatabase.h" #include "llvm/Support/Path.h" namespace clang { @@ -74,64 +72,13 @@ /// FIXME(ioeric): we might not want to insert an absolute include path if the /// path is not shortened. -llvm::Expected -calculateIncludePath(llvm::StringRef File, llvm::StringRef Code, - const HeaderFile &DeclaringHeader, - const HeaderFile &InsertedHeader, - const tooling::CompileCommand &CompileCommand, - IntrusiveRefCntPtr FS) { - assert(llvm::sys::path::is_absolute(File)); +llvm::Expected calculateIncludePath( + PathRef File, StringRef BuildDir, HeaderSearch &HeaderSearchInfo, + const std::vector &Inclusions, const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader) { assert(DeclaringHeader.valid() && InsertedHeader.valid()); if (File == DeclaringHeader.File || File == InsertedHeader.File) return ""; - FS->setCurrentWorkingDirectory(CompileCommand.Directory); - - // Set up a CompilerInstance and create a preprocessor to collect existing - // #include headers in \p Code. Preprocesor also provides HeaderSearch with - // which we can calculate the shortest include path for \p Header. - std::vector Argv; - for (const auto &S : CompileCommand.CommandLine) - Argv.push_back(S.c_str()); - IgnoringDiagConsumer IgnoreDiags; - auto CI = clang::createInvocationFromCommandLine( - Argv, - CompilerInstance::createDiagnostics(new DiagnosticOptions(), &IgnoreDiags, - false), - FS); - if (!CI) - return llvm::make_error( - "Failed to create a compiler instance for " + File, - llvm::inconvertibleErrorCode()); - CI->getFrontendOpts().DisableFree = false; - // Parse the main file to get all existing #includes in the file, and then we - // can make sure the same header (even with different include path) is not - // added more than once. - CI->getPreprocessorOpts().SingleFileParseMode = true; - - // The diagnostic options must be set before creating a CompilerInstance. - CI->getDiagnosticOpts().IgnoreWarnings = true; - auto Clang = prepareCompilerInstance( - std::move(CI), /*Preamble=*/nullptr, - llvm::MemoryBuffer::getMemBuffer(Code, File), - std::make_shared(), FS, IgnoreDiags); - - if (Clang->getFrontendOpts().Inputs.empty()) - return llvm::make_error( - "Empty frontend action inputs empty for file " + File, - llvm::inconvertibleErrorCode()); - PreprocessOnlyAction Action; - if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) - return llvm::make_error( - "Failed to begin preprocessor only action for file " + File, - llvm::inconvertibleErrorCode()); - std::vector Inclusions; - Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback( - Clang->getSourceManager(), - [&Inclusions](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); })); - if (!Action.Execute()) - return llvm::make_error( - "Failed to execute preprocessor only action for file " + File, - llvm::inconvertibleErrorCode()); llvm::StringSet<> IncludedHeaders; for (const auto &Inc : Inclusions) { IncludedHeaders.insert(Inc.Written); @@ -144,14 +91,13 @@ if (Included(DeclaringHeader.File) || Included(InsertedHeader.File)) return ""; - auto &HeaderSearchInfo = Clang->getPreprocessor().getHeaderSearchInfo(); bool IsSystem = false; if (InsertedHeader.Verbatim) return InsertedHeader.File; std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics( - InsertedHeader.File, CompileCommand.Directory, &IsSystem); + InsertedHeader.File, BuildDir, &IsSystem); if (IsSystem) Suggested = "<" + Suggested + ">"; else @@ -161,5 +107,35 @@ return Suggested; } +Expected> +IncludeInserter::insert(const HeaderFile &DeclaringHeader, + const HeaderFile &InsertedHeader) const { + auto Validate = [](const HeaderFile &Header) { + return Header.valid() + ? llvm::Error::success() + : llvm::make_error( + "Invalid HeaderFile: " + Header.File + + " (verbatim=" + std::to_string(Header.Verbatim) + ").", + llvm::inconvertibleErrorCode()); + }; + if (auto Err = Validate(DeclaringHeader)) + return std::move(Err); + if (auto Err = Validate(InsertedHeader)) + return std::move(Err); + auto Include = + calculateIncludePath(FileName, BuildDir, HeaderSearchInfo, Inclusions, + DeclaringHeader, InsertedHeader); + if (!Include) + return Include.takeError(); + if (Include->empty()) + return llvm::None; + StringRef IncludeRef = *Include; + auto Insertion = + Inserter.insert(IncludeRef.trim("\"<>"), IncludeRef.startswith("<")); + if (!Insertion) + return llvm::None; + return replacementToEdit(Code, *Insertion); +} + } // namespace clangd } // namespace clang Index: clangd/Protocol.h =================================================================== --- clangd/Protocol.h +++ clangd/Protocol.h @@ -99,6 +99,9 @@ return std::tie(LHS.line, LHS.character) == std::tie(RHS.line, RHS.character); } + friend bool operator!=(const Position &LHS, const Position &RHS) { + return !(LHS == RHS); + } friend bool operator<(const Position &LHS, const Position &RHS) { return std::tie(LHS.line, LHS.character) < std::tie(RHS.line, RHS.character); Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -45,6 +45,16 @@ MATCHER_P(Filter, F, "") { return arg.filterText == F; } MATCHER_P(Doc, D, "") { return arg.documentation == D; } MATCHER_P(Detail, D, "") { return arg.detail == D; } +MATCHER_P(InsertInclude, IncludeHeader, "") { + if (arg.additionalTextEdits.size() != 1) + return false; + const auto &Edit = arg.additionalTextEdits[0]; + if (Edit.range.start != Edit.range.end) + return false; + SmallVector Matches; + llvm::Regex RE(R"(#include[ ]*(["<][^">]*[">]))"); + return RE.match(Edit.newText, &Matches) && Matches[1] == IncludeHeader; +} MATCHER_P(PlainText, Text, "") { return arg.insertTextFormat == clangd::InsertTextFormat::PlainText && arg.insertText == Text; @@ -58,6 +68,8 @@ return true; return llvm::StringRef(arg.insertText).contains(arg.filterText); } +MATCHER(HasAdditionalEdits, "") { return !arg.additionalTextEdits.empty(); } + // Shorthand for Contains(Named(Name)). Matcher &> Has(std::string Name) { return Contains(Named(std::move(Name))); @@ -75,9 +87,7 @@ return MemIndex::build(std::move(Slab).build()); } -// Builds a server and runs code completion. -// If IndexSymbols is non-empty, an index will be built and passed to opts. -CompletionList completions(StringRef Text, +CompletionList completions(ClangdServer &Server, StringRef Text, std::vector IndexSymbols = {}, clangd::CodeCompleteOptions Opts = {}) { std::unique_ptr OverrideIndex; @@ -87,10 +97,6 @@ Opts.Index = OverrideIndex.get(); } - MockFSProvider FS; - MockCompilationDatabase CDB; - IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); auto File = testPath("foo.cpp"); Annotations Test(Text); runAddDocument(Server, File, Test.code()); @@ -101,6 +107,18 @@ return CompletionList; } +// Builds a server and runs code completion. +// If IndexSymbols is non-empty, an index will be built and passed to opts. +CompletionList completions(StringRef Text, + std::vector IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + return completions(Server, Text, std::move(IndexSymbols), std::move(Opts)); +} + std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) { std::string Result; raw_string_ostream OS(Result); @@ -505,6 +523,42 @@ EXPECT_TRUE(Results.isIncomplete); } +TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) { + MockFSProvider FS; + MockCompilationDatabase CDB; + std::string Subdir = testPath("sub"); + std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str(); + CDB.ExtraClangFlags = {SearchDirArg.c_str()}; + std::string BarHeader = testPath("sub/bar.h"); + FS.Files[BarHeader] = ""; + + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + Symbol::Details Scratch; + auto BarURI = URI::createFile(BarHeader).toString(); + Symbol Sym = cls("ns::X"); + Sym.CanonicalDeclaration.FileURI = BarURI; + Scratch.IncludeHeader = BarURI; + Sym.Detail = &Scratch; + // Shoten include path based on search dirctory and insert. + auto Results = completions(Server, + R"cpp( + int main() { ns::^ } + )cpp", + {Sym}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), InsertInclude("\"bar.h\"")))); + // Duplicate based on inclusions in preamble. + Results = completions(Server, + R"cpp( + #include "sub/bar.h" // not shortest, so should only match resolved. + int main() { ns::^ } + )cpp", + {Sym}); + EXPECT_THAT(Results.items, + ElementsAre(AllOf(Named("X"), Not(HasAdditionalEdits())))); +} + TEST(CompletionTest, IndexSuppressesPreambleCompletions) { MockFSProvider FS; MockCompilationDatabase CDB; Index: unittests/clangd/HeadersTests.cpp =================================================================== --- unittests/clangd/HeadersTests.cpp +++ unittests/clangd/HeadersTests.cpp @@ -8,7 +8,13 @@ //===----------------------------------------------------------------------===// #include "Headers.h" + +#include "Compiler.h" #include "TestFS.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/CompilerInvocation.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Lex/PreprocessorOptions.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -16,30 +22,83 @@ namespace clangd { namespace { +using ::testing::AllOf; +using ::testing::UnorderedElementsAre; + class HeadersTest : public ::testing::Test { public: HeadersTest() { CDB.ExtraClangFlags = {SearchDirArg.c_str()}; FS.Files[MainFile] = ""; + // Make sure directory sub/ exists. + FS.Files[testPath("sub/EMPTY")] = ""; + } + +private: + std::unique_ptr setupClang() { + auto Cmd = CDB.getCompileCommand(MainFile); + assert(static_cast(Cmd)); + auto VFS = FS.getFileSystem(); + VFS->setCurrentWorkingDirectory(Cmd->Directory); + + std::vector Argv; + for (const auto &S : Cmd->CommandLine) + Argv.push_back(S.c_str()); + auto CI = clang::createInvocationFromCommandLine( + Argv, + CompilerInstance::createDiagnostics(new DiagnosticOptions(), + &IgnoreDiags, false), + VFS); + EXPECT_TRUE(static_cast(CI)); + CI->getFrontendOpts().DisableFree = false; + + // The diagnostic options must be set before creating a CompilerInstance. + CI->getDiagnosticOpts().IgnoreWarnings = true; + auto Clang = prepareCompilerInstance( + std::move(CI), /*Preamble=*/nullptr, + llvm::MemoryBuffer::getMemBuffer(FS.Files[MainFile], MainFile), + std::make_shared(), VFS, IgnoreDiags); + + EXPECT_FALSE(Clang->getFrontendOpts().Inputs.empty()); + return Clang; } protected: + std::vector collectIncludes() { + auto Clang = setupClang(); + PreprocessOnlyAction Action; + EXPECT_TRUE( + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])); + std::vector Inclusions; + Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback( + Clang->getSourceManager(), + [&](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); })); + EXPECT_TRUE(Action.Execute()); + Action.EndSourceFile(); + return Inclusions; + } + // Calculates the include path, or returns "" on error. std::string calculate(PathRef Original, PathRef Preferred = "", + const std::vector &Inclusions = {}, bool ExpectError = false) { + auto Clang = setupClang(); + PreprocessOnlyAction Action; + EXPECT_TRUE( + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])); + if (Preferred.empty()) Preferred = Original; - auto VFS = FS.getFileSystem(); - auto Cmd = CDB.getCompileCommand(MainFile); - assert(static_cast(Cmd)); - VFS->setCurrentWorkingDirectory(Cmd->Directory); auto ToHeaderFile = [](llvm::StringRef Header) { return HeaderFile{Header, /*Verbatim=*/!llvm::sys::path::is_absolute(Header)}; }; - auto Path = calculateIncludePath(MainFile, FS.Files[MainFile], - ToHeaderFile(Original), - ToHeaderFile(Preferred), *Cmd, VFS); + + auto Path = calculateIncludePath( + MainFile, CDB.getCompileCommand(MainFile)->Directory, + Clang->getPreprocessor().getHeaderSearchInfo(), Inclusions, + ToHeaderFile(Original), ToHeaderFile(Preferred)); + Action.EndSourceFile(); if (!Path) { llvm::consumeError(Path.takeError()); EXPECT_TRUE(ExpectError); @@ -49,52 +108,50 @@ } return std::move(*Path); } + + Expected> + insert(const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader, + const std::vector &ExistingInclusions = {}) { + auto Clang = setupClang(); + PreprocessOnlyAction Action; + EXPECT_TRUE( + Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])); + + IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(), + CDB.getCompileCommand(MainFile)->Directory, + Clang->getPreprocessor().getHeaderSearchInfo()); + for (const auto &Inc : ExistingInclusions) + Inserter.addExisting(Inc); + + auto Edit = Inserter.insert(DeclaringHeader, InsertedHeader); + Action.EndSourceFile(); + return Edit; + } + MockFSProvider FS; MockCompilationDatabase CDB; std::string MainFile = testPath("main.cpp"); std::string Subdir = testPath("sub"); std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str(); + IgnoringDiagConsumer IgnoreDiags; }; -TEST_F(HeadersTest, InsertInclude) { - std::string Path = testPath("sub/bar.h"); - FS.Files[Path] = ""; - EXPECT_EQ(calculate(Path), "\"bar.h\""); -} +MATCHER_P(Written, Name, "") { return arg.Written == Name; } +MATCHER_P(Resolved, Name, "") { return arg.Resolved == Name; } -TEST_F(HeadersTest, DontInsertDuplicateSameName) { +TEST_F(HeadersTest, CollectRewrittenAndResolved) { FS.Files[MainFile] = R"cpp( -#include "bar.h" +#include "sub/bar.h" // not shortest )cpp"; std::string BarHeader = testPath("sub/bar.h"); FS.Files[BarHeader] = ""; - EXPECT_EQ(calculate(BarHeader), ""); -} -TEST_F(HeadersTest, DontInsertDuplicateDifferentName) { - std::string BarHeader = testPath("sub/bar.h"); - FS.Files[BarHeader] = ""; - FS.Files[MainFile] = R"cpp( -#include "sub/bar.h" // not shortest -)cpp"; - EXPECT_EQ(calculate("\"sub/bar.h\""), ""); // Duplicate rewritten. - EXPECT_EQ(calculate(BarHeader), ""); // Duplicate resolved. - EXPECT_EQ(calculate(BarHeader, "\"BAR.h\""), ""); // Do not insert preferred. + EXPECT_THAT(collectIncludes(), + UnorderedElementsAre( + AllOf(Written("\"sub/bar.h\""), Resolved(BarHeader)))); } -TEST_F(HeadersTest, DontInsertDuplicatePreferred) { - std::string BarHeader = testPath("sub/bar.h"); - FS.Files[BarHeader] = ""; - FS.Files[MainFile] = R"cpp( -#include "sub/bar.h" // not shortest -)cpp"; - // Duplicate written. - EXPECT_EQ(calculate("\"original.h\"", "\"sub/bar.h\""), ""); - // Duplicate resolved. - EXPECT_EQ(calculate("\"original.h\"", BarHeader), ""); -} - -TEST_F(HeadersTest, StillInsertIfTrasitivelyIncluded) { +TEST_F(HeadersTest, OnlyCollectInclusionsInMain) { std::string BazHeader = testPath("sub/baz.h"); FS.Files[BazHeader] = ""; std::string BarHeader = testPath("sub/bar.h"); @@ -104,27 +161,92 @@ FS.Files[MainFile] = R"cpp( #include "bar.h" )cpp"; - EXPECT_EQ(calculate(BazHeader), "\"baz.h\""); + EXPECT_THAT( + collectIncludes(), + UnorderedElementsAre(AllOf(Written("\"bar.h\""), Resolved(BarHeader)))); +} + +TEST_F(HeadersTest, UnResolvedInclusion) { + FS.Files[MainFile] = R"cpp( +#include "foo.h" +)cpp"; + + EXPECT_THAT(collectIncludes(), + UnorderedElementsAre(AllOf(Written("\"foo.h\""), Resolved("")))); +} + +TEST_F(HeadersTest, InsertInclude) { + std::string Path = testPath("sub/bar.h"); + FS.Files[Path] = ""; + EXPECT_EQ(calculate(Path), "\"bar.h\""); } TEST_F(HeadersTest, DoNotInsertIfInSameFile) { MainFile = testPath("main.h"); - FS.Files[MainFile] = ""; EXPECT_EQ(calculate(MainFile), ""); } +TEST_F(HeadersTest, ShortenedInclude) { + std::string BarHeader = testPath("sub/bar.h"); + EXPECT_EQ(calculate(BarHeader), "\"bar.h\""); +} + +TEST_F(HeadersTest, NotShortenedInclude) { + std::string BarHeader = testPath("sub-2/bar.h"); + EXPECT_EQ(calculate(BarHeader, ""), "\"" + BarHeader + "\""); +} + TEST_F(HeadersTest, PreferredHeader) { - FS.Files[MainFile] = ""; std::string BarHeader = testPath("sub/bar.h"); - FS.Files[BarHeader] = ""; EXPECT_EQ(calculate(BarHeader, ""), ""); std::string BazHeader = testPath("sub/baz.h"); - FS.Files[BazHeader] = ""; EXPECT_EQ(calculate(BarHeader, BazHeader), "\"baz.h\""); } +TEST_F(HeadersTest, DontInsertDuplicatePreferred) { + std::vector Inclusions = { + {Range(), /*Written*/ "\"bar.h\"", /*Resolved*/ ""}}; + EXPECT_EQ(calculate(testPath("sub/bar.h"), "\"bar.h\"", Inclusions), ""); + EXPECT_EQ(calculate("\"x.h\"", "\"bar.h\"", Inclusions), ""); +} + +TEST_F(HeadersTest, DontInsertDuplicateResolved) { + std::string BarHeader = testPath("sub/bar.h"); + std::vector Inclusions = { + {Range(), /*Written*/ "fake-bar.h", /*Resolved*/ BarHeader}}; + EXPECT_EQ(calculate(BarHeader, "", Inclusions), ""); + // Do not insert preferred. + EXPECT_EQ(calculate(BarHeader, "\"BAR.h\"", Inclusions), ""); +} + +HeaderFile literal(StringRef Include) { + HeaderFile H{Include, /*Verbatim=*/true}; + assert(H.valid()); + return H; +} + +TEST_F(HeadersTest, PreferInserted) { + auto Edit = insert(literal(""), literal("")); + EXPECT_TRUE(static_cast(Edit)); + EXPECT_TRUE(llvm::StringRef((*Edit)->newText).contains("")); +} + +TEST_F(HeadersTest, ExistingInclusion) { + Inclusion Existing{Range(), /*Written=*/"", /*Resolved=*/""}; + auto Edit = insert(literal(""), literal(""), {Existing}); + EXPECT_TRUE(static_cast(Edit)); + EXPECT_FALSE(static_cast(*Edit)); +} + +TEST_F(HeadersTest, ValidateHeaders) { + HeaderFile InvalidHeader{"a.h", /*Verbatim=*/true}; + assert(!InvalidHeader.valid()); + auto Edit = insert(InvalidHeader, literal("\"c.h\"")); + EXPECT_FALSE(static_cast(Edit)); + llvm::consumeError(Edit.takeError()); +} + } // namespace } // namespace clangd } // namespace clang -