From 83f63e42b2e15e4bc120253b9deae00a232a787d Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Mon, 3 Sep 2018 10:18:21 +0000 Subject: [PATCH] [clangd] Support multiple #include headers in one symbol. Summary: Currently, a symbol can have only one #include header attached, which might not work well if the symbol can be imported via different #includes depending on where it's used. This patch stores multiple #include headers (with # references) for each symbol, so that CodeCompletion can decide which include to insert. In this patch, code completion simply picks the most popular include as the default inserted header. We also return all possible includes and their edits in the `CodeCompletion` results. Reviewers: sammccall Reviewed By: sammccall Subscribers: mgrang, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits Differential Revision: https://reviews.llvm.org/D51291 llvm-svn: 341304 --- clang-tools-extra/clangd/CodeComplete.cpp | 103 ++++++++++++------ clang-tools-extra/clangd/CodeComplete.h | 21 +++- clang-tools-extra/clangd/index/Index.cpp | 5 +- clang-tools-extra/clangd/index/Index.h | 35 ++++-- clang-tools-extra/clangd/index/Merge.cpp | 18 ++- .../clangd/index/SymbolCollector.cpp | 7 +- clang-tools-extra/clangd/index/SymbolYAML.cpp | 13 ++- .../unittests/clangd/CodeCompleteTests.cpp | 61 +++++++++-- .../unittests/clangd/FileIndexTests.cpp | 2 +- .../unittests/clangd/IndexTests.cpp | 40 +++++++ .../unittests/clangd/SymbolCollectorTests.cpp | 24 +++- 11 files changed, 263 insertions(+), 66 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 6cda2f0b9ef6..617046b69e94 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -44,10 +44,13 @@ #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/Error.h" #include "llvm/Support/Format.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/ScopedPrinter.h" +#include +#include #include // We log detailed candidate here if you run with -debug-only=codecomplete. @@ -247,6 +250,7 @@ struct CompletionCandidate { // We may have a result from Sema, from the index, or both. const CodeCompletionResult *SemaResult = nullptr; const Symbol *IndexResult = nullptr; + llvm::SmallVector RankedIncludeHeaders; // States whether this item is an override suggestion. bool IsOverride = false; @@ -267,7 +271,7 @@ struct CompletionCandidate { // This could break #include insertion. return hash_combine( (IndexResult->Scope + IndexResult->Name).toStringRef(Scratch), - headerToInsertIfNotPresent().getValueOr("")); + headerToInsertIfAllowed().getValueOr("")); default: return 0; } @@ -281,11 +285,12 @@ struct CompletionCandidate { llvm::raw_svector_ostream OS(Scratch); D->printQualifiedName(OS); } - return hash_combine(Scratch, headerToInsertIfNotPresent().getValueOr("")); + return hash_combine(Scratch, headerToInsertIfAllowed().getValueOr("")); } - llvm::Optional headerToInsertIfNotPresent() const { - if (!IndexResult || IndexResult->IncludeHeader.empty()) + // The best header to include if include insertion is allowed. + llvm::Optional headerToInsertIfAllowed() const { + if (RankedIncludeHeaders.empty()) return llvm::None; if (SemaResult && SemaResult->Declaration) { // Avoid inserting new #include if the declaration is found in the current @@ -295,7 +300,7 @@ struct CompletionCandidate { if (SM.isInMainFile(SM.getExpansionLoc(RD->getBeginLoc()))) return llvm::None; } - return IndexResult->IncludeHeader; + return RankedIncludeHeaders[0]; } using Bundle = llvm::SmallVector; @@ -358,31 +363,41 @@ struct CodeCompletionBuilder { if (Completion.Name.empty()) Completion.Name = C.IndexResult->Name; } - if (auto Inserted = C.headerToInsertIfNotPresent()) { - // Turn absolute path into a literal string that can be #included. - auto Include = [&]() -> Expected> { - auto ResolvedDeclaring = - toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName); - if (!ResolvedDeclaring) - return ResolvedDeclaring.takeError(); - auto ResolvedInserted = toHeaderFile(*Inserted, FileName); - if (!ResolvedInserted) - return ResolvedInserted.takeError(); - return std::make_pair(Includes.calculateIncludePath(*ResolvedDeclaring, - *ResolvedInserted), - Includes.shouldInsertInclude(*ResolvedDeclaring, - *ResolvedInserted)); - }(); - if (Include) { - Completion.Header = Include->first; - if (Include->second) - Completion.HeaderInsertion = Includes.insert(Include->first); + + // Turn absolute path into a literal string that can be #included. + auto Inserted = + [&](StringRef Header) -> Expected> { + auto ResolvedDeclaring = + toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName); + if (!ResolvedDeclaring) + return ResolvedDeclaring.takeError(); + auto ResolvedInserted = toHeaderFile(Header, FileName); + if (!ResolvedInserted) + return ResolvedInserted.takeError(); + return std::make_pair( + Includes.calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted), + Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted)); + }; + bool ShouldInsert = C.headerToInsertIfAllowed().hasValue(); + // Calculate include paths and edits for all possible headers. + for (const auto &Inc : C.RankedIncludeHeaders) { + if (auto ToInclude = Inserted(Inc)) { + CodeCompletion::IncludeCandidate Include; + Include.Header = ToInclude->first; + if (ToInclude->second && ShouldInsert) + Include.Insertion = Includes.insert(ToInclude->first); + Completion.Includes.push_back(std::move(Include)); } else log("Failed to generate include insertion edits for adding header " "(FileURI='{0}', IncludeHeader='{1}') into {2}", - C.IndexResult->CanonicalDeclaration.FileURI, - C.IndexResult->IncludeHeader, FileName); + C.IndexResult->CanonicalDeclaration.FileURI, Inc, FileName); } + // Prefer includes that do not need edits (i.e. already exist). + std::stable_partition(Completion.Includes.begin(), + Completion.Includes.end(), + [](const CodeCompletion::IncludeCandidate &I) { + return !I.Insertion.hasValue(); + }); } void add(const CompletionCandidate &C, CodeCompletionString *SemaCCS) { @@ -1135,6 +1150,26 @@ clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const { return Result; } +// 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. + std::sort(Includes.begin(), Includes.end(), + [](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: @@ -1383,6 +1418,8 @@ private: CompletionCandidate C; C.SemaResult = SemaResult; C.IndexResult = IndexResult; + if (C.IndexResult) + C.RankedIncludeHeaders = getRankedIncludes(*C.IndexResult); C.IsOverride = IsOverride; C.Name = IndexResult ? IndexResult->Name : Recorder->getName(*SemaResult); if (auto OverloadSet = Opts.BundleOverloads ? C.overloadSet() : 0) { @@ -1576,16 +1613,18 @@ bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) { CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const { CompletionItem LSP; - LSP.label = (HeaderInsertion ? Opts.IncludeIndicator.Insert - : Opts.IncludeIndicator.NoInsert) + + const auto *InsertInclude = Includes.empty() ? nullptr : &Includes[0]; + LSP.label = ((InsertInclude && InsertInclude->Insertion) + ? Opts.IncludeIndicator.Insert + : Opts.IncludeIndicator.NoInsert) + (Opts.ShowOrigins ? "[" + llvm::to_string(Origin) + "]" : "") + RequiredQualifier + Name + Signature; LSP.kind = Kind; LSP.detail = BundleSize > 1 ? llvm::formatv("[{0} overloads]", BundleSize) : ReturnType; - if (!Header.empty()) - LSP.detail += "\n" + Header; + if (InsertInclude) + LSP.detail += "\n" + InsertInclude->Header; LSP.documentation = Documentation; LSP.sortText = sortText(Score.Total, Name); LSP.filterText = Name; @@ -1613,8 +1652,8 @@ CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const { LSP.insertText = LSP.textEdit->newText; LSP.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet : InsertTextFormat::PlainText; - if (HeaderInsertion) - LSP.additionalTextEdits.push_back(*HeaderInsertion); + if (InsertInclude && InsertInclude->Insertion) + LSP.additionalTextEdits.push_back(*InsertInclude->Insertion); return LSP; } diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h index 1785fb6a68e0..3213acc7a31e 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -26,6 +26,7 @@ #include "clang/Sema/CodeCompleteOptions.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include @@ -131,12 +132,20 @@ struct CodeCompletion { // Other fields should apply equally to all bundled completions. unsigned BundleSize = 1; SymbolOrigin Origin = SymbolOrigin::Unknown; - // The header through which this symbol could be included. - // Quoted string as expected by an #include directive, e.g. "". - // Empty for non-symbol completions, or when not known. - std::string Header; - // Present if Header is set and should be inserted to use this item. - llvm::Optional HeaderInsertion; + + struct IncludeCandidate { + // The header through which this symbol could be included. + // Quoted string as expected by an #include directive, e.g. "". + // Empty for non-symbol completions, or when not known. + std::string Header; + // Present if Header should be inserted to use this item. + llvm::Optional Insertion; + }; + // All possible include headers ranked by preference. By default, the first + // include is used. + // If we've bundled together overloads that have different sets of includes, + // thse includes may not be accurate for all of them. + llvm::SmallVector Includes; /// Holds information about small corrections that needs to be done. Like /// converting '->' to '.' on member access. diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp index 20e4b44cc400..1120b22bb9f0 100644 --- a/clang-tools-extra/clangd/index/Index.cpp +++ b/clang-tools-extra/clangd/index/Index.cpp @@ -9,6 +9,7 @@ #include "Index.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/SHA1.h" #include "llvm/Support/raw_ostream.h" @@ -90,9 +91,11 @@ static void own(Symbol &S, llvm::UniqueStringSaver &Strings, Intern(S.Signature); Intern(S.CompletionSnippetSuffix); + Intern(S.Documentation); Intern(S.ReturnType); - Intern(S.IncludeHeader); + for (auto &I : S.IncludeHeaders) + Intern(I.IncludeHeader); } void SymbolSlab::Builder::insert(const Symbol &S) { diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index 63e87a23d995..08f8530e0059 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -16,7 +16,9 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/Hashing.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/StringSaver.h" #include #include @@ -213,14 +215,31 @@ struct Symbol { /// Type when this symbol is used in an expression. (Short display form). /// e.g. return type of a function, or type of a variable. llvm::StringRef ReturnType; - /// This can be either a URI of the header to be #include'd for this symbol, - /// or a literal header quoted with <> or "" that is suitable to be included - /// directly. When this is a URI, the exact #include path needs to be - /// calculated according to the URI scheme. - /// - /// This is a canonical include for the symbol and can be different from - /// FileURI in the CanonicalDeclaration. - llvm::StringRef IncludeHeader; + + struct IncludeHeaderWithReferences { + IncludeHeaderWithReferences() = default; + + IncludeHeaderWithReferences(llvm::StringRef IncludeHeader, + unsigned References) + : IncludeHeader(IncludeHeader), References(References) {} + + /// This can be either a URI of the header to be #include'd + /// for this symbol, or a literal header quoted with <> or "" that is + /// suitable to be included directly. When it is a URI, the exact #include + /// path needs to be calculated according to the URI scheme. + /// + /// Note that the include header is a canonical include for the symbol and + /// can be different from FileURI in the CanonicalDeclaration. + llvm::StringRef IncludeHeader = ""; + /// The number of translation units that reference this symbol and include + /// this header. This number is only meaningful if aggregated in an index. + unsigned References = 0; + }; + /// One Symbol can potentially be incuded via different headers. + /// - If we haven't seen a definition, this covers all declarations. + /// - If we have seen a definition, this covers declarations visible from + /// any definition. + llvm::SmallVector IncludeHeaders; // FIXME: add all occurrences support. // FIXME: add extra fields for index scoring signals. diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index 1bca61e79830..6c384bfd6a47 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -118,6 +118,10 @@ Symbol mergeSymbol(const Symbol &L, const Symbol &R) { // Classes: this is the def itself. Functions: hopefully the header decl. // If both did (or both didn't), continue to prefer L over R. bool PreferR = R.Definition && !L.Definition; + // Merge include headers only if both have definitions or both have no + // definition; otherwise, only accumulate references of common includes. + bool MergeIncludes = + L.Definition.FileURI.empty() == R.Definition.FileURI.empty(); Symbol S = PreferR ? R : L; // The target symbol we're merging into. const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol. @@ -136,8 +140,18 @@ Symbol mergeSymbol(const Symbol &L, const Symbol &R) { S.Documentation = O.Documentation; if (S.ReturnType == "") S.ReturnType = O.ReturnType; - if (S.IncludeHeader == "") - S.IncludeHeader = O.IncludeHeader; + for (const auto &OI : O.IncludeHeaders) { + bool Found = false; + for (auto &SI : S.IncludeHeaders) { + if (SI.IncludeHeader == OI.IncludeHeader) { + Found = true; + SI.References += OI.References; + break; + } + } + if (!Found && MergeIncludes) + S.IncludeHeaders.emplace_back(OI.IncludeHeader, OI.References); + } S.Origin |= O.Origin | SymbolOrigin::Merge; return S; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 214522145324..912c65085975 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -422,7 +422,9 @@ bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name, } S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; - S.IncludeHeader = Include; + if (!Include.empty()) + S.IncludeHeaders.emplace_back(Include, 1); + Symbols.insert(S); return true; } @@ -530,7 +532,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, S.CompletionSnippetSuffix = SnippetSuffix; S.Documentation = Documentation; S.ReturnType = ReturnType; - S.IncludeHeader = Include; + if (!Include.empty()) + S.IncludeHeaders.emplace_back(Include, 1); S.Origin = Opts.Origin; Symbols.insert(S); diff --git a/clang-tools-extra/clangd/index/SymbolYAML.cpp b/clang-tools-extra/clangd/index/SymbolYAML.cpp index 6d9c84b73377..d3947fa1416a 100644 --- a/clang-tools-extra/clangd/index/SymbolYAML.cpp +++ b/clang-tools-extra/clangd/index/SymbolYAML.cpp @@ -10,11 +10,13 @@ #include "SymbolYAML.h" #include "Index.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/Errc.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/raw_ostream.h" LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(clang::clangd::Symbol) +LLVM_YAML_IS_SEQUENCE_VECTOR(clang::clangd::Symbol::IncludeHeaderWithReferences) namespace llvm { namespace yaml { @@ -66,6 +68,15 @@ template <> struct MappingTraits { } }; +template <> +struct MappingTraits { + static void mapping(IO &io, + clang::clangd::Symbol::IncludeHeaderWithReferences &Inc) { + io.mapRequired("Header", Inc.IncludeHeader); + io.mapRequired("References", Inc.References); + } +}; + template <> struct MappingTraits { static void mapping(IO &IO, Symbol &Sym) { MappingNormalization NSymbolID(IO, Sym.ID); @@ -83,7 +94,7 @@ template <> struct MappingTraits { IO.mapOptional("CompletionSnippetSuffix", Sym.CompletionSnippetSuffix); IO.mapOptional("Documentation", Sym.Documentation); IO.mapOptional("ReturnType", Sym.ReturnType); - IO.mapOptional("IncludeHeader", Sym.IncludeHeader); + IO.mapOptional("IncludeHeaders", Sym.IncludeHeaders); } }; diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp index 284149a5e05f..80e5f91430e8 100644 --- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp +++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp @@ -55,10 +55,16 @@ MATCHER_P(SigHelpLabeled, Label, "") { return arg.label == Label; } MATCHER_P(Kind, K, "") { return arg.Kind == K; } MATCHER_P(Doc, D, "") { return arg.Documentation == D; } MATCHER_P(ReturnType, D, "") { return arg.ReturnType == D; } -MATCHER_P(InsertInclude, IncludeHeader, "") { - return arg.Header == IncludeHeader && bool(arg.HeaderInsertion); +MATCHER_P(HasInclude, IncludeHeader, "") { + return !arg.Includes.empty() && arg.Includes[0].Header == IncludeHeader; +} +MATCHER_P(InsertInclude, IncludeHeader, "") { + return !arg.Includes.empty() && arg.Includes[0].Header == IncludeHeader && + bool(arg.Includes[0].Insertion); +} +MATCHER(InsertInclude, "") { + return !arg.Includes.empty() && bool(arg.Includes[0].Insertion); } -MATCHER(InsertInclude, "") { return bool(arg.HeaderInsertion); } MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; } MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; } @@ -568,7 +574,7 @@ TEST(CompletionTest, IncludeInsertionPreprocessorIntegrationTests) { auto BarURI = URI::createFile(BarHeader).toString(); Symbol Sym = cls("ns::X"); Sym.CanonicalDeclaration.FileURI = BarURI; - Sym.IncludeHeader = BarURI; + Sym.IncludeHeaders.emplace_back(BarURI, 1); // Shoten include path based on search dirctory and insert. auto Results = completions(Server, R"cpp( @@ -600,8 +606,8 @@ TEST(CompletionTest, NoIncludeInsertionWhenDeclFoundInFile) { auto BarURI = URI::createFile(BarHeader).toString(); SymX.CanonicalDeclaration.FileURI = BarURI; SymY.CanonicalDeclaration.FileURI = BarURI; - SymX.IncludeHeader = ""; - SymY.IncludeHeader = ""; + SymX.IncludeHeaders.emplace_back("", 1); + SymY.IncludeHeaders.emplace_back("", 1); // Shoten include path based on search dirctory and insert. auto Results = completions(Server, R"cpp( @@ -1178,7 +1184,7 @@ TEST(CompletionTest, OverloadBundling) { // Differences in header-to-insert suppress bundling. std::string DeclFile = URI::createFile(testPath("foo")).toString(); NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile; - NoArgsGFunc.IncludeHeader = ""; + NoArgsGFunc.IncludeHeaders.emplace_back("", 1); EXPECT_THAT( completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).Completions, UnorderedElementsAre(AllOf(Named("GFuncC"), InsertInclude("")), @@ -1345,7 +1351,9 @@ TEST(CompletionTest, Render) { C.RequiredQualifier = "Foo::"; C.Scope = "ns::Foo::"; C.Documentation = "This is x()."; - C.Header = "\"foo.h\""; + C.Includes.emplace_back(); + auto &Include = C.Includes.back(); + Include.Header = "\"foo.h\""; C.Kind = CompletionItemKind::Method; C.Score.Total = 1.0; C.Origin = SymbolOrigin::AST | SymbolOrigin::Static; @@ -1370,7 +1378,7 @@ TEST(CompletionTest, Render) { EXPECT_EQ(R.insertText, "Foo::x(${0:bool})"); EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet); - C.HeaderInsertion.emplace(); + Include.Insertion.emplace(); R = C.render(Opts); EXPECT_EQ(R.label, "^Foo::x(bool) const"); EXPECT_THAT(R.additionalTextEdits, Not(IsEmpty())); @@ -1826,6 +1834,41 @@ TEST(CompletionTest, EnableSpeculativeIndexRequest) { ASSERT_EQ(Reqs3.size(), 2u); } +TEST(CompletionTest, InsertTheMostPopularHeader) { + std::string DeclFile = URI::createFile(testPath("foo")).toString(); + Symbol sym = func("Func"); + sym.CanonicalDeclaration.FileURI = DeclFile; + sym.IncludeHeaders.emplace_back("\"foo.h\"", 2); + sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000); + + auto Results = completions("Fun^", {sym}).Completions; + assert(!Results.empty()); + EXPECT_THAT(Results[0], AllOf(Named("Func"), InsertInclude("\"bar.h\""))); + EXPECT_EQ(Results[0].Includes.size(), 2u); +} + +TEST(CompletionTest, NoInsertIncludeIfOnePresent) { + MockFSProvider FS; + MockCompilationDatabase CDB; + + std::string FooHeader = testPath("foo.h"); + FS.Files[FooHeader] = ""; + + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + std::string DeclFile = URI::createFile(testPath("foo")).toString(); + Symbol sym = func("Func"); + sym.CanonicalDeclaration.FileURI = DeclFile; + sym.IncludeHeaders.emplace_back("\"foo.h\"", 2); + sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000); + + EXPECT_THAT( + completions(Server, "#include \"foo.h\"\nFun^", {sym}).Completions, + UnorderedElementsAre( + AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude())))); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/unittests/clangd/FileIndexTests.cpp b/clang-tools-extra/unittests/clangd/FileIndexTests.cpp index 727240d74989..d18f1809644c 100644 --- a/clang-tools-extra/unittests/clangd/FileIndexTests.cpp +++ b/clang-tools-extra/unittests/clangd/FileIndexTests.cpp @@ -223,7 +223,7 @@ TEST(FileIndexTest, NoIncludeCollected) { Req.Query = ""; bool SeenSymbol = false; M.fuzzyFind(Req, [&](const Symbol &Sym) { - EXPECT_TRUE(Sym.IncludeHeader.empty()); + EXPECT_TRUE(Sym.IncludeHeaders.empty()); SeenSymbol = true; }); EXPECT_TRUE(SeenSymbol); diff --git a/clang-tools-extra/unittests/clangd/IndexTests.cpp b/clang-tools-extra/unittests/clangd/IndexTests.cpp index 1ac710340655..7fd39e60e7d9 100644 --- a/clang-tools-extra/unittests/clangd/IndexTests.cpp +++ b/clang-tools-extra/unittests/clangd/IndexTests.cpp @@ -306,6 +306,46 @@ TEST(MergeIndexTest, FindOccurrences) { FileURI("unittest:///test2.cc")))); } +MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") { + return (arg.IncludeHeader == IncludeHeader) && (arg.References == References); +} + +TEST(MergeTest, MergeIncludesOnDifferentDefinitions) { + Symbol L, R; + L.Name = "left"; + R.Name = "right"; + L.ID = R.ID = SymbolID("hello"); + L.IncludeHeaders.emplace_back("common", 1); + R.IncludeHeaders.emplace_back("common", 1); + R.IncludeHeaders.emplace_back("new", 1); + + // Both have no definition. + Symbol M = mergeSymbol(L, R); + EXPECT_THAT(M.IncludeHeaders, + UnorderedElementsAre(IncludeHeaderWithRef("common", 2u), + IncludeHeaderWithRef("new", 1u))); + + // Only merge references of the same includes but do not merge new #includes. + L.Definition.FileURI = "file:/left.h"; + M = mergeSymbol(L, R); + EXPECT_THAT(M.IncludeHeaders, + UnorderedElementsAre(IncludeHeaderWithRef("common", 2u))); + + // Definitions are the same. + R.Definition.FileURI = "file:/right.h"; + M = mergeSymbol(L, R); + EXPECT_THAT(M.IncludeHeaders, + UnorderedElementsAre(IncludeHeaderWithRef("common", 2u), + IncludeHeaderWithRef("new", 1u))); + + // Definitions are different. + R.Definition.FileURI = "file:/right.h"; + M = mergeSymbol(L, R); + EXPECT_THAT(M.IncludeHeaders, + UnorderedElementsAre(IncludeHeaderWithRef("common", 2u), + IncludeHeaderWithRef("new", 1u))); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp index 5769a2668742..b9efa5ba4077 100644 --- a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp +++ b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp @@ -54,7 +54,13 @@ MATCHER_P(Snippet, S, "") { MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; } MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; } -MATCHER_P(IncludeHeader, P, "") { return arg.IncludeHeader == P; } +MATCHER_P(IncludeHeader, P, "") { + return (arg.IncludeHeaders.size() == 1) && + (arg.IncludeHeaders.begin()->IncludeHeader == P); +} +MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") { + return (arg.IncludeHeader == IncludeHeader) && (arg.References == References); +} MATCHER_P(DeclRange, Pos, "") { return std::tie(arg.CanonicalDeclaration.Start.Line, arg.CanonicalDeclaration.Start.Column, @@ -760,6 +766,11 @@ CanonicalDeclaration: IsIndexedForCodeCompletion: true Documentation: 'Foo doc' ReturnType: 'int' +IncludeHeaders: + - Header: 'include1' + References: 7 + - Header: 'include2' + References: 3 ... )"; const std::string YAML2 = R"( @@ -791,6 +802,10 @@ CompletionSnippetSuffix: '-snippet' Doc("Foo doc"), ReturnType("int"), DeclURI("file:///path/foo.h"), ForCodeCompletion(true)))); + auto &Sym1 = *Symbols1.begin(); + EXPECT_THAT(Sym1.IncludeHeaders, + UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u), + IncludeHeaderWithRef("include2", 3u))); auto Symbols2 = symbolsFromYAML(YAML2); EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf( QName("clang::Foo2"), Labeled("Foo2-sig"), @@ -812,9 +827,10 @@ CompletionSnippetSuffix: '-snippet' TEST_F(SymbolCollectorTest, IncludeHeaderSameAsFileURI) { CollectorOpts.CollectIncludePath = true; runSymbolCollector("class Foo {};", /*Main=*/""); - EXPECT_THAT(Symbols, - UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI), - IncludeHeader(TestHeaderURI)))); + EXPECT_THAT(Symbols, UnorderedElementsAre( + AllOf(QName("Foo"), DeclURI(TestHeaderURI)))); + EXPECT_THAT(Symbols.begin()->IncludeHeaders, + UnorderedElementsAre(IncludeHeaderWithRef(TestHeaderURI, 1u))); } #ifndef _WIN32