[clangd] Uprank delcarations when "using q::name" is present in the main file

kirillbobyrev · kirillbobyrev · commit 47d7f52dea9b · 2018-07-11T14:49:49.000Z
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 llvm-svn: 336810
diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp
@@ -41,6 +41,17 @@ static bool hasDeclInMainFile(const Decl &D) {
   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 CodeCompletionResult &SemaCCResult) {
     // 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);
   }
 
diff --git a/clang-tools-extra/unittests/clangd/QualityTests.cpp b/clang-tools-extra/unittests/clangd/QualityTests.cpp
@@ -77,18 +77,31 @@ TEST(QualityTests, SymbolQualitySignalExtraction) {
 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;
 
-    [[deprecated]]
-    int deprecated() { return 0; }
+  using flags::FLAGS_FOO;
+
+  int ::header_main() {}
+  int main();
 
-    namespace { struct X { void y() { int z; } }; }
-    struct S{}
+  [[deprecated]]
+  int deprecated() { return 0; }
+
+  namespace { struct X { void y() { int z; } }; }
+  struct S{}
   )cpp";
   auto AST = Test.build();
 
@@ -111,6 +124,32 @@ TEST(QualityTests, SymbolRelevanceSignalExtraction) {
   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, SymbolRelevanceSignalsSanity) {
 }
 
 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));
diff --git a/clang/include/clang/Sema/CodeCompleteConsumer.h b/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -45,8 +45,9 @@ class LangOptions;
 class NamedDecl;
 class NestedNameSpecifier;
 class Preprocessor;
-class Sema;
 class RawComment;
+class Sema;
+class UsingShadowDecl;
 
 /// Default priority values for code-completion results based
 /// on their kind.
@@ -836,6 +837,12 @@ class CodeCompletionResult {
   /// informative rather than required.
   NestedNameSpecifier *Qualifier = nullptr;
 
+  /// If this Decl was unshadowed by using declaration, this can store a
+  /// pointer to the UsingShadowDecl which was used in the unshadowing process.
+  /// This information can be used to uprank CodeCompletionResults / which have
+  /// corresponding `using decl::qualified::name;` nearby.
+  const UsingShadowDecl *ShadowDecl = nullptr;
+
   /// Build a result that refers to a declaration.
   CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority,
                        NestedNameSpecifier *Qualifier = nullptr,
@@ -847,7 +854,7 @@ class CodeCompletionResult {
         QualifierIsInformative(QualifierIsInformative),
         StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
         DeclaringEntity(false), Qualifier(Qualifier) {
-    //FIXME: Add assert to check FixIts range requirements.
+    // FIXME: Add assert to check FixIts range requirements.
     computeCursorKindAndAvailability(Accessible);
   }
 
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -859,12 +859,12 @@ void ResultBuilder::MaybeAddResult(Result R, DeclContext *CurContext) {
   }
 
   // Look through using declarations.
-  if (const UsingShadowDecl *Using =
-          dyn_cast<UsingShadowDecl>(R.Declaration)) {
-    MaybeAddResult(Result(Using->getTargetDecl(),
-                          getBasePriority(Using->getTargetDecl()),
-                          R.Qualifier),
-                   CurContext);
+  if (const UsingShadowDecl *Using = dyn_cast<UsingShadowDecl>(R.Declaration)) {
+    CodeCompletionResult Result(Using->getTargetDecl(),
+                                getBasePriority(Using->getTargetDecl()),
+                                R.Qualifier);
+    Result.ShadowDecl = Using;
+    MaybeAddResult(Result, CurContext);
     return;
   }
 
@@ -977,10 +977,11 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
 
   // Look through using declarations.
   if (const UsingShadowDecl *Using = dyn_cast<UsingShadowDecl>(R.Declaration)) {
-    AddResult(Result(Using->getTargetDecl(),
-                     getBasePriority(Using->getTargetDecl()),
-                     R.Qualifier),
-              CurContext, Hiding);
+    CodeCompletionResult Result(Using->getTargetDecl(),
+                                getBasePriority(Using->getTargetDecl()),
+                                R.Qualifier);
+    Result.ShadowDecl = Using;
+    AddResult(Result, CurContext, Hiding);
     return;
   }
 
@@ -1004,10 +1005,10 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
   if (AsNestedNameSpecifier) {
     R.StartsNestedNameSpecifier = true;
     R.Priority = CCP_NestedNameSpecifier;
-  }
-  else if (Filter == &ResultBuilder::IsMember && !R.Qualifier && InBaseClass &&
-           isa<CXXRecordDecl>(R.Declaration->getDeclContext()
-                                                  ->getRedeclContext()))
+  } else if (Filter == &ResultBuilder::IsMember && !R.Qualifier &&
+             InBaseClass &&
+             isa<CXXRecordDecl>(
+                 R.Declaration->getDeclContext()->getRedeclContext()))
     R.QualifierIsInformative = true;
 
   // If this result is supposed to have an informative qualifier, add one.
