[clangd] Adjust symbol score based on crude symbol type.

[sammccall]

Summary

Numbers are guesses to be adjusted later.

[ioeric]

lgtm

clangd/Quality.cpp

33 ↗

(On Diff #150000)

Have you considered index::getSymbolInfo(ND) and reusing the other categorize?

51 ↗

(On Diff #150000)

ni: #undefine MAP?

clang-tools-extra/trunk/clangd/Quality.h

 /// Attributes of a symbol that affect how much we like it.
 struct SymbolQualitySignals {
   unsigned SemaCCPriority = 0; // 1-80, 1 is best. 0 means absent.
                                // FIXME: this is actually a mix of symbol
                                //        quality and relevance. Untangle this.
   bool Deprecated = false;
   unsigned References = 0;
 
+  enum SymbolCategory {
+    Variable,
+    Macro,
+    Type,
+    Function,
+    Namespace,
+    Unknown,
+  } Category = Unknown;
+
   void merge(const CodeCompletionResult &SemaCCResult);
   void merge(const Symbol &IndexResult);
 
   // Condense these signals down to a single number, higher is better.
   float evaluate() const;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &,
                               const SymbolQualitySignals &);

clang-tools-extra/trunk/clangd/Quality.cpp

 //===--- Quality.cpp --------------------------------------------*- C++-*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 #include "Quality.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclVisitor.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 
 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;
 }
 
+static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) {
+  class Switch
+      : public ConstDeclVisitor<Switch, SymbolQualitySignals::SymbolCategory> {
+  public:
+#define MAP(DeclType, Category)                                                \
+  SymbolQualitySignals::SymbolCategory Visit##DeclType(const DeclType *) {     \
+    return SymbolQualitySignals::Category;                                     \
+  }
+    MAP(NamespaceDecl, Namespace);
+    MAP(NamespaceAliasDecl, Namespace);
+    MAP(TypeDecl, Type);
+    MAP(TypeAliasTemplateDecl, Type);
+    MAP(ClassTemplateDecl, Type);
+    MAP(ValueDecl, Variable);
+    MAP(VarTemplateDecl, Variable);
+    MAP(FunctionDecl, Function);
+    MAP(FunctionTemplateDecl, Function);
+    MAP(Decl, Unknown);
+#undef MAP
+  };
+  return Switch().Visit(&ND);
+}
+
+static SymbolQualitySignals::SymbolCategory
+categorize(const index::SymbolInfo &D) {
+  switch (D.Kind) {
+    case index::SymbolKind::Namespace:
+    case index::SymbolKind::NamespaceAlias:
+      return SymbolQualitySignals::Namespace;
+    case index::SymbolKind::Macro:
+      return SymbolQualitySignals::Macro;
+    case index::SymbolKind::Enum:
+    case index::SymbolKind::Struct:
+    case index::SymbolKind::Class:
+    case index::SymbolKind::Protocol:
+    case index::SymbolKind::Extension:
+    case index::SymbolKind::Union:
+    case index::SymbolKind::TypeAlias:
+      return SymbolQualitySignals::Type;
+    case index::SymbolKind::Function:
+    case index::SymbolKind::ClassMethod:
+    case index::SymbolKind::InstanceMethod:
+    case index::SymbolKind::StaticMethod:
+    case index::SymbolKind::InstanceProperty:
+    case index::SymbolKind::ClassProperty:
+    case index::SymbolKind::StaticProperty:
+    case index::SymbolKind::Constructor:
+    case index::SymbolKind::Destructor:
+    case index::SymbolKind::ConversionFunction:
+      return SymbolQualitySignals::Function;
+    case index::SymbolKind::Variable:
+    case index::SymbolKind::Field:
+    case index::SymbolKind::EnumConstant:
+    case index::SymbolKind::Parameter:
+      return SymbolQualitySignals::Variable;
+    case index::SymbolKind::Using:
+    case index::SymbolKind::Module:
+    case index::SymbolKind::Unknown:
+      return SymbolQualitySignals::Unknown;
+  }
+}
+
 void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
   SemaCCPriority = SemaCCResult.Priority;
   if (SemaCCResult.Availability == CXAvailability_Deprecated)
     Deprecated = true;
+
+  if (SemaCCResult.Declaration)
+    Category = categorize(*SemaCCResult.Declaration);
+  else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
+    Category = Macro;
 }
 
 void SymbolQualitySignals::merge(const Symbol &IndexResult) {
   References = std::max(IndexResult.References, References);
+  Category = categorize(IndexResult.SymInfo);
 }
 
 float SymbolQualitySignals::evaluate() const {
   float Score = 1;
 
   // This avoids a sharp gradient for tail symbols, and also neatly avoids the
   // question of whether 0 references means a bad symbol or missing data.
   if (References >= 3)
     Score *= std::log(References);
 
   if (SemaCCPriority)
     // Map onto a 0-2 interval, so we don't reward/penalize non-Sema results.
     // Priority 80 is a really bad score.
     Score *= 2 - std::min<float>(80, SemaCCPriority) / 40;
 
   if (Deprecated)
     Score *= 0.1f;
 
+  switch (Category) {
+    case Type:
+    case Function:
+    case Variable:
+      Score *= 1.1;
+      break;
+    case Namespace:
+    case Macro:
+      Score *= 0.2;
+      break;
+    case Unknown:
+      break;
+  }
+
   return Score;
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const SymbolQualitySignals &S) {
   OS << formatv("=== Symbol quality: {0}\n", S.evaluate());
   if (S.SemaCCPriority)
     OS << formatv("\tSemaCCPriority: {0}\n", S.SemaCCPriority);
   OS << formatv("\tReferences: {0}\n", S.References);
   OS << formatv("\tDeprecated: {0}\n", S.Deprecated);
+  OS << formatv("\tCategory: {0}\n", static_cast<int>(S.Category));
   return OS;
 }
 
 static SymbolRelevanceSignals::AccessibleScope
 ComputeScope(const NamedDecl &D) {
   bool InClass = false;
   for (const DeclContext *DC = D.getDeclContext(); !DC->isFileContext();
        DC = DC->getParent()) {

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

TEST(QualityTests, SymbolQualitySignalExtraction) {
   auto Symbols = Header.headerSymbols();
   auto AST = Header.build();
 
   SymbolQualitySignals Quality;
   Quality.merge(findSymbol(Symbols, "x"));
   EXPECT_FALSE(Quality.Deprecated);
   EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority);
   EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
+  EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
 
   Symbol F = findSymbol(Symbols, "f");
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
   EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
   EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority);
   EXPECT_EQ(Quality.References, 24u);
+  EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
 
   Quality = {};
   Quality.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42));
   EXPECT_TRUE(Quality.Deprecated);
   EXPECT_EQ(Quality.SemaCCPriority, 42u);
   EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
+  EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
 }
 
 TEST(QualityTests, SymbolRelevanceSignalExtraction) {
   TestTU Test;
   Test.HeaderCode = R"cpp(
     int header();
     int header_main();
     )cpp";
TEST(QualityTests, SymbolQualitySignalsSanity) {
   EXPECT_GT(WithReferences.evaluate(), Default.evaluate());
   EXPECT_GT(ManyReferences.evaluate(), WithReferences.evaluate());
 
   SymbolQualitySignals LowPriority, HighPriority;
   LowPriority.SemaCCPriority = 60;
   HighPriority.SemaCCPriority = 20;
   EXPECT_GT(HighPriority.evaluate(), Default.evaluate());
   EXPECT_LT(LowPriority.evaluate(), Default.evaluate());
+
+  SymbolQualitySignals Variable, Macro;
+  Variable.Category = SymbolQualitySignals::Variable;
+  Macro.Category = SymbolQualitySignals::Macro;
+  EXPECT_GT(Variable.evaluate(), Default.evaluate());
+  EXPECT_LT(Macro.evaluate(), Default.evaluate());
 }
 
 TEST(QualityTests, SymbolRelevanceSignalsSanity) {
   SymbolRelevanceSignals Default;
   EXPECT_EQ(Default.evaluate(), 1);
 
   SymbolRelevanceSignals Forbidden;
   Forbidden.Forbidden = true;