[clang-tools-extra] r352868 - [clangd] Penalize file-scope symbols in the ranking for non-completion queries

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Feb  1 05:07:37 2019
New Revision: 352868

URL: http://llvm.org/viewvc/llvm-project?rev=352868&view=rev
Log:
[clangd] Penalize file-scope symbols in the ranking for non-completion queries

Patch by Nathan Ridge!

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

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=352868&r1=352867&r2=352868&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Quality.cpp (original)
+++ clang-tools-extra/trunk/clangd/Quality.cpp Fri Feb  1 05:07:37 2019
@@ -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()
     case GlobalScope:
       break;
     case FileScope:
-      Score *= 1.5;
+      Score *= 1.5f;
       break;
     case ClassScope:
       Score *= 2;
@@ -374,6 +374,19 @@ float SymbolRelevanceSignals::evaluate()
       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)

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=352868&r1=352867&r2=352868&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Fri Feb  1 05:07:37 2019
@@ -178,6 +178,16 @@ TEST(QualityTests, SymbolRelevanceSignal
   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, SymbolRelevanceSignal
 
   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());