Index: include/clang/Tooling/Core/Replacement.h =================================================================== --- include/clang/Tooling/Core/Replacement.h +++ include/clang/Tooling/Core/Replacement.h @@ -123,14 +123,13 @@ /// \brief Returns a human readable string representation. std::string toString() const; - private: - void setFromSourceLocation(const SourceManager &Sources, - SourceLocation Start, unsigned Length, - StringRef ReplacementText); - void setFromSourceRange(const SourceManager &Sources, - const CharSourceRange &Range, - StringRef ReplacementText, - const LangOptions &LangOpts); +private: + void setFromSourceLocation(const SourceManager &Sources, SourceLocation Start, + unsigned Length, StringRef ReplacementText); + void setFromSourceRange(const SourceManager &Sources, + const CharSourceRange &Range, + StringRef ReplacementText, + const LangOptions &LangOpts); std::string FilePath; Range ReplacementRange; @@ -143,17 +142,70 @@ /// \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 Maintains a set of replacements that are conflict-free. +/// Two replacements are considered conflicts if they overlap or have the same +/// offset (i.e. order-dependent). +class Replacements { + private: + typedef std::set ReplacementsImpl; -/// \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); + public: + typedef ReplacementsImpl::const_iterator const_iterator; + + Replacements() = default; + + explicit Replacements(const Replacement &R) { Replaces.insert(R); } + + /// \brief Adds a new replacement \p R to the current set of replacements. + /// \p R must have the same file path as all existing replacements. + /// Returns true if the replacement is successfully inserted; otherwise, + /// it returns an llvm::Error, i.e. there is a conflict between R and the + /// existing replacements or R's file path is different from the filepath of + /// existing replacements. Callers must explicitly check the Error returned. + /// This prevents users from adding order-dependent replacements. To control + /// the order in which order-dependent replacements are applied, use + /// merge({R}) with R referring to the changed code after applying all + /// existing replacements. + /// Replacements with offset UINT_MAX are special - we do not detect conflicts + /// for such replacements since users may add them intentionally as a special + /// category of replacements. + llvm::Error add(const Replacement &R); + + /// \brief Merges \p Replaces into the current replacements. \p Replaces + /// refers to code after applying the current replacements. + Replacements merge(const Replacements &Replaces) const; + + // Returns the affected ranges in the changed code. + std::vector getAffectedRanges() const; + + // Returns the new offset in the code after replacements being applied. + // Note that if there is an insertion at Offset in the current replacements, + // \p Offset will be shifted to Offset + Length in inserted text. + unsigned getShiftedCodePosition(unsigned Position) const; + + unsigned size() const { return Replaces.size(); } + + void clear() { Replaces.clear(); } + + bool empty() const { return Replaces.empty(); } + + const_iterator begin() const { return Replaces.begin(); } + + const_iterator end() const { return Replaces.end(); } + + bool operator==(const Replacements &RHS) const { + return Replaces == RHS.Replaces; + } + + +private: + Replacements(const_iterator Begin, const_iterator End) + : Replaces(Begin, End) {} + + Replacements mergeReplacements(const ReplacementsImpl &Second) const; + + ReplacementsImpl Replaces; +}; /// \brief Apply all replacements in \p Replaces to the Rewriter \p Rewrite. /// @@ -161,8 +213,7 @@ /// other applications. /// /// \returns true if all replacements apply. false otherwise. -bool applyAllReplacements(const std::vector &Replaces, - Rewriter &Rewrite); +bool applyAllReplacements(const Replacements &Replaces, Rewriter &Rewrite); /// \brief Applies all replacements in \p Replaces to \p Code. /// @@ -174,27 +225,6 @@ llvm::Expected applyAllReplacements(StringRef Code, const Replacements &Replaces); -/// \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); - -/// \brief Removes duplicate Replacements and reports if Replacements conflict -/// with one another. All Replacements are assumed to be in the same file. -/// -/// \post Replaces[i].getOffset() <= Replaces[i+1].getOffset(). -/// -/// 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); - /// \brief Collection of Replacements generated from a single translation unit. struct TranslationUnitReplacements { /// Name of the main source for the translation unit. @@ -208,14 +238,6 @@ std::vector Replacements; }; -/// \brief Calculates the ranges in a single file that are affected by the -/// Replacements. Overlapping ranges will be merged. -/// -/// \pre Replacements must be for the same file. -/// -/// \returns a non-overlapping and sorted ranges. -std::vector calculateChangedRanges(const Replacements &Replaces); - /// \brief Calculates the new ranges after \p Replaces are applied. These /// include both the original \p Ranges and the affected ranges of \p Replaces /// in the new code. @@ -233,12 +255,6 @@ std::map 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); - template Replacement::Replacement(const SourceManager &Sources, const Node &NodeToReplace, StringRef ReplacementText, Index: include/clang/Tooling/Refactoring.h =================================================================== --- include/clang/Tooling/Refactoring.h +++ include/clang/Tooling/Refactoring.h @@ -21,6 +21,7 @@ #include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Tooling.h" +#include #include namespace clang { @@ -42,9 +43,9 @@ std::shared_ptr PCHContainerOps = std::make_shared()); - /// \brief Returns the set of replacements to which replacements should - /// be added during the run of the tool. - Replacements &getReplacements(); + /// \brief Returns the file path to replacements map to which replacements + /// should be added during the run of the tool. + std::map &getReplacements(); /// \brief Call run(), apply all generated replacements, and immediately save /// the results to disk. @@ -65,7 +66,7 @@ int saveRewrittenFiles(Rewriter &Rewrite); private: - Replacements Replace; + std::map FileToReplaces; }; /// \brief Groups \p Replaces by the file path and applies each group of @@ -77,14 +78,15 @@ /// Replacement applications happen independently of the success of other /// applications. /// -/// \param[in] Replaces Replacements to apply. +/// \param[in] FileToReplaces Replacements (grouped by files) to apply. /// \param[in] Rewrite The `Rewritter` to apply replacements on. /// \param[in] Style The style name used for reformatting. See ```getStyle``` in /// "include/clang/Format/Format.h" for all possible style forms. /// /// \returns true if all replacements applied and formatted. false otherwise. -bool formatAndApplyAllReplacements(const Replacements &Replaces, - Rewriter &Rewrite, StringRef Style = "file"); +bool formatAndApplyAllReplacements( + const std::map &FileToReplaces, + Rewriter &Rewrite, StringRef Style = "file"); } // end namespace tooling } // end namespace clang Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -854,8 +854,13 @@ SourceLocation Start = FormatTok->Tok.getLocation(); auto Replace = [&](SourceLocation Start, unsigned Length, StringRef ReplacementText) { - Result.insert(tooling::Replacement(Env.getSourceManager(), Start, - Length, ReplacementText)); + auto Err = Result.add(tooling::Replacement( + Env.getSourceManager(), Start, Length, ReplacementText)); + // FIXME: handle error. For now, print error message and skip the + // replacement for release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); }; Replace(Start, 1, IsSingle ? "'" : "\""); Replace(FormatTok->Tok.getEndLoc().getLocWithOffset(-1), 1, @@ -1163,7 +1168,13 @@ } auto SR = CharSourceRange::getCharRange(Tokens[St]->Tok.getLocation(), Tokens[End]->Tok.getEndLoc()); - Fixes.insert(tooling::Replacement(Env.getSourceManager(), SR, "")); + auto Err = + Fixes.add(tooling::Replacement(Env.getSourceManager(), SR, "")); + // FIXME: better error handling. for now just print error message and skip + // for the release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err && "Fixes must not conflict!"); Idx = End + 1; } @@ -1256,8 +1267,13 @@ Includes.back().Offset + Includes.back().Text.size() - Includes.front().Offset); - Replaces.insert(tooling::Replacement(FileName, Includes.front().Offset, - result.size(), result)); + auto Err = Replaces.add(tooling::Replacement( + FileName, Includes.front().Offset, result.size(), result)); + // FIXME: better error handling. For now, just skip the replacement for the + // release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); } namespace { @@ -1402,14 +1418,13 @@ auto NewCode = applyAllReplacements(Code, Replaces); if (!NewCode) return NewCode.takeError(); - std::vector ChangedRanges = - tooling::calculateChangedRanges(Replaces); + std::vector ChangedRanges = Replaces.getAffectedRanges(); StringRef FileName = Replaces.begin()->getFilePath(); tooling::Replacements FormatReplaces = ProcessFunc(Style, *NewCode, ChangedRanges, FileName); - return mergeReplacements(Replaces, FormatReplaces); + return Replaces.merge(FormatReplaces); } llvm::Expected @@ -1497,20 +1512,22 @@ return Replaces; tooling::Replacements HeaderInsertions; + tooling::Replacements Result; for (const auto &R : Replaces) { - if (isHeaderInsertion(R)) - HeaderInsertions.insert(R); - else if (R.getOffset() == UINT_MAX) + if (isHeaderInsertion(R)) { + // Replacements from \p Replaces must be conflict-free already, so we can + // simply consume the error. + llvm::consumeError(HeaderInsertions.add(R)); + } else if (R.getOffset() == UINT_MAX) { llvm::errs() << "Insertions other than header #include insertion are " "not supported! " << R.getReplacementText() << "\n"; + } else { + llvm::consumeError(Result.add(R)); + } } if (HeaderInsertions.empty()) return Replaces; - tooling::Replacements Result; - std::set_difference(Replaces.begin(), Replaces.end(), - HeaderInsertions.begin(), HeaderInsertions.end(), - std::inserter(Result, Result.begin())); llvm::Regex IncludeRegex(IncludeRegexPattern); llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*[^\\]*$)"); @@ -1587,7 +1604,12 @@ std::string NewInclude = !IncludeDirective.endswith("\n") ? (IncludeDirective + "\n").str() : IncludeDirective.str(); - Result.insert(tooling::Replacement(FileName, Offset, 0, NewInclude)); + auto NewReplace = tooling::Replacement(FileName, Offset, 0, NewInclude); + auto Err = Result.add(NewReplace); + if (Err) { + llvm::consumeError(std::move(Err)); + Result = Result.merge(tooling::Replacements(NewReplace)); + } } return Result; } Index: lib/Format/SortJavaScriptImports.cpp =================================================================== --- lib/Format/SortJavaScriptImports.cpp +++ lib/Format/SortJavaScriptImports.cpp @@ -127,7 +127,8 @@ tooling::Replacements analyze(TokenAnnotator &Annotator, SmallVectorImpl &AnnotatedLines, - FormatTokenLexer &Tokens, tooling::Replacements &Result) override { + FormatTokenLexer &Tokens, tooling::Replacements &) override { + tooling::Replacements Result; AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(), AnnotatedLines.end()); @@ -192,9 +193,14 @@ DEBUG(llvm::dbgs() << "Replacing imports:\n" << getSourceText(InsertionPoint) << "\nwith:\n" << ReferencesText << "\n"); - Result.insert(tooling::Replacement( + auto Err = Result.add(tooling::Replacement( Env.getSourceManager(), CharSourceRange::getCharRange(InsertionPoint), ReferencesText)); + // FIXME: better error handling. For now, just print error message and skip + // the replacement for the release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); return Result; } Index: lib/Format/TokenAnalyzer.cpp =================================================================== --- lib/Format/TokenAnalyzer.cpp +++ lib/Format/TokenAnalyzer.cpp @@ -111,8 +111,8 @@ DEBUG({ llvm::dbgs() << "Replacements for run " << Run << ":\n"; - for (tooling::Replacements::iterator I = RunResult.begin(), - E = RunResult.end(); + for (tooling::Replacements::const_iterator I = RunResult.begin(), + E = RunResult.end(); I != E; ++I) { llvm::dbgs() << I->toString() << "\n"; } @@ -120,7 +120,15 @@ for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) { delete AnnotatedLines[i]; } - Result.insert(RunResult.begin(), RunResult.end()); + for (auto R : RunResult) { + auto Err = Result.add(R); + // FIXME: better error handling here. For now, simply return an empty + // Replacements to indicate failure. + if (Err) { + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + return tooling::Replacements(); + } + } } return Result; } Index: lib/Format/WhitespaceManager.cpp =================================================================== --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -502,8 +502,13 @@ if (StringRef(SourceMgr.getCharacterData(Range.getBegin()), WhitespaceLength) == Text) return; - Replaces.insert(tooling::Replacement( + auto Err = Replaces.add(tooling::Replacement( SourceMgr, CharSourceRange::getCharRange(Range), Text)); + // FIXME: better error handling. For now, just print an error message in the + // release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); } void WhitespaceManager::appendNewlineText(std::string &Text, Index: lib/Tooling/Core/Replacement.cpp =================================================================== --- lib/Tooling/Core/Replacement.cpp +++ lib/Tooling/Core/Replacement.cpp @@ -137,200 +137,30 @@ ReplacementText); } -template -unsigned shiftedCodePositionInternal(const T &Replaces, unsigned Position) { - unsigned Offset = 0; - for (const auto& R : Replaces) { - if (R.getOffset() + R.getLength() <= Position) { - Offset += R.getReplacementText().size() - R.getLength(); - continue; - } - if (R.getOffset() < Position && - R.getOffset() + R.getReplacementText().size() <= Position) { - Position = R.getOffset() + R.getReplacementText().size() - 1; - } - break; - } - 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) { - if (Replaces.empty()) - return; - - 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.getReplacementText() < RHS.getReplacementText(); - }; - - auto EqualNoPath = [](const Replacement &LHS, const Replacement &RHS) { - return LHS.getOffset() == RHS.getOffset() && - LHS.getLength() == RHS.getLength() && - LHS.getReplacementText() == RHS.getReplacementText(); - }; - - // 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); - Replaces.erase(std::unique(Replaces.begin(), Replaces.end(), EqualNoPath), - Replaces.end()); - - // Detect conflicts - Range ConflictRange(Replaces.front().getOffset(), - Replaces.front().getLength()); - unsigned ConflictStart = 0; - unsigned ConflictLength = 1; - for (unsigned i = 1; i < Replaces.size(); ++i) { - Range Current(Replaces[i].getOffset(), Replaces[i].getLength()); - if (ConflictRange.overlapsWith(Current)) { - // Extend conflicted range - ConflictRange = Range(ConflictRange.getOffset(), - std::max(ConflictRange.getLength(), - Current.getOffset() + Current.getLength() - - ConflictRange.getOffset())); - ++ConflictLength; - } else { - if (ConflictLength > 1) - Conflicts.push_back(Range(ConflictStart, ConflictLength)); - ConflictRange = Current; - ConflictStart = i; - ConflictLength = 1; - } - } - - if (ConflictLength > 1) - Conflicts.push_back(Range(ConflictStart, ConflictLength)); -} - -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(); - I != E; ++I) { - if (I->isApplicable()) { - Result = I->apply(Rewrite) && Result; - } else { - Result = false; - } - } - return Result; -} - -llvm::Expected applyAllReplacements(StringRef Code, - const Replacements &Replaces) { - if (Replaces.empty()) - return Code.str(); - - IntrusiveRefCntPtr InMemoryFileSystem( - new vfs::InMemoryFileSystem); - FileManager Files(FileSystemOptions(), InMemoryFileSystem); - DiagnosticsEngine Diagnostics( - IntrusiveRefCntPtr(new DiagnosticIDs), - new DiagnosticOptions); - SourceManager SourceMgr(Diagnostics, Files); - Rewriter Rewrite(SourceMgr, LangOptions()); - InMemoryFileSystem->addFile( - "", 0, llvm::MemoryBuffer::getMemBuffer(Code, "")); - FileID ID = SourceMgr.createFileID(Files.getFile(""), SourceLocation(), - clang::SrcMgr::C_User); - for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end(); - I != E; ++I) { - Replacement Replace("", I->getOffset(), I->getLength(), - I->getReplacementText()); - if (!Replace.apply(Rewrite)) - return llvm::make_error( - "Failed to apply replacement: " + Replace.toString(), - llvm::inconvertibleErrorCode()); - } - std::string Result; - llvm::raw_string_ostream OS(Result); - Rewrite.getEditBuffer(ID).write(OS); - OS.flush(); - return Result; -} - -// Merge and sort overlapping ranges in \p Ranges. -static std::vector mergeAndSortRanges(std::vector Ranges) { - std::sort(Ranges.begin(), Ranges.end(), - [](const Range &LHS, const Range &RHS) { - if (LHS.getOffset() != RHS.getOffset()) - return LHS.getOffset() < RHS.getOffset(); - return LHS.getLength() < RHS.getLength(); - }); - std::vector Result; - for (const auto &R : Ranges) { - if (Result.empty() || - Result.back().getOffset() + Result.back().getLength() < R.getOffset()) { - Result.push_back(R); - } else { - unsigned NewEnd = - std::max(Result.back().getOffset() + Result.back().getLength(), - R.getOffset() + R.getLength()); - Result[Result.size() - 1] = - Range(Result.back().getOffset(), NewEnd - Result.back().getOffset()); +llvm::Error Replacements::add(const Replacement &R) { + if (R.getOffset() != UINT_MAX) + for (auto Replace : Replaces) { + if (R.getFilePath() != Replace.getFilePath()) + return llvm::make_error( + "All replacements must have the same file path. New replacement: " + + R.getFilePath() + ", existing replacements: " + + Replace.getFilePath() + "\n", + llvm::inconvertibleErrorCode()); + if (R.getOffset() == Replace.getOffset() || + Range(R.getOffset(), R.getLength()) + .overlapsWith(Range(Replace.getOffset(), Replace.getLength()))) + return llvm::make_error( + "New replacement:\n" + R.toString() + + "\nconflicts with existing replacement:\n" + Replace.toString(), + llvm::inconvertibleErrorCode()); } - } - return Result; -} -std::vector calculateChangedRanges(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 mergeAndSortRanges(ChangedRanges); -} - -std::vector -calculateRangesAfterReplacements(const Replacements &Replaces, - const std::vector &Ranges) { - auto MergedRanges = mergeAndSortRanges(Ranges); - tooling::Replacements FakeReplaces; - for (const auto &R : MergedRanges) - FakeReplaces.insert(Replacement(Replaces.begin()->getFilePath(), - R.getOffset(), R.getLength(), - std::string(R.getLength(), ' '))); - tooling::Replacements NewReplaces = mergeReplacements(FakeReplaces, Replaces); - return calculateChangedRanges(NewReplaces); + Replaces.insert(R); + return llvm::Error::success(); } namespace { + // Represents a merged replacement, i.e. a replacement consisting of multiple // overlapping replacements from 'First' and 'Second' in mergeReplacements. // @@ -424,26 +254,19 @@ unsigned Length; std::string Text; }; -} // namespace -std::map -groupReplacementsByFile(const Replacements &Replaces) { - std::map FileToReplaces; - for (const auto &Replace : Replaces) { - FileToReplaces[Replace.getFilePath()].insert(Replace); - } - return FileToReplaces; -} +} // namespace -Replacements mergeReplacements(const Replacements &First, - const Replacements &Second) { - if (First.empty() || Second.empty()) - return First.empty() ? Second : First; +Replacements Replacements::merge(const Replacements &ReplacesToMerge) const { + if (empty() || ReplacesToMerge.empty()) + return empty() ? ReplacesToMerge : *this; + auto &First = Replaces; + auto &Second = ReplacesToMerge.Replaces; // 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; - Replacements Result; + ReplacementsImpl Result; // Iterate over both sets and always add the next element (smallest total // Offset) from either 'First' or 'Second'. Merge that element with @@ -469,8 +292,141 @@ Delta -= Merged.deltaFirst(); Result.insert(Merged.asReplacement()); } + return Replacements(Result.begin(), Result.end()); +} + +// Combines overlapping ranges in \p Ranges and sorts the combined ranges. +// Returns a set of non-overlapping and sorted ranges that is equivalent to +// \p Ranges. +static std::vector combineAndSortRanges(std::vector Ranges) { + std::sort(Ranges.begin(), Ranges.end(), + [](const Range &LHS, const Range &RHS) { + if (LHS.getOffset() != RHS.getOffset()) + return LHS.getOffset() < RHS.getOffset(); + return LHS.getLength() < RHS.getLength(); + }); + std::vector Result; + for (const auto &R : Ranges) { + if (Result.empty() || + Result.back().getOffset() + Result.back().getLength() < R.getOffset()) { + Result.push_back(R); + } else { + unsigned NewEnd = + std::max(Result.back().getOffset() + Result.back().getLength(), + R.getOffset() + R.getLength()); + Result[Result.size() - 1] = + Range(Result.back().getOffset(), NewEnd - Result.back().getOffset()); + } + } return Result; } +std::vector +calculateRangesAfterReplacements(const Replacements &Replaces, + const std::vector &Ranges) { + // To calculate the new ranges, + // - Turn \p Ranges into Replacements at (offset, length) with an empty + // (unimportant) replacement text of length "length". + // - Merge with \p Replaces. + // - The new ranges will be the affected ranges of the merged replacements. + auto MergedRanges = combineAndSortRanges(Ranges); + tooling::Replacements FakeReplaces; + for (const auto &R : MergedRanges) { + auto Err = FakeReplaces.add(Replacement(Replaces.begin()->getFilePath(), + R.getOffset(), R.getLength(), + std::string(R.getLength(), ' '))); + assert(!Err && + "Replacements must not conflict since ranges have been merged."); + (void)Err; + } + return FakeReplaces.merge(Replaces).getAffectedRanges(); +} + +std::vector Replacements::getAffectedRanges() const { + 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 combineAndSortRanges(ChangedRanges); +} + +unsigned Replacements::getShiftedCodePosition(unsigned Position) const { + unsigned Offset = 0; + for (const auto& R : Replaces) { + if (R.getOffset() + R.getLength() <= Position) { + Offset += R.getReplacementText().size() - R.getLength(); + continue; + } + if (R.getOffset() < Position && + R.getOffset() + R.getReplacementText().size() <= Position) { + Position = R.getOffset() + R.getReplacementText().size(); + if (R.getReplacementText().size() > 0) + Position--; + } + break; + } + return Position + Offset; +} + +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; +} + +llvm::Expected applyAllReplacements(StringRef Code, + const Replacements &Replaces) { + if (Replaces.empty()) + return Code.str(); + + IntrusiveRefCntPtr InMemoryFileSystem( + new vfs::InMemoryFileSystem); + FileManager Files(FileSystemOptions(), InMemoryFileSystem); + DiagnosticsEngine Diagnostics( + IntrusiveRefCntPtr(new DiagnosticIDs), + new DiagnosticOptions); + SourceManager SourceMgr(Diagnostics, Files); + Rewriter Rewrite(SourceMgr, LangOptions()); + InMemoryFileSystem->addFile( + "", 0, llvm::MemoryBuffer::getMemBuffer(Code, "")); + FileID ID = SourceMgr.createFileID(Files.getFile(""), SourceLocation(), + clang::SrcMgr::C_User); + for (Replacements::const_iterator I = Replaces.begin(), E = Replaces.end(); + I != E; ++I) { + Replacement Replace("", I->getOffset(), I->getLength(), + I->getReplacementText()); + if (!Replace.apply(Rewrite)) + return llvm::make_error( + "Failed to apply replacement: " + Replace.toString(), + llvm::inconvertibleErrorCode()); + } + std::string Result; + llvm::raw_string_ostream OS(Result); + Rewrite.getEditBuffer(ID).write(OS); + OS.flush(); + return Result; +} + +std::map +groupReplacementsByFile(const Replacements &Replaces) { + std::map FileToReplaces; + for (const auto &Replace : Replaces) + // We can ignore the Error here since \p Replaces is already conflict-free. + FileToReplaces[Replace.getFilePath()].add(Replace); + return FileToReplaces; +} + } // end namespace tooling } // end namespace clang Index: lib/Tooling/Refactoring.cpp =================================================================== --- lib/Tooling/Refactoring.cpp +++ lib/Tooling/Refactoring.cpp @@ -30,7 +30,9 @@ std::shared_ptr PCHContainerOps) : ClangTool(Compilations, SourcePaths, PCHContainerOps) {} -Replacements &RefactoringTool::getReplacements() { return Replace; } +std::map &RefactoringTool::getReplacements() { + return FileToReplaces; +} int RefactoringTool::runAndSave(FrontendActionFactory *ActionFactory) { if (int Result = run(ActionFactory)) { @@ -54,20 +56,22 @@ } bool RefactoringTool::applyAllReplacements(Rewriter &Rewrite) { - return tooling::applyAllReplacements(Replace, Rewrite); + bool Result = true; + for (const auto &Entry : FileToReplaces) + Result = tooling::applyAllReplacements(Entry.second, Rewrite) && Result; + return Result; } int RefactoringTool::saveRewrittenFiles(Rewriter &Rewrite) { return Rewrite.overwriteChangedFiles() ? 1 : 0; } -bool formatAndApplyAllReplacements(const Replacements &Replaces, - Rewriter &Rewrite, StringRef Style) { +bool formatAndApplyAllReplacements( + const std::map &FileToReplaces, Rewriter &Rewrite, + StringRef Style) { SourceManager &SM = Rewrite.getSourceMgr(); FileManager &Files = SM.getFileManager(); - auto FileToReplaces = groupReplacementsByFile(Replaces); - bool Result = true; for (const auto &FileAndReplaces : FileToReplaces) { const std::string &FilePath = FileAndReplaces.first; Index: lib/Tooling/RefactoringCallbacks.cpp =================================================================== --- lib/Tooling/RefactoringCallbacks.cpp +++ lib/Tooling/RefactoringCallbacks.cpp @@ -40,10 +40,14 @@ void ReplaceStmtWithText::run( const ast_matchers::MatchFinder::MatchResult &Result) { if (const Stmt *FromMatch = Result.Nodes.getStmtAs(FromId)) { - Replace.insert(tooling::Replacement( + auto Err = Replace.add(tooling::Replacement( *Result.SourceManager, - CharSourceRange::getTokenRange(FromMatch->getSourceRange()), - ToText)); + CharSourceRange::getTokenRange(FromMatch->getSourceRange()), ToText)); + // FIXME: better error handling. For now, just print error message in the + // release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); } } @@ -54,9 +58,15 @@ const ast_matchers::MatchFinder::MatchResult &Result) { const Stmt *FromMatch = Result.Nodes.getStmtAs(FromId); const Stmt *ToMatch = Result.Nodes.getStmtAs(ToId); - if (FromMatch && ToMatch) - Replace.insert(replaceStmtWithStmt( - *Result.SourceManager, *FromMatch, *ToMatch)); + if (FromMatch && ToMatch) { + auto Err = Replace.add( + replaceStmtWithStmt(*Result.SourceManager, *FromMatch, *ToMatch)); + // FIXME: better error handling. For now, just print error message in the + // release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); + } } ReplaceIfStmtWithItsBody::ReplaceIfStmtWithItsBody(StringRef Id, @@ -68,11 +78,23 @@ 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)); + auto Err = + Replace.add(replaceStmtWithStmt(*Result.SourceManager, *Node, *Body)); + // FIXME: better error handling. For now, just print error message in the + // release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); } 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, "")); + auto Err = + Replace.add(replaceStmtWithText(*Result.SourceManager, *Node, "")); + // FIXME: better error handling. For now, just print error message in the + // release version. + if (Err) + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + assert(!Err); } } } Index: tools/clang-format/ClangFormat.cpp =================================================================== --- tools/clang-format/ClangFormat.cpp +++ tools/clang-format/ClangFormat.cpp @@ -266,17 +266,17 @@ bool IncompleteFormat = false; Replacements FormatChanges = reformat(FormatStyle, *ChangedCode, Ranges, AssumedFileName, &IncompleteFormat); - Replaces = tooling::mergeReplacements(Replaces, FormatChanges); + Replaces = Replaces.merge(FormatChanges); if (OutputXML) { outs() << "\n\n"; if (Cursor.getNumOccurrences() != 0) outs() << "" - << tooling::shiftedCodePosition(FormatChanges, CursorPosition) + << FormatChanges.getShiftedCodePosition(CursorPosition) << "\n"; - outputReplacementsXML(Replaces); + outputReplacementsXML(Replaces); outs() << "\n"; } else { IntrusiveRefCntPtr InMemoryFileSystem( @@ -298,7 +298,7 @@ } else { if (Cursor.getNumOccurrences() != 0) outs() << "{ \"Cursor\": " - << tooling::shiftedCodePosition(FormatChanges, CursorPosition) + << FormatChanges.getShiftedCodePosition(CursorPosition) << ", \"IncompleteFormat\": " << (IncompleteFormat ? "true" : "false") << " }\n"; Rewrite.getEditBuffer(ID).write(outs()); Index: unittests/Format/CleanupTest.cpp =================================================================== --- unittests/Format/CleanupTest.cpp +++ unittests/Format/CleanupTest.cpp @@ -9,11 +9,15 @@ #include "clang/Format/Format.h" +#include "../Tooling/ReplacementTest.h" #include "../Tooling/RewriterTestContext.h" #include "clang/Tooling/Core/Replacement.h" #include "gtest/gtest.h" +using clang::tooling::ReplacementTest; +using clang::tooling::toReplacements; + namespace clang { namespace format { namespace { @@ -241,7 +245,7 @@ EXPECT_EQ(Expected, Result); } -class CleanUpReplacementsTest : public ::testing::Test { +class CleanUpReplacementsTest : public ReplacementTest { protected: tooling::Replacement createReplacement(unsigned Offset, unsigned Length, StringRef Text) { @@ -304,9 +308,9 @@ "namespace D { int i; }\n\n" "int x= 0;" "}"; - tooling::Replacements Replaces = { - createReplacement(getOffset(Code, 3, 3), 6, ""), - createReplacement(getOffset(Code, 9, 34), 6, "")}; + tooling::Replacements Replaces = + toReplacements({createReplacement(getOffset(Code, 3, 3), 6, ""), + createReplacement(getOffset(Code, 9, 34), 6, "")}); EXPECT_EQ(Expected, formatAndApply(Code, Replaces)); } @@ -315,7 +319,8 @@ std::string Code = "int main() {}"; std::string Expected = "#include \"a.h\"\n" "int main() {}"; - tooling::Replacements Replaces = {createInsertion("#include \"a.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"a.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -332,7 +337,8 @@ "#define MMM 123\n" "#endif"; - tooling::Replacements Replaces = {createInsertion("#include \"b.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"b.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -357,7 +363,8 @@ "#define MMM 123\n" "#endif"; - tooling::Replacements Replaces = {createInsertion("#include \"a.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"a.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -369,7 +376,8 @@ "#include \n" "\n" "int main() {}"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -382,7 +390,8 @@ "#include \n" "\n" "int main() {}"; - tooling::Replacements Replaces = {createInsertion("#include \"z.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"z.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -394,7 +403,8 @@ "#include \"z.h\"\n" "\n" "int main() {}"; - tooling::Replacements Replaces = {createInsertion("#include \"z.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"z.h\"")}); Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -412,8 +422,9 @@ "#include \"clang/Format/Format.h\"\n" "#include \"llvm/x/y.h\"\n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include \"d.h\""), - createInsertion("#include \"llvm/x/y.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"d.h\""), + createInsertion("#include \"llvm/x/y.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -430,8 +441,9 @@ "#include \"clang/Format/Format.h\"\n" "#include \n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include "), - createInsertion("#include \"new/new.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include "), + createInsertion("#include \"new/new.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -447,7 +459,8 @@ "\n" "#include \"y/a.h\"\n" "#include \"z/b.h\"\n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -467,8 +480,9 @@ "#include \"y/a.h\"\n" "#include \"z/b.h\"\n" "#include \"x/x.h\"\n"; - tooling::Replacements Replaces = {createInsertion("#include "), - createInsertion("#include \"x/x.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include "), + createInsertion("#include \"x/x.h\"")}); Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -482,12 +496,11 @@ "#include \n" "#include \n" "int x;"; - tooling::Replacements Replaces = {createInsertion("#include \"a.h\""), - createInsertion("#include \"c.h\""), - createInsertion("#include \"b.h\""), - createInsertion("#include "), - createInsertion("#include "), - createInsertion("#include \"fix.h\"")}; + tooling::Replacements Replaces = toReplacements( + {createInsertion("#include \"a.h\""), createInsertion("#include \"c.h\""), + createInsertion("#include \"b.h\""), + createInsertion("#include "), createInsertion("#include "), + createInsertion("#include \"fix.h\"")}); EXPECT_EQ(Expected, formatAndApply(Code, Replaces)); } @@ -500,12 +513,11 @@ "#include \"b.h\"\n" "#include \"c.h\"\n" "int x;"; - tooling::Replacements Replaces = {createInsertion("#include \"a.h\""), - createInsertion("#include \"c.h\""), - createInsertion("#include \"b.h\""), - createInsertion("#include "), - createInsertion("#include "), - createInsertion("#include \"fix.h\"")}; + tooling::Replacements Replaces = toReplacements( + {createInsertion("#include \"a.h\""), createInsertion("#include \"c.h\""), + createInsertion("#include \"b.h\""), + createInsertion("#include "), createInsertion("#include "), + createInsertion("#include \"fix.h\"")}); Style = format::getGoogleStyle(format::FormatStyle::LanguageKind::LK_Cpp); EXPECT_EQ(Expected, formatAndApply(Code, Replaces)); } @@ -526,13 +538,12 @@ "int a;\n" "int b;\n" "int a;"; - tooling::Replacements Replaces = { - createReplacement(getOffset(Code, 4, 8), 1, "b"), - createInsertion("#include "), - createInsertion("#include "), - createInsertion("#include \"clang/x/x.h\""), - createInsertion("#include \"y.h\""), - createInsertion("#include \"x.h\"")}; + tooling::Replacements Replaces = toReplacements( + {createReplacement(getOffset(Code, 4, 8), 1, "b"), + createInsertion("#include "), createInsertion("#include "), + createInsertion("#include \"clang/x/x.h\""), + createInsertion("#include \"y.h\""), + createInsertion("#include \"x.h\"")}); EXPECT_EQ(Expected, formatAndApply(Code, Replaces)); } @@ -544,7 +555,8 @@ "void f() {}\n" "#define A \\\n" " int i;"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, formatAndApply(Code, Replaces)); } @@ -556,7 +568,8 @@ "\n" " // comment\n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -574,7 +587,8 @@ "* comment\n" "*/\n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -592,7 +606,8 @@ "\n\n" "/* c1 */ /*c2 */\n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -614,7 +629,8 @@ "\n" "#include \n" "int x;\n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -626,7 +642,8 @@ "#include \n" "#ifdef X\n" "#define X\n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -642,7 +659,8 @@ "#include \n" "int x;\n" "#define Y 1\n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -656,7 +674,8 @@ "#ifndef X\n" "int x;\n" "#define Y 1\n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -678,14 +697,16 @@ "#include \n" "int x;\n" "#define Y 1\n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } TEST_F(CleanUpReplacementsTest, EmptyCode) { std::string Code = ""; std::string Expected = "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -694,7 +715,8 @@ TEST_F(CleanUpReplacementsTest, NoNewLineAtTheEndOfCode) { std::string Code = "#include "; std::string Expected = "#include #include \n"; - tooling::Replacements Replaces = {createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -703,8 +725,9 @@ "#include \n"; std::string Expected = "#include \"a.h\"\n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include "), - createInsertion("#include \"a.h\"")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include "), + createInsertion("#include \"a.h\"")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } @@ -716,8 +739,9 @@ "#include \"vector\"\n" "#include \n" "#include \n"; - tooling::Replacements Replaces = {createInsertion("#include \"vector\""), - createInsertion("#include ")}; + tooling::Replacements Replaces = + toReplacements({createInsertion("#include \"vector\""), + createInsertion("#include ")}); EXPECT_EQ(Expected, apply(Code, Replaces)); } Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -9,7 +9,7 @@ #include "clang/Format/Format.h" -#include "../Tooling/RewriterTestContext.h" +#include "../Tooling/ReplacementTest.h" #include "FormatTestUtils.h" #include "clang/Frontend/TextDiagnosticPrinter.h" @@ -19,6 +19,9 @@ #define DEBUG_TYPE "format-test" +using clang::tooling::ReplacementTest; +using clang::tooling::toReplacements; + namespace clang { namespace format { namespace { @@ -11520,17 +11523,6 @@ #endif // _MSC_VER -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" @@ -11545,15 +11537,15 @@ " 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")); + tooling::Replacements Replaces = toReplacements( + {tooling::Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 6, + "auto "), + tooling::Replacement(Context.Sources, Context.getLocation(ID, 3, 10), 1, + "nullptr"), + tooling::Replacement(Context.Sources, Context.getLocation(ID, 4, 3), 1, + "nullptr"), + 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. @@ -11580,9 +11572,9 @@ " return 0;\n" "}"; FileID ID = Context.createInMemoryFile("fix.cpp", Code); - tooling::Replacements Replaces; - Replaces.insert(tooling::Replacement( - Context.Sources, Context.getLocation(ID, 1, 1), 0, "#include \"b.h\"\n")); + tooling::Replacements Replaces = toReplacements( + {tooling::Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 0, + "#include \"b.h\"\n")}); format::FormatStyle Style = format::getLLVMStyle(); Style.SortIncludes = true; Index: unittests/Tooling/RefactoringTest.cpp =================================================================== --- unittests/Tooling/RefactoringTest.cpp +++ unittests/Tooling/RefactoringTest.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "ReplacementTest.h" #include "RewriterTestContext.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" @@ -31,16 +32,6 @@ namespace clang { namespace tooling { -class ReplacementTest : public ::testing::Test { - protected: - Replacement createReplacement(SourceLocation Start, unsigned Length, - llvm::StringRef ReplacementText) { - return Replacement(Context.Sources, Start, Length, ReplacementText); - } - - RewriterTestContext Context; -}; - TEST_F(ReplacementTest, CanDeleteAllText) { FileID ID = Context.createInMemoryFile("input.cpp", "text"); SourceLocation Location = Context.getLocation(ID, 1, 1); @@ -108,29 +99,30 @@ EXPECT_TRUE(Replace2.getFilePath().empty()); } -TEST_F(ReplacementTest, CanApplyReplacements) { - FileID ID = Context.createInMemoryFile("input.cpp", - "line1\nline2\nline3\nline4"); +TEST_F(ReplacementTest, FailAddReplacements) { 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)); + auto Err = Replaces.add(Replacement("x.cc", 0, 10, "3")); + EXPECT_TRUE(!Err); + llvm::consumeError(std::move(Err)); + Err = Replaces.add(Replacement("x.cc", 0, 2, "")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); + Err = Replaces.add(Replacement("x.cc", 2, 2, "")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); + Err = Replaces.add(Replacement("y.cc", 20, 2, "")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); } -// 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) { +TEST_F(ReplacementTest, CanApplyReplacements) { 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( - Replacement(Context.Sources, Context.getLocation(ID, 3, 1), 5, "other")); + Replacements Replaces = + toReplacements({Replacement(Context.Sources, + Context.getLocation(ID, 2, 1), 5, "replaced"), + 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)); } @@ -138,32 +130,28 @@ TEST_F(ReplacementTest, SkipsDuplicateReplacements) { 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")); + auto Replaces = toReplacements({Replacement( + Context.Sources, Context.getLocation(ID, 2, 1), 5, "replaced")}); + + auto Err = Replaces.add(Replacement( + Context.Sources, Context.getLocation(ID, 2, 1), 5, "replaced")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); + + Err = Replaces.add(Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 5, "replaced")); + EXPECT_TRUE((bool)Err); + llvm::consumeError(std::move(Err)); + EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); EXPECT_EQ("line1\nreplaced\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")); +TEST_F(ReplacementTest, InvalidSourceLocationFailsApplyAll) { + Replacements Replaces = + toReplacements({Replacement(Context.Sources, SourceLocation(), 5, "2")}); + EXPECT_FALSE(applyAllReplacements(Replaces, Context.Rewrite)); - EXPECT_EQ("a", Context.getRewrittenText(IDa)); - EXPECT_EQ("z", Context.getRewrittenText(IDz)); } TEST_F(ReplacementTest, MultipleFilesReplaceAndFormat) { @@ -179,76 +167,66 @@ std::string Expected2 = "int x =\n" " 1234567890123;\n" "int y = 10;"; - FileID ID1 = Context.createInMemoryFile("format_1.cpp", Code1); - FileID ID2 = Context.createInMemoryFile("format_2.cpp", Code2); + StringRef File1 = "format_1.cpp"; + StringRef File2 = "format_2.cpp"; + FileID ID1 = Context.createInMemoryFile(File1, Code1); + FileID ID2 = Context.createInMemoryFile(File2, Code2); - tooling::Replacements Replaces; // Scrambled the order of replacements. - Replaces.insert(tooling::Replacement( - Context.Sources, Context.getLocation(ID2, 1, 12), 0, "4567890123")); - Replaces.insert(tooling::Replacement( - Context.Sources, Context.getLocation(ID1, 1, 1), 6, "auto ")); - Replaces.insert(tooling::Replacement( - Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10")); - Replaces.insert(tooling::Replacement( - Context.Sources, Context.getLocation(ID1, 3, 10), 1, "12345678901")); - - EXPECT_TRUE(formatAndApplyAllReplacements( - Replaces, Context.Rewrite, "{BasedOnStyle: LLVM, ColumnLimit: 20}")); + std::map FileToReplaces; + FileToReplaces[File1] = toReplacements( + {tooling::Replacement(Context.Sources, Context.getLocation(ID1, 1, 1), 6, + "auto "), + tooling::Replacement(Context.Sources, Context.getLocation(ID1, 3, 10), 1, + "12345678901")}); + FileToReplaces[File2] = toReplacements( + {tooling::Replacement(Context.Sources, Context.getLocation(ID2, 1, 12), 0, + "4567890123"), + tooling::Replacement(Context.Sources, Context.getLocation(ID2, 2, 9), 1, + "10")}); + EXPECT_TRUE( + formatAndApplyAllReplacements(FileToReplaces, Context.Rewrite, + "{BasedOnStyle: LLVM, ColumnLimit: 20}")); EXPECT_EQ(Expected1, Context.getRewrittenText(ID1)); EXPECT_EQ(Expected2, Context.getRewrittenText(ID2)); } 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, " ")); + Replacements Replaces = + toReplacements({Replacement("", 0, 1, ""), 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| + EXPECT_EQ(0u, Replaces.getShiftedCodePosition(0)); // |int i; + EXPECT_EQ(0u, Replaces.getShiftedCodePosition(1)); // |nt i; + EXPECT_EQ(1u, Replaces.getShiftedCodePosition(2)); // i|t i; + EXPECT_EQ(2u, Replaces.getShiftedCodePosition(3)); // in| i; + EXPECT_EQ(3u, Replaces.getShiftedCodePosition(4)); // int| i; + EXPECT_EQ(3u, Replaces.getShiftedCodePosition(5)); // int | i; + EXPECT_EQ(3u, Replaces.getShiftedCodePosition(6)); // int |i; + EXPECT_EQ(4u, Replaces.getShiftedCodePosition(7)); // int |; + EXPECT_EQ(5u, Replaces.getShiftedCodePosition(8)); // int i| } TEST(ShiftedCodePositionTest, FindsNewCodePositionWithInserts) { - Replacements Replaces; - Replaces.insert(Replacement("", 4, 0, "\"\n\"")); + Replacements Replaces = toReplacements({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" - EXPECT_EQ(8u, shiftedCodePosition(Replaces, 5)); // "12345|78" + EXPECT_EQ(3u, Replaces.getShiftedCodePosition(3)); // "123|5678" + EXPECT_EQ(7u, Replaces.getShiftedCodePosition(4)); // "1234|678" + EXPECT_EQ(8u, Replaces.getShiftedCodePosition(5)); // "12345|78" } TEST(ShiftedCodePositionTest, FindsNewCodePositionInReplacedText) { - Replacements Replaces; // Replace the first four characters with "abcd". - Replaces.insert(Replacement("", 0, 4, "abcd")); + auto Replaces = toReplacements({Replacement("", 0, 4, "abcd")}); for (unsigned i = 0; i < 3; ++i) - EXPECT_EQ(i, shiftedCodePosition(Replaces, i)); + EXPECT_EQ(i, Replaces.getShiftedCodePosition(i)); +} + +TEST(ShiftedCodePositionTest, NoReplacementText) { + Replacements Replaces = toReplacements({Replacement("", 0, 42, "")}); + EXPECT_EQ(0u, Replaces.getShiftedCodePosition(0)); + EXPECT_EQ(0u, Replaces.getShiftedCodePosition(39)); + EXPECT_EQ(3u, Replaces.getShiftedCodePosition(45)); + EXPECT_EQ(0u, Replaces.getShiftedCodePosition(42)); } class FlushRewrittenFilesTest : public ::testing::Test { @@ -304,9 +282,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")); + Replacements Replaces = toReplacements({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", @@ -454,12 +431,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")); + Replacements Replaces = toReplacements( + {Replacement("foo", 0, 4, ""), Replacement("foo", 10, 1, "zzzzzz"), + Replacement("foo", 11, 0, "oooooooooooooooo")}); - std::vector Ranges = calculateChangedRanges(Replaces); + std::vector Ranges = Replaces.getAffectedRanges(); EXPECT_EQ(2ul, Ranges.size()); EXPECT_TRUE(Ranges[0].getOffset() == 0); @@ -470,23 +446,23 @@ TEST(Range, RangesAfterReplacements) { std::vector Ranges = {Range(5, 2), Range(10, 5)}; - Replacements Replaces = {Replacement("foo", 0, 2, "1234")}; + Replacements Replaces = toReplacements({Replacement("foo", 0, 2, "1234")}); std::vector Expected = {Range(0, 4), Range(7, 2), Range(12, 5)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } TEST(Range, RangesBeforeReplacements) { std::vector Ranges = {Range(5, 2), Range(10, 5)}; - Replacements Replaces = {Replacement("foo", 20, 2, "1234")}; + Replacements Replaces = toReplacements({Replacement("foo", 20, 2, "1234")}); std::vector Expected = {Range(5, 2), Range(10, 5), Range(20, 4)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } TEST(Range, NotAffectedByReplacements) { std::vector Ranges = {Range(0, 2), Range(5, 2), Range(10, 5)}; - Replacements Replaces = {Replacement("foo", 3, 2, "12"), - Replacement("foo", 12, 2, "12"), - Replacement("foo", 20, 5, "")}; + Replacements Replaces = toReplacements({Replacement("foo", 3, 2, "12"), + Replacement("foo", 12, 2, "12"), + Replacement("foo", 20, 5, "")}); std::vector Expected = {Range(0, 2), Range(3, 4), Range(10, 5), Range(20, 0)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); @@ -494,9 +470,9 @@ TEST(Range, RangesWithNonOverlappingReplacements) { std::vector Ranges = {Range(0, 2), Range(5, 2), Range(10, 5)}; - Replacements Replaces = {Replacement("foo", 3, 1, ""), - Replacement("foo", 6, 1, "123"), - Replacement("foo", 20, 2, "12345")}; + Replacements Replaces = toReplacements({Replacement("foo", 3, 1, ""), + Replacement("foo", 6, 1, "123"), + Replacement("foo", 20, 2, "12345")}); std::vector Expected = {Range(0, 2), Range(3, 0), Range(4, 4), Range(11, 5), Range(21, 5)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); @@ -505,9 +481,9 @@ TEST(Range, RangesWithOverlappingReplacements) { std::vector Ranges = {Range(0, 2), Range(5, 2), Range(15, 5), Range(30, 5)}; - Replacements Replaces = { - Replacement("foo", 1, 3, ""), Replacement("foo", 6, 1, "123"), - Replacement("foo", 13, 3, "1"), Replacement("foo", 25, 15, "")}; + Replacements Replaces = toReplacements( + {Replacement("foo", 1, 3, ""), Replacement("foo", 6, 1, "123"), + Replacement("foo", 13, 3, "1"), Replacement("foo", 25, 15, "")}); std::vector Expected = {Range(0, 1), Range(2, 4), Range(12, 5), Range(22, 0)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); @@ -515,122 +491,52 @@ TEST(Range, MergeIntoOneRange) { std::vector Ranges = {Range(0, 2), Range(5, 2), Range(15, 5)}; - Replacements Replaces = {Replacement("foo", 1, 15, "1234567890")}; + Replacements Replaces = + toReplacements({Replacement("foo", 1, 15, "1234567890")}); std::vector Expected = {Range(0, 15)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } TEST(Range, ReplacementsStartingAtRangeOffsets) { std::vector Ranges = {Range(0, 2), Range(5, 5), Range(15, 5)}; - Replacements Replaces = { - Replacement("foo", 0, 2, "12"), Replacement("foo", 5, 1, "123"), - Replacement("foo", 7, 4, "12345"), Replacement("foo", 15, 10, "12")}; + Replacements Replaces = toReplacements( + {Replacement("foo", 0, 2, "12"), Replacement("foo", 5, 1, "123"), + Replacement("foo", 7, 4, "12345"), Replacement("foo", 15, 10, "12")}); std::vector Expected = {Range(0, 2), Range(5, 9), Range(18, 2)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } TEST(Range, ReplacementsEndingAtRangeEnds) { std::vector Ranges = {Range(0, 2), Range(5, 2), Range(15, 5)}; - Replacements Replaces = {Replacement("foo", 6, 1, "123"), - Replacement("foo", 17, 3, "12")}; + Replacements Replaces = toReplacements( + {Replacement("foo", 6, 1, "123"), Replacement("foo", 17, 3, "12")}); std::vector Expected = {Range(0, 2), Range(5, 4), Range(17, 4)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } TEST(Range, AjacentReplacements) { std::vector Ranges = {Range(0, 0), Range(15, 5)}; - Replacements Replaces = {Replacement("foo", 1, 2, "123"), - Replacement("foo", 12, 3, "1234")}; + Replacements Replaces = toReplacements( + {Replacement("foo", 1, 2, "123"), Replacement("foo", 12, 3, "1234")}); std::vector Expected = {Range(0, 0), Range(1, 3), Range(13, 9)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } TEST(Range, MergeRangesAfterReplacements) { std::vector Ranges = {Range(8, 0), Range(5, 2), Range(9, 0), Range(0, 1)}; - Replacements Replaces = {Replacement("foo", 1, 3, ""), - Replacement("foo", 7, 0, "12"), Replacement("foo", 9, 2, "")}; - std::vector Expected = {Range(0, 1), Range(2, 4), Range(7, 0), Range(8, 0)}; + Replacements Replaces = toReplacements({Replacement("foo", 1, 3, ""), + Replacement("foo", 7, 0, "12"), + Replacement("foo", 9, 2, "")}); + std::vector Expected = {Range(0, 1), Range(2, 4), Range(7, 0), + Range(8, 0)}; EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } -TEST(DeduplicateTest, removesDuplicates) { - std::vector 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", 50, 0, " foo ")); // Duplicate - Input.push_back(Replacement("fileA", 51, 3, " bar ")); - Input.push_back(Replacement("fileB", 51, 3, " bar ")); // Filename differs! - Input.push_back(Replacement("fileB", 60, 1, " bar ")); - Input.push_back(Replacement("fileA", 60, 2, " bar ")); - 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 ")); - Expected.push_back(Replacement("fileA", 10, 3, " 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 ")); - - std::vector Conflicts; // Ignored for this test - deduplicate(Input, Conflicts); - - EXPECT_EQ(3U, Conflicts.size()); - EXPECT_EQ(Expected, Input); -} - -TEST(DeduplicateTest, detectsConflicts) { - { - std::vector Input; - Input.push_back(Replacement("fileA", 0, 5, " foo ")); - Input.push_back(Replacement("fileA", 0, 5, " foo ")); // Duplicate not a - // conflict. - Input.push_back(Replacement("fileA", 2, 6, " bar ")); - Input.push_back(Replacement("fileA", 7, 3, " moo ")); - - std::vector Conflicts; - deduplicate(Input, Conflicts); - - // One duplicate is removed and the remaining three items form one - // conflicted range. - ASSERT_EQ(3u, Input.size()); - ASSERT_EQ(1u, Conflicts.size()); - ASSERT_EQ(0u, Conflicts.front().getOffset()); - ASSERT_EQ(3u, Conflicts.front().getLength()); - } - { - std::vector 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", 15, 5, " golf ")); // 5 - Input.push_back(Replacement("fileA", 16, 5, " bag ")); // 6 - Input.push_back(Replacement("fileA", 10, 3, " club ")); // 7 - - // #3 is special in that it is completely contained by another conflicting - // Replacement. #4 ensures #3 hasn't messed up the conflicting range size. - - std::vector Conflicts; - deduplicate(Input, Conflicts); - - // No duplicates - ASSERT_EQ(8u, Input.size()); - ASSERT_EQ(2u, Conflicts.size()); - ASSERT_EQ(1u, Conflicts[0].getOffset()); - ASSERT_EQ(4u, Conflicts[0].getLength()); - ASSERT_EQ(6u, Conflicts[1].getOffset()); - ASSERT_EQ(2u, Conflicts[1].getLength()); - } +TEST(Range, ConflictingRangesBeforeReplacements) { + std::vector Ranges = {Range(8, 3), Range(5, 4), Range(9, 1)}; + Replacements Replaces = toReplacements({Replacement("foo", 1, 3, "")}); + std::vector Expected = {Range(1, 0), Range(2, 6)}; + EXPECT_EQ(Expected, calculateRangesAfterReplacements(Replaces, Ranges)); } class MergeReplacementsTest : public ::testing::Test { @@ -646,7 +552,7 @@ EXPECT_EQ(Intermediate, *AfterFirst); EXPECT_EQ(Result, *InSequenceRewrite); - tooling::Replacements Merged = mergeReplacements(First, Second); + tooling::Replacements Merged = First.merge(Second); auto MergedRewrite = applyAllReplacements(Code, Merged); EXPECT_TRUE(static_cast(MergedRewrite)); EXPECT_EQ(*InSequenceRewrite, *MergedRewrite); @@ -660,7 +566,7 @@ auto AfterFirst = applyAllReplacements(Code, First); EXPECT_TRUE(static_cast(AfterFirst)); auto InSequenceRewrite = applyAllReplacements(*AfterFirst, Second); - tooling::Replacements Merged = mergeReplacements(First, Second); + tooling::Replacements Merged = First.merge(Second); auto MergedRewrite = applyAllReplacements(Code, Merged); EXPECT_TRUE(static_cast(MergedRewrite)); EXPECT_EQ(*InSequenceRewrite, *MergedRewrite); @@ -673,62 +579,82 @@ TEST_F(MergeReplacementsTest, Offsets) { mergeAndTestRewrite("aaa", "aabab", "cacabab", - {{"", 2, 0, "b"}, {"", 3, 0, "b"}}, - {{"", 0, 0, "c"}, {"", 1, 0, "c"}}); + toReplacements({{"", 2, 0, "b"}, {"", 3, 0, "b"}}), + toReplacements({{"", 0, 0, "c"}, {"", 1, 0, "c"}})); mergeAndTestRewrite("aaa", "babaa", "babacac", - {{"", 0, 0, "b"}, {"", 1, 0, "b"}}, - {{"", 4, 0, "c"}, {"", 5, 0, "c"}}); - mergeAndTestRewrite("aaaa", "aaa", "aac", {{"", 1, 1, ""}}, - {{"", 2, 1, "c"}}); + toReplacements({{"", 0, 0, "b"}, {"", 1, 0, "b"}}), + toReplacements({{"", 4, 0, "c"}, {"", 5, 0, "c"}})); + mergeAndTestRewrite("aaaa", "aaa", "aac", toReplacements({{"", 1, 1, ""}}), + toReplacements({{"", 2, 1, "c"}})); mergeAndTestRewrite("aa", "bbabba", "bbabcba", - {{"", 0, 0, "bb"}, {"", 1, 0, "bb"}}, {{"", 4, 0, "c"}}); + toReplacements({{"", 0, 0, "bb"}, {"", 1, 0, "bb"}}), + toReplacements({{"", 4, 0, "c"}})); } TEST_F(MergeReplacementsTest, Concatenations) { // Basic concatenations. It is important to merge these into a single // replacement to ensure the correct order. - EXPECT_EQ((Replacements{{"", 0, 0, "ab"}}), - mergeReplacements({{"", 0, 0, "a"}}, {{"", 1, 0, "b"}})); - EXPECT_EQ((Replacements{{"", 0, 0, "ba"}}), - mergeReplacements({{"", 0, 0, "a"}}, {{"", 0, 0, "b"}})); - mergeAndTestRewrite("", "a", "ab", {{"", 0, 0, "a"}}, {{"", 1, 0, "b"}}); - mergeAndTestRewrite("", "a", "ba", {{"", 0, 0, "a"}}, {{"", 0, 0, "b"}}); + { + auto First = toReplacements({{"", 0, 0, "a"}}); + auto Second = toReplacements({{"", 1, 0, "b"}}); + EXPECT_EQ(toReplacements({{"", 0, 0, "ab"}}), First.merge(Second)); + } + { + auto First = toReplacements({{"", 0, 0, "a"}}); + auto Second = toReplacements({{"", 0, 0, "b"}}); + EXPECT_EQ(toReplacements({{"", 0, 0, "ba"}}), First.merge(Second)); + } + mergeAndTestRewrite("", "a", "ab", toReplacements({{"", 0, 0, "a"}}), + toReplacements({{"", 1, 0, "b"}})); + mergeAndTestRewrite("", "a", "ba", toReplacements({{"", 0, 0, "a"}}), + toReplacements({{"", 0, 0, "b"}})); } TEST_F(MergeReplacementsTest, NotChangingLengths) { - mergeAndTestRewrite("aaaa", "abba", "acca", {{"", 1, 2, "bb"}}, - {{"", 1, 2, "cc"}}); - mergeAndTestRewrite("aaaa", "abba", "abcc", {{"", 1, 2, "bb"}}, - {{"", 2, 2, "cc"}}); - mergeAndTestRewrite("aaaa", "abba", "ccba", {{"", 1, 2, "bb"}}, - {{"", 0, 2, "cc"}}); + mergeAndTestRewrite("aaaa", "abba", "acca", + toReplacements({{"", 1, 2, "bb"}}), + toReplacements({{"", 1, 2, "cc"}})); + mergeAndTestRewrite("aaaa", "abba", "abcc", + toReplacements({{"", 1, 2, "bb"}}), + toReplacements({{"", 2, 2, "cc"}})); + mergeAndTestRewrite("aaaa", "abba", "ccba", + toReplacements({{"", 1, 2, "bb"}}), + toReplacements({{"", 0, 2, "cc"}})); mergeAndTestRewrite("aaaaaa", "abbdda", "abccda", - {{"", 1, 2, "bb"}, {"", 3, 2, "dd"}}, {{"", 2, 2, "cc"}}); + toReplacements({{"", 1, 2, "bb"}, {"", 3, 2, "dd"}}), + toReplacements({{"", 2, 2, "cc"}})); } TEST_F(MergeReplacementsTest, OverlappingRanges) { mergeAndTestRewrite("aaa", "bbd", "bcbcd", - {{"", 0, 1, "bb"}, {"", 1, 2, "d"}}, - {{"", 1, 0, "c"}, {"", 2, 0, "c"}}); + toReplacements({{"", 0, 1, "bb"}, {"", 1, 2, "d"}}), + toReplacements({{"", 1, 0, "c"}, {"", 2, 0, "c"}})); - mergeAndTestRewrite("aaaa", "aabbaa", "acccca", {{"", 2, 0, "bb"}}, - {{"", 1, 4, "cccc"}}); + mergeAndTestRewrite("aaaa", "aabbaa", "acccca", + toReplacements({{"", 2, 0, "bb"}}), + toReplacements({{"", 1, 4, "cccc"}})); mergeAndTestRewrite("aaaa", "aababa", "acccca", - {{"", 2, 0, "b"}, {"", 3, 0, "b"}}, {{"", 1, 4, "cccc"}}); - mergeAndTestRewrite("aaaaaa", "abbbba", "abba", {{"", 1, 4, "bbbb"}}, - {{"", 2, 2, ""}}); - mergeAndTestRewrite("aaaa", "aa", "cc", {{"", 1, 1, ""}, {"", 2, 1, ""}}, - {{"", 0, 2, "cc"}}); - mergeAndTestRewrite("aa", "abbba", "abcbcba", {{"", 1, 0, "bbb"}}, - {{"", 2, 0, "c"}, {"", 3, 0, "c"}}); - - mergeAndTestRewrite("aaa", "abbab", "ccdd", - {{"", 0, 1, ""}, {"", 2, 0, "bb"}, {"", 3, 0, "b"}}, - {{"", 0, 2, "cc"}, {"", 2, 3, "dd"}}); - mergeAndTestRewrite("aa", "babbab", "ccdd", - {{"", 0, 0, "b"}, {"", 1, 0, "bb"}, {"", 2, 0, "b"}}, - {{"", 0, 3, "cc"}, {"", 3, 3, "dd"}}); + toReplacements({{"", 2, 0, "b"}, {"", 3, 0, "b"}}), + toReplacements({{"", 1, 4, "cccc"}})); + mergeAndTestRewrite("aaaaaa", "abbbba", "abba", + toReplacements({{"", 1, 4, "bbbb"}}), + toReplacements({{"", 2, 2, ""}})); + mergeAndTestRewrite("aaaa", "aa", "cc", + toReplacements({{"", 1, 1, ""}, {"", 2, 1, ""}}), + toReplacements({{"", 0, 2, "cc"}})); + mergeAndTestRewrite("aa", "abbba", "abcbcba", + toReplacements({{"", 1, 0, "bbb"}}), + toReplacements({{"", 2, 0, "c"}, {"", 3, 0, "c"}})); + + mergeAndTestRewrite( + "aaa", "abbab", "ccdd", + toReplacements({{"", 0, 1, ""}, {"", 2, 0, "bb"}, {"", 3, 0, "b"}}), + toReplacements({{"", 0, 2, "cc"}, {"", 2, 3, "dd"}})); + mergeAndTestRewrite( + "aa", "babbab", "ccdd", + toReplacements({{"", 0, 0, "b"}, {"", 1, 0, "bb"}, {"", 2, 0, "b"}}), + toReplacements({{"", 0, 3, "cc"}, {"", 3, 3, "dd"}})); } } // end namespace tooling Index: unittests/Tooling/ReplacementTest.h =================================================================== --- /dev/null +++ unittests/Tooling/ReplacementTest.h @@ -0,0 +1,56 @@ +//===- unittest/Tooling/ReplacementTest.h - Replacements related test------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines utility class and function for Replacement related tests. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_UNITTESTS_TOOLING_REPLACEMENTTESTBASE_H +#define LLVM_CLANG_UNITTESTS_TOOLING_REPLACEMENTTESTBASE_H + +#include "RewriterTestContext.h" +#include "clang/Tooling/Core/Replacement.h" +#include "gtest/gtest.h" + +namespace clang { +namespace tooling { + +/// \brief Converts a set of replacements to Replacements class. +/// \return A Replacements class containing \p Replaces on success; otherwise, +/// an empty Replacements is returned. +static tooling::Replacements +toReplacements(const std::set &Replaces) { + tooling::Replacements Result; + for (const auto &R : Replaces) { + auto Err = Result.add(R); + EXPECT_TRUE(!Err); + if (Err) { + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + return tooling::Replacements(); + } + } + return Result; +} + +/// \brief A utility class for replacement related tests. +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; +}; + +} // namespace tooling +} // namespace clang + +#endif // LLVM_CLANG_UNITTESTS_TOOLING_REPLACEMENTTESTBASE_H Index: unittests/Tooling/RewriterTest.cpp =================================================================== --- unittests/Tooling/RewriterTest.cpp +++ unittests/Tooling/RewriterTest.cpp @@ -39,8 +39,11 @@ TEST(Rewriter, AdjacentInsertAndDelete) { Replacements Replaces; - Replaces.insert(Replacement("", 6, 6, "")); - Replaces.insert(Replacement("", 6, 0, "replaced\n")); + auto Err = Replaces.add(Replacement("", 6, 6, "")); + EXPECT_TRUE(!Err); + Replaces = + Replaces.merge(Replacements(Replacement("", 6, 0, "replaced\n"))); + auto Rewritten = applyAllReplacements("line1\nline2\nline3\nline4", Replaces); EXPECT_TRUE(static_cast(Rewritten)); EXPECT_EQ("line1\nreplaced\nline3\nline4", *Rewritten);