sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ilya-biryukov, mgorny. Herald added a project: clang.
Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D61596 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/refactor/Rename.cpp clangd/refactor/Rename.h clangd/unittests/CMakeLists.txt clangd/unittests/RenameTests.cpp
Index: clangd/unittests/RenameTests.cpp =================================================================== --- /dev/null +++ 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 "gtest/gtest.h" +#include "gmock/gmock.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 Index: clangd/unittests/CMakeLists.txt =================================================================== --- clangd/unittests/CMakeLists.txt +++ clangd/unittests/CMakeLists.txt @@ -48,6 +48,7 @@ JSONTransportTests.cpp PrintASTTests.cpp QualityTests.cpp + RenameTests.cpp RIFFTests.cpp SelectionTests.cpp SerializationTests.cpp Index: clangd/refactor/Rename.h =================================================================== --- /dev/null +++ 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 "llvm/Support/Error.h" +#include "clang/Tooling/Core/Replacement.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 Index: clangd/refactor/Rename.cpp =================================================================== --- /dev/null +++ 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 Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -162,8 +162,8 @@ /// 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. Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -613,8 +613,8 @@ 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); Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ 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 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) @@ -294,52 +261,18 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName, Callback<std::vector<TextEdit>> CB) { - auto Action = [Pos](Path File, std::string NewName, + auto Action = [Pos](std::string File, std::string NewName, Callback<std::vector<TextEdit>> CB, 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( Index: clangd/CMakeLists.txt =================================================================== --- clangd/CMakeLists.txt +++ clangd/CMakeLists.txt @@ -89,6 +89,7 @@ index/dex/PostingList.cpp index/dex/Trigram.cpp + refactor/Rename.cpp refactor/Tweak.cpp LINK_LIBS
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
