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

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48933

Files:
  clangd/Quality.cpp
  unittests/clangd/QualityTests.cpp


Index: unittests/clangd/QualityTests.cpp
===================================================================
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -21,6 +21,8 @@
 #include "Quality.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -199,6 +201,39 @@
   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));
+
+  // `findDecl` would return the implicit injected class for "Foo::Foo"; simply
+  // look for a constructor decl under the class.
+  const DeclContext *FooCtx = llvm::dyn_cast<DeclContext>(Foo);
+  assert(FooCtx);
+  const NamedDecl *CtorDecl = nullptr;
+  for (const auto *D : FooCtx->decls())
+    if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
+      if (llvm::isa<CXXConstructorDecl>(D)) {
+        CtorDecl = ND;
+        break;
+      }
+  assert(CtorDecl);
+  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
Index: clangd/Quality.cpp
===================================================================
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,10 +11,12 @@
 #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"
@@ -195,6 +197,9 @@
   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())



Index: unittests/clangd/QualityTests.cpp
===================================================================
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -21,6 +21,8 @@
 #include "Quality.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -199,6 +201,39 @@
   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));
+
+  // `findDecl` would return the implicit injected class for "Foo::Foo"; simply
+  // look for a constructor decl under the class.
+  const DeclContext *FooCtx = llvm::dyn_cast<DeclContext>(Foo);
+  assert(FooCtx);
+  const NamedDecl *CtorDecl = nullptr;
+  for (const auto *D : FooCtx->decls())
+    if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
+      if (llvm::isa<CXXConstructorDecl>(D)) {
+        CtorDecl = ND;
+        break;
+      }
+  assert(CtorDecl);
+  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
Index: clangd/Quality.cpp
===================================================================
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,10 +11,12 @@
 #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"
@@ -195,6 +197,9 @@
   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())

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits