[clangd] Expose qualified symbol names in CompletionItem (C++ structure only, no json).

[ioeric]

Summary

The qualified name can be used to match a completion item to its corresponding
symbol. This can be useful for tools that measure code completion quality.
Qualified names are not precise for identifying symbols; we need to figure out a
better way to identify completion items.

[ioeric]

• Fix build

[sammccall]

clangd/CodeComplete.cpp

247

can skip the bool() here if you like

clangd/Protocol.h

743

So this is always set to scope + filterText (except for non-decl completions, where you're not setting it in this patch).

WDYT about just making that "scope", and for now relying on the fact that scope + filtertext gives you a nice readable name?

Advantages:

• it's a bit smaller
• as we add more structure to the C++ completion API, I think we'll want to expose scope+name split, so this would be closer to the desired state.

[ioeric]

• store scope in completion item. Add splitQualifiedName for NamedDecl.

clangd/Protocol.h

743

Sounds good. Done.

[sammccall]

clangd/SourceCode.h

60 ↗

(On Diff #152331)

This is a bit non-orthogonal: the printing of the qname doesn't have much to do with splitting it.
Also, the printing doesn't really belong in SourceCode.h as it's basically AST manipulation.

What about adding e.g. printQualifiedName to AST.h, and calling splitQualifiedName(printQualifiedName(ND))?(

unittests/clangd/CodeCompleteTests.cpp

47

maybe test scope explicitly/directly?

[ioeric]

• Add printQualifiedName in AST.h

clangd/AST.h

 //===--- AST.h - Utility AST functions  -------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Various code that examines C++ source code using AST.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
 
+#include "clang/AST/Decl.h"
 #include "clang/Basic/SourceLocation.h"
 
 namespace clang {
 class SourceManager;
 class Decl;
 
 namespace clangd {
 
 /// Find the identifier source location of the given D.
 ///
 /// The returned location is usually the spelling location where the name of the
 /// decl occurs in the code.
 SourceLocation findNameLoc(const clang::Decl *D);
 
+/// Returns the qualified name of ND. The scope doesn't contain unwritten scopes
+/// like inline namespaces.
+std::string printQualifiedName(const NamedDecl &ND);
+
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_

clangd/AST.cpp

if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
       //   * symbols controlled and defined by a compile command-line option
       //     `-DName=foo`, the spelling location will be "<command line>".
       SpellingLoc = SM.getExpansionRange(D->getLocation()).getBegin();
     }
   }
   return SpellingLoc;
 }
 
+std::string printQualifiedName(const NamedDecl &ND) {
+  std::string QName;
+  llvm::raw_string_ostream OS(QName);
+  PrintingPolicy Policy(ND.getASTContext().getLangOpts());
+  // Note that inline namespaces are treated as transparent scopes. This
+  // reflects the way they're most commonly used for lookup. Ideally we'd
+  // include them, but at query time it's hard to find all the inline
+  // namespaces to query: the preamble doesn't have a dedicated list.
+  Policy.SuppressUnwrittenScope = true;
+  ND.printQualifiedName(OS, Policy);
+  OS.flush();
+  assert(!StringRef(QName).startswith("::"));
+  return QName;
+}
+
 } // namespace clangd
 } // namespace clang

clangd/CodeComplete.cpp

 //  - results must be scored and ranked (see Quality.h) before rendering
 //
 // Signature help works in a similar way as code completion, but it is simpler:
 // it's purely AST-based, and there are few candidates.
 //
 //===---------------------------------------------------------------------===//
 
 #include "CodeComplete.h"
+#include "AST.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
 #include "FuzzyMatch.h"
 #include "Headers.h"
 #include "Logger.h"
 #include "Quality.h"
 #include "SourceCode.h"
 #include "Trace.h"
struct CompletionCandidate {
 
   // Builds an LSP completion item.
   CompletionItem build(StringRef FileName, const CompletionItemScores &Scores,
                        const CodeCompleteOptions &Opts,
                        CodeCompletionString *SemaCCS,
                        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;
       I.documentation = formatDocumentation(*SemaCCS, SemaDocComment);
       I.detail = getDetail(*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;
     }
     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;
       if (I.filterText.empty())
         I.filterText = IndexResult->Name;
 

clangd/Protocol.h

struct CompletionItem {
   /// themselves.
   std::vector<TextEdit> additionalTextEdits;
 
   // TODO(krasimir): The following optional fields defined by the language
   // server protocol are unsupported:
   //
   // data?: any - A data entry field that is preserved on a completion item
   //              between a completion and a completion resolve request.
+
+  // C++ extension that is only expected to be used by users of ClangdServer's
+  // C++ API. Not serialized from/to json.
+  /// The containing scope (e.g. namespace) of the symbol this item corresponds
+  /// to, e.g. "" (global scope), "ns::" (top-level namespace).
+  std::string SymbolScope;
 };
 json::Expr toJSON(const CompletionItem &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CompletionItem &);
 
 bool operator<(const CompletionItem &, const CompletionItem &);
 
 /// Represents a collection of completion items to be presented in the editor.
 struct CompletionList {
   /// The list is not complete. Further typing should result in recomputing the

clangd/index/SymbolCollector.cpp

void SymbolCollector::finish() {
   ReferencedDecls.clear();
 }
 
 const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
                                               SymbolID ID) {
   auto &Ctx = ND.getASTContext();
   auto &SM = Ctx.getSourceManager();
 
-  std::string QName;
-  llvm::raw_string_ostream OS(QName);
-  PrintingPolicy Policy(ASTCtx->getLangOpts());
-  // Note that inline namespaces are treated as transparent scopes. This
-  // reflects the way they're most commonly used for lookup. Ideally we'd
-  // include them, but at query time it's hard to find all the inline
-  // namespaces to query: the preamble doesn't have a dedicated list.
-  Policy.SuppressUnwrittenScope = true;
-  ND.printQualifiedName(OS, Policy);
-  OS.flush();
-  assert(!StringRef(QName).startswith("::"));
-
   Symbol S;
   S.ID = std::move(ID);
+  std::string QName = printQualifiedName(ND);
   std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
   // FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
   // for consistency with CodeCompletionString and a clean name/signature split.
 
   S.IsIndexedForCodeCompletion = isIndexedForCodeCompletion(ND, Ctx);
   S.SymInfo = index::getSymbolInfo(&ND);
   std::string FileURI;
   if (auto DeclLoc =

unittests/clangd/CodeCompleteTests.cpp

 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
   void onDiagnosticsReady(PathRef File,
                           std::vector<Diag> Diagnostics) override {}
 };
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.insertText == Name; }
+MATCHER_P(Scope, Name, "") { return arg.SymbolScope == Name; }
 MATCHER_P(Labeled, Label, "") {
   std::string Indented;
   if (!StringRef(Label).startswith(
           CodeCompleteOptions().IncludeIndicator.Insert) &&
       !StringRef(Label).startswith(
           CodeCompleteOptions().IncludeIndicator.NoInsert))
     Indented =
         (Twine(CodeCompleteOptions().IncludeIndicator.NoInsert) + Label).str();
TEST(CompletionTest, CompleteOnInvalidLine) {
   Position Pos;
   Pos.line = 100;
   Pos.character = 0;
   EXPECT_THAT_EXPECTED(
       runCodeComplete(Server, FooCpp, Pos, clangd::CodeCompleteOptions()),
       Failed());
 }
 
+TEST(CompletionTest, QualifiedNames) {
+  auto Results = completions(
+      R"cpp(
+          namespace ns { int local; void both(); }
+          void f() { ::ns::^ }
+      )cpp",
+      {func("ns::both"), cls("ns::Index")});
+  // We get results from both index and sema, with no duplicates.
+  EXPECT_THAT(Results.items, UnorderedElementsAre(Scope("ns::"), Scope("ns::"),
+                                                  Scope("ns::")));
+}
 
 } // namespace
 } // namespace clangd
 } // namespace clang