[clangd] Fetch documentation from the Index during signature help
authorIlya Biryukov <ibiryukov@google.com>
Fri, 17 Aug 2018 09:32:30 +0000 (09:32 +0000)
committerIlya Biryukov <ibiryukov@google.com>
Fri, 17 Aug 2018 09:32:30 +0000 (09:32 +0000)
Summary:
Sema can only be used for documentation in the current file, other doc
comments should be fetched from the index.

Reviewers: hokein, ioeric, kadircet

Reviewed By: hokein, kadircet

Subscribers: MaskRay, jkorous, mgrang, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D50727

llvm-svn: 340005

clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp

index 01cc823..104a86f 100644 (file)
@@ -177,15 +177,16 @@ void ClangdServer::signatureHelp(PathRef File, Position Pos,
 
   auto PCHs = this->PCHs;
   auto FS = FSProvider.getFileSystem();
-  auto Action = [Pos, FS, PCHs](Path File, Callback<SignatureHelp> CB,
-                                llvm::Expected<InputsAndPreamble> IP) {
+  auto *Index = this->Index;
+  auto Action = [Pos, FS, PCHs, Index](Path File, Callback<SignatureHelp> CB,
+                                       llvm::Expected<InputsAndPreamble> IP) {
     if (!IP)
       return CB(IP.takeError());
 
     auto PreambleData = IP->Preamble;
     CB(clangd::signatureHelp(File, IP->Command,
                              PreambleData ? &PreambleData->Preamble : nullptr,
-                             IP->Contents, Pos, FS, PCHs));
+                             IP->Contents, Pos, FS, PCHs, Index));
   };
 
   WorkScheduler.runWithPreamble("SignatureHelp", File,
index 02c6575..36d0f21 100644 (file)
@@ -687,18 +687,23 @@ private:
   llvm::unique_function<void()> ResultsCallback;
 };
 
-using ScoredSignature =
-    std::pair<SignatureQualitySignals, SignatureInformation>;
+struct ScoredSignature {
+  // When set, requires documentation to be requested from the index with this
+  // ID.
+  llvm::Optional<SymbolID> IDForDoc;
+  SignatureInformation Signature;
+  SignatureQualitySignals Quality;
+};
 
 class SignatureHelpCollector final : public CodeCompleteConsumer {
-
 public:
   SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
-                         SignatureHelp &SigHelp)
-      : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false),
+                         SymbolIndex *Index, SignatureHelp &SigHelp)
+      : CodeCompleteConsumer(CodeCompleteOpts,
+                             /*OutputIsBinary=*/false),
         SigHelp(SigHelp),
         Allocator(std::make_shared<clang::GlobalCodeCompletionAllocator>()),
-        CCTUInfo(Allocator) {}
+        CCTUInfo(Allocator), Index(Index) {}
 
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
                                  OverloadCandidate *Candidates,
@@ -726,47 +731,74 @@ public:
       const auto *CCS = Candidate.CreateSignatureString(
           CurrentArg, S, *Allocator, CCTUInfo, true);
       assert(CCS && "Expected the CodeCompletionString to be non-null");
-      // FIXME: for headers, we need to get a comment from the index.
       ScoredSignatures.push_back(processOverloadCandidate(
           Candidate, *CCS,
           Candidate.getFunction()
               ? getDeclComment(S.getASTContext(), *Candidate.getFunction())
               : ""));
     }
-    std::sort(ScoredSignatures.begin(), ScoredSignatures.end(),
-              [](const ScoredSignature &L, const ScoredSignature &R) {
-                // Ordering follows:
-                // - Less number of parameters is better.
-                // - Function is better than FunctionType which is better than
-                // Function Template.
-                // - High score is better.
-                // - Shorter signature is better.
-                // - Alphebatically smaller is better.
-                if (L.first.NumberOfParameters != R.first.NumberOfParameters)
-                  return L.first.NumberOfParameters <
-                         R.first.NumberOfParameters;
-                if (L.first.NumberOfOptionalParameters !=
-                    R.first.NumberOfOptionalParameters)
-                  return L.first.NumberOfOptionalParameters <
-                         R.first.NumberOfOptionalParameters;
-                if (L.first.Kind != R.first.Kind) {
-                  using OC = CodeCompleteConsumer::OverloadCandidate;
-                  switch (L.first.Kind) {
-                  case OC::CK_Function:
-                    return true;
-                  case OC::CK_FunctionType:
-                    return R.first.Kind != OC::CK_Function;
-                  case OC::CK_FunctionTemplate:
-                    return false;
-                  }
-                  llvm_unreachable("Unknown overload candidate type.");
-                }
-                if (L.second.label.size() != R.second.label.size())
-                  return L.second.label.size() < R.second.label.size();
-                return L.second.label < R.second.label;
-              });
-    for (const auto &SS : ScoredSignatures)
-      SigHelp.signatures.push_back(SS.second);
+
+    // Sema does not load the docs from the preamble, so we need to fetch extra
+    // docs from the index instead.
+    llvm::DenseMap<SymbolID, std::string> FetchedDocs;
+    if (Index) {
+      LookupRequest IndexRequest;
+      for (const auto &S : ScoredSignatures) {
+        if (!S.IDForDoc)
+          continue;
+        IndexRequest.IDs.insert(*S.IDForDoc);
+      }
+      Index->lookup(IndexRequest, [&](const Symbol &S) {
+        if (!S.Detail || S.Detail->Documentation.empty())
+          return;
+        FetchedDocs[S.ID] = S.Detail->Documentation;
+      });
+      log("SigHelp: requested docs for {0} symbols from the index, got {1} "
+          "symbols with non-empty docs in the response",
+          IndexRequest.IDs.size(), FetchedDocs.size());
+    }
+
+    std::sort(
+        ScoredSignatures.begin(), ScoredSignatures.end(),
+        [](const ScoredSignature &L, const ScoredSignature &R) {
+          // Ordering follows:
+          // - Less number of parameters is better.
+          // - Function is better than FunctionType which is better than
+          // Function Template.
+          // - High score is better.
+          // - Shorter signature is better.
+          // - Alphebatically smaller is better.
+          if (L.Quality.NumberOfParameters != R.Quality.NumberOfParameters)
+            return L.Quality.NumberOfParameters < R.Quality.NumberOfParameters;
+          if (L.Quality.NumberOfOptionalParameters !=
+              R.Quality.NumberOfOptionalParameters)
+            return L.Quality.NumberOfOptionalParameters <
+                   R.Quality.NumberOfOptionalParameters;
+          if (L.Quality.Kind != R.Quality.Kind) {
+            using OC = CodeCompleteConsumer::OverloadCandidate;
+            switch (L.Quality.Kind) {
+            case OC::CK_Function:
+              return true;
+            case OC::CK_FunctionType:
+              return R.Quality.Kind != OC::CK_Function;
+            case OC::CK_FunctionTemplate:
+              return false;
+            }
+            llvm_unreachable("Unknown overload candidate type.");
+          }
+          if (L.Signature.label.size() != R.Signature.label.size())
+            return L.Signature.label.size() < R.Signature.label.size();
+          return L.Signature.label < R.Signature.label;
+        });
+
+    for (auto &SS : ScoredSignatures) {
+      auto IndexDocIt =
+          SS.IDForDoc ? FetchedDocs.find(*SS.IDForDoc) : FetchedDocs.end();
+      if (IndexDocIt != FetchedDocs.end())
+        SS.Signature.documentation = IndexDocIt->second;
+
+      SigHelp.signatures.push_back(std::move(SS.Signature));
+    }
   }
 
   GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
@@ -779,11 +811,11 @@ private:
   ScoredSignature processOverloadCandidate(const OverloadCandidate &Candidate,
                                            const CodeCompletionString &CCS,
                                            llvm::StringRef DocComment) const {
-    SignatureInformation Result;
+    SignatureInformation Signature;
     SignatureQualitySignals Signal;
     const char *ReturnType = nullptr;
 
-    Result.documentation = formatDocumentation(CCS, DocComment);
+    Signature.documentation = formatDocumentation(CCS, DocComment);
     Signal.Kind = Candidate.getKind();
 
     for (const auto &Chunk : CCS) {
@@ -802,10 +834,10 @@ private:
         // A piece of text that describes the parameter that corresponds to
         // the code-completion location within a function call, message send,
         // macro invocation, etc.
-        Result.label += Chunk.Text;
+        Signature.label += Chunk.Text;
         ParameterInformation Info;
         Info.label = Chunk.Text;
-        Result.parameters.push_back(std::move(Info));
+        Signature.parameters.push_back(std::move(Info));
         Signal.NumberOfParameters++;
         Signal.ContainsActiveParameter = true;
         break;
@@ -814,29 +846,36 @@ private:
         // The rest of the parameters are defaulted/optional.
         assert(Chunk.Optional &&
                "Expected the optional code completion string to be non-null.");
-        Result.label +=
-            getOptionalParameters(*Chunk.Optional, Result.parameters, Signal);
+        Signature.label += getOptionalParameters(*Chunk.Optional,
+                                                 Signature.parameters, Signal);
         break;
       }
       case CodeCompletionString::CK_VerticalSpace:
         break;
       default:
-        Result.label += Chunk.Text;
+        Signature.label += Chunk.Text;
         break;
       }
     }
     if (ReturnType) {
-      Result.label += " -> ";
-      Result.label += ReturnType;
+      Signature.label += " -> ";
+      Signature.label += ReturnType;
     }
-    dlog("Signal for {0}: {1}", Result, Signal);
-    return {Signal, Result};
+    dlog("Signal for {0}: {1}", Signature, Signal);
+    ScoredSignature Result;
+    Result.Signature = std::move(Signature);
+    Result.Quality = Signal;
+    Result.IDForDoc =
+        Result.Signature.documentation.empty() && Candidate.getFunction()
+            ? clangd::getSymbolID(Candidate.getFunction())
+            : llvm::None;
+    return Result;
   }
 
   SignatureHelp &SigHelp;
   std::shared_ptr<clang::GlobalCodeCompletionAllocator> Allocator;
   CodeCompletionTUInfo CCTUInfo;
-
+  const SymbolIndex *Index;
 }; // SignatureHelpCollector
 
 struct SemaCompleteInput {
@@ -1315,7 +1354,8 @@ SignatureHelp signatureHelp(PathRef FileName,
                             PrecompiledPreamble const *Preamble,
                             StringRef Contents, Position Pos,
                             IntrusiveRefCntPtr<vfs::FileSystem> VFS,
-                            std::shared_ptr<PCHContainerOperations> PCHs) {
+                            std::shared_ptr<PCHContainerOperations> PCHs,
+                            SymbolIndex *Index) {
   SignatureHelp Result;
   clang::CodeCompleteOptions Options;
   Options.IncludeGlobals = false;
@@ -1323,10 +1363,11 @@ SignatureHelp signatureHelp(PathRef FileName,
   Options.IncludeCodePatterns = false;
   Options.IncludeBriefComments = false;
   IncludeStructure PreambleInclusions; // Unused for signatureHelp
-  semaCodeComplete(llvm::make_unique<SignatureHelpCollector>(Options, Result),
-                   Options,
-                   {FileName, Command, Preamble, Contents, Pos, std::move(VFS),
-                    std::move(PCHs)});
+  semaCodeComplete(
+      llvm::make_unique<SignatureHelpCollector>(Options, Index, Result),
+      Options,
+      {FileName, Command, Preamble, Contents, Pos, std::move(VFS),
+       std::move(PCHs)});
   return Result;
 }
 
index 227b49f..9ef2b1f 100644 (file)
@@ -172,12 +172,11 @@ CodeCompleteResult codeComplete(PathRef FileName,
                                 CodeCompleteOptions Opts);
 
 /// Get signature help at a specified \p Pos in \p FileName.
-SignatureHelp signatureHelp(PathRef FileName,
-                            const tooling::CompileCommand &Command,
-                            PrecompiledPreamble const *Preamble,
-                            StringRef Contents, Position Pos,
-                            IntrusiveRefCntPtr<vfs::FileSystem> VFS,
-                            std::shared_ptr<PCHContainerOperations> PCHs);
+SignatureHelp
+signatureHelp(PathRef FileName, const tooling::CompileCommand &Command,
+              PrecompiledPreamble const *Preamble, StringRef Contents,
+              Position Pos, IntrusiveRefCntPtr<vfs::FileSystem> VFS,
+              std::shared_ptr<PCHContainerOperations> PCHs, SymbolIndex *Index);
 
 // For index-based completion, we only consider:
 //   * symbols in namespaces or translation unit scopes (e.g. no class
index a181750..a95c91e 100644 (file)
@@ -826,11 +826,19 @@ TEST(CompletionTest, IgnoreCompleteInExcludedPPBranchWithRecoveryContext) {
   EXPECT_TRUE(Results.Completions.empty());
 }
 
-SignatureHelp signatures(StringRef Text) {
+SignatureHelp signatures(StringRef Text,
+                         std::vector<Symbol> IndexSymbols = {}) {
+  std::unique_ptr<SymbolIndex> Index;
+  if (!IndexSymbols.empty())
+    Index = memIndex(IndexSymbols);
+
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
-  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  Opts.StaticIndex = Index.get();
+
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
   auto File = testPath("foo.cpp");
   Annotations Test(Text);
   runAddDocument(Server, File, Test.code());
@@ -845,6 +853,7 @@ MATCHER_P(ParamsAre, P, "") {
       return false;
   return true;
 }
+MATCHER_P(SigDoc, Doc, "") { return arg.documentation == Doc; }
 
 Matcher<SignatureInformation> Sig(std::string Label,
                                   std::vector<std::string> Params) {
@@ -1594,6 +1603,51 @@ TEST(SignatureHelpTest, InstantiatedSignatures) {
               ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
 }
 
+TEST(SignatureHelpTest, IndexDocumentation) {
+  Symbol::Details DocDetails;
+  DocDetails.Documentation = "Doc from the index";
+
+  Symbol Foo0 = sym("foo", index::SymbolKind::Function, "@F@\\0#");
+  Foo0.Detail = &DocDetails;
+  Symbol Foo1 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#");
+  Foo1.Detail = &DocDetails;
+  Symbol Foo2 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#I#");
+
+  EXPECT_THAT(
+      signatures(R"cpp(
+    int foo();
+    int foo(double);
+
+    void test() {
+      foo(^);
+    }
+  )cpp",
+                 {Foo0})
+          .signatures,
+      ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Doc from the index")),
+                  AllOf(Sig("foo(double) -> int", {"double"}), SigDoc(""))));
+
+  EXPECT_THAT(
+      signatures(R"cpp(
+    int foo();
+    // Overriden doc from sema
+    int foo(int);
+    // Doc from sema
+    int foo(int, int);
+
+    void test() {
+      foo(^);
+    }
+  )cpp",
+                 {Foo0, Foo1, Foo2})
+          .signatures,
+      ElementsAre(AllOf(Sig("foo() -> int", {}), SigDoc("Doc from the index")),
+                  AllOf(Sig("foo(int) -> int", {"int"}),
+                        SigDoc("Overriden doc from sema")),
+                  AllOf(Sig("foo(int, int) -> int", {"int", "int"}),
+                        SigDoc("Doc from sema"))));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang