[clangd] More precise representation of symbol names/labels in the index.

[sammccall]

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.

[ioeric]

lgtm

clangd/CodeCompletionStrings.cpp

122

Qualifier seems a bit ambiguous for this. The documentation for CK_TypedText says:

/// The piece of text that the user is expected to type to
/// match the code-completion string, typically a keyword or the name of a
/// declarator or macro.
CK_TypedText,

For example, it could also be the keyword in "typedef ^". Maybe just TypedText would be more straightforward?

unittests/clangd/CodeCompletionStringsTests.cpp

70–72

nit: I find getSignature a bit awkward here because we are not getting anything back. Maybe setSignature?

[sammccall]

clangd/CodeCompletionStrings.cpp

122

Changed to "RequiredQualifiers" and specified it more clearly in the method comment.

Also added an implementation comment here as it's a little subtle. We're not outputting the current chunk to Qualifiers, but rather moving all the text we recorded so far.

unittests/clangd/CodeCompletionStringsTests.cpp

70–72

Agreed. Changed to computeSignature.

(I'm not sure the current test structure with the inputs and outputs being fixture members is great, but I don't really want to shave that yak now)

clangd/CodeComplete.cpp

CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
                        const IncludeInserter &Includes,
                        llvm::StringRef SemaDocComment) const {
     assert(bool(SemaResult) == bool(SemaCCS));
     assert(SemaResult || IndexResult);
 
     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<NamedDecl>(D))
             I.SymbolScope = splitQualifiedName(printQualifiedName(*ND)).first;
+      I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration);
     }
     if (IndexResult) {
       if (I.SymbolScope.empty())
         I.SymbolScope = IndexResult->Scope;
       if (I.kind == CompletionItemKind::Missing)
         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<std::string> {
             auto ResolvedDeclaring = toHeaderFile(
                 IndexResult->CanonicalDeclaration.FileURI, FileName);
             if (!ResolvedDeclaring)
               return ResolvedDeclaring.takeError();
             auto ResolvedInserted = toHeaderFile(*Inserted, FileName);
             if (!ResolvedInserted)

clangd/CodeCompletionStrings.h

 /// If \p CommentsFromHeaders parameter is set, only comments from the main
 /// file will be returned. It is used to workaround crashes when parsing
 /// comments in the stale headers, coming from completion preamble.
 std::string
 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<T>::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.
 ///
 /// \param DocComment is a documentation comment for the original declaration,
 ///        it should be obtained via getDocComment or getParameterDocComment.
 std::string formatDocumentation(const CodeCompletionString &CCS,
                                 llvm::StringRef DocComment);
 
 /// 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
 
 #endif

clangd/CodeCompletionStrings.cpp

 
 namespace {
 
 bool isInformativeQualifierChunk(CodeCompletionString::Chunk const &Chunk) {
   return Chunk.Kind == CodeCompletionString::CK_Informative &&
          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 == '\\')
       Out->push_back('\\');
     Out->push_back(Character);
   }
 }
 
-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)
     return true;
   auto &SourceMgr = Ctx.getSourceManager();
   // Accessing comments for decls from  invalid preamble can lead to crashes.
   // So we only return comments from the main file when doing code completion.
   // For indexing, we still read all the comments.
getParameterDocComment(const ASTContext &Ctx,
   if (!RC)
     return "";
   std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
   if (!LooksLikeDocComment(Doc))
     return "";
   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 <qualifiers><name><signature>.
+      // 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,
                                 llvm::StringRef DocComment) {
   // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
   // information in the documentation field.
   std::string Result;
   const unsigned AnnotationCount = CCS.getAnnotationCount();
if (!Result.empty()) {
       // character to make the annotations stand out.
       Result.push_back('\n');
     }
     Result += DocComment;
   }
   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 "";
 }
 
 } // namespace clangd
 } // namespace clang

clangd/index/Index.h

 // The class presents a C++ symbol, e.g. class, function.
 //
 // WARNING: Symbols do not own much of their underlying data - typically strings
 // are owned by a SymbolSlab. They should be treated as non-owning references.
 // Copies are shallow.
 // 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<k, t>::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;
   // The symbol information, like symbol kind.
   index::SymbolInfo SymInfo;
   // The unqualified name of the symbol, e.g. "bar" (for ns::bar).
   llvm::StringRef Name;
   // The containing namespace. e.g. "" (global), "ns::" (top-level namespace).
struct Symbol {
   //     in the ".h" file corresponding to the definition.
   SymbolLocation CanonicalDeclaration;
   // The number of translation units that reference this symbol from their main
   // file. This number is only meaningful if aggregated in an index.
   unsigned References = 0;
   /// 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
   /// is preferable to send only core symbol information in the batched results
   /// and have clients resolve full symbol information for a specific candidate
   /// if needed.
   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
     /// 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;

clangd/index/Index.cpp

static void own(Symbol &S, DenseSet<StringRef> &Strings,
   };
 
   // We need to copy every StringRef field onto the arena.
   Intern(S.Name);
   Intern(S.Scope);
   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.
     auto *Detail = Arena.Allocate<Symbol::Details>();
     *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;
   }
 }
 
 void SymbolSlab::Builder::insert(const Symbol &S) {
   auto R = SymbolIndex.try_emplace(S.ID, Symbols.size());

clangd/index/Merge.cpp

mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch) {
 
   // For each optional field, fill it from O if missing in S.
   // (It might be missing in O too, but that's a no-op).
   if (!S.Definition)
     S.Definition = O.Definition;
   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) {
       // Copy into scratch space so we can merge.
       *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;
     } else
       S.Detail = O.Detail;
   }
   return S;
 }
 
 std::unique_ptr<SymbolIndex> mergeIndex(const SymbolIndex *Dynamic,
                                         const SymbolIndex *Static) {
   return llvm::make_unique<MergedIndex>(Dynamic, Static);
 }
 } // namespace clangd
 } // namespace clang

clangd/index/SymbolCollector.cpp

const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
   // FIXME: we may want to choose a different redecl, or combine from several.
   assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
   // We use the primary template, as clang does during code completion.
   CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0);
   const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
       *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)) {
     // Use the expansion location to get the #include header since this is
     // where the symbol is exposed.
     if (auto Header = getIncludeHeader(
             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;
 
   Symbols.insert(S);
   return Symbols.find(S.ID);
 }
 
 void SymbolCollector::addDefinition(const NamedDecl &ND,

clangd/index/SymbolYAML.cpp

static void mapping(IO &io, SymbolInfo &SymInfo) {
     io.mapRequired("Kind", SymInfo.Kind);
     io.mapRequired("Lang", SymInfo.Lang);
   }
 };
 
 template <> struct MappingTraits<Symbol::Details> {
   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);
   }
 };
 
 // A YamlIO normalizer for fields of type "const T*" allocated on an arena.
 // Normalizes to Optional<T>, so traits should be provided for T.
 template <typename T> struct ArenaPtr {
   ArenaPtr(IO &) {}
static void mapping(IO &IO, Symbol &Sym) {
     IO.mapRequired("Scope", Sym.Scope);
     IO.mapRequired("SymInfo", Sym.SymInfo);
     IO.mapOptional("CanonicalDeclaration", Sym.CanonicalDeclaration,
                    SymbolLocation());
     IO.mapOptional("Definition", Sym.Definition, SymbolLocation());
     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);
   }
 };
 
 template <> struct ScalarEnumerationTraits<SymbolLanguage> {
   static void enumeration(IO &IO, SymbolLanguage &Value) {
     IO.enumCase(Value, "C", SymbolLanguage::C);
     IO.enumCase(Value, "Cpp", SymbolLanguage::CXX);

unittests/clangd/CodeCompleteTests.cpp

if (Pos == llvm::StringRef::npos) {
     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 += 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;
 }
 Symbol func(StringRef Name) { // Assumes the function has no args.
   return sym(Name, index::SymbolKind::Function, "@F@\\0#"); // no args
 }
 Symbol cls(StringRef Name) {

unittests/clangd/CodeCompletionStringsTests.cpp

 
 class CompletionStringTest : public ::testing::Test {
 public:
   CompletionStringTest()
       : Allocator(std::make_shared<clang::GlobalCodeCompletionAllocator>()),
         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<clang::GlobalCodeCompletionAllocator> 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) {
   Builder.addBriefComment("This is ignored");
   EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
             "Is this brief?");
 }
 
 TEST_F(CompletionStringTest, DocumentationWithAnnotation) {
   Builder.addBriefComment("This is ignored");
   Builder.AddAnnotation("Ano");
   EXPECT_EQ(formatDocumentation(*Builder.TakeString(), "Is this brief?"),
             "Annotation: Ano\n\nIs this brief?");
 }
 
 TEST_F(CompletionStringTest, MultipleAnnotations) {
   Builder.AddAnnotation("Ano1");
   Builder.AddAnnotation("Ano2");
   Builder.AddAnnotation("Ano3");
 
   EXPECT_EQ(formatDocumentation(*Builder.TakeString(), ""),
             "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");
   Builder.AddChunk(CodeCompletionString::CK_LeftParen);
   Builder.AddPlaceholderChunk("p1");
   Builder.AddChunk(CodeCompletionString::CK_Comma);
   Builder.AddPlaceholderChunk("p2");
   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");
 }
 
 TEST_F(CompletionStringTest, EscapeSnippet) {
   Builder.AddTypedTextChunk("Foo");
   Builder.AddChunk(CodeCompletionString::CK_LeftParen);
   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
 } // namespace clangd
 } // namespace clang

unittests/clangd/FileIndexTests.cpp

)cpp";
   update(M, "f", Source);
 
   FuzzyFindRequest Req;
   Req.Query = "";
   bool SeenVector = false;
   bool SeenMakeVector = false;
   M.fuzzyFind(Req, [&](const Symbol &Sym) {
     if (Sym.Name == "vector") {
-      EXPECT_EQ(Sym.CompletionLabel, "vector<class Ty>");
-      EXPECT_EQ(Sym.CompletionSnippetInsertText, "vector<${1:class Ty}>");
-      EXPECT_EQ(Sym.CompletionPlainInsertText, "vector");
+      EXPECT_EQ(Sym.Signature, "<class Ty>");
+      EXPECT_EQ(Sym.CompletionSnippetSuffix, "<${1:class Ty}>");
       SeenVector = true;
       return;
     }
 
     if (Sym.Name == "make_vector") {
-      EXPECT_EQ(Sym.CompletionLabel, "make_vector<class Ty>(Arg A)");
-      EXPECT_EQ(Sym.CompletionSnippetInsertText,
-                "make_vector<${1:class Ty}>(${2:Arg A})");
-      EXPECT_EQ(Sym.CompletionPlainInsertText, "make_vector");
+      EXPECT_EQ(Sym.Signature, "<class Ty>(Arg A)");
+      EXPECT_EQ(Sym.CompletionSnippetSuffix, "<${1:class Ty}>(${2:Arg A})");
       SeenMakeVector = true;
     }
   });
   EXPECT_TRUE(SeenVector);
   EXPECT_TRUE(SeenMakeVector);
 }
 
 TEST(FileIndexTest, RebuildWithPreamble) {

unittests/clangd/IndexTests.cpp

 TEST(MergeTest, Merge) {
   Symbol L, R;
   L.ID = R.ID = SymbolID("hello");
   L.Name = R.Name = "Foo";                    // same in both
   L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs
   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;
 
   Symbol::Details Scratch;
   Symbol M = mergeSymbol(L, R, &Scratch);
   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--");
 }
 
 TEST(MergeTest, PreferSymbolWithDefn) {
   Symbol L, R;
   Symbol::Details Scratch;
 
   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
 } // namespace clangd
 } // namespace clang

unittests/clangd/SymbolCollectorTests.cpp

 using testing::AllOf;
 using testing::Eq;
 using testing::Field;
 using testing::Not;
 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; }
 MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
 MATCHER_P(IncludeHeader, P, "") {
   return arg.Detail && arg.Detail->IncludeHeader == P;
 }
 MATCHER_P(DeclRange, Pos, "") {
const std::string Header = R"(
     int ff(int x, double y) { return 0; }
     }
   )";
   runSymbolCollector(Header, /*Main=*/"");
   EXPECT_THAT(
       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() {}
     int ff(int x, double y) { return 0; }
     }
   )";
   runSymbolCollector(Header, /*Main=*/"");
-  EXPECT_THAT(
-      Symbols,
+  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"),
+                  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) {
   const std::string YAML1 = R"(
 ---
 ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
 Name:   'Foo1'
 Scope:   'clang::'
 SymInfo:
   Kind:            Function
   Lang:            Cpp
 CanonicalDeclaration:
   FileURI:        file:///path/foo.h
   Start:
     Line: 1
     Column: 0
   End:
     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"(
 ---
 ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
 Name:   'Foo2'
 Scope:   'clang::'
 SymInfo:
   Kind:            Function
   Lang:            Cpp
 CanonicalDeclaration:
   FileURI:        file:///path/bar.h
   Start:
     Line: 1
     Column: 0
   End:
     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"),
+              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;
   {
     llvm::raw_string_ostream OS(ConcatenatedYAML);
     SymbolsToYAML(Symbols1, OS);
     SymbolsToYAML(Symbols2, OS);
   }
   auto ConcatenatedSymbols = SymbolsFromYAML(ConcatenatedYAML);