hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Failing case:

  #include "foo.h"
  void fo^o() {}

getRenameDecl() returns the decl of the symbol under the cursor (which is
in the current main file), instead, we use the canonical decl to determine
whether a symbol is declared in #included header.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63872

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -78,7 +78,6 @@
   for (const Test &T : Tests) {
     Annotations Code(T.Before);
     auto TU = TestTU::withCode(Code.code());
-    TU.HeaderCode = "void foo();"; // outside main file, will not be touched.
     auto AST = TU.build();
     auto RenameResult =
         renameWithinFile(AST, testPath(TU.Filename), Code.point(), "abcde");
@@ -92,58 +91,68 @@
 }
 
 TEST(RenameTest, Renameable) {
-  // Test cases where the symbol is declared in header.
   struct Case {
-    const char* HeaderCode;
+    const char *Code;
     const char* ErrorMessage; // null if no error
+    bool IsHeaderFile;
   };
+  const bool HeaderFile = true;
   Case Cases[] = {
       {R"cpp(// allow -- function-local
         void f(int [[Lo^cal]]) {
           [[Local]] = 2;
         }
       )cpp",
-       nullptr},
+       nullptr, HeaderFile},
 
       {R"cpp(// allow -- symbol is indexable and has no refs in index.
         void [[On^lyInThisFile]]();
       )cpp",
-       nullptr},
+       nullptr, HeaderFile},
 
       {R"cpp(// disallow -- symbol is indexable and has other refs in index.
         void f() {
           Out^side s;
         }
       )cpp",
-       "used outside main file"},
+       "used outside main file", HeaderFile},
 
       {R"cpp(// disallow -- symbol is not indexable.
         namespace {
         class Unin^dexable {};
         }
       )cpp",
-       "not eligible for indexing"},
+       "not eligible for indexing", HeaderFile},
 
       {R"cpp(// disallow -- namespace symbol isn't supported
         namespace fo^o {}
       )cpp",
-       "not a supported kind"},
+       "not a supported kind", HeaderFile},
+
+      {R"cpp(// foo is declared outside the file.
+        void fo^o() {}
+      )cpp", "used outside main file", !HeaderFile/*cc file*/},
   };
-  const char *CommonHeader = "class Outside {};";
-  TestTU OtherFile = TestTU::withCode("Outside s;");
+  const char *CommonHeader = R"cpp(
+    class Outside {};
+    void foo();
+  )cpp";
+  TestTU OtherFile = TestTU::withCode("Outside s; auto ss = &foo;");
   OtherFile.HeaderCode = CommonHeader;
   OtherFile.Filename = "other.cc";
-  // The index has a "Outside" reference.
+  // The index has a "Outside" reference and a "foo" reference.
   auto OtherFileIndex = OtherFile.index();
 
   for (const auto& Case : Cases) {
-    Annotations T(Case.HeaderCode);
-    // We open the .h file as the main file.
+    Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
-    TU.Filename = "test.h";
     TU.HeaderCode = CommonHeader;
-    // Parsing the .h file as C++ include.
-    TU.ExtraArgs.push_back("-xobjective-c++-header");
+    if (Case.IsHeaderFile) {
+      // We open the .h file as the main file.
+      TU.Filename = "test.h";
+      // Parsing the .h file as C++ include.
+      TU.ExtraArgs.push_back("-xobjective-c++-header");
+    }
     auto AST = TU.build();
 
     auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(),
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -154,6 +154,7 @@
 
   const auto *RenameDecl = Rename->getRenameDecl();
   assert(RenameDecl && "symbol must be found at this point");
+  RenameDecl = cast<NamedDecl>(RenameDecl->getCanonicalDecl());
   if (auto Reject = renamableWithinFile(*RenameDecl, File, Index)) {
     auto Message = [](ReasonToReject Reason) {
       switch (Reason) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to