From e30483a5c54cae335d9d5b9461cb8753e4d1a57f Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Tue, 1 Mar 2016 12:37:30 +0000 Subject: [PATCH] Add functions to apply replacements and reformat them. This is a commonly useful feature to have, and we have implemented it multiple times with different kinds of bugs. This implementation centralizes the idea in a set of functions that we can then use from the various tools. Reverts r262234, which is a revert of r262232, and puts the functions into FOrmat.h, as they are closely coupled to clang-format, and we otherwise introduce a cyclic dependency between libFormat and libTooling. Patch by Eric Liu. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@262323 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Format/Format.h | 22 ++++++++++++ include/clang/Tooling/Core/Replacement.h | 7 ++++ lib/Format/Format.cpp | 28 +++++++++++++++ lib/Tooling/Core/Replacement.cpp | 17 +++++++-- unittests/Format/CMakeLists.txt | 1 + unittests/Format/FormatTest.cpp | 46 +++++++++++++++++++++++- unittests/Tooling/RefactoringTest.cpp | 19 ++++++++++ 7 files changed, 137 insertions(+), 3 deletions(-) diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h index b32fc3adbd..f0636ac384 100644 --- a/include/clang/Format/Format.h +++ b/include/clang/Format/Format.h @@ -731,6 +731,28 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, StringRef FileName, unsigned *Cursor = nullptr); +/// \brief Returns the replacements corresponding to applying and formatting +/// \p Replaces. +tooling::Replacements formatReplacements(StringRef Code, + const tooling::Replacements &Replaces, + const FormatStyle &Style); + +/// \brief In addition to applying all replacements in \p Replaces to \p Code, +/// this function also reformats the changed code after applying replacements. +/// +/// \pre Replacements must be for the same file and conflict-free. +/// +/// Replacement applications happen independently of the success of +/// other applications. +/// +/// \returns the changed code with all replacements applied and formatted, if +/// successful. An empty string otherwise. +/// +/// See also "include/clang/Tooling/Core/Replacements.h". +std::string applyAllReplacementsAndFormat(StringRef Code, + const tooling::Replacements &Replaces, + const FormatStyle &Style); + /// \brief Reformats the given \p Ranges in the file \p ID. /// /// Each range is extended on either end to its next bigger logic unit, i.e. diff --git a/include/clang/Tooling/Core/Replacement.h b/include/clang/Tooling/Core/Replacement.h index 37389ac915..4ef7184961 100644 --- a/include/clang/Tooling/Core/Replacement.h +++ b/include/clang/Tooling/Core/Replacement.h @@ -220,6 +220,13 @@ bool applyAllReplacements(const std::vector &Replaces, /// replacements cannot be applied, this returns an empty \c string. std::string applyAllReplacements(StringRef Code, const Replacements &Replaces); +/// \brief Calculate the ranges in a single file that are affected by the +/// Replacements. +/// +/// \pre Replacements must be for the same file. +std::vector +calculateChangedRangesInFile(const tooling::Replacements &Replaces); + /// \brief Merges two sets of replacements with the second set referring to the /// code after applying the first set. Within both 'First' and 'Second', /// replacements must not overlap. diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 85a0152e29..9d5c0bca33 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -1884,6 +1884,34 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, return Replaces; } +tooling::Replacements formatReplacements(StringRef Code, + const tooling::Replacements &Replaces, + const FormatStyle &Style) { + if (Replaces.empty()) + return tooling::Replacements(); + + std::string NewCode = applyAllReplacements(Code, Replaces); + std::vector ChangedRanges = + tooling::calculateChangedRangesInFile(Replaces); + StringRef FileName = Replaces.begin()->getFilePath(); + tooling::Replacements FormatReplaces = + reformat(Style, NewCode, ChangedRanges, FileName); + + tooling::Replacements MergedReplacements = + mergeReplacements(Replaces, FormatReplaces); + return MergedReplacements; +} + +std::string applyAllReplacementsAndFormat(StringRef Code, + const tooling::Replacements &Replaces, + const FormatStyle &Style) { + tooling::Replacements NewReplacements = + formatReplacements(Code, Replaces, Style); + if (NewReplacements.empty()) + return Code; // Exit early to avoid overhead in `applyAllReplacements`. + return applyAllReplacements(Code, NewReplacements); +} + tooling::Replacements reformat(const FormatStyle &Style, SourceManager &SourceMgr, FileID ID, ArrayRef Ranges, diff --git a/lib/Tooling/Core/Replacement.cpp b/lib/Tooling/Core/Replacement.cpp index 47bbdeb470..6226cf285b 100644 --- a/lib/Tooling/Core/Replacement.cpp +++ b/lib/Tooling/Core/Replacement.cpp @@ -11,6 +11,8 @@ // //===----------------------------------------------------------------------===// +#include "clang/Tooling/Core/Replacement.h" + #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" @@ -18,7 +20,6 @@ #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" #include "clang/Rewrite/Core/Rewriter.h" -#include "clang/Tooling/Core/Replacement.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/raw_os_ostream.h" @@ -281,6 +282,18 @@ std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) { return Result; } +std::vector calculateChangedRangesInFile(const Replacements &Replaces) { + std::vector ChangedRanges; + int Shift = 0; + for (const Replacement &R : Replaces) { + unsigned Offset = R.getOffset() + Shift; + unsigned Length = R.getReplacementText().size(); + Shift += Length - R.getLength(); + ChangedRanges.push_back(Range(Offset, Length)); + } + return ChangedRanges; +} + namespace { // Represents a merged replacement, i.e. a replacement consisting of multiple // overlapping replacements from 'First' and 'Second' in mergeReplacements. @@ -314,7 +327,7 @@ public: // Merges the next element 'R' into this merged element. As we always merge // from 'First' into 'Second' or vice versa, the MergedReplacement knows what - // set the next element is coming from. + // set the next element is coming from. void merge(const Replacement &R) { if (MergeSecond) { unsigned REnd = R.getOffset() + Delta + R.getLength(); diff --git a/unittests/Format/CMakeLists.txt b/unittests/Format/CMakeLists.txt index 01af435fff..de26a4704a 100644 --- a/unittests/Format/CMakeLists.txt +++ b/unittests/Format/CMakeLists.txt @@ -13,5 +13,6 @@ add_clang_unittest(FormatTests target_link_libraries(FormatTests clangFormat + clangFrontend clangToolingCore ) diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 164909f868..4872565e04 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -7,8 +7,12 @@ // //===----------------------------------------------------------------------===// -#include "FormatTestUtils.h" #include "clang/Format/Format.h" + +#include "../Tooling/RewriterTestContext.h" +#include "FormatTestUtils.h" + +#include "clang/Frontend/TextDiagnosticPrinter.h" #include "llvm/Support/Debug.h" #include "gtest/gtest.h" @@ -11171,6 +11175,46 @@ TEST_F(FormatTest, FormatsTableGenCode) { verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style); } +class ReplacementTest : public ::testing::Test { +protected: + tooling::Replacement createReplacement(SourceLocation Start, unsigned Length, + llvm::StringRef ReplacementText) { + return tooling::Replacement(Context.Sources, Start, Length, + ReplacementText); + } + + RewriterTestContext Context; +}; + +TEST_F(ReplacementTest, FormatCodeAfterReplacements) { + // Column limit is 20. + std::string Code = "Type *a =\n" + " new Type();\n" + "g(iiiii, 0, jjjjj,\n" + " 0, kkkkk, 0, mm);\n" + "int bad = format ;"; + std::string Expected = "auto a = new Type();\n" + "g(iiiii, nullptr,\n" + " jjjjj, nullptr,\n" + " kkkkk, nullptr,\n" + " mm);\n" + "int bad = format ;"; + FileID ID = Context.createInMemoryFile("format.cpp", Code); + tooling::Replacements Replaces; + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID, 1, 1), 6, "auto ")); + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID, 3, 10), 1, "nullptr")); + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID, 4, 3), 1, "nullptr")); + Replaces.insert(tooling::Replacement( + Context.Sources, Context.getLocation(ID, 4, 13), 1, "nullptr")); + + format::FormatStyle Style = format::getLLVMStyle(); + Style.ColumnLimit = 20; // Set column limit to 20 to increase readibility. + EXPECT_EQ(Expected, applyAllReplacementsAndFormat(Code, Replaces, Style)); +} + } // end namespace } // end namespace format } // end namespace clang diff --git a/unittests/Tooling/RefactoringTest.cpp b/unittests/Tooling/RefactoringTest.cpp index ff11aeae11..73da5a82cb 100644 --- a/unittests/Tooling/RefactoringTest.cpp +++ b/unittests/Tooling/RefactoringTest.cpp @@ -418,6 +418,25 @@ TEST(Range, contains) { EXPECT_FALSE(Range(0, 10).contains(Range(0, 11))); } +TEST(Range, CalculateRangesOfReplacements) { + // Before: aaaabbbbbbz + // After : bbbbbbzzzzzzoooooooooooooooo + Replacements Replaces; + Replaces.insert(Replacement("foo", 0, 4, "")); + Replaces.insert(Replacement("foo", 10, 1, "zzzzzz")); + Replaces.insert(Replacement("foo", 11, 0, "oooooooooooooooo")); + + std::vector Ranges = calculateChangedRangesInFile(Replaces); + + EXPECT_EQ(3ul, Ranges.size()); + EXPECT_TRUE(Ranges[0].getOffset() == 0); + EXPECT_TRUE(Ranges[0].getLength() == 0); + EXPECT_TRUE(Ranges[1].getOffset() == 6); + EXPECT_TRUE(Ranges[1].getLength() == 6); + EXPECT_TRUE(Ranges[2].getOffset() == 12); + EXPECT_TRUE(Ranges[2].getLength() == 16); +} + TEST(DeduplicateTest, removesDuplicates) { std::vector Input; Input.push_back(Replacement("fileA", 50, 0, " foo ")); -- 2.40.0