[clang-tools-extra] r336810 - [clangd] Uprank delcarations when "using q::name" is present in the main file

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Wed Jul 11 07:49:49 2018
New Revision: 336810

URL: http://llvm.org/viewvc/llvm-project?rev=336810&view=rev
Log:
[clangd] Uprank delcarations when "using q::name" is present in the main file

Having `using qualified::name;` for some symbol is an important signal
for clangd code completion as the user is more likely to use such
symbol.  This patch helps to uprank the relevant symbols by saving
UsingShadowDecl in the new field of CodeCompletionResult and checking
whether the corresponding UsingShadowDecl is located in the main file
later in ClangD code completion routine. While the relative importance
of such signal is a subject to change in the future, this patch simply
bumps DeclProximity score to the value of 1.0 which should be enough for
now.

The patch was tested using

`$ ninja check-clang check-clang-tools`

No unexpected failures were noticed after running the relevant testsets.

Reviewers: sammccall, ioeric

Subscribers: MaskRay, jkorous, cfe-commits

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

Modified:
    clang-tools-extra/trunk/clangd/Quality.cpp
    clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp

Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=336810&r1=336809&r2=336810&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Wed Jul 11 07:49:49 2018
@@ -41,6 +41,17 @@ static bool hasDeclInMainFile(const Decl
   return false;
 }
 
+static bool hasUsingDeclInMainFile(const CodeCompletionResult &R) {
+  const auto &Context = R.Declaration->getASTContext();
+  const auto &SourceMgr = Context.getSourceManager();
+  if (R.ShadowDecl) {
+    const auto Loc = SourceMgr.getExpansionLoc(R.ShadowDecl->getLocation());
+    if (SourceMgr.isWrittenInMainFile(Loc))
+      return true;
+  }
+  return false;
+}
+
 static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) {
   class Switch
       : public ConstDeclVisitor<Switch, SymbolQualitySignals::SymbolCategory> {
@@ -231,8 +242,10 @@ void SymbolRelevanceSignals::merge(const
     // We boost things that have decls in the main file. We give a fixed score
     // for all other declarations in sema as they are already included in the
     // translation unit.
-    float DeclProximity =
-        hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.6;
+    float DeclProximity = (hasDeclInMainFile(*SemaCCResult.Declaration) ||
+                           hasUsingDeclInMainFile(SemaCCResult))
+                              ? 1.0
+                              : 0.6;
     SemaProximityScore = std::max(DeclProximity, SemaProximityScore);
   }
 

Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=336810&r1=336809&r2=336810&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Wed Jul 11 07:49:49 2018
@@ -77,18 +77,31 @@ TEST(QualityTests, SymbolQualitySignalEx
 TEST(QualityTests, SymbolRelevanceSignalExtraction) {
   TestTU Test;
   Test.HeaderCode = R"cpp(
-    int header();
-    int header_main();
-    )cpp";
+  int header();
+  int header_main();
+
+  namespace hdr { class Bar {}; } // namespace hdr
+
+  #define DEFINE_FLAG(X) \
+  namespace flags { \
+  int FLAGS_##X; \
+  } \
+
+  DEFINE_FLAG(FOO)
+  )cpp";
   Test.Code = R"cpp(
-    int ::header_main() {}
-    int main();
+  using hdr::Bar;
+
+  using flags::FLAGS_FOO;
+
+  int ::header_main() {}
+  int main();
 
-    [[deprecated]]
-    int deprecated() { return 0; }
+  [[deprecated]]
+  int deprecated() { return 0; }
 
-    namespace { struct X { void y() { int z; } }; }
-    struct S{}
+  namespace { struct X { void y() { int z; } }; }
+  struct S{}
   )cpp";
   auto AST = Test.build();
 
@@ -111,6 +124,32 @@ TEST(QualityTests, SymbolRelevanceSignal
   EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
       << "Current file and header";
 
+  auto constructShadowDeclCompletionResult = [&](const std::string DeclName) {
+    auto *Shadow =
+        *dyn_cast<UsingDecl>(
+             &findAnyDecl(AST,
+                          [&](const NamedDecl &ND) {
+                            if (const UsingDecl *Using =
+                                    dyn_cast<UsingDecl>(&ND))
+                              if (Using->shadow_size() &&
+                                  Using->getQualifiedNameAsString() == DeclName)
+                                return true;
+                            return false;
+                          }))
+             ->shadow_begin();
+    CodeCompletionResult Result(Shadow->getTargetDecl(), 42);
+    Result.ShadowDecl = Shadow;
+    return Result;
+  };
+
+  Relevance = {};
+  Relevance.merge(constructShadowDeclCompletionResult("Bar"));
+  EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
+      << "Using declaration in main file";
+  Relevance.merge(constructShadowDeclCompletionResult("FLAGS_FOO"));
+  EXPECT_FLOAT_EQ(Relevance.SemaProximityScore, 1.0f)
+      << "Using declaration in main file";
+
   Relevance = {};
   Relevance.merge(CodeCompletionResult(&findAnyDecl(AST, "X"), 42));
   EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope);
@@ -191,7 +230,8 @@ TEST(QualityTests, SymbolRelevanceSignal
 }
 
 TEST(QualityTests, SortText) {
-  EXPECT_LT(sortText(std::numeric_limits<float>::infinity()), sortText(1000.2f));
+  EXPECT_LT(sortText(std::numeric_limits<float>::infinity()),
+            sortText(1000.2f));
   EXPECT_LT(sortText(1000.2f), sortText(1));
   EXPECT_LT(sortText(1), sortText(0.3f));
   EXPECT_LT(sortText(0.3f), sortText(0));