From a68951e37e3df3ba19d9df0dde3bf6353e7109ad Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Fri, 22 Jun 2018 16:11:35 +0000 Subject: [PATCH] [clangd] More precise representation of symbol names/labels in the index. Summary: Previously, the strings matched LSP completion pretty closely. The completion label was a single string, for instance. This made implementing completion itself easy but makes it hard to use the names in other way, e.g. pretty-printed name in synthesized documentation/hover. It also limits our introspection into completion items, which can only be as precise as the indexed symbols. This change is a prerequisite to improvements to overload bundling which need to inspect e.g. signature structure. Reviewers: ioeric Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D48475 llvm-svn: 335360 --- clang-tools-extra/clangd/CodeComplete.cpp | 29 +-- .../clangd/CodeCompletionStrings.cpp | 179 +++++++----------- .../clangd/CodeCompletionStrings.h | 18 +- clang-tools-extra/clangd/index/Index.cpp | 7 +- clang-tools-extra/clangd/index/Index.h | 27 +-- clang-tools-extra/clangd/index/Merge.cpp | 14 +- .../clangd/index/SymbolCollector.cpp | 20 +- clang-tools-extra/clangd/index/SymbolYAML.cpp | 9 +- .../unittests/clangd/CodeCompleteTests.cpp | 3 - .../clangd/CodeCompletionStringsTests.cpp | 65 +++---- .../unittests/clangd/FileIndexTests.cpp | 11 +- .../unittests/clangd/IndexTests.cpp | 22 +-- .../unittests/clangd/SymbolCollectorTests.cpp | 59 +++--- 13 files changed, 199 insertions(+), 264 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 647e02da7288..d2e71b6e4727 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -249,17 +249,21 @@ struct CompletionCandidate { CompletionItem I; bool InsertingInclude = false; // Whether a new #include will be added. if (SemaResult) { - I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration); - getLabelAndInsertText(*SemaCCS, &I.label, &I.insertText, - Opts.EnableSnippets); - if (const char* Text = SemaCCS->getTypedText()) - I.filterText = Text; + llvm::StringRef Name(SemaCCS->getTypedText()); + std::string Signature, SnippetSuffix, Qualifiers; + getSignature(*SemaCCS, &Signature, &SnippetSuffix, &Qualifiers); + I.label = (Qualifiers + Name + Signature).str(); + I.filterText = Name; + I.insertText = (Qualifiers + Name).str(); + if (Opts.EnableSnippets) + I.insertText += SnippetSuffix; I.documentation = formatDocumentation(*SemaCCS, SemaDocComment); - I.detail = getDetail(*SemaCCS); + I.detail = getReturnType(*SemaCCS); if (SemaResult->Kind == CodeCompletionResult::RK_Declaration) if (const auto *D = SemaResult->getDeclaration()) if (const auto *ND = llvm::dyn_cast(D)) I.SymbolScope = splitQualifiedName(printQualifiedName(*ND)).first; + I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration); } if (IndexResult) { if (I.SymbolScope.empty()) @@ -268,21 +272,22 @@ struct CompletionCandidate { I.kind = toCompletionItemKind(IndexResult->SymInfo.Kind); // FIXME: reintroduce a way to show the index source for debugging. if (I.label.empty()) - I.label = IndexResult->CompletionLabel; + I.label = (IndexResult->Name + IndexResult->Signature).str(); if (I.filterText.empty()) I.filterText = IndexResult->Name; // FIXME(ioeric): support inserting/replacing scope qualifiers. - if (I.insertText.empty()) - I.insertText = Opts.EnableSnippets - ? IndexResult->CompletionSnippetInsertText - : IndexResult->CompletionPlainInsertText; + if (I.insertText.empty()) { + I.insertText = IndexResult->Name; + if (Opts.EnableSnippets) + I.insertText += IndexResult->CompletionSnippetSuffix; + } if (auto *D = IndexResult->Detail) { if (I.documentation.empty()) I.documentation = D->Documentation; if (I.detail.empty()) - I.detail = D->CompletionDetail; + I.detail = D->ReturnType; if (auto Inserted = headerToInsertIfNotPresent()) { auto IncludePath = [&]() -> Expected { auto ResolvedDeclaring = toHeaderFile( diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index 814866e5a429..504ab6c95bf0 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -24,31 +24,6 @@ bool isInformativeQualifierChunk(CodeCompletionString::Chunk const &Chunk) { StringRef(Chunk.Text).endswith("::"); } -void processPlainTextChunks(const CodeCompletionString &CCS, - std::string *LabelOut, std::string *InsertTextOut) { - std::string &Label = *LabelOut; - std::string &InsertText = *InsertTextOut; - for (const auto &Chunk : CCS) { - // Informative qualifier chunks only clutter completion results, skip - // them. - if (isInformativeQualifierChunk(Chunk)) - continue; - - switch (Chunk.Kind) { - case CodeCompletionString::CK_ResultType: - case CodeCompletionString::CK_Optional: - break; - case CodeCompletionString::CK_TypedText: - InsertText += Chunk.Text; - Label += Chunk.Text; - break; - default: - Label += Chunk.Text; - break; - } - } -} - void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) { for (const auto Character : Text) { if (Character == '$' || Character == '}' || Character == '\\') @@ -57,73 +32,6 @@ void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) { } } -void processSnippetChunks(const CodeCompletionString &CCS, - std::string *LabelOut, std::string *InsertTextOut) { - std::string &Label = *LabelOut; - std::string &InsertText = *InsertTextOut; - - unsigned ArgCount = 0; - for (const auto &Chunk : CCS) { - // Informative qualifier chunks only clutter completion results, skip - // them. - if (isInformativeQualifierChunk(Chunk)) - continue; - - switch (Chunk.Kind) { - case CodeCompletionString::CK_TypedText: - case CodeCompletionString::CK_Text: - Label += Chunk.Text; - InsertText += Chunk.Text; - break; - case CodeCompletionString::CK_Optional: - // FIXME: Maybe add an option to allow presenting the optional chunks? - break; - case CodeCompletionString::CK_Placeholder: - ++ArgCount; - InsertText += "${" + std::to_string(ArgCount) + ':'; - appendEscapeSnippet(Chunk.Text, &InsertText); - InsertText += '}'; - Label += Chunk.Text; - break; - case CodeCompletionString::CK_Informative: - // For example, the word "const" for a const method, or the name of - // the base class for methods that are part of the base class. - Label += Chunk.Text; - // Don't put the informative chunks in the insertText. - break; - case CodeCompletionString::CK_ResultType: - // This is retrieved as detail. - break; - case CodeCompletionString::CK_CurrentParameter: - // This should never be present while collecting completion items, - // only while collecting overload candidates. - llvm_unreachable("Unexpected CK_CurrentParameter while collecting " - "CompletionItems"); - break; - case CodeCompletionString::CK_LeftParen: - case CodeCompletionString::CK_RightParen: - case CodeCompletionString::CK_LeftBracket: - case CodeCompletionString::CK_RightBracket: - case CodeCompletionString::CK_LeftBrace: - case CodeCompletionString::CK_RightBrace: - case CodeCompletionString::CK_LeftAngle: - case CodeCompletionString::CK_RightAngle: - case CodeCompletionString::CK_Comma: - case CodeCompletionString::CK_Colon: - case CodeCompletionString::CK_SemiColon: - case CodeCompletionString::CK_Equal: - case CodeCompletionString::CK_HorizontalSpace: - InsertText += Chunk.Text; - Label += Chunk.Text; - break; - case CodeCompletionString::CK_VerticalSpace: - InsertText += Chunk.Text; - // Don't even add a space to the label. - break; - } - } -} - bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D, bool CommentsFromHeaders) { if (CommentsFromHeaders) @@ -199,10 +107,76 @@ getParameterDocComment(const ASTContext &Ctx, return Doc; } -void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label, - std::string *InsertText, bool EnableSnippets) { - return EnableSnippets ? processSnippetChunks(CCS, Label, InsertText) - : processPlainTextChunks(CCS, Label, InsertText); +void getSignature(const CodeCompletionString &CCS, std::string *Signature, + std::string *Snippet, std::string *RequiredQualifiers) { + unsigned ArgCount = 0; + for (const auto &Chunk : CCS) { + // Informative qualifier chunks only clutter completion results, skip + // them. + if (isInformativeQualifierChunk(Chunk)) + continue; + + switch (Chunk.Kind) { + case CodeCompletionString::CK_TypedText: + // The typed-text chunk is the actual name. We don't record this chunk. + // In general our string looks like . + // So once we see the name, any text we recorded so far should be + // reclassified as qualifiers. + if (RequiredQualifiers) + *RequiredQualifiers = std::move(*Signature); + Signature->clear(); + Snippet->clear(); + break; + case CodeCompletionString::CK_Text: + *Signature += Chunk.Text; + *Snippet += Chunk.Text; + break; + case CodeCompletionString::CK_Optional: + break; + case CodeCompletionString::CK_Placeholder: + *Signature += Chunk.Text; + ++ArgCount; + *Snippet += "${" + std::to_string(ArgCount) + ':'; + appendEscapeSnippet(Chunk.Text, Snippet); + *Snippet += '}'; + break; + case CodeCompletionString::CK_Informative: + // For example, the word "const" for a const method, or the name of + // the base class for methods that are part of the base class. + *Signature += Chunk.Text; + // Don't put the informative chunks in the snippet. + break; + case CodeCompletionString::CK_ResultType: + // This is not part of the signature. + break; + case CodeCompletionString::CK_CurrentParameter: + // This should never be present while collecting completion items, + // only while collecting overload candidates. + llvm_unreachable("Unexpected CK_CurrentParameter while collecting " + "CompletionItems"); + break; + case CodeCompletionString::CK_LeftParen: + case CodeCompletionString::CK_RightParen: + case CodeCompletionString::CK_LeftBracket: + case CodeCompletionString::CK_RightBracket: + case CodeCompletionString::CK_LeftBrace: + case CodeCompletionString::CK_RightBrace: + case CodeCompletionString::CK_LeftAngle: + case CodeCompletionString::CK_RightAngle: + case CodeCompletionString::CK_Comma: + case CodeCompletionString::CK_Colon: + case CodeCompletionString::CK_SemiColon: + case CodeCompletionString::CK_Equal: + case CodeCompletionString::CK_HorizontalSpace: + *Signature += Chunk.Text; + *Snippet += Chunk.Text; + break; + case CodeCompletionString::CK_VerticalSpace: + *Snippet += Chunk.Text; + // Don't even add a space to the signature. + break; + } + } } std::string formatDocumentation(const CodeCompletionString &CCS, @@ -235,17 +209,10 @@ std::string formatDocumentation(const CodeCompletionString &CCS, return Result; } -std::string getDetail(const CodeCompletionString &CCS) { - for (const auto &Chunk : CCS) { - // Informative qualifier chunks only clutter completion results, skip - // them. - switch (Chunk.Kind) { - case CodeCompletionString::CK_ResultType: +std::string getReturnType(const CodeCompletionString &CCS) { + for (const auto &Chunk : CCS) + if (Chunk.Kind == CodeCompletionString::CK_ResultType) return Chunk.Text; - default: - break; - } - } return ""; } diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index fd81db40e762..dac4ba56676e 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -45,13 +45,15 @@ getParameterDocComment(const ASTContext &Ctx, const CodeCompleteConsumer::OverloadCandidate &Result, unsigned ArgIndex, bool CommentsFromHeaders); -/// Gets label and insert text for a completion item. For example, for function -/// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled. -/// -/// If \p EnableSnippets is true, this will try to use snippet for the insert -/// text. Otherwise, the insert text will always be plain text. -void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label, - std::string *InsertText, bool EnableSnippets); +/// Formats the signature for an item, as a display string and snippet. +/// e.g. for const_reference std::vector::at(size_type) const, this returns: +/// *Signature = "(size_type) const" +/// *Snippet = "(${0:size_type})" +/// If set, RequiredQualifiers is the text that must be typed before the name. +/// e.g "Base::" when calling a base class member function that's hidden. +void getSignature(const CodeCompletionString &CCS, std::string *Signature, + std::string *Snippet, + std::string *RequiredQualifiers = nullptr); /// Assembles formatted documentation for a completion result. This includes /// documentation comments and other relevant information like annotations. @@ -63,7 +65,7 @@ std::string formatDocumentation(const CodeCompletionString &CCS, /// Gets detail to be used as the detail field in an LSP completion item. This /// is usually the return type of a function. -std::string getDetail(const CodeCompletionString &CCS); +std::string getReturnType(const CodeCompletionString &CCS); } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp index 68c8242d1bee..d9cf2cd34298 100644 --- a/clang-tools-extra/clangd/index/Index.cpp +++ b/clang-tools-extra/clangd/index/Index.cpp @@ -84,9 +84,8 @@ static void own(Symbol &S, DenseSet &Strings, Intern(S.CanonicalDeclaration.FileURI); Intern(S.Definition.FileURI); - Intern(S.CompletionLabel); - Intern(S.CompletionPlainInsertText); - Intern(S.CompletionSnippetInsertText); + Intern(S.Signature); + Intern(S.CompletionSnippetSuffix); if (S.Detail) { // Copy values of StringRefs into arena. @@ -94,7 +93,7 @@ static void own(Symbol &S, DenseSet &Strings, *Detail = *S.Detail; // Intern the actual strings. Intern(Detail->Documentation); - Intern(Detail->CompletionDetail); + Intern(Detail->ReturnType); Intern(Detail->IncludeHeader); // Replace the detail pointer with our copy. S.Detail = Detail; diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index 376ec87d7ce9..5ca76f2b6c4f 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -125,6 +125,11 @@ namespace clangd { // When adding new unowned data fields to Symbol, remember to update: // - SymbolSlab::Builder in Index.cpp, to copy them to the slab's storage. // - mergeSymbol in Merge.cpp, to properly combine two Symbols. +// +// A fully documented symbol can be split as: +// size_type std::map::count(const K& key) const +// | Return | Scope |Name| Signature | +// We split up these components to allow display flexibility later. struct Symbol { // The ID of the symbol. SymbolID ID; @@ -152,15 +157,13 @@ struct Symbol { /// Whether or not this symbol is meant to be used for the code completion. /// See also isIndexedForCodeCompletion(). bool IsIndexedForCodeCompletion = false; - /// A brief description of the symbol that can be displayed in the completion - /// candidate list. For example, "Foo(X x, Y y) const" is a label for a - /// function. - llvm::StringRef CompletionLabel; - /// What to insert when completing this symbol (plain text version). - llvm::StringRef CompletionPlainInsertText; - /// What to insert when completing this symbol (snippet version). This is - /// empty if it is the same as the plain insert text above. - llvm::StringRef CompletionSnippetInsertText; + /// A brief description of the symbol that can be appended in the completion + /// candidate list. For example, "(X x, Y y) const" is a function signature. + llvm::StringRef Signature; + /// What to insert when completing this symbol, after the symbol name. + /// This is in LSP snippet syntax (e.g. "({$0})" for a no-args function). + /// (When snippets are disabled, the symbol name alone is used). + llvm::StringRef CompletionSnippetSuffix; /// Optional symbol details that are not required to be set. For example, an /// index fuzzy match can return a large number of symbol candidates, and it @@ -170,9 +173,9 @@ struct Symbol { struct Details { /// Documentation including comment for the symbol declaration. llvm::StringRef Documentation; - /// This is what goes into the LSP detail field in a completion item. For - /// example, the result type of a function. - llvm::StringRef CompletionDetail; + /// 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 diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index f2255ce3401a..2a5cfa9f5e33 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -96,12 +96,10 @@ mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch) { if (!S.CanonicalDeclaration) S.CanonicalDeclaration = O.CanonicalDeclaration; S.References += O.References; - if (S.CompletionLabel == "") - S.CompletionLabel = O.CompletionLabel; - if (S.CompletionPlainInsertText == "") - S.CompletionPlainInsertText = O.CompletionPlainInsertText; - if (S.CompletionSnippetInsertText == "") - S.CompletionSnippetInsertText = O.CompletionSnippetInsertText; + if (S.Signature == "") + S.Signature = O.Signature; + if (S.CompletionSnippetSuffix == "") + S.CompletionSnippetSuffix = O.CompletionSnippetSuffix; if (O.Detail) { if (S.Detail) { @@ -109,8 +107,8 @@ mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch) { *Scratch = *S.Detail; if (Scratch->Documentation == "") Scratch->Documentation = O.Detail->Documentation; - if (Scratch->CompletionDetail == "") - Scratch->CompletionDetail = O.Detail->CompletionDetail; + if (Scratch->ReturnType == "") + Scratch->ReturnType = O.Detail->ReturnType; if (Scratch->IncludeHeader == "") Scratch->IncludeHeader = O.Detail->IncludeHeader; S.Detail = Scratch; diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 2d1e0d5bba6a..22d7b3df69ce 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -386,18 +386,13 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator, *CompletionTUInfo, /*IncludeBriefComments*/ false); - std::string Label; - std::string SnippetInsertText; - std::string IgnoredLabel; - std::string PlainInsertText; - getLabelAndInsertText(*CCS, &Label, &SnippetInsertText, - /*EnableSnippets=*/true); - getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText, - /*EnableSnippets=*/false); + std::string Signature; + std::string SnippetSuffix; + getSignature(*CCS, &Signature, &SnippetSuffix); std::string Documentation = formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion, /*CommentsFromHeaders=*/true)); - std::string CompletionDetail = getDetail(*CCS); + std::string ReturnType = getReturnType(*CCS); std::string Include; if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) { @@ -407,12 +402,11 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, QName, SM, SM.getExpansionLoc(ND.getLocation()), Opts)) Include = std::move(*Header); } - S.CompletionLabel = Label; - S.CompletionPlainInsertText = PlainInsertText; - S.CompletionSnippetInsertText = SnippetInsertText; + S.Signature = Signature; + S.CompletionSnippetSuffix = SnippetSuffix; Symbol::Details Detail; Detail.Documentation = Documentation; - Detail.CompletionDetail = CompletionDetail; + Detail.ReturnType = ReturnType; Detail.IncludeHeader = Include; S.Detail = &Detail; diff --git a/clang-tools-extra/clangd/index/SymbolYAML.cpp b/clang-tools-extra/clangd/index/SymbolYAML.cpp index 4120611d5521..304443e68e51 100644 --- a/clang-tools-extra/clangd/index/SymbolYAML.cpp +++ b/clang-tools-extra/clangd/index/SymbolYAML.cpp @@ -69,7 +69,7 @@ template <> struct MappingTraits { template <> struct MappingTraits { static void mapping(IO &io, Symbol::Details &Detail) { io.mapOptional("Documentation", Detail.Documentation); - io.mapOptional("CompletionDetail", Detail.CompletionDetail); + io.mapOptional("ReturnType", Detail.ReturnType); io.mapOptional("IncludeHeader", Detail.IncludeHeader); } }; @@ -110,11 +110,8 @@ template <> struct MappingTraits { IO.mapOptional("References", Sym.References, 0u); IO.mapOptional("IsIndexedForCodeCompletion", Sym.IsIndexedForCodeCompletion, false); - IO.mapRequired("CompletionLabel", Sym.CompletionLabel); - IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText); - - IO.mapOptional("CompletionSnippetInsertText", - Sym.CompletionSnippetInsertText); + IO.mapOptional("Signature", Sym.Signature); + IO.mapOptional("CompletionSnippetSuffix", Sym.CompletionSnippetSuffix); IO.mapOptional("Detail", NDetail->Opt); } }; diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp index 45ea505df0fd..eed9433d6532 100644 --- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp +++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp @@ -164,9 +164,6 @@ Symbol sym(StringRef QName, index::SymbolKind Kind, StringRef USRFormat) { } USR += Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func# Sym.ID = SymbolID(USR); - Sym.CompletionPlainInsertText = Sym.Name; - Sym.CompletionSnippetInsertText = Sym.Name; - Sym.CompletionLabel = Sym.Name; Sym.SymInfo.Kind = Kind; Sym.IsIndexedForCodeCompletion = true; return Sym; diff --git a/clang-tools-extra/unittests/clangd/CodeCompletionStringsTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompletionStringsTests.cpp index 82ab0f57a010..a6038a732d1c 100644 --- a/clang-tools-extra/unittests/clangd/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/unittests/clangd/CodeCompletionStringsTests.cpp @@ -23,24 +23,23 @@ public: CCTUInfo(Allocator), Builder(*Allocator, CCTUInfo) {} protected: - void labelAndInsertText(const CodeCompletionString &CCS, - bool EnableSnippets = false) { - Label.clear(); - InsertText.clear(); - getLabelAndInsertText(CCS, &Label, &InsertText, EnableSnippets); + void computeSignature(const CodeCompletionString &CCS) { + Signature.clear(); + Snippet.clear(); + getSignature(CCS, &Signature, &Snippet); } std::shared_ptr Allocator; CodeCompletionTUInfo CCTUInfo; CodeCompletionBuilder Builder; - std::string Label; - std::string InsertText; + std::string Signature; + std::string Snippet; }; -TEST_F(CompletionStringTest, Detail) { +TEST_F(CompletionStringTest, ReturnType) { Builder.AddResultTypeChunk("result"); Builder.AddResultTypeChunk("redundant result no no"); - EXPECT_EQ(getDetail(*Builder.TakeString()), "result"); + EXPECT_EQ(getReturnType(*Builder.TakeString()), "result"); } TEST_F(CompletionStringTest, Documentation) { @@ -65,31 +64,15 @@ TEST_F(CompletionStringTest, MultipleAnnotations) { "Annotations: Ano1 Ano2 Ano3\n"); } -TEST_F(CompletionStringTest, SimpleLabelAndInsert) { +TEST_F(CompletionStringTest, EmptySignature) { Builder.AddTypedTextChunk("X"); Builder.AddResultTypeChunk("result no no"); - labelAndInsertText(*Builder.TakeString()); - EXPECT_EQ(Label, "X"); - EXPECT_EQ(InsertText, "X"); + computeSignature(*Builder.TakeString()); + EXPECT_EQ(Signature, ""); + EXPECT_EQ(Snippet, ""); } -TEST_F(CompletionStringTest, FunctionPlainText) { - Builder.AddResultTypeChunk("result no no"); - Builder.AddTypedTextChunk("Foo"); - Builder.AddChunk(CodeCompletionString::CK_LeftParen); - Builder.AddPlaceholderChunk("p1"); - Builder.AddChunk(CodeCompletionString::CK_Comma); - Builder.AddPlaceholderChunk("p2"); - Builder.AddChunk(CodeCompletionString::CK_RightParen); - Builder.AddChunk(CodeCompletionString::CK_HorizontalSpace); - Builder.AddInformativeChunk("const"); - - labelAndInsertText(*Builder.TakeString()); - EXPECT_EQ(Label, "Foo(p1, p2) const"); - EXPECT_EQ(InsertText, "Foo"); -} - -TEST_F(CompletionStringTest, FunctionSnippet) { +TEST_F(CompletionStringTest, Function) { Builder.AddResultTypeChunk("result no no"); Builder.addBriefComment("This comment is ignored"); Builder.AddTypedTextChunk("Foo"); @@ -100,13 +83,9 @@ TEST_F(CompletionStringTest, FunctionSnippet) { Builder.AddChunk(CodeCompletionString::CK_RightParen); auto *CCS = Builder.TakeString(); - labelAndInsertText(*CCS); - EXPECT_EQ(Label, "Foo(p1, p2)"); - EXPECT_EQ(InsertText, "Foo"); - - labelAndInsertText(*CCS, /*EnableSnippets=*/true); - EXPECT_EQ(Label, "Foo(p1, p2)"); - EXPECT_EQ(InsertText, "Foo(${1:p1}, ${2:p2})"); + computeSignature(*CCS); + EXPECT_EQ(Signature, "(p1, p2)"); + EXPECT_EQ(Snippet, "(${1:p1}, ${2:p2})"); EXPECT_EQ(formatDocumentation(*CCS, "Foo's comment"), "Foo's comment"); } @@ -116,18 +95,18 @@ TEST_F(CompletionStringTest, EscapeSnippet) { Builder.AddPlaceholderChunk("$p}1\\"); Builder.AddChunk(CodeCompletionString::CK_RightParen); - labelAndInsertText(*Builder.TakeString(), /*EnableSnippets=*/true); - EXPECT_EQ(Label, "Foo($p}1\\)"); - EXPECT_EQ(InsertText, "Foo(${1:\\$p\\}1\\\\})"); + computeSignature(*Builder.TakeString()); + EXPECT_EQ(Signature, "($p}1\\)"); + EXPECT_EQ(Snippet, "(${1:\\$p\\}1\\\\})"); } TEST_F(CompletionStringTest, IgnoreInformativeQualifier) { Builder.AddTypedTextChunk("X"); Builder.AddInformativeChunk("info ok"); Builder.AddInformativeChunk("info no no::"); - labelAndInsertText(*Builder.TakeString()); - EXPECT_EQ(Label, "Xinfo ok"); - EXPECT_EQ(InsertText, "X"); + computeSignature(*Builder.TakeString()); + EXPECT_EQ(Signature, "info ok"); + EXPECT_EQ(Snippet, ""); } } // namespace diff --git a/clang-tools-extra/unittests/clangd/FileIndexTests.cpp b/clang-tools-extra/unittests/clangd/FileIndexTests.cpp index 1f00e4a4d302..08e85689ebeb 100644 --- a/clang-tools-extra/unittests/clangd/FileIndexTests.cpp +++ b/clang-tools-extra/unittests/clangd/FileIndexTests.cpp @@ -202,18 +202,15 @@ vector make_vector(Arg A) {} bool SeenMakeVector = false; M.fuzzyFind(Req, [&](const Symbol &Sym) { if (Sym.Name == "vector") { - EXPECT_EQ(Sym.CompletionLabel, "vector"); - EXPECT_EQ(Sym.CompletionSnippetInsertText, "vector<${1:class Ty}>"); - EXPECT_EQ(Sym.CompletionPlainInsertText, "vector"); + EXPECT_EQ(Sym.Signature, ""); + EXPECT_EQ(Sym.CompletionSnippetSuffix, "<${1:class Ty}>"); SeenVector = true; return; } if (Sym.Name == "make_vector") { - EXPECT_EQ(Sym.CompletionLabel, "make_vector(Arg A)"); - EXPECT_EQ(Sym.CompletionSnippetInsertText, - "make_vector<${1:class Ty}>(${2:Arg A})"); - EXPECT_EQ(Sym.CompletionPlainInsertText, "make_vector"); + EXPECT_EQ(Sym.Signature, "(Arg A)"); + EXPECT_EQ(Sym.CompletionSnippetSuffix, "<${1:class Ty}>(${2:Arg A})"); SeenMakeVector = true; } }); diff --git a/clang-tools-extra/unittests/clangd/IndexTests.cpp b/clang-tools-extra/unittests/clangd/IndexTests.cpp index 4d7c43027416..6314268672b4 100644 --- a/clang-tools-extra/unittests/clangd/IndexTests.cpp +++ b/clang-tools-extra/unittests/clangd/IndexTests.cpp @@ -274,11 +274,11 @@ TEST(MergeTest, Merge) { R.CanonicalDeclaration.FileURI = "file:///right.h"; L.References = 1; R.References = 2; - L.CompletionPlainInsertText = "f00"; // present in left only - R.CompletionSnippetInsertText = "f0{$1:0}"; // present in right only + L.Signature = "()"; // present in left only + R.CompletionSnippetSuffix = "{$1:0}"; // present in right only Symbol::Details DetL, DetR; - DetL.CompletionDetail = "DetL"; - DetR.CompletionDetail = "DetR"; + DetL.ReturnType = "DetL"; + DetR.ReturnType = "DetR"; DetR.Documentation = "--doc--"; L.Detail = &DetL; R.Detail = &DetR; @@ -288,10 +288,10 @@ TEST(MergeTest, Merge) { EXPECT_EQ(M.Name, "Foo"); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h"); EXPECT_EQ(M.References, 3u); - EXPECT_EQ(M.CompletionPlainInsertText, "f00"); - EXPECT_EQ(M.CompletionSnippetInsertText, "f0{$1:0}"); + EXPECT_EQ(M.Signature, "()"); + EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}"); ASSERT_TRUE(M.Detail); - EXPECT_EQ(M.Detail->CompletionDetail, "DetL"); + EXPECT_EQ(M.Detail->ReturnType, "DetL"); EXPECT_EQ(M.Detail->Documentation, "--doc--"); } @@ -302,19 +302,19 @@ TEST(MergeTest, PreferSymbolWithDefn) { L.ID = R.ID = SymbolID("hello"); L.CanonicalDeclaration.FileURI = "file:/left.h"; R.CanonicalDeclaration.FileURI = "file:/right.h"; - L.CompletionPlainInsertText = "left-insert"; - R.CompletionPlainInsertText = "right-insert"; + L.Name = "left"; + R.Name = "right"; Symbol M = mergeSymbol(L, R, &Scratch); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h"); EXPECT_EQ(M.Definition.FileURI, ""); - EXPECT_EQ(M.CompletionPlainInsertText, "left-insert"); + EXPECT_EQ(M.Name, "left"); R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored. M = mergeSymbol(L, R, &Scratch); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h"); EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp"); - EXPECT_EQ(M.CompletionPlainInsertText, "right-insert"); + EXPECT_EQ(M.Name, "right"); } } // namespace diff --git a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp index 8605cb2139a0..dadfe5fd5e08 100644 --- a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp +++ b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp @@ -36,15 +36,18 @@ using testing::UnorderedElementsAre; using testing::UnorderedElementsAreArray; // GMock helpers for matching Symbol. -MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; } -MATCHER(HasDetail, "") { return arg.Detail; } -MATCHER_P(Detail, D, "") { - return arg.Detail && arg.Detail->CompletionDetail == D; +MATCHER_P(Labeled, Label, "") { + return (arg.Name + arg.Signature).str() == Label; +} +MATCHER(HasReturnType, "") { + return arg.Detail && !arg.Detail->ReturnType.empty(); +} +MATCHER_P(ReturnType, D, "") { + return arg.Detail && arg.Detail->ReturnType == D; } MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; } -MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; } MATCHER_P(Snippet, S, "") { - return arg.CompletionSnippetInsertText == S; + return (arg.Name + arg.CompletionSnippetSuffix).str() == S; } MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; } @@ -656,10 +659,10 @@ TEST_F(SymbolCollectorTest, SymbolWithDocumentation) { Symbols, UnorderedElementsAre( QName("nx"), AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), - Detail("int"), Doc("Foo comment.")))); + ReturnType("int"), Doc("Foo comment.")))); } -TEST_F(SymbolCollectorTest, PlainAndSnippet) { +TEST_F(SymbolCollectorTest, Snippet) { const std::string Header = R"( namespace nx { void f() {} @@ -667,13 +670,12 @@ TEST_F(SymbolCollectorTest, PlainAndSnippet) { } )"; runSymbolCollector(Header, /*Main=*/""); - EXPECT_THAT( - Symbols, - UnorderedElementsAre( - QName("nx"), - AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")), - AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"), - Snippet("ff(${1:int x}, ${2:double y})")))); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + QName("nx"), + AllOf(QName("nx::f"), Labeled("f()"), Snippet("f()")), + AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), + Snippet("ff(${1:int x}, ${2:double y})")))); } TEST_F(SymbolCollectorTest, YAMLConversions) { @@ -694,12 +696,9 @@ CanonicalDeclaration: Line: 1 Column: 1 IsIndexedForCodeCompletion: true -CompletionLabel: 'Foo1-label' -CompletionFilterText: 'filter' -CompletionPlainInsertText: 'plain' Detail: Documentation: 'Foo doc' - CompletionDetail: 'int' + ReturnType: 'int' ... )"; const std::string YAML2 = R"( @@ -719,25 +718,23 @@ CanonicalDeclaration: Line: 1 Column: 1 IsIndexedForCodeCompletion: false -CompletionLabel: 'Foo2-label' -CompletionFilterText: 'filter' -CompletionPlainInsertText: 'plain' -CompletionSnippetInsertText: 'snippet' +Signature: '-sig' +CompletionSnippetSuffix: '-snippet' ... )"; auto Symbols1 = SymbolsFromYAML(YAML1); EXPECT_THAT(Symbols1, - UnorderedElementsAre(AllOf( - QName("clang::Foo1"), Labeled("Foo1-label"), Doc("Foo doc"), - Detail("int"), DeclURI("file:///path/foo.h"), - ForCodeCompletion(true)))); + UnorderedElementsAre(AllOf(QName("clang::Foo1"), Labeled("Foo1"), + Doc("Foo doc"), ReturnType("int"), + DeclURI("file:///path/foo.h"), + ForCodeCompletion(true)))); auto Symbols2 = SymbolsFromYAML(YAML2); - EXPECT_THAT(Symbols2, - UnorderedElementsAre(AllOf( - QName("clang::Foo2"), Labeled("Foo2-label"), Not(HasDetail()), - DeclURI("file:///path/bar.h"), ForCodeCompletion(false)))); + EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf( + QName("clang::Foo2"), Labeled("Foo2-sig"), + Not(HasReturnType()), DeclURI("file:///path/bar.h"), + ForCodeCompletion(false)))); std::string ConcatenatedYAML; {