[clangd] Penalize file-scope symbols in the ranking for non-completion queries

Patch by Nathan Ridge!

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

llvm-svn: 352868

diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp
index e9e8ff0..50e3f7e 100644 (file)
--- a/clang-tools-extra/clangd/Quality.cpp
+++ b/clang-tools-extra/clangd/Quality.cpp
@@ -282,12 +282,12 @@ computeScope(const NamedDecl *D) {
 }
 
 void SymbolRelevanceSignals::merge(const Symbol &IndexResult) {
-  // FIXME: Index results always assumed to be at global scope. If Scope becomes
-  // relevant to non-completion requests, we should recognize class members etc.
-
   SymbolURI = IndexResult.CanonicalDeclaration.FileURI;
   SymbolScope = IndexResult.Scope;
   IsInstanceMember |= isInstanceMember(IndexResult.SymInfo);
+  if (!(IndexResult.Flags & Symbol::VisibleOutsideFile)) {
+    Scope = AccessibleScope::FileScope;
+  }
 }
 
 void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
@@ -365,7 +365,7 @@ float SymbolRelevanceSignals::evaluate() const {
     case GlobalScope:
       break;
     case FileScope:
-      Score *= 1.5;
+      Score *= 1.5f;
       break;
     case ClassScope:
       Score *= 2;
@@ -374,6 +374,19 @@ float SymbolRelevanceSignals::evaluate() const {
       Score *= 4;
       break;
     }
+  } else {
+    // For non-completion queries, the wider the scope where a symbol is
+    // visible, the more likely it is to be relevant.
+    switch (Scope) {
+    case GlobalScope:
+      break;
+    case FileScope:
+      Score *= 0.5f;
+      break;
+    default:
+      // TODO: Handle other scopes as we start to use them for index results.
+      break;
+    }
   }
 
   if (TypeMatchesPreferred)

diff --git a/clang-tools-extra/unittests/clangd/QualityTests.cpp b/clang-tools-extra/unittests/clangd/QualityTests.cpp
index 259d296..5ae6dd5 100644 (file)
--- a/clang-tools-extra/unittests/clangd/QualityTests.cpp
+++ b/clang-tools-extra/unittests/clangd/QualityTests.cpp
@@ -178,6 +178,16 @@ TEST(QualityTests, SymbolRelevanceSignalExtraction) {
   BaseMember.InBaseClass = true;
   Relevance.merge(BaseMember);
   EXPECT_TRUE(Relevance.InBaseClass);
+
+  auto Index = Test.index();
+  Symbol X;
+  FuzzyFindRequest Req;
+  Req.Query = "X";
+  Req.AnyScope = true;
+  Index->fuzzyFind(Req, [&X](const Symbol& S){ X = S; });
+  Relevance = {};
+  Relevance.merge(X);
+  EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::FileScope);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -264,7 +274,7 @@ TEST(QualityTests, SymbolRelevanceSignalsSanity) {
 
   SymbolRelevanceSignals Scoped;
   Scoped.Scope = SymbolRelevanceSignals::FileScope;
-  EXPECT_EQ(Scoped.evaluate(), Default.evaluate());
+  EXPECT_LT(Scoped.evaluate(), Default.evaluate());
   Scoped.Query = SymbolRelevanceSignals::CodeComplete;
   EXPECT_GT(Scoped.evaluate(), Default.evaluate());