Skip to content

Commit

Permalink
[clangd] Move Rename into its own file, and add unit test. NFC
Browse files Browse the repository at this point in the history
Reviewers: kadircet

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

Tags: #clang

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

llvm-svn: 360116
  • Loading branch information
sam-mccall authored and MrSidims committed May 17, 2019
1 parent f0d9ebe commit 2fe7f71
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 79 deletions.
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/CMakeLists.txt
Expand Up @@ -89,6 +89,7 @@ add_clang_library(clangDaemon
index/dex/PostingList.cpp
index/dex/Trigram.cpp

refactor/Rename.cpp
refactor/Tweak.cpp

LINK_LIBS
Expand Down
83 changes: 8 additions & 75 deletions clang-tools-extra/clangd/ClangdServer.cpp
Expand Up @@ -18,15 +18,14 @@
#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"
#include "clang/Frontend/CompilerInvocation.h"
#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"
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/ClangdUnit.cpp
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/ClangdUnit.h
Expand Up @@ -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.
Expand Down
79 changes: 79 additions & 0 deletions 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
24 changes: 24 additions & 0 deletions 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
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/unittests/CMakeLists.txt
Expand Up @@ -48,6 +48,7 @@ add_unittest(ClangdUnitTests ClangdTests
JSONTransportTests.cpp
PrintASTTests.cpp
QualityTests.cpp
RenameTests.cpp
RIFFTests.cpp
SelectionTests.cpp
SerializationTests.cpp
Expand Down
47 changes: 47 additions & 0 deletions 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

0 comments on commit 2fe7f71

Please sign in to comment.