[clangd] Downrank symbols with reserved names (score *= 0.1)

Reviewers: ilya-biryukov

Subscribers: klimek, ioeric, MaskRay, jkorous, cfe-commits

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

llvm-svn: 334274
This commit is contained in:
Sam McCall 2018-06-08 09:36:34 +00:00
parent 257ff33989
commit e018b36cea
3 changed files with 30 additions and 5 deletions

View File

@ -9,6 +9,7 @@
#include "Quality.h"
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/CharInfo.h"
#include "clang/AST/DeclVisitor.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Sema/CodeCompleteConsumer.h"
@ -19,6 +20,11 @@
namespace clang {
namespace clangd {
using namespace llvm;
static bool IsReserved(StringRef Name) {
// FIXME: Should we exclude _Bool and others recognized by the standard?
return Name.size() >= 2 && Name[0] == '_' &&
(isUppercase(Name[1]) || Name[1] == '_');
}
static bool hasDeclInMainFile(const Decl &D) {
auto &SourceMgr = D.getASTContext().getSourceManager();
@ -101,11 +107,18 @@ void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
Category = categorize(*SemaCCResult.Declaration);
else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
Category = Macro;
if (SemaCCResult.Declaration) {
if (auto *ID = SemaCCResult.Declaration->getIdentifier())
ReservedName = ReservedName || IsReserved(ID->getName());
} else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
ReservedName = ReservedName || IsReserved(SemaCCResult.Macro->getName());
}
void SymbolQualitySignals::merge(const Symbol &IndexResult) {
References = std::max(IndexResult.References, References);
Category = categorize(IndexResult.SymInfo);
ReservedName = ReservedName || IsReserved(IndexResult.Name);
}
float SymbolQualitySignals::evaluate() const {
@ -118,6 +131,8 @@ float SymbolQualitySignals::evaluate() const {
if (Deprecated)
Score *= 0.1f;
if (ReservedName)
Score *= 0.1f;
switch (Category) {
case Type:
@ -142,6 +157,7 @@ raw_ostream &operator<<(raw_ostream &OS, const SymbolQualitySignals &S) {
OS << formatv("=== Symbol quality: {0}\n", S.evaluate());
OS << formatv("\tReferences: {0}\n", S.References);
OS << formatv("\tDeprecated: {0}\n", S.Deprecated);
OS << formatv("\tReserved name: {0}\n", S.ReservedName);
OS << formatv("\tCategory: {0}\n", static_cast<int>(S.Category));
return OS;
}

View File

@ -45,6 +45,8 @@ struct Symbol;
/// Attributes of a symbol that affect how much we like it.
struct SymbolQualitySignals {
bool Deprecated = false;
bool ReservedName = false; // __foo, _Foo are usually implementation details.
// FIXME: make these findable once user types _.
unsigned References = 0;
enum SymbolCategory {

View File

@ -28,31 +28,34 @@ namespace {
TEST(QualityTests, SymbolQualitySignalExtraction) {
auto Header = TestTU::withHeaderCode(R"cpp(
int x;
int _X;
[[deprecated]]
int f() { return x; }
int _f() { return _X; }
)cpp");
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
SymbolQualitySignals Quality;
Quality.merge(findSymbol(Symbols, "x"));
Quality.merge(findSymbol(Symbols, "_X"));
EXPECT_FALSE(Quality.Deprecated);
EXPECT_TRUE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
Symbol F = findSymbol(Symbols, "f");
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_FALSE(Quality.ReservedName);
EXPECT_EQ(Quality.References, 24u);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
Quality = {};
Quality.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42));
Quality.merge(CodeCompletionResult(&findDecl(AST, "_f"), /*Priority=*/42));
EXPECT_TRUE(Quality.Deprecated);
EXPECT_FALSE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
}
@ -112,6 +115,10 @@ TEST(QualityTests, SymbolQualitySignalsSanity) {
Deprecated.Deprecated = true;
EXPECT_LT(Deprecated.evaluate(), Default.evaluate());
SymbolQualitySignals ReservedName;
ReservedName.ReservedName = true;
EXPECT_LT(ReservedName.evaluate(), Default.evaluate());
SymbolQualitySignals WithReferences, ManyReferences;
WithReferences.References = 10;
ManyReferences.References = 1000;