Index: include/clang/Format/Format.h =================================================================== --- include/clang/Format/Format.h +++ include/clang/Format/Format.h @@ -763,22 +763,29 @@ std::string configurationAsText(const FormatStyle &Style); /// \brief Returns the replacements necessary to sort all ``#include`` blocks -/// that are affected by ``Ranges``. +/// that are affected by ``Ranges``. Replacements will be sorted, +/// non-overlapping, and ready to be applied. tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, ArrayRef Ranges, StringRef FileName, unsigned *Cursor = nullptr); /// \brief Returns the replacements corresponding to applying and formatting -/// \p Replaces. +/// \p Replaces. Replacements will be sorted, non-overlapping, and ready to be +/// applied. +/// \pre \p Replaces must be ready to be applied. tooling::Replacements formatReplacements(StringRef Code, const tooling::Replacements &Replaces, const FormatStyle &Style); /// \brief Returns the replacements corresponding to applying \p Replaces and -/// cleaning up the code after that. +/// cleaning up the code after that. Replacements will be sorted, +/// non-overlapping, and ready to be applied. +/// /// This also inserts a C++ #include directive into the correct block if the /// replacement corresponding to the header insertion has offset UINT_MAX. +/// +/// \pre \p Replaces must be ready to be applied. tooling::Replacements cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces, const FormatStyle &Style); @@ -789,8 +796,9 @@ /// everything that might influence its formatting or might be influenced by its /// formatting. /// -/// Returns the ``Replacements`` necessary to make all \p Ranges comply with -/// \p Style. +/// Returns the ``Replacements`` necessary to make all \p Ranges comply with \p +/// Style. ``Replacements`` will be sorted, non-overlapping, and ready to be +/// applied. /// /// If ``IncompleteFormat`` is non-null, its value will be set to true if any /// of the affected ranges were not formatted due to a non-recoverable syntax @@ -811,7 +819,9 @@ /// \brief Clean up any erroneous/redundant code in the given \p Ranges in the /// file \p ID. /// -/// Returns the ``Replacements`` that clean up all \p Ranges in the file \p ID. +/// Returns the ``Replacements`` (ready to be applied) that clean up all \p +/// Ranges in the file \p ID. ``Replacements`` will be sorted, non-overlapping, +/// and ready to be applied. tooling::Replacements cleanup(const FormatStyle &Style, SourceManager &SourceMgr, FileID ID, ArrayRef Ranges); Index: include/clang/Tooling/Core/Replacement.h =================================================================== --- include/clang/Tooling/Core/Replacement.h +++ include/clang/Tooling/Core/Replacement.h @@ -137,11 +137,13 @@ /// \brief Equal-to operator between two Replacements. bool operator==(const Replacement &LHS, const Replacement &RHS); -/// \brief A set of Replacements. -/// FIXME: Change to a vector and deduplicate in the RefactoringTool. -typedef std::set Replacements; +/// \brief A vector of Replacements. +typedef std::vector Replacements; /// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite. +/// It is callers' responsibility to ensure replacements are good to apply (i.e +/// deduplicated or sorted). This function simply applies all replacements +/// without preprocessing them. /// /// Replacement applications happen independently of the success of /// other applications. @@ -149,16 +151,10 @@ /// \returns true if all replacements apply. false otherwise. bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite); -/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite. -/// -/// Replacement applications happen independently of the success of -/// other applications. -/// -/// \returns true if all replacements apply. false otherwise. -bool applyAllReplacements(const std::vector &Replaces, - Rewriter &Rewrite); - /// \brief Applies all replacements in \p Replaces to \p Code. +/// It is callers' responsibility to ensure replacements are good to apply (i.e +/// deduplicated or sorted). This function simply applies all replacements +/// without preprocessing them. /// /// This completely ignores the path stored in each replacement. If one or more /// replacements cannot be applied, this returns an empty \c string. @@ -166,14 +162,9 @@ /// \brief Calculates how a code \p Position is shifted when \p Replaces are /// applied. -unsigned shiftedCodePosition(const Replacements& Replaces, unsigned Position); - -/// \brief Calculates how a code \p Position is shifted when \p Replaces are -/// applied. /// /// \pre Replaces[i].getOffset() <= Replaces[i+1].getOffset(). -unsigned shiftedCodePosition(const std::vector &Replaces, - unsigned Position); +unsigned shiftedCodePosition(const Replacements& Replaces, unsigned Position); /// \brief Removes duplicate Replacements and reports if Replacements conflict /// with one another. All Replacements are assumed to be in the same file. @@ -182,8 +173,7 @@ /// /// This function sorts \p Replaces so that conflicts can be reported simply by /// offset into \p Replaces and number of elements in the conflict. -void deduplicate(std::vector &Replaces, - std::vector &Conflicts); +void deduplicate(Replacements &Replaces, std::vector &Conflicts); /// \brief Collection of Replacements generated from a single translation unit. struct TranslationUnitReplacements { @@ -198,34 +188,11 @@ std::vector Replacements; }; -/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite. -/// -/// Replacement applications happen independently of the success of -/// other applications. -/// -/// \returns true if all replacements apply. false otherwise. -bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite); - -/// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite. -/// -/// Replacement applications happen independently of the success of -/// other applications. -/// -/// \returns true if all replacements apply. false otherwise. -bool applyAllReplacements(const std::vector &Replaces, - Rewriter &Rewrite); - -/// \brief Applies all replacements in \p Replaces to \p Code. -/// -/// This completely ignores the path stored in each replacement. If one or more -/// replacements cannot be applied, this returns an empty \c string. -std::string applyAllReplacements(StringRef Code, const Replacements &Replaces); - /// \brief Calculates the ranges in a single file that are affected by the /// Replacements. /// /// \pre Replacements must be for the same file. -std::vector calculateChangedRanges(const Replacements &Replaces); +std::vector calculateChangedRanges(Replacements Replaces); /// \brief Groups a random set of replacements by file path. Replacements /// related to the same file entry are put into the same vector. @@ -233,10 +200,10 @@ groupReplacementsByFile(const 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. -Replacements mergeReplacements(const Replacements &First, - const Replacements &Second); +/// code after applying the first set. +/// +/// \pre Within both 'First' and 'Second', replacements must not overlap. +Replacements mergeReplacements(Replacements First, Replacements Second); template Replacement::Replacement(const SourceManager &Sources, Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -855,8 +855,8 @@ SourceLocation Start = FormatTok->Tok.getLocation(); auto Replace = [&](SourceLocation Start, unsigned Length, StringRef ReplacementText) { - Result.insert(tooling::Replacement(Env.getSourceManager(), Start, - Length, ReplacementText)); + Result.push_back(tooling::Replacement(Env.getSourceManager(), Start, + Length, ReplacementText)); }; Replace(Start, 1, IsSingle ? "'" : "\""); Replace(FormatTok->Tok.getEndLoc().getLocWithOffset(-1), 1, @@ -1164,10 +1164,14 @@ } auto SR = CharSourceRange::getCharRange(Tokens[St]->Tok.getLocation(), Tokens[End]->Tok.getEndLoc()); - Fixes.insert(tooling::Replacement(Env.getSourceManager(), SR, "")); + Fixes.push_back(tooling::Replacement(Env.getSourceManager(), SR, "")); Idx = End + 1; } + std::vector Conflicts; + deduplicate(Fixes, Conflicts); + assert(Conflicts.empty() && "Fixes must be conflict-free!"); + return Fixes; } @@ -1257,8 +1261,8 @@ Includes.back().Offset + Includes.back().Text.size() - Includes.front().Offset); - Replaces.insert(tooling::Replacement(FileName, Includes.front().Offset, - result.size(), result)); + Replaces.push_back(tooling::Replacement(FileName, Includes.front().Offset, + result.size(), result)); } namespace { @@ -1389,9 +1393,15 @@ if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript) return sortJavaScriptImports(Style, Code, Ranges, FileName); sortCppIncludes(Style, Code, Ranges, FileName, Replaces, Cursor); + + std::vector Conflicts; + deduplicate(Replaces, Conflicts); + assert(Conflicts.empty() && "Fixes must be conflict-free!"); + return Replaces; } +// \p Replaces must be ready to be applied. template static tooling::Replacements processReplacements(T ProcessFunc, StringRef Code, @@ -1496,7 +1506,7 @@ tooling::Replacements HeaderInsertions; for (const auto &R : Replaces) { if (isHeaderInsertion(R)) - HeaderInsertions.insert(R); + HeaderInsertions.push_back(R); else if (R.getOffset() == UINT_MAX) llvm::errs() << "Insertions other than header #include insertion are " "not supported! " @@ -1584,8 +1594,13 @@ std::string NewInclude = !IncludeDirective.endswith("\n") ? (IncludeDirective + "\n").str() : IncludeDirective.str(); - Result.insert(tooling::Replacement(FileName, Offset, 0, NewInclude)); + Result.push_back(tooling::Replacement(FileName, Offset, 0, NewInclude)); } + + std::vector Conflicts; + deduplicate(Result, Conflicts); + assert(Conflicts.empty() && "Result must be conflict-free!"); + return Result; } Index: lib/Format/SortJavaScriptImports.cpp =================================================================== --- lib/Format/SortJavaScriptImports.cpp +++ lib/Format/SortJavaScriptImports.cpp @@ -192,7 +192,7 @@ DEBUG(llvm::dbgs() << "Replacing imports:\n" << getSourceText(InsertionPoint) << "\nwith:\n" << ReferencesText << "\n"); - Result.insert(tooling::Replacement( + Result.push_back(tooling::Replacement( Env.getSourceManager(), CharSourceRange::getCharRange(InsertionPoint), ReferencesText)); Index: lib/Format/TokenAnalyzer.cpp =================================================================== --- lib/Format/TokenAnalyzer.cpp +++ lib/Format/TokenAnalyzer.cpp @@ -120,8 +120,11 @@ for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { delete AnnotatedLines[i]; } - Result.insert(RunResult.begin(), RunResult.end()); + Result.insert(Result.end(), RunResult.begin(), RunResult.end()); } + std::vector Conflicts; + deduplicate(Result, Conflicts); + assert(Conflicts.empty() && "Result must be conflict-free!"); return Result; } Index: lib/Format/WhitespaceManager.h =================================================================== --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -78,7 +78,8 @@ unsigned Newlines, unsigned IndentLevel, int Spaces); - /// \brief Returns all the \c Replacements created during formatting. + /// \brief Returns all the \c Replacements (sorted and non-overlapping) + /// created during formatting. const tooling::Replacements &generateReplacements(); /// \brief Represents a change before a token, a break inside a token, Index: lib/Format/WhitespaceManager.cpp =================================================================== --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -102,6 +102,9 @@ alignEscapedNewlines(); generateChanges(); + std::vector Conflicts; + deduplicate(Replaces, Conflicts); + assert(Conflicts.empty() && "Replcements must be conflict-free!"); return Replaces; } @@ -502,7 +505,7 @@ if (StringRef(SourceMgr.getCharacterData(Range.getBegin()), WhitespaceLength) == Text) return; - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( SourceMgr, CharSourceRange::getCharRange(Range), Text)); } Index: lib/Tooling/Core/Replacement.cpp =================================================================== --- lib/Tooling/Core/Replacement.cpp +++ lib/Tooling/Core/Replacement.cpp @@ -138,10 +138,9 @@ ReplacementText); } -template -unsigned shiftedCodePositionInternal(const T &Replaces, unsigned Position) { +unsigned shiftedCodePosition(const Replacements &Replaces, unsigned Position) { unsigned Offset = 0; - for (const auto& R : Replaces) { + for (const auto &R : Replaces) { if (R.getOffset() + R.getLength() <= Position) { Offset += R.getReplacementText().size() - R.getLength(); continue; @@ -155,27 +154,16 @@ return Position + Offset; } -unsigned shiftedCodePosition(const Replacements &Replaces, unsigned Position) { - return shiftedCodePositionInternal(Replaces, Position); -} - -// FIXME: Remove this function when Replacements is implemented as std::vector -// instead of std::set. -unsigned shiftedCodePosition(const std::vector &Replaces, - unsigned Position) { - return shiftedCodePositionInternal(Replaces, Position); -} - -void deduplicate(std::vector &Replaces, - std::vector &Conflicts) { +void deduplicate(Replacements &Replaces, std::vector &Conflicts) { if (Replaces.empty()) return; + // This does not compare file path, otherwise it is the same as operator<. auto LessNoPath = [](const Replacement &LHS, const Replacement &RHS) { if (LHS.getOffset() != RHS.getOffset()) return LHS.getOffset() < RHS.getOffset(); if (LHS.getLength() != RHS.getLength()) - return LHS.getLength() < RHS.getLength(); + return LHS.getLength() > RHS.getLength(); return LHS.getReplacementText() < RHS.getReplacementText(); }; @@ -187,7 +175,7 @@ // Deduplicate. We don't want to deduplicate based on the path as we assume // that all replacements refer to the same file (or are symlinks). - std::sort(Replaces.begin(), Replaces.end(), LessNoPath); + std::stable_sort(Replaces.begin(), Replaces.end(), LessNoPath); Replaces.erase(std::unique(Replaces.begin(), Replaces.end(), EqualNoPath), Replaces.end()); @@ -220,25 +208,7 @@ bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite) { bool Result = true; - for (Replacements::const_iterator I = Replaces.begin(), - E = Replaces.end(); - I != E; ++I) { - if (I->isApplicable()) { - Result = I->apply(Rewrite) && Result; - } else { - Result = false; - } - } - return Result; -} - -// FIXME: Remove this function when Replacements is implemented as std::vector -// instead of std::set. -bool applyAllReplacements(const std::vector &Replaces, - Rewriter &Rewrite) { - bool Result = true; - for (std::vector::const_iterator I = Replaces.begin(), - E = Replaces.end(); + for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end(); I != E; ++I) { if (I->isApplicable()) { Result = I->apply(Rewrite) && Result; @@ -250,8 +220,8 @@ } std::string applyAllReplacements(StringRef Code, const Replacements &Replaces) { - if (Replaces.empty()) return Code; - + if (Replaces.empty()) + return Code; IntrusiveRefCntPtr InMemoryFileSystem( new vfs::InMemoryFileSystem); FileManager Files(FileSystemOptions(), InMemoryFileSystem); @@ -278,8 +248,9 @@ return Result; } -std::vector calculateChangedRanges(const Replacements &Replaces) { +std::vector calculateChangedRanges(Replacements Replaces) { std::vector ChangedRanges; + std::stable_sort(Replaces.begin(), Replaces.end()); int Shift = 0; for (const Replacement &R : Replaces) { unsigned Offset = R.getOffset() + Shift; @@ -390,16 +361,21 @@ groupReplacementsByFile(const Replacements &Replaces) { std::map FileToReplaces; for (const auto &Replace : Replaces) { - FileToReplaces[Replace.getFilePath()].insert(Replace); + FileToReplaces[Replace.getFilePath()].push_back(Replace); } return FileToReplaces; } -Replacements mergeReplacements(const Replacements &First, - const Replacements &Second) { +Replacements mergeReplacements(Replacements First, Replacements Second) { if (First.empty() || Second.empty()) return First.empty() ? Second : First; + std::vector Conflicts; + deduplicate(First, Conflicts); + assert(Conflicts.empty() && "First must be conflict-free!"); + deduplicate(Second, Conflicts); + assert(Conflicts.empty() && "Second must be conflict-free!"); + // Delta is the amount of characters that replacements from 'Second' need to // be shifted so that their offsets refer to the original text. int Delta = 0; @@ -427,7 +403,7 @@ ++I; } Delta -= Merged.deltaFirst(); - Result.insert(Merged.asReplacement()); + Result.push_back(Merged.asReplacement()); } return Result; } Index: lib/Tooling/RefactoringCallbacks.cpp =================================================================== --- lib/Tooling/RefactoringCallbacks.cpp +++ lib/Tooling/RefactoringCallbacks.cpp @@ -40,10 +40,9 @@ void ReplaceStmtWithText::run( const ast_matchers::MatchFinder::MatchResult &Result) { if (const Stmt *FromMatch = Result.Nodes.getStmtAs(FromId)) { - Replace.insert(tooling::Replacement( + Replace.push_back(tooling::Replacement( *Result.SourceManager, - CharSourceRange::getTokenRange(FromMatch->getSourceRange()), - ToText)); + CharSourceRange::getTokenRange(FromMatch->getSourceRange()), ToText)); } } @@ -55,8 +54,8 @@ const Stmt *FromMatch = Result.Nodes.getStmtAs(FromId); const Stmt *ToMatch = Result.Nodes.getStmtAs(ToId); if (FromMatch && ToMatch) - Replace.insert(replaceStmtWithStmt( - *Result.SourceManager, *FromMatch, *ToMatch)); + Replace.push_back( + replaceStmtWithStmt(*Result.SourceManager, *FromMatch, *ToMatch)); } ReplaceIfStmtWithItsBody::ReplaceIfStmtWithItsBody(StringRef Id, @@ -68,11 +67,12 @@ if (const IfStmt *Node = Result.Nodes.getStmtAs(Id)) { const Stmt *Body = PickTrueBranch ? Node->getThen() : Node->getElse(); if (Body) { - Replace.insert(replaceStmtWithStmt(*Result.SourceManager, *Node, *Body)); + Replace.push_back( + replaceStmtWithStmt(*Result.SourceManager, *Node, *Body)); } else if (!PickTrueBranch) { // If we want to use the 'else'-branch, but it doesn't exist, delete // the whole 'if'. - Replace.insert(replaceStmtWithText(*Result.SourceManager, *Node, "")); + Replace.push_back(replaceStmtWithText(*Result.SourceManager, *Node, "")); } } } Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -11542,13 +11542,13 @@ "int bad = format ;"; FileID ID = Context.createInMemoryFile("format.cpp", Code); tooling::Replacements Replaces; - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID, 1, 1), 6, "auto ")); - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID, 3, 10), 1, "nullptr")); - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID, 4, 3), 1, "nullptr")); - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID, 4, 13), 1, "nullptr")); format::FormatStyle Style = format::getLLVMStyle(); @@ -11573,7 +11573,7 @@ "}"; FileID ID = Context.createInMemoryFile("fix.cpp", Code); tooling::Replacements Replaces; - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID, 1, 1), 0, "#include \"b.h\"\n")); format::FormatStyle Style = format::getLLVMStyle(); Index: unittests/Tooling/RefactoringTest.cpp =================================================================== --- unittests/Tooling/RefactoringTest.cpp +++ unittests/Tooling/RefactoringTest.cpp @@ -33,7 +33,7 @@ namespace tooling { class ReplacementTest : public ::testing::Test { - protected: +protected: Replacement createReplacement(SourceLocation Start, unsigned Length, llvm::StringRef ReplacementText) { return Replacement(Context.Sources, Start, Length, ReplacementText); @@ -67,8 +67,8 @@ } TEST_F(ReplacementTest, CanReplaceTextAtPosition) { - FileID ID = Context.createInMemoryFile("input.cpp", - "line1\nline2\nline3\nline4"); + FileID ID = + Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4"); SourceLocation Location = Context.getLocation(ID, 2, 3); Replacement Replace(createReplacement(Location, 12, "x")); EXPECT_TRUE(Replace.apply(Context.Rewrite)); @@ -76,8 +76,8 @@ } TEST_F(ReplacementTest, CanReplaceTextAtPositionMultipleTimes) { - FileID ID = Context.createInMemoryFile("input.cpp", - "line1\nline2\nline3\nline4"); + FileID ID = + Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4"); SourceLocation Location1 = Context.getLocation(ID, 2, 3); Replacement Replace1(createReplacement(Location1, 12, "x\ny\n")); EXPECT_TRUE(Replace1.apply(Context.Rewrite)); @@ -110,24 +110,9 @@ } TEST_F(ReplacementTest, CanApplyReplacements) { - FileID ID = Context.createInMemoryFile("input.cpp", - "line1\nline2\nline3\nline4"); + FileID ID = + Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4"); Replacements Replaces; - Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), - 5, "replaced")); - Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 3, 1), - 5, "other")); - EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); - EXPECT_EQ("line1\nreplaced\nother\nline4", Context.getRewrittenText(ID)); -} - -// FIXME: Remove this test case when Replacements is implemented as std::vector -// instead of std::set. The other ReplacementTest tests will need to be updated -// at that point as well. -TEST_F(ReplacementTest, VectorCanApplyReplacements) { - FileID ID = Context.createInMemoryFile("input.cpp", - "line1\nline2\nline3\nline4"); - std::vector Replaces; Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 5, "replaced")); Replaces.push_back( @@ -137,31 +122,48 @@ } TEST_F(ReplacementTest, SkipsDuplicateReplacements) { - FileID ID = Context.createInMemoryFile("input.cpp", - "line1\nline2\nline3\nline4"); + FileID ID = + Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4"); Replacements Replaces; - Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), - 5, "replaced")); - Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), - 5, "replaced")); - Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), - 5, "replaced")); + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 5, "replaced")); + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 5, "replaced")); + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 5, "replaced")); + std::vector Conflicts; + deduplicate(Replaces, Conflicts); + EXPECT_TRUE(Conflicts.empty()); EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); EXPECT_EQ("line1\nreplaced\nline3\nline4", Context.getRewrittenText(ID)); } +TEST_F(ReplacementTest, CanApplyDuplicateReplacements) { + FileID ID = + Context.createInMemoryFile("input.cpp", "line1\nline2\nline3\nline4"); + Replacements Replaces; + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 0, "insert\n")); + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 0, "insert\n")); + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 0, "insert\n")); + EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); + EXPECT_EQ("line1\ninsert\ninsert\ninsert\nline2\nline3\nline4", + Context.getRewrittenText(ID)); +} + TEST_F(ReplacementTest, ApplyAllFailsIfOneApplyFails) { // This test depends on the value of the file name of an invalid source // location being in the range ]a, z[. FileID IDa = Context.createInMemoryFile("a.cpp", "text"); FileID IDz = Context.createInMemoryFile("z.cpp", "text"); Replacements Replaces; - Replaces.insert(Replacement(Context.Sources, Context.getLocation(IDa, 1, 1), - 4, "a")); - Replaces.insert(Replacement(Context.Sources, SourceLocation(), - 5, "2")); - Replaces.insert(Replacement(Context.Sources, Context.getLocation(IDz, 1, 1), - 4, "z")); + Replaces.push_back( + Replacement(Context.Sources, Context.getLocation(IDa, 1, 1), 4, "a")); + Replaces.push_back(Replacement(Context.Sources, SourceLocation(), 5, "2")); + Replaces.push_back( + Replacement(Context.Sources, Context.getLocation(IDz, 1, 1), 4, "z")); EXPECT_FALSE(applyAllReplacements(Replaces, Context.Rewrite)); EXPECT_EQ("a", Context.getRewrittenText(IDa)); EXPECT_EQ("z", Context.getRewrittenText(IDz)); @@ -185,13 +187,13 @@ tooling::Replacements Replaces; // Scrambled the order of replacements. - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID2, 1, 12), 0, "4567890123")); - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID1, 1, 1), 6, "auto ")); - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10")); - Replaces.insert(tooling::Replacement( + Replaces.push_back(tooling::Replacement( Context.Sources, Context.getLocation(ID1, 3, 10), 1, "12345678901")); EXPECT_TRUE(formatAndApplyAllReplacements( @@ -202,25 +204,6 @@ TEST(ShiftedCodePositionTest, FindsNewCodePosition) { Replacements Replaces; - Replaces.insert(Replacement("", 0, 1, "")); - Replaces.insert(Replacement("", 4, 3, " ")); - // Assume ' int i;' is turned into 'int i;' and cursor is located at '|'. - EXPECT_EQ(0u, shiftedCodePosition(Replaces, 0)); // |int i; - EXPECT_EQ(0u, shiftedCodePosition(Replaces, 1)); // |nt i; - EXPECT_EQ(1u, shiftedCodePosition(Replaces, 2)); // i|t i; - EXPECT_EQ(2u, shiftedCodePosition(Replaces, 3)); // in| i; - EXPECT_EQ(3u, shiftedCodePosition(Replaces, 4)); // int| i; - EXPECT_EQ(3u, shiftedCodePosition(Replaces, 5)); // int | i; - EXPECT_EQ(3u, shiftedCodePosition(Replaces, 6)); // int |i; - EXPECT_EQ(4u, shiftedCodePosition(Replaces, 7)); // int |; - EXPECT_EQ(5u, shiftedCodePosition(Replaces, 8)); // int i| -} - -// FIXME: Remove this test case when Replacements is implemented as std::vector -// instead of std::set. The other ReplacementTest tests will need to be updated -// at that point as well. -TEST(ShiftedCodePositionTest, VectorFindsNewCodePositionWithInserts) { - std::vector Replaces; Replaces.push_back(Replacement("", 0, 1, "")); Replaces.push_back(Replacement("", 4, 3, " ")); // Assume ' int i;' is turned into 'int i;' and cursor is located at '|'. @@ -237,7 +220,7 @@ TEST(ShiftedCodePositionTest, FindsNewCodePositionWithInserts) { Replacements Replaces; - Replaces.insert(Replacement("", 4, 0, "\"\n\"")); + Replaces.push_back(Replacement("", 4, 0, "\"\n\"")); // Assume '"12345678"' is turned into '"1234"\n"5678"'. EXPECT_EQ(3u, shiftedCodePosition(Replaces, 3)); // "123|5678" EXPECT_EQ(7u, shiftedCodePosition(Replaces, 4)); // "1234|678" @@ -247,16 +230,16 @@ TEST(ShiftedCodePositionTest, FindsNewCodePositionInReplacedText) { Replacements Replaces; // Replace the first four characters with "abcd". - Replaces.insert(Replacement("", 0, 4, "abcd")); + Replaces.push_back(Replacement("", 0, 4, "abcd")); for (unsigned i = 0; i < 3; ++i) EXPECT_EQ(i, shiftedCodePosition(Replaces, i)); } class FlushRewrittenFilesTest : public ::testing::Test { public: - FlushRewrittenFilesTest() {} + FlushRewrittenFilesTest() {} - ~FlushRewrittenFilesTest() override { + ~FlushRewrittenFilesTest() override { for (llvm::StringMap::iterator I = TemporaryFiles.begin(), E = TemporaryFiles.end(); I != E; ++I) { @@ -306,8 +289,8 @@ TEST_F(FlushRewrittenFilesTest, StoresChangesOnDisk) { FileID ID = createFile("input.cpp", "line1\nline2\nline3\nline4"); Replacements Replaces; - Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), - 5, "replaced")); + Replaces.push_back(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 5, "replaced")); EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); EXPECT_FALSE(Context.Rewrite.overwriteChangedFiles()); EXPECT_EQ("line1\nreplaced\nline3\nline4", @@ -315,8 +298,7 @@ } namespace { -template -class TestVisitor : public clang::RecursiveASTVisitor { +template class TestVisitor : public clang::RecursiveASTVisitor { public: bool runOver(StringRef Code) { return runToolOnCode(new TestAction(this), Code); @@ -358,8 +340,8 @@ }; } // end namespace -void expectReplacementAt(const Replacement &Replace, - StringRef File, unsigned Offset, unsigned Length) { +void expectReplacementAt(const Replacement &Replace, StringRef File, + unsigned Offset, unsigned Length) { ASSERT_TRUE(Replace.isApplicable()); EXPECT_EQ(File, Replace.getFilePath()); EXPECT_EQ(Offset, Replace.getOffset()); @@ -409,7 +391,7 @@ TEST(Replacement, TemplatedFunctionCall) { CallToFVisitor CallToF; EXPECT_TRUE(CallToF.runOver( - "template void F(); void G() { F(); }")); + "template void F(); void G() { F(); }")); expectReplacementAt(CallToF.Replace, "input.cc", 43, 8); } @@ -418,14 +400,15 @@ public: bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) { if (NNSLoc.getNestedNameSpecifier()) { - if (const NamespaceDecl* NS = NNSLoc.getNestedNameSpecifier()->getAsNamespace()) { + if (const NamespaceDecl *NS = + NNSLoc.getNestedNameSpecifier()->getAsNamespace()) { if (NS->getName() == "a") { Replace = Replacement(*SM, &NNSLoc, "", Context->getLangOpts()); } } } - return TestVisitor::TraverseNestedNameSpecifierLoc( - NNSLoc); + return TestVisitor< + NestedNameSpecifierAVisitor>::TraverseNestedNameSpecifierLoc(NNSLoc); } Replacement Replace; }; @@ -456,9 +439,9 @@ // 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")); + Replaces.push_back(Replacement("foo", 0, 4, "")); + Replaces.push_back(Replacement("foo", 10, 1, "zzzzzz")); + Replaces.push_back(Replacement("foo", 11, 0, "oooooooooooooooo")); std::vector Ranges = calculateChangedRanges(Replaces); @@ -472,11 +455,11 @@ } TEST(DeduplicateTest, removesDuplicates) { - std::vector Input; + Replacements Input; Input.push_back(Replacement("fileA", 50, 0, " foo ")); Input.push_back(Replacement("fileA", 10, 3, " bar ")); Input.push_back(Replacement("fileA", 10, 2, " bar ")); // Length differs - Input.push_back(Replacement("fileA", 9, 3, " bar ")); // Offset differs + Input.push_back(Replacement("fileA", 9, 3, " bar ")); // Offset differs Input.push_back(Replacement("fileA", 50, 0, " foo ")); // Duplicate Input.push_back(Replacement("fileA", 51, 3, " bar ")); Input.push_back(Replacement("fileB", 51, 3, " bar ")); // Filename differs! @@ -485,15 +468,15 @@ Input.push_back(Replacement("fileA", 51, 3, " moo ")); // Replacement text // differs! - std::vector Expected; - Expected.push_back(Replacement("fileA", 9, 3, " bar ")); - Expected.push_back(Replacement("fileA", 10, 2, " bar ")); + Replacements Expected; + Expected.push_back(Replacement("fileA", 9, 3, " bar ")); Expected.push_back(Replacement("fileA", 10, 3, " bar ")); + Expected.push_back(Replacement("fileA", 10, 2, " bar ")); Expected.push_back(Replacement("fileA", 50, 0, " foo ")); Expected.push_back(Replacement("fileA", 51, 3, " bar ")); Expected.push_back(Replacement("fileA", 51, 3, " moo ")); - Expected.push_back(Replacement("fileB", 60, 1, " bar ")); Expected.push_back(Replacement("fileA", 60, 2, " bar ")); + Expected.push_back(Replacement("fileB", 60, 1, " bar ")); std::vector Conflicts; // Ignored for this test deduplicate(Input, Conflicts); @@ -504,7 +487,7 @@ TEST(DeduplicateTest, detectsConflicts) { { - std::vector Input; + Replacements Input; Input.push_back(Replacement("fileA", 0, 5, " foo ")); Input.push_back(Replacement("fileA", 0, 5, " foo ")); // Duplicate not a // conflict. @@ -522,15 +505,15 @@ ASSERT_EQ(3u, Conflicts.front().getLength()); } { - std::vector Input; + Replacements Input; // Expected sorted order is shown. It is the sorted order to which the // returned conflict info refers to. - Input.push_back(Replacement("fileA", 0, 5, " foo ")); // 0 - Input.push_back(Replacement("fileA", 5, 5, " bar ")); // 1 - Input.push_back(Replacement("fileA", 6, 0, " bar ")); // 3 - Input.push_back(Replacement("fileA", 5, 5, " moo ")); // 2 - Input.push_back(Replacement("fileA", 7, 2, " bar ")); // 4 + Input.push_back(Replacement("fileA", 0, 5, " foo ")); // 0 + Input.push_back(Replacement("fileA", 5, 5, " bar ")); // 1 + Input.push_back(Replacement("fileA", 6, 0, " bar ")); // 3 + Input.push_back(Replacement("fileA", 5, 5, " moo ")); // 2 + Input.push_back(Replacement("fileA", 7, 2, " bar ")); // 4 Input.push_back(Replacement("fileA", 15, 5, " golf ")); // 5 Input.push_back(Replacement("fileA", 16, 5, " bag ")); // 6 Input.push_back(Replacement("fileA", 10, 3, " club ")); // 7 Index: unittests/Tooling/RewriterTest.cpp =================================================================== --- unittests/Tooling/RewriterTest.cpp +++ unittests/Tooling/RewriterTest.cpp @@ -39,8 +39,8 @@ TEST(Rewriter, AdjacentInsertAndDelete) { Replacements Replaces; - Replaces.insert(Replacement("", 6, 6, "")); - Replaces.insert(Replacement("", 6, 0, "replaced\n")); + Replaces.push_back(Replacement("", 6, 6, "")); + Replaces.push_back(Replacement("", 6, 0, "replaced\n")); EXPECT_EQ("line1\nreplaced\nline3\nline4", applyAllReplacements("line1\nline2\nline3\nline4", Replaces)); }