diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp index 00490274ad35..e42fca4e3449 100644 --- a/clang-tools-extra/clangd/Quality.cpp +++ b/clang-tools-extra/clangd/Quality.cpp @@ -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(S.Category)); return OS; } diff --git a/clang-tools-extra/clangd/Quality.h b/clang-tools-extra/clangd/Quality.h index c60686286751..ca1f1bf9126e 100644 --- a/clang-tools-extra/clangd/Quality.h +++ b/clang-tools-extra/clangd/Quality.h @@ -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 { diff --git a/clang-tools-extra/unittests/clangd/QualityTests.cpp b/clang-tools-extra/unittests/clangd/QualityTests.cpp index 4b16dd59c011..097fd3231556 100644 --- a/clang-tools-extra/unittests/clangd/QualityTests.cpp +++ b/clang-tools-extra/unittests/clangd/QualityTests.cpp @@ -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;