[clangd] Implement a smart version of HeaderSource switch.

Summary:
This patch implements another version header-source switch by incorporating the
AST and index, it will be used:
  - to improve the current header-source switch feature (layer with the
    existing file heuristic);
  - by the incoming define-outline code action;

Reviewers: kadircet

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

llvm-svn: 373197

clang-tools-extra/clangd/HeaderSourceSwitch.cpp

@@ -7,6 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "HeaderSourceSwitch.h"
+#include "AST.h"
+#include "Logger.h"
+#include "index/SymbolCollector.h"
+#include "clang/AST/Decl.h"
 
 namespace clang {
 namespace clangd {
@@ -64,5 +68,86 @@ llvm::Optional<Path> getCorrespondingHeaderOrSource(
   return None;
 }
 
+llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
+                                                    ParsedAST &AST,
+                                                    const SymbolIndex *Index) {
+  if (!Index) {
+    // FIXME: use the AST to do the inference.
+    return None;
+  }
+  LookupRequest Request;
+  // Find all symbols present in the original file.
+  for (const auto *D : getIndexableLocalDecls(AST)) {
+    if (auto ID = getSymbolID(D))
+      Request.IDs.insert(*ID);
+  }
+  llvm::StringMap<int> Candidates; // Target path => score.
+  auto AwardTarget = [&](const char *TargetURI) {
+    if (auto TargetPath = URI::resolve(TargetURI, OriginalFile)) {
+      if (*TargetPath != OriginalFile) // exclude the original file.
+        ++Candidates[*TargetPath];
+    };
+  };
+  // If we switch from a header, we are looking for the implementation
+  // file, so we use the definition loc; otherwise we look for the header file,
+  // we use the decl loc;
+  //
+  // For each symbol in the original file, we get its target location (decl or
+  // def) from the index, then award that target file.
+  bool IsHeader = AST.getASTContext().getLangOpts().IsHeaderFile;
+  Index->lookup(Request, [&](const Symbol &Sym) {
+    if (IsHeader)
+      AwardTarget(Sym.Definition.FileURI);
+    else
+      AwardTarget(Sym.CanonicalDeclaration.FileURI);
+  });
+  // FIXME: our index doesn't have any interesting information (this could be
+  // that the background-index is not finished), we should use the decl/def
+  // locations from the AST to do the inference (from .cc to .h).
+  if (Candidates.empty())
+    return None;
+
+  // Pickup the winner, who contains most of symbols.
+  // FIXME: should we use other signals (file proximity) to help score?
+  auto Best = Candidates.begin();
+  for (auto It = Candidates.begin(); It != Candidates.end(); ++It) {
+    if (It->second > Best->second)
+      Best = It;
+    else if (It->second == Best->second && It->first() < Best->first())
+      // Select the first one in the lexical order if we have multiple
+      // candidates.
+      Best = It;
+  }
+  return Path(Best->first());
+}
+
+std::vector<const Decl *> getIndexableLocalDecls(ParsedAST &AST) {
+  std::vector<const Decl *> Results;
+  std::function<void(Decl *)> TraverseDecl = [&](Decl *D) {
+    auto *ND = llvm::dyn_cast<NamedDecl>(D);
+    if (!ND || ND->isImplicit())
+      return;
+    if (!SymbolCollector::shouldCollectSymbol(*ND, D->getASTContext(), {},
+                                              /*IsMainFileSymbol=*/false))
+      return;
+    if (!llvm::isa<FunctionDecl>(ND)) {
+      // Visit the children, but we skip function decls as we are not interested
+      // in the function body.
+      if (auto *Scope = llvm::dyn_cast<DeclContext>(ND)) {
+        for (auto *D : Scope->decls())
+          TraverseDecl(D);
+      }
+    }
+    if (llvm::isa<NamespaceDecl>(D))
+      return; // namespace is indexable, but we're not interested.
+    Results.push_back(D);
+  };
+  // Traverses the ParsedAST directly to collect all decls present in the main
+  // file.
+  for (auto *TopLevel : AST.getLocalTopLevelDecls())
+    TraverseDecl(TopLevel);
+  return Results;
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/HeaderSourceSwitch.h

@@ -21,6 +21,16 @@ llvm::Optional<Path> getCorrespondingHeaderOrSource(
     const Path &OriginalFile,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS);
 
+/// Given a header file, returns the best matching source file, and vice visa.
+/// The heuristics incorporate with the AST and the index (if provided).
+llvm::Optional<Path> getCorrespondingHeaderOrSource(const Path &OriginalFile,
+                                                    ParsedAST &AST,
+                                                    const SymbolIndex *Index);
+
+/// Returns all indexable decls that are present in the main file of the AST.
+/// Exposed for unittests.
+std::vector<const Decl *> getIndexableLocalDecls(ParsedAST &AST);
+
 } // namespace clangd
 } // namespace clang
 

clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp

@@ -10,6 +10,7 @@
 
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/MemIndex.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -71,6 +72,174 @@ TEST(HeaderSourceSwitchTest, FileHeuristic) {
   EXPECT_FALSE(PathResult.hasValue());
 }
 
+MATCHER_P(DeclNamed, Name, "") {
+  if (const NamedDecl *ND = dyn_cast<NamedDecl>(arg))
+    if (ND->getQualifiedNameAsString() == Name)
+      return true;
+  return false;
+}
+
+TEST(HeaderSourceSwitchTest, GetLocalDecls) {
+  TestTU TU;
+  TU.HeaderCode = R"cpp(
+  void HeaderOnly();
+  )cpp";
+  TU.Code = R"cpp(
+  void MainF1();
+  class Foo {};
+  namespace ns {
+  class Foo {
+    void method();
+    int field;
+  };
+  } // namespace ns
+
+  // Non-indexable symbols
+  namespace {
+  void Ignore1() {}
+  }
+
+  )cpp";
+
+  auto AST = TU.build();
+  EXPECT_THAT(getIndexableLocalDecls(AST),
+              testing::UnorderedElementsAre(
+                  DeclNamed("MainF1"), DeclNamed("Foo"), DeclNamed("ns::Foo"),
+                  DeclNamed("ns::Foo::method"), DeclNamed("ns::Foo::field")));
+}
+
+TEST(HeaderSourceSwitchTest, FromHeaderToSource) {
+  // build a proper index, which contains symbols:
+  //   A_Sym1, declared in TestTU.h, defined in a.cpp
+  //   B_Sym[1-2], declared in TestTU.h, defined in b.cpp
+  SymbolSlab::Builder AllSymbols;
+  TestTU Testing;
+  Testing.HeaderFilename = "TestTU.h";
+  Testing.HeaderCode = "void A_Sym1();";
+  Testing.Filename = "a.cpp";
+  Testing.Code = "void A_Sym1() {};";
+  for (auto &Sym : Testing.headerSymbols())
+    AllSymbols.insert(Sym);
+
+  Testing.HeaderCode = R"cpp(
+  void B_Sym1();
+  void B_Sym2();
+  )cpp";
+  Testing.Filename = "b.cpp";
+  Testing.Code = R"cpp(
+  void B_Sym1() {}
+  void B_Sym2() {}
+  )cpp";
+  for (auto &Sym : Testing.headerSymbols())
+    AllSymbols.insert(Sym);
+  auto Index = MemIndex::build(std::move(AllSymbols).build(), {}, {});
+
+  // Test for swtich from .h header to .cc source
+  struct {
+    llvm::StringRef HeaderCode;
+    llvm::Optional<std::string> ExpectedSource;
+  } TestCases[] = {
+      {"// empty, no header found", llvm::None},
+      {R"cpp(
+         // no definition found in the index.
+         void NonDefinition();
+       )cpp",
+       llvm::None},
+      {R"cpp(
+         void A_Sym1();
+       )cpp",
+       testPath("a.cpp")},
+      {R"cpp(
+         // b.cpp wins.
+         void A_Sym1();
+         void B_Sym1();
+         void B_Sym2();
+       )cpp",
+       testPath("b.cpp")},
+      {R"cpp(
+         // a.cpp and b.cpp have same scope, but a.cpp because "a.cpp" < "b.cpp".
+         void A_Sym1();
+         void B_Sym1();
+       )cpp",
+       testPath("a.cpp")},
+  };
+  for (const auto &Case : TestCases) {
+    TestTU TU = TestTU::withCode(Case.HeaderCode);
+    TU.Filename = "TestTU.h";
+    TU.ExtraArgs.push_back("-xc++-header"); // inform clang this is a header.
+    auto HeaderAST = TU.build();
+    EXPECT_EQ(Case.ExpectedSource,
+              getCorrespondingHeaderOrSource(testPath(TU.Filename), HeaderAST,
+                                             Index.get()));
+  }
+}
+
+TEST(HeaderSourceSwitchTest, FromSourceToHeader) {
+  // build a proper index, which contains symbols:
+  //   A_Sym1, declared in a.h, defined in TestTU.cpp
+  //   B_Sym[1-2], declared in b.h, defined in TestTU.cpp
+  TestTU TUForIndex = TestTU::withCode(R"cpp(
+  #include "a.h"
+  #include "b.h"
+
+  void A_Sym1() {}
+
+  void B_Sym1() {}
+  void B_Sym2() {}
+  )cpp");
+  TUForIndex.AdditionalFiles["a.h"] = R"cpp(
+  void A_Sym1();
+  )cpp";
+  TUForIndex.AdditionalFiles["b.h"] = R"cpp(
+  void B_Sym1();
+  void B_Sym2();
+  )cpp";
+  TUForIndex.Filename = "TestTU.cpp";
+  auto Index = TUForIndex.index();
+
+  // Test for switching from .cc source file to .h header.
+  struct {
+    llvm::StringRef SourceCode;
+    llvm::Optional<std::string> ExpectedResult;
+  } TestCases[] = {
+      {"// empty, no header found", llvm::None},
+      {R"cpp(
+         // symbol not in index, no header found
+         void Local() {}
+       )cpp",
+       llvm::None},
+
+      {R"cpp(
+         // a.h wins.
+         void A_Sym1() {}
+       )cpp",
+       testPath("a.h")},
+
+      {R"cpp(
+         // b.h wins.
+         void A_Sym1() {}
+         void B_Sym1() {}
+         void B_Sym2() {}
+       )cpp",
+       testPath("b.h")},
+
+      {R"cpp(
+         // a.h and b.h have same scope, but a.h wins because "a.h" < "b.h".
+         void A_Sym1() {}
+         void B_Sym1() {}
+       )cpp",
+       testPath("a.h")},
+  };
+  for (const auto &Case : TestCases) {
+    TestTU TU = TestTU::withCode(Case.SourceCode);
+    TU.Filename = "Test.cpp";
+    auto AST = TU.build();
+    EXPECT_EQ(Case.ExpectedResult,
+              getCorrespondingHeaderOrSource(testPath(TU.Filename), AST,
+                                             Index.get()));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang