[clangd] Move Rename into its own file, and add unit test. NFC

Reviewers: kadircet

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, jfb, cfe-commits

Tags: #clang

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

llvm-svn: 360116

clang-tools-extra/clangd/CMakeLists.txt

@@ -89,6 +89,7 @@ add_clang_library(clangDaemon
   index/dex/PostingList.cpp
   index/dex/Trigram.cpp
 
+  refactor/Rename.cpp
   refactor/Tweak.cpp
 
   LINK_LIBS

clang-tools-extra/clangd/ClangdServer.cpp

@@ -18,6 +18,7 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
+#include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -25,8 +26,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
-#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -44,38 +43,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-// Expand a DiagnosticError to make it print-friendly (print the detailed
-// message, rather than "clang diagnostic").
-llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
-  if (auto Diag = DiagnosticError::take(Err)) {
-    llvm::cantFail(std::move(Err));
-    SmallVector<char, 128> DiagMessage;
-    Diag->second.EmitToString(DE, DiagMessage);
-    return llvm::make_error<llvm::StringError>(DiagMessage,
-                                               llvm::inconvertibleErrorCode());
-  }
-  return Err;
-}
-
-class RefactoringResultCollector final
-    : public tooling::RefactoringResultConsumer {
-public:
-  void handleError(llvm::Error Err) override {
-    assert(!Result.hasValue());
-    Result = std::move(Err);
-  }
-
-  // Using the handle(SymbolOccurrences) from parent class.
-  using tooling::RefactoringResultConsumer::handle;
-
-  void handle(tooling::AtomicChanges SourceReplacements) override {
-    assert(!Result.hasValue());
-    Result = std::move(SourceReplacements);
-  }
-
-  llvm::Optional<llvm::Expected<tooling::AtomicChanges>> Result;
-};
-
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex, DiagnosticsConsumer &DiagConsumer)
@@ -299,47 +266,13 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
                       llvm::Expected<InputsAndAST> InpAST) {
     if (!InpAST)
       return CB(InpAST.takeError());
-    auto &AST = InpAST->AST;
-
-    RefactoringResultCollector ResultCollector;
-    const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
-    SourceLocation SourceLocationBeg =
-        clangd::getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
-    tooling::RefactoringRuleContext Context(
-        AST.getASTContext().getSourceManager());
-    Context.setASTContext(AST.getASTContext());
-    auto Rename = clang::tooling::RenameOccurrences::initiate(
-        Context, SourceRange(SourceLocationBeg), NewName);
-    if (!Rename)
-      return CB(expandDiagnostics(Rename.takeError(),
-                                  AST.getASTContext().getDiagnostics()));
-
-    Rename->invoke(ResultCollector, Context);
-
-    assert(ResultCollector.Result.hasValue());
-    if (!ResultCollector.Result.getValue())
-      return CB(expandDiagnostics(ResultCollector.Result->takeError(),
-                                  AST.getASTContext().getDiagnostics()));
-
-    std::vector<TextEdit> Replacements;
-    for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
-      tooling::Replacements ChangeReps = Change.getReplacements();
-      for (const auto &Rep : ChangeReps) {
-        // FIXME: Right now we only support renaming the main file, so we
-        // drop replacements not for the main file. In the future, we might
-        // consider to support:
-        //   * rename in any included header
-        //   * rename only in the "main" header
-        //   * provide an error if there are symbols we won't rename (e.g.
-        //     std::vector)
-        //   * rename globally in project
-        //   * rename in open files
-        if (Rep.getFilePath() == File)
-          Replacements.push_back(
-              replacementToEdit(InpAST->Inputs.Contents, Rep));
-      }
-    }
-    return CB(std::move(Replacements));
+    auto Changes = renameWithinFile(InpAST->AST, File, Pos, NewName);
+    if (!Changes)
+      return CB(Changes.takeError());
+    std::vector<TextEdit> Edits;
+    for (const auto &Rep : *Changes)
+      Edits.push_back(replacementToEdit(InpAST->Inputs.Contents, Rep));
+    return CB(std::move(Edits));
   };
 
   WorkScheduler.runWithAST(

clang-tools-extra/clangd/ClangdUnit.cpp

@@ -613,8 +613,8 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
                           std::move(VFS), Inputs.Index, Inputs.Opts);
 }
 
-SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
-                                        const FileID FID) {
+SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit,
+                                        const Position &Pos, const FileID FID) {
   const ASTContext &AST = Unit.getASTContext();
   const SourceManager &SourceMgr = AST.getSourceManager();
   auto Offset = positionToOffset(SourceMgr.getBufferData(FID), Pos);

clang-tools-extra/clangd/ClangdUnit.h

@@ -162,8 +162,8 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
 
 /// Get the beginning SourceLocation at a specified \p Pos.
 /// May be invalid if Pos is, or if there's no identifier.
-SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
-                                        const FileID FID);
+SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit,
+                                        const Position &Pos, const FileID FID);
 
 /// For testing/debugging purposes. Note that this method deserializes all
 /// unserialized Decls, so use with care.

clang-tools-extra/clangd/refactor/Rename.cpp

@@ -0,0 +1,79 @@
+#include "refactor/Rename.h"
+#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
+#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+class RefactoringResultCollector final
+    : public tooling::RefactoringResultConsumer {
+public:
+  void handleError(llvm::Error Err) override {
+    assert(!Result.hasValue());
+    Result = std::move(Err);
+  }
+
+  // Using the handle(SymbolOccurrences) from parent class.
+  using tooling::RefactoringResultConsumer::handle;
+
+  void handle(tooling::AtomicChanges SourceReplacements) override {
+    assert(!Result.hasValue());
+    Result = std::move(SourceReplacements);
+  }
+
+  llvm::Optional<llvm::Expected<tooling::AtomicChanges>> Result;
+};
+
+// Expand a DiagnosticError to make it print-friendly (print the detailed
+// message, rather than "clang diagnostic").
+llvm::Error expandDiagnostics(llvm::Error Err, DiagnosticsEngine &DE) {
+  if (auto Diag = DiagnosticError::take(Err)) {
+    llvm::cantFail(std::move(Err));
+    SmallVector<char, 128> DiagMessage;
+    Diag->second.EmitToString(DE, DiagMessage);
+    return llvm::make_error<llvm::StringError>(DiagMessage,
+                                               llvm::inconvertibleErrorCode());
+  }
+  return Err;
+}
+
+} // namespace
+
+llvm::Expected<tooling::Replacements>
+renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
+                 llvm::StringRef NewName) {
+  RefactoringResultCollector ResultCollector;
+  ASTContext &ASTCtx = AST.getASTContext();
+  const SourceManager &SourceMgr = ASTCtx.getSourceManager();
+  SourceLocation SourceLocationBeg =
+      clangd::getBeginningOfIdentifier(AST, Pos, SourceMgr.getMainFileID());
+  tooling::RefactoringRuleContext Context(ASTCtx.getSourceManager());
+  Context.setASTContext(ASTCtx);
+  auto Rename = clang::tooling::RenameOccurrences::initiate(
+      Context, SourceRange(SourceLocationBeg), NewName);
+  if (!Rename)
+    return expandDiagnostics(Rename.takeError(), ASTCtx.getDiagnostics());
+
+  Rename->invoke(ResultCollector, Context);
+
+  assert(ResultCollector.Result.hasValue());
+  if (!ResultCollector.Result.getValue())
+    return expandDiagnostics(ResultCollector.Result->takeError(),
+                             ASTCtx.getDiagnostics());
+
+  tooling::Replacements FilteredChanges;
+  // Right now we only support renaming the main file, so we
+  // drop replacements not for the main file. In the future, we might
+  // also support rename with wider scope.
+  for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) {
+    for (const auto &Rep : Change.getReplacements()) {
+      if (Rep.getFilePath() == File)
+        cantFail(FilteredChanges.add(Rep));
+    }
+  }
+  return FilteredChanges;
+}
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/refactor/Rename.h

@@ -0,0 +1,24 @@
+//===--- Rename.h - Symbol-rename refactorings -------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangdUnit.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+/// Renames all occurrences of the symbol at \p Pos to \p NewName.
+/// Occurrences outside the current file are not modified.
+llvm::Expected<tooling::Replacements> renameWithinFile(ParsedAST &AST,
+                                                       llvm::StringRef File,
+                                                       Position Pos,
+                                                       llvm::StringRef NewName);
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/CMakeLists.txt

@@ -48,6 +48,7 @@ add_unittest(ClangdUnitTests ClangdTests
   JSONTransportTests.cpp
   PrintASTTests.cpp
   QualityTests.cpp
+  RenameTests.cpp
   RIFFTests.cpp
   SelectionTests.cpp
   SerializationTests.cpp

clang-tools-extra/clangd/unittests/RenameTests.cpp

@@ -0,0 +1,47 @@
+//===-- RenameTests.cpp -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "refactor/Rename.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(RenameTest, SingleFile) {
+  Annotations Code(R"cpp(
+    void foo() {
+      fo^o();
+    }
+  )cpp");
+  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");
+  ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+  auto ApplyResult = tooling::applyAllReplacements(Code.code(), *RenameResult);
+  ASSERT_TRUE(bool(ApplyResult)) << ApplyResult.takeError();
+
+  const char *Want = R"cpp(
+    void abcde() {
+      abcde();
+    }
+  )cpp";
+  EXPECT_EQ(Want, *ApplyResult);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang