[clangd] Treat class constructor as in the same scope as the class in ranking.

[sammccall]

unittests/clangd/QualityTests.cpp

219

hmm, maybe neater to add an overload of findAnyDecl() that takes a filter function<bool(const NamedDecl&)>? (and implement the findAnyDecl in terms of it)

Call site should be slightly more readable, and you'll also get an assertion that there's exactly one match "for free".

[ioeric]

• Refactored findAnyDecl.

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 "FileDistance.h"
 #include "URI.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclVisitor.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cmath>
 
 namespace clang {
 namespace clangd {
 using namespace llvm;

 
 static SymbolRelevanceSignals::AccessibleScope
 ComputeScope(const NamedDecl *D) {
   // Injected "Foo" within the class "Foo" has file scope, not class scope.
   const DeclContext *DC = D->getDeclContext();
   if (auto *R = dyn_cast_or_null<RecordDecl>(D))
     if (R->isInjectedClassName())
       DC = DC->getParent();
+  // Class constructor should have the same scope as the class.
+  if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(D))
+    DC = DC->getParent();
   bool InClass = false;
   for (; !DC->isFileContext(); DC = DC->getParent()) {
     if (DC->isFunctionOrMethod())
       return SymbolRelevanceSignals::FunctionScope;
     InClass = InClass || DC->isRecord();
   }
   if (InClass)
     return SymbolRelevanceSignals::ClassScope;

unittests/clangd/QualityTests.cpp

 // we can compute from a corpus of queries and preferred rankings.
 //
 //===----------------------------------------------------------------------===//
 
 #include "FileDistance.h"
 #include "Quality.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 
 // Force the unittest URI scheme to be linked,
 static int LLVM_ATTRIBUTE_UNUSED UnittestSchemeAnchorDest =
TEST(QualityTests, SortText) {
   EXPECT_LT(sortText(0.3f), sortText(0));
   EXPECT_LT(sortText(0), sortText(-10));
   EXPECT_LT(sortText(-10), sortText(-std::numeric_limits<float>::infinity()));
 
   EXPECT_LT(sortText(1, "z"), sortText(0, "a"));
   EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
 }
 
+TEST(QualityTests, NoBoostForClassConstructor) {
+  auto Header = TestTU::withHeaderCode(R"cpp(
+    class Foo {
+    public:
+      Foo(int);
+    };
+  )cpp");
+  auto Symbols = Header.headerSymbols();
+  auto AST = Header.build();
+
+  const NamedDecl *Foo = &findDecl(AST, "Foo");
+  SymbolRelevanceSignals Cls;
+  Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
+
+  const NamedDecl *CtorDecl = &findAnyDecl(AST, [](const NamedDecl &ND) {
+    return (ND.getQualifiedNameAsString() == "Foo::Foo") &&
+           llvm::isa<CXXConstructorDecl>(&ND);
+  });
+  SymbolRelevanceSignals Ctor;
+  Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+
+  EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
+  EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/TestTU.h

struct TestTU {
   SymbolSlab headerSymbols() const;
   std::unique_ptr<SymbolIndex> index() const;
 };
 
 // Look up an index symbol by qualified name, which must be unique.
 const Symbol &findSymbol(const SymbolSlab &, llvm::StringRef QName);
 // Look up an AST symbol by qualified name, which must be unique and top-level.
 const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName);
+// Look up a main-file AST symbol that satisfies \p Filter.
+const NamedDecl &findAnyDecl(ParsedAST &AST,
+                             std::function<bool(const NamedDecl &)> Filter);
 // Look up a main-file AST symbol by unqualified name, which must be unique.
 const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name);
 
 } // namespace clangd
 } // namespace clang
 #endif

unittests/clangd/TestTU.cpp

const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
   const DeclContext *Scope = Ctx.getTranslationUnitDecl();
   for (auto NameIt = Components.begin(), End = Components.end() - 1;
        NameIt != End; ++NameIt) {
     Scope = &cast<DeclContext>(LookupDecl(*Scope, *NameIt));
   }
   return LookupDecl(*Scope, Components.back());
 }
 
-const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name) {
+const NamedDecl &findAnyDecl(ParsedAST &AST,
+                             std::function<bool(const NamedDecl &)> Callback) {
   struct Visitor : RecursiveASTVisitor<Visitor> {
-    llvm::StringRef Name;
+    decltype(Callback) CB;
     llvm::SmallVector<const NamedDecl *, 1> Decls;
     bool VisitNamedDecl(const NamedDecl *ND) {
-      if (auto *ID = ND->getIdentifier())
-        if (ID->getName() == Name)
-          Decls.push_back(ND);
+      if (CB(*ND))
       return true;
     }
   } Visitor;
-  Visitor.Name = Name;
+  Visitor.CB = Callback;
   for (Decl *D : AST.getLocalTopLevelDecls())
     Visitor.TraverseDecl(D);
   if (Visitor.Decls.size() != 1) {
-    ADD_FAILURE() << Visitor.Decls.size() << " symbols named " << Name;
+    ADD_FAILURE() << Visitor.Decls.size() << " symbols matched.";
     assert(Visitor.Decls.size() == 1);
   }
   return *Visitor.Decls.front();
 }
 
+const NamedDecl &findAnyDecl(ParsedAST &AST, llvm::StringRef Name) {
+  return findAnyDecl(AST, [Name](const NamedDecl &ND) {
+    if (auto *ID = ND.getIdentifier())
+      if (ID->getName() == Name)
+        return true;
+    return false;
+  });
+}
+
 } // namespace clangd
 } // namespace clang