[clangd] Boost scores for decls from current file in completion

Summary: This should, arguably, give better ranking.

Reviewers: ioeric, sammccall

Reviewed By: sammccall

Subscribers: mgorny, klimek, MaskRay, jkorous, mgrang, cfe-commits

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

llvm-svn: 333906

clang-tools-extra/clangd/Quality.cpp

@@ -8,6 +8,8 @@
 //===---------------------------------------------------------------------===//
 #include "Quality.h"
 #include "index/Index.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
@@ -17,9 +19,18 @@ namespace clang {
 namespace clangd {
 using namespace llvm;
 
+static bool hasDeclInMainFile(const Decl &D) {
+  auto &SourceMgr = D.getASTContext().getSourceManager();
+  for (auto *Redecl : D.redecls()) {
+    auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
+    if (SourceMgr.isWrittenInMainFile(Loc))
+      return true;
+  }
+  return false;
+}
+
 void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
   SemaCCPriority = SemaCCResult.Priority;
-
   if (SemaCCResult.Availability == CXAvailability_Deprecated)
     Deprecated = true;
 }
@@ -60,12 +71,25 @@ void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
   if (SemaCCResult.Availability == CXAvailability_NotAvailable ||
       SemaCCResult.Availability == CXAvailability_NotAccessible)
     Forbidden = true;
+
+  if (SemaCCResult.Declaration) {
+    // We boost things that have decls in the main file.
+    // The real proximity scores would be more general when we have them.
+    float DeclProximity =
+        hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.0;
+    ProximityScore = std::max(DeclProximity, ProximityScore);
+  }
 }
 
 float SymbolRelevanceSignals::evaluate() const {
   if (Forbidden)
     return 0;
-  return NameMatch;
+
+  float Score = NameMatch;
+  // Proximity scores are [0,1] and we translate them into a multiplier in the
+  // range from 1 to 2.
+  Score *= 1 + ProximityScore;
+  return Score;
 }
 raw_ostream &operator<<(raw_ostream &OS, const SymbolRelevanceSignals &S) {
   OS << formatv("=== Symbol relevance: {0}\n", S.evaluate());

clang-tools-extra/clangd/Quality.h

@@ -64,6 +64,9 @@ struct SymbolRelevanceSignals {
   // 0-1 fuzzy-match score for unqualified name. Must be explicitly assigned.
   float NameMatch = 1;
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
+  /// Proximity between the best declaration and the query location. [0-1] score
+  /// where 1 is closest
+  float ProximityScore = 0;
 
   void merge(const CodeCompletionResult &SemaResult);
 

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

@@ -58,17 +58,46 @@ TEST(QualityTests, SymbolQualitySignalExtraction) {
 }
 
 TEST(QualityTests, SymbolRelevanceSignalExtraction) {
-  auto AST = TestTU::withHeaderCode(R"cpp(
-    [[deprecated]]
-    int f() { return 0; }
-  )cpp")
-                 .build();
+  TestTU Test;
+  Test.HeaderCode = R"cpp(
+    int test_func_in_header();
+    int test_func_in_header_and_cpp();
+    )cpp";
+  Test.Code = R"cpp(
+    int ::test_func_in_header_and_cpp() {
+    }
+    int test_func_in_cpp();
 
-  SymbolRelevanceSignals Relevance;
-  Relevance.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42,
-                                       nullptr, false, /*Accessible=*/false));
-  EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals().NameMatch);
-  EXPECT_TRUE(Relevance.Forbidden);
+    [[deprecated]]
+    int test_deprecated() { return 0; }
+  )cpp";
+  auto AST = Test.build();
+
+  SymbolRelevanceSignals Deprecated;
+  Deprecated.merge(CodeCompletionResult(&findDecl(AST, "test_deprecated"),
+                                        /*Priority=*/42, nullptr, false,
+                                        /*Accessible=*/false));
+  EXPECT_EQ(Deprecated.NameMatch, SymbolRelevanceSignals().NameMatch);
+  EXPECT_TRUE(Deprecated.Forbidden);
+
+  // Test proximity scores.
+  SymbolRelevanceSignals FuncInCpp;
+  FuncInCpp.merge(CodeCompletionResult(&findDecl(AST, "test_func_in_cpp"),
+                                       CCP_Declaration));
+  /// Decls in the current file should get a proximity score of 1.0.
+  EXPECT_FLOAT_EQ(FuncInCpp.ProximityScore, 1.0);
+
+  SymbolRelevanceSignals FuncInHeader;
+  FuncInHeader.merge(CodeCompletionResult(&findDecl(AST, "test_func_in_header"),
+                                          CCP_Declaration));
+  /// Decls outside current file currently don't get a proximity score boost.
+  EXPECT_FLOAT_EQ(FuncInHeader.ProximityScore, 0.0);
+
+  SymbolRelevanceSignals FuncInHeaderAndCpp;
+  FuncInHeaderAndCpp.merge(CodeCompletionResult(
+      &findDecl(AST, "test_func_in_header_and_cpp"), CCP_Declaration));
+  /// Decls in both header **and** the main file get the same boost.
+  EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.ProximityScore, 1.0);
 }
 
 // Do the signals move the scores in the direction we expect?
@@ -104,6 +133,10 @@ TEST(QualityTests, SymbolRelevanceSignalsSanity) {
   SymbolRelevanceSignals PoorNameMatch;
   PoorNameMatch.NameMatch = 0.2f;
   EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate());
+
+  SymbolRelevanceSignals WithProximity;
+  WithProximity.ProximityScore = 0.2;
+  EXPECT_LT(Default.evaluate(), WithProximity.evaluate());
 }
 
 TEST(QualityTests, SortText) {