[clang-tools-extra] 902dc6c - [clangd] Fix a regression issue in local rename.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2019-12-02T10:32:55+01:00
New Revision: 902dc6c69ce7985427efa103a7c4099c372da6fa

URL: https://github.com/llvm/llvm-project/commit/902dc6c69ce7985427efa103a7c4099c372da6fa
DIFF: https://github.com/llvm/llvm-project/commit/902dc6c69ce7985427efa103a7c4099c372da6fa.diff

LOG: [clangd] Fix a regression issue in local rename.

Summary:
The regression is that we can't rename symbols in annonymous
namespaces.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D70853

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 6a3439cc0612..7d74641be719 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -123,20 +123,26 @@ llvm::Optional<ReasonToReject> renameable(const Decl &RenameDecl,
   if (RenameDecl.getParentFunctionOrMethod())
     return None;
 
+  // Check whether the symbol being rename is indexable.
+  auto &ASTCtx = RenameDecl.getASTContext();
+  bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts());
+  bool DeclaredInMainFile =
+      isInsideMainFile(RenameDecl.getBeginLoc(), ASTCtx.getSourceManager());
+  bool IsMainFileOnly = true;
+  if (MainFileIsHeader)
+    // main file is a header, the symbol can't be main file only.
+    IsMainFileOnly = false;
+  else if (!DeclaredInMainFile)
+    IsMainFileOnly = false;
   bool IsIndexable =
       isa<NamedDecl>(RenameDecl) &&
       SymbolCollector::shouldCollectSymbol(
           cast<NamedDecl>(RenameDecl), RenameDecl.getASTContext(),
-          SymbolCollector::Options(), CrossFile);
+          SymbolCollector::Options(), IsMainFileOnly);
   if (!IsIndexable) // If the symbol is not indexable, we disallow rename.
     return ReasonToReject::NonIndexable;
 
   if (!CrossFile) {
-    auto &ASTCtx = RenameDecl.getASTContext();
-    const auto &SM = ASTCtx.getSourceManager();
-    bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts());
-    bool DeclaredInMainFile = isInsideMainFile(RenameDecl.getBeginLoc(), SM);
-
     if (!DeclaredInMainFile)
       // We are sure the symbol is used externally, bail out early.
       return ReasonToReject::UsedOutsideFile;

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 0615272de372..1ade0c0443bc 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -450,13 +450,20 @@ TEST(RenameTest, Renameable) {
       )cpp",
        "used outside main file", HeaderFile, Index},
 
-      {R"cpp(// disallow -- symbol is not indexable.
+      {R"cpp(// disallow -- symbol in annonymous namespace in header is not indexable.
         namespace {
         class Unin^dexable {};
         }
       )cpp",
        "not eligible for indexing", HeaderFile, Index},
 
+      {R"cpp(// allow -- symbol in annonymous namespace in non-header file is indexable.
+        namespace {
+        class [[F^oo]] {};
+        }
+      )cpp",
+       nullptr, !HeaderFile, Index},
+
       {R"cpp(// disallow -- namespace symbol isn't supported
         namespace n^s {}
       )cpp",