diff --git a/clang-tools-extra/clangd/Quality.cpp b/clang-tools-extra/clangd/Quality.cpp index 1e9211b9cb14..fe7a969eb3a7 100644 --- a/clang-tools-extra/clangd/Quality.cpp +++ b/clang-tools-extra/clangd/Quality.cpp @@ -62,6 +62,10 @@ static bool hasUsingDeclInMainFile(const CodeCompletionResult &R) { } static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) { + if (const auto *FD = dyn_cast(&ND)) { + if (FD->isOverloadedOperator()) + return SymbolQualitySignals::Operator; + } class Switch : public ConstDeclVisitor { public: @@ -75,6 +79,7 @@ static SymbolQualitySignals::SymbolCategory categorize(const NamedDecl &ND) { MAP(TypeAliasTemplateDecl, Type); MAP(ClassTemplateDecl, Type); MAP(CXXConstructorDecl, Constructor); + MAP(CXXDestructorDecl, Destructor); MAP(ValueDecl, Variable); MAP(VarTemplateDecl, Variable); MAP(FunctionDecl, Function); @@ -134,9 +139,10 @@ categorize(const index::SymbolInfo &D) { case index::SymbolKind::InstanceProperty: case index::SymbolKind::ClassProperty: case index::SymbolKind::StaticProperty: - case index::SymbolKind::Destructor: case index::SymbolKind::ConversionFunction: return SymbolQualitySignals::Function; + case index::SymbolKind::Destructor: + return SymbolQualitySignals::Destructor; case index::SymbolKind::Constructor: return SymbolQualitySignals::Constructor; case index::SymbolKind::Variable: @@ -231,10 +237,12 @@ float SymbolQualitySignals::evaluate() const { Score *= 0.8f; break; case Macro: + case Destructor: + case Operator: Score *= 0.5f; break; - case Unknown: case Constructor: // No boost constructors so they are after class types. + case Unknown: break; } diff --git a/clang-tools-extra/clangd/Quality.h b/clang-tools-extra/clangd/Quality.h index f85e427fbe17..9cb0c2f3085d 100644 --- a/clang-tools-extra/clangd/Quality.h +++ b/clang-tools-extra/clangd/Quality.h @@ -68,8 +68,10 @@ struct SymbolQualitySignals { Type, Function, Constructor, + Destructor, Namespace, Keyword, + Operator, } Category = Unknown; void merge(const CodeCompletionResult &SemaCCResult); diff --git a/clang-tools-extra/unittests/clangd/QualityTests.cpp b/clang-tools-extra/unittests/clangd/QualityTests.cpp index cc330f28a523..f1a1fdae6b4e 100644 --- a/clang-tools-extra/unittests/clangd/QualityTests.cpp +++ b/clang-tools-extra/unittests/clangd/QualityTests.cpp @@ -205,16 +205,22 @@ TEST(QualityTests, SymbolQualitySignalsSanity) { EXPECT_GT(WithReferences.evaluate(), Default.evaluate()); EXPECT_GT(ManyReferences.evaluate(), WithReferences.evaluate()); - SymbolQualitySignals Keyword, Variable, Macro, Constructor, Function; + SymbolQualitySignals Keyword, Variable, Macro, Constructor, Function, + Destructor, Operator; Keyword.Category = SymbolQualitySignals::Keyword; Variable.Category = SymbolQualitySignals::Variable; Macro.Category = SymbolQualitySignals::Macro; Constructor.Category = SymbolQualitySignals::Constructor; + Destructor.Category = SymbolQualitySignals::Destructor; + Destructor.Category = SymbolQualitySignals::Destructor; + Operator.Category = SymbolQualitySignals::Operator; Function.Category = SymbolQualitySignals::Function; EXPECT_GT(Variable.evaluate(), Default.evaluate()); EXPECT_GT(Keyword.evaluate(), Variable.evaluate()); EXPECT_LT(Macro.evaluate(), Default.evaluate()); + EXPECT_LT(Operator.evaluate(), Default.evaluate()); EXPECT_LT(Constructor.evaluate(), Function.evaluate()); + EXPECT_LT(Destructor.evaluate(), Constructor.evaluate()); } TEST(QualityTests, SymbolRelevanceSignalsSanity) { @@ -385,11 +391,12 @@ TEST(QualityTests, IsInstanceMember) { EXPECT_TRUE(Rel.IsInstanceMember); } -TEST(QualityTests, ConstructorQuality) { +TEST(QualityTests, ConstructorDestructor) { auto Header = TestTU::withHeaderCode(R"cpp( class Foo { public: Foo(int); + ~Foo(); }; )cpp"); auto Symbols = Header.headerSymbols(); @@ -399,15 +406,43 @@ TEST(QualityTests, ConstructorQuality) { return (ND.getQualifiedNameAsString() == "Foo::Foo") && isa(&ND); }); + const NamedDecl *DtorDecl = &findDecl(AST, [](const NamedDecl &ND) { + return (ND.getQualifiedNameAsString() == "Foo::~Foo") && + isa(&ND); + }); - SymbolQualitySignals Q; - Q.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0)); - EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor); + SymbolQualitySignals CtorQ; + CtorQ.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0)); + EXPECT_EQ(CtorQ.Category, SymbolQualitySignals::Constructor); - Q.Category = SymbolQualitySignals::Unknown; + CtorQ.Category = SymbolQualitySignals::Unknown; const Symbol &CtorSym = findSymbol(Symbols, "Foo::Foo"); - Q.merge(CtorSym); - EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor); + CtorQ.merge(CtorSym); + EXPECT_EQ(CtorQ.Category, SymbolQualitySignals::Constructor); + + SymbolQualitySignals DtorQ; + DtorQ.merge(CodeCompletionResult(DtorDecl, /*Priority=*/0)); + EXPECT_EQ(DtorQ.Category, SymbolQualitySignals::Destructor); +} + +TEST(QualityTests, Operator) { + auto Header = TestTU::withHeaderCode(R"cpp( + class Foo { + public: + bool operator<(const Foo& f1, const Foo& f2); + }; + )cpp"); + auto AST = Header.build(); + + const NamedDecl *Operator = &findDecl(AST, [](const NamedDecl &ND) { + if (const auto *OD = dyn_cast(&ND)) + if (OD->isOverloadedOperator()) + return true; + return false; + }); + SymbolQualitySignals Q; + Q.merge(CodeCompletionResult(Operator, /*Priority=*/0)); + EXPECT_EQ(Q.Category, SymbolQualitySignals::Operator); } TEST(QualityTests, ItemWithFixItsRankedDown) {