Index: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h =================================================================== --- clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h +++ clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h @@ -18,6 +18,7 @@ #include "clang/Tooling/Core/Diagnostic.h" #include "clang/Tooling/Refactoring.h" +#include "clang/Tooling/Refactoring/AtomicChange.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include @@ -29,15 +30,8 @@ class DiagnosticsEngine; class Rewriter; -namespace format { -struct FormatStyle; -} // end namespace format - namespace replace { -/// \brief Collection of source ranges. -typedef std::vector RangeVector; - /// \brief Collection of TranslationUnitReplacements. typedef std::vector TUReplacements; @@ -47,10 +41,10 @@ /// \brief Collection of TranslationUniDiagnostics. typedef std::vector TUDiagnostics; -/// \brief Map mapping file name to Replacements targeting that file. +/// \brief Map mapping file name to a set of AtomicChange targeting that file. typedef llvm::DenseMap> - FileToReplacementsMap; + std::vector> + FileToChangesMap; /// \brief Recursively descends through a directory structure rooted at \p /// Directory and attempts to deserialize *.yaml files as @@ -77,65 +71,36 @@ const llvm::StringRef Directory, TUDiagnostics &TUs, TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics); -/// \brief Deduplicate, check for conflicts, and apply all Replacements stored -/// in \c TUs. If conflicts occur, no Replacements are applied. -/// -/// \post For all (key,value) in GroupedReplacements, value[i].getOffset() <= -/// value[i+1].getOffset(). +/// \brief Deduplicate, check for conflicts, and convert all Replacements stored +/// in \c TUs to AtomicChange. If conflicts occur, no Replacements are applied. /// /// \param[in] TUs Collection of TranslationUnitReplacements or -/// TranslationUnitDiagnostics to merge, -/// deduplicate, and test for conflicts. -/// \param[out] GroupedReplacements Container grouping all Replacements by the +/// TranslationUnitDiagnostics to merge, deduplicate, and test for conflicts. +/// \param[out] FileChanges Container grouping all changes by the /// file they target. /// \param[in] SM SourceManager required for conflict reporting. /// /// \returns \parblock -/// \li true If all changes were applied successfully. +/// \li true If all changes were converted successfully. /// \li false If there were conflicts. -bool mergeAndDeduplicate(const TUReplacements &TUs, - FileToReplacementsMap &GroupedReplacements, +bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs, + FileToChangesMap &FileChanges, clang::SourceManager &SM); -bool mergeAndDeduplicate(const TUDiagnostics &TUs, - FileToReplacementsMap &GroupedReplacements, - clang::SourceManager &SM); - -// FIXME: Remove this function after changing clang-apply-replacements to use -// Replacements class. -bool applyAllReplacements(const std::vector &Replaces, - Rewriter &Rewrite); - -/// \brief Apply all replacements in \c GroupedReplacements. +/// \brief Apply \c AtomicChange on File and rewrite it. /// -/// \param[in] GroupedReplacements Deduplicated and conflict free Replacements -/// to apply. -/// \param[out] Rewrites The results of applying replacements will be applied -/// to this Rewriter. +/// \param[in] File Path of the file where to apply AtomicChange. +/// \param[in] Changes to apply. +/// \param[in] SM Source Manager where to read the file. +/// \param[in] Spec For code cleanup and formatting. /// /// \returns \parblock -/// \li true If all changes were applied successfully. -/// \li false If a replacement failed to apply. -bool applyReplacements(const FileToReplacementsMap &GroupedReplacements, - clang::Rewriter &Rewrites); - -/// \brief Given a collection of Replacements for a single file, produces a list -/// of source ranges that enclose those Replacements. -/// -/// \pre Replacements[i].getOffset() <= Replacements[i+1].getOffset(). -/// -/// \param[in] Replacements Replacements from a single file. -/// -/// \returns Collection of source ranges that enclose all given Replacements. -/// One range is created for each replacement. -RangeVector calculateChangedRanges( - const std::vector &Replacements); - -/// \brief Write the contents of \c FileContents to disk. Keys of the map are -/// filenames and values are the new contents for those files. -/// -/// \param[in] Rewrites Rewriter containing written files to write to disk. -bool writeFiles(const clang::Rewriter &Rewrites); +/// \li true if all changes applied successfully. +/// \li false if something wrong occurs. +bool applyChanges(StringRef File, + const std::vector &Changes, + const SourceManager &SM, + const tooling::ApplyChangesSpec &Spec); /// \brief Delete the replacement files. /// Index: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp =================================================================== --- clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -80,10 +80,9 @@ return ErrorCode; } -std::error_code -collectReplacementsFromDirectory(const llvm::StringRef Directory, - TUDiagnostics &TUs, TUReplacementFiles &TUFiles, - clang::DiagnosticsEngine &Diagnostics) { +std::error_code collectReplacementsFromDirectory( + const llvm::StringRef Directory, TUDiagnostics &TUs, + TUReplacementFiles &TUFiles, clang::DiagnosticsEngine &Diagnostics) { using namespace llvm::sys::fs; using namespace llvm::sys::path; @@ -125,23 +124,21 @@ return ErrorCode; } -/// \brief Dumps information for a sequence of conflicting Replacements. +/// \brief Dumps information for a conflicting Replacement. /// -/// \param[in] File FileEntry for the file the conflicting Replacements are +/// \param[in] File FileEntry for the file the conflicting Replacement are /// for. -/// \param[in] ConflictingReplacements List of conflicting Replacements. +/// \param[in] Conflict A conflicting Replacement. /// \param[in] SM SourceManager used for reporting. -static void reportConflict( - const FileEntry *File, - const llvm::ArrayRef ConflictingReplacements, - SourceManager &SM) { +static void reportConflict(const FileEntry *File, + const tooling::ReplacementError &Conflict, + SourceManager &SM) { FileID FID = SM.translateFile(File); if (FID.isInvalid()) FID = SM.createFileID(File, SourceLocation(), SrcMgr::C_User); - // FIXME: Output something a little more user-friendly (e.g. unified diff?) - errs() << "The following changes conflict:\n"; - for (const tooling::Replacement &R : ConflictingReplacements) { + + auto PrintConflit = [&](const tooling::Replacement &R) { if (R.getLength() == 0) { errs() << " Insert at " << SM.getLineNumber(FID, R.getOffset()) << ":" << SM.getColumnNumber(FID, R.getOffset()) << " " @@ -162,221 +159,244 @@ else errs() << " with \"" << R.getReplacementText() << "\"\n"; } - } -} - -// FIXME: Remove this function after changing clang-apply-replacements to use -// Replacements class. -bool applyAllReplacements(const std::vector &Replaces, - Rewriter &Rewrite) { - bool Result = true; - for (auto I = Replaces.begin(), E = Replaces.end(); I != E; ++I) { - if (I->isApplicable()) { - Result = I->apply(Rewrite) && Result; - } else { - Result = false; - } - } - return Result; -} - -// FIXME: moved from libToolingCore. remove this when std::vector -// is replaced with tooling::Replacements class. -static void deduplicate(std::vector &Replaces, - std::vector &Conflicts) { - if (Replaces.empty()) - return; - - auto LessNoPath = [](const tooling::Replacement &LHS, - const tooling::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 tooling::Replacement &LHS, - const tooling::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 - tooling::Range ConflictRange(Replaces.front().getOffset(), - Replaces.front().getLength()); - unsigned ConflictStart = 0; - unsigned ConflictLength = 1; - for (unsigned i = 1; i < Replaces.size(); ++i) { - tooling::Range Current(Replaces[i].getOffset(), Replaces[i].getLength()); - if (ConflictRange.overlapsWith(Current)) { - // Extend conflicted range - ConflictRange = - tooling::Range(ConflictRange.getOffset(), - std::max(ConflictRange.getLength(), - Current.getOffset() + Current.getLength() - - ConflictRange.getOffset())); - ++ConflictLength; - } else { - if (ConflictLength > 1) - Conflicts.push_back(tooling::Range(ConflictStart, ConflictLength)); - ConflictRange = Current; - ConflictStart = i; - ConflictLength = 1; - } - } - - if (ConflictLength > 1) - Conflicts.push_back(tooling::Range(ConflictStart, ConflictLength)); + errs() << "The following changes conflict:\n"; + if (auto R = Conflict.getExistingReplacement()) + PrintConflit(*R); + if (auto R = Conflict.getNewReplacement()) + PrintConflit(*R); } -/// \brief Deduplicates and tests for conflicts among the replacements for each -/// file in \c Replacements. Any conflicts found are reported. -/// -/// \post Replacements[i].getOffset() <= Replacements[i+1].getOffset(). -/// -/// \param[in,out] Replacements Container of all replacements grouped by file -/// to be deduplicated and checked for conflicts. -/// \param[in] SM SourceManager required for conflict reporting. +/// \brief Dumps information for all conflicting replacements per file. /// -/// \returns \parblock -/// \li true if conflicts were detected -/// \li false if no conflicts were detected -static bool deduplicateAndDetectConflicts(FileToReplacementsMap &Replacements, - SourceManager &SM) { - bool conflictsFound = false; - - for (auto &FileAndReplacements : Replacements) { - const FileEntry *Entry = FileAndReplacements.first; - auto &Replacements = FileAndReplacements.second; - assert(Entry != nullptr && "No file entry!"); - - std::vector Conflicts; - deduplicate(FileAndReplacements.second, Conflicts); - - if (Conflicts.empty()) - continue; - - conflictsFound = true; - +/// \param[in] Conflicts Map where FileEntry is the file the conflicting +/// Replacement are for. +/// \param[in] SM SourceManager used for reporting. +static void reportConflicts( + const llvm::DenseMap> &Conflicts, + SourceManager &SM) { + for (const auto &FileConflicts : Conflicts) { + const FileEntry *Entry = FileConflicts.first; errs() << "There are conflicting changes to " << Entry->getName() << ":\n"; - - for (const tooling::Range &Conflict : Conflicts) { - auto ConflictingReplacements = llvm::makeArrayRef( - &Replacements[Conflict.getOffset()], Conflict.getLength()); - reportConflict(Entry, ConflictingReplacements, SM); - } + for (const auto &Conflict : FileConflicts.second) + reportConflict(Entry, Conflict, SM); } - - return conflictsFound; } -bool mergeAndDeduplicate(const TUReplacements &TUs, - FileToReplacementsMap &GroupedReplacements, - clang::SourceManager &SM) { - - // Group all replacements by target file. +static llvm::DenseMap> +groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, + FileToChangesMap &FileChanges, clang::SourceManager &SM) { std::set Warned; + llvm::DenseMap> + GroupedReplacements; + for (const auto &TU : TUs) { for (const tooling::Replacement &R : TU.Replacements) { // Use the file manager to deduplicate paths. FileEntries are // automatically canonicalized. - if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) { + if (const FileEntry *Entry = + SM.getFileManager().getFile(R.getFilePath())) { GroupedReplacements[Entry].push_back(R); } else if (Warned.insert(R.getFilePath()).second) { - errs() << "Described file '" << R.getFilePath() - << "' doesn't exist. Ignoring...\n"; + errs() << "Described file '" << R.getFilePath() + << "' doesn't exist. Ignoring...\n"; } } } - // Ask clang to deduplicate and report conflicts. - return !deduplicateAndDetectConflicts(GroupedReplacements, SM); -} - -bool mergeAndDeduplicate(const TUDiagnostics &TUs, - FileToReplacementsMap &GroupedReplacements, - clang::SourceManager &SM) { - - // Group all replacements by target file. - std::set Warned; - for (const auto &TU : TUs) { + for (const auto &TU : TUDs) { for (const auto &D : TU.Diagnostics) { for (const auto &Fix : D.Fix) { for (const tooling::Replacement &R : Fix.second) { // Use the file manager to deduplicate paths. FileEntries are // automatically canonicalized. - if (const FileEntry *Entry = SM.getFileManager().getFile(R.getFilePath())) { + if (const FileEntry *Entry = + SM.getFileManager().getFile(R.getFilePath())) { GroupedReplacements[Entry].push_back(R); } else if (Warned.insert(R.getFilePath()).second) { - errs() << "Described file '" << R.getFilePath() - << "' doesn't exist. Ignoring...\n"; + errs() << "Described file '" << R.getFilePath() + << "' doesn't exist. Ignoring...\n"; } } } } } - - // Ask clang to deduplicate and report conflicts. - return !deduplicateAndDetectConflicts(GroupedReplacements, SM); + return GroupedReplacements; } -bool applyReplacements(const FileToReplacementsMap &GroupedReplacements, - clang::Rewriter &Rewrites) { +namespace { - // Apply all changes - // - // FIXME: No longer certain GroupedReplacements is really the best kind of - // data structure for applying replacements. Rewriter certainly doesn't care. - // However, until we nail down the design of ReplacementGroups, might as well - // leave this as is. - for (const auto &FileAndReplacements : GroupedReplacements) { - if (!applyAllReplacements(FileAndReplacements.second, Rewrites)) - return false; +// Carries extra error information when converting Replacements to AtomicChange. +class ConflictError : public llvm::ErrorInfo { +public: + ConflictError(const FileEntry *Entry, + std::vector Conflicts) + : Entry(Entry), Conflicts(Conflicts) {} + + void log(raw_ostream &OS) const override {} + + // Corresponding FileEntry. + const FileEntry *getEntry() const { return Entry; } + + // All the conflict in ReplacementError form. + const std::vector &getConflicts() const { + return Conflicts; } - return true; -} + static char ID; -RangeVector calculateChangedRanges( - const std::vector &Replaces) { - RangeVector ChangedRanges; - - // Generate the new ranges from the replacements. - int Shift = 0; - for (const tooling::Replacement &R : Replaces) { - unsigned Offset = R.getOffset() + Shift; - unsigned Length = R.getReplacementText().size(); - Shift += Length - R.getLength(); - ChangedRanges.push_back(tooling::Range(Offset, Length)); +private: + std::error_code convertToErrorCode() const override { + return llvm::inconvertibleErrorCode(); } - return ChangedRanges; -} + const FileEntry *Entry; + std::vector Conflicts; +}; + +char ConflictError::ID = 0; + +// Store replacements for a given file and deduplicate them using one unique +// AtomicChange. Final changes will be retrive as a set of AtomicChange or an +// error containing all conflicts. +class ReplacementsToAtomicChanges { +public: + explicit ReplacementsToAtomicChanges(clang::SourceManager &SM, + const FileEntry *Entry) + : SM(&SM), Entry(Entry), + BeginLoc(SM.getLocForStartOfFile( + SM.getOrCreateFileID(Entry, SrcMgr::C_User))), + AllChanges(Entry->getName(), Entry->getName()) {} + + // Store a replacement. Deduplication will be handle at this level. + // A replacement with a length of 0 is considered as an insertion. + void add(const tooling::Replacement &R) { + if (R.getLength() == 0) + addInsertion(R); + else + addReplacement(R); + } + + // Retrive all the stored replacement as set of AtomicChange. + // Is a conflict has been occured, Expected will contain the resulting error + // in form of ConflictError. + llvm::Expected> getChanges() const { + if (!Conflicts.empty()) + return llvm::make_error(Entry, Conflicts); + + std::vector Changes; + for (const auto &R : AllChanges.getReplacements()) { + tooling::AtomicChange Change(Entry->getName(), Entry->getName()); + if (llvm::Error Err = + Change.replace(*SM, BeginLoc.getLocWithOffset(R.getOffset()), + R.getLength(), R.getReplacementText())) + llvm_unreachable( + "Failed to add replacement when converting to AtomicChange."); + Changes.push_back(Change); + } + return Changes; + } + +private: + void addReplacement(const tooling::Replacement &R) { + if (llvm::Error Error = + AllChanges.replace(*SM, BeginLoc.getLocWithOffset(R.getOffset()), + R.getLength(), R.getReplacementText())) { + // Store conflicting replacement for errors reporting per file. + handleAllErrors(std::move(Error), + [&](const tooling::ReplacementError &Err) { + Conflicts.push_back(Err); + }); + } + } + + void addInsertion(const tooling::Replacement &R) { + if (llvm::Error Error = + AllChanges.insert(*SM, BeginLoc.getLocWithOffset(R.getOffset()), + R.getReplacementText())) { + // Store conflicting replacement for errors reporting per file. + handleAllErrors(std::move(Error), + [&](const tooling::ReplacementError &Err) { + Conflicts.push_back(Err); + }); + } + } + + clang::SourceManager *SM; + const FileEntry *Entry; + const SourceLocation BeginLoc; + tooling::AtomicChange AllChanges; + std::vector Conflicts; +}; + +} // namespace -bool writeFiles(const clang::Rewriter &Rewrites) { +bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs, + FileToChangesMap &FileChanges, + clang::SourceManager &SM) { + + llvm::DenseMap> + GroupedReplacements = groupReplacements(TUs, TUDs, FileChanges, SM); - for (auto BufferI = Rewrites.buffer_begin(), BufferE = Rewrites.buffer_end(); - BufferI != BufferE; ++BufferI) { - StringRef FileName = - Rewrites.getSourceMgr().getFileEntryForID(BufferI->first)->getName(); + llvm::DenseMap> + Conflicts; - std::error_code EC; - llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::F_Text); - if (EC) { - errs() << "Warning: Could not write to " << EC.message() << "\n"; + // To keep current clang-apply-replacement behavior, report conflicts per file + // and skip file containing conflicts, all replacements are store into 1 big + // AtomicChange. + // Once done and if no conflicts have been occured, a set of AtomicChange is + // generated else conflicts are stored for reporting. + for (const auto &FileAndReplacements : GroupedReplacements) { + const FileEntry *Entry = FileAndReplacements.first; + ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry); + for (const auto &R : FileAndReplacements.second) + DeduplicatedChanges.add(R); + + llvm::Expected> Changes = + DeduplicatedChanges.getChanges(); + if (!Changes) { + handleAllErrors(Changes.takeError(), [&](const ConflictError &Error) { + Conflicts.insert( + std::make_pair(Error.getEntry(), Error.getConflicts())); + }); continue; } - BufferI->second.write(FileStream); + FileChanges.insert(std::make_pair(Entry, *Changes)); } + reportConflicts(Conflicts, SM); + return Conflicts.empty(); +} +bool applyChanges(StringRef File, + const std::vector &Changes, + const SourceManager &SM, + const tooling::ApplyChangesSpec &Spec) { + + llvm::ErrorOr> Buffer = + SM.getFileManager().getBufferForFile(File); + if (!Buffer) { + errs() << "Can't get buffer for file " << File << ": " + << Buffer.getError().message() << "\n"; + return false; + } + + StringRef Code = Buffer.get()->getBuffer(); + llvm::Expected NewCode = + tooling::applyAtomicChanges(File, Code, Changes, Spec); + if (!NewCode) { + errs() << llvm::toString(NewCode.takeError()) << "\n"; + return false; + } + // Write new file to disk + std::error_code EC; + llvm::raw_fd_ostream FileStream(File, EC, llvm::sys::fs::F_None); + if (EC) { + errs() << "Could not open " << File << " for writing\n"; + return false; + } + FileStream << *NewCode; return true; } Index: clang-apply-replacements/tool/CMakeLists.txt =================================================================== --- clang-apply-replacements/tool/CMakeLists.txt +++ clang-apply-replacements/tool/CMakeLists.txt @@ -12,6 +12,7 @@ clangFormat clangRewrite clangToolingCore + clangToolingRefactor ) install(TARGETS clang-apply-replacements Index: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp =================================================================== --- clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp @@ -1,4 +1,4 @@ -//===-- ClangApplyReplacementsMain.cpp - Main file for the tool -----------===// + //===-- ClangApplyReplacementsMain.cpp - Main file for the tool -----------===// // // The LLVM Compiler Infrastructure // @@ -86,116 +86,6 @@ OS << "clang-apply-replacements version " CLANG_VERSION_STRING << "\n"; } -/// \brief Convenience function to get rewritten content for \c Filename from -/// \c Rewrites. -/// -/// \pre Replacements[i].getFilePath() == Replacements[i+1].getFilePath(). -/// \post Replacements.empty() -> Result.empty() -/// -/// \param[in] Replacements Replacements to apply -/// \param[in] Rewrites Rewriter to use to apply replacements. -/// \param[out] Result Contents of the file after applying replacements if -/// replacements were provided. -/// -/// \returns \parblock -/// \li true if all replacements were applied successfully. -/// \li false if at least one replacement failed to apply. -static bool -getRewrittenData(const std::vector &Replacements, - Rewriter &Rewrites, std::string &Result) { - if (Replacements.empty()) - return true; - - if (!applyAllReplacements(Replacements, Rewrites)) - return false; - - SourceManager &SM = Rewrites.getSourceMgr(); - FileManager &Files = SM.getFileManager(); - - StringRef FileName = Replacements.begin()->getFilePath(); - const clang::FileEntry *Entry = Files.getFile(FileName); - assert(Entry && "Expected an existing file"); - FileID ID = SM.translateFile(Entry); - assert(ID.isValid() && "Expected a valid FileID"); - const RewriteBuffer *Buffer = Rewrites.getRewriteBufferFor(ID); - Result = std::string(Buffer->begin(), Buffer->end()); - - return true; -} - -/// \brief Apply \c Replacements and return the new file contents. -/// -/// \pre Replacements[i].getFilePath() == Replacements[i+1].getFilePath(). -/// \post Replacements.empty() -> Result.empty() -/// -/// \param[in] Replacements Replacements to apply. -/// \param[out] Result Contents of the file after applying replacements if -/// replacements were provided. -/// \param[in] Diagnostics For diagnostic output. -/// -/// \returns \parblock -/// \li true if all replacements applied successfully. -/// \li false if at least one replacement failed to apply. -static bool -applyReplacements(const std::vector &Replacements, - std::string &Result, DiagnosticsEngine &Diagnostics) { - FileManager Files((FileSystemOptions())); - SourceManager SM(Diagnostics, Files); - Rewriter Rewrites(SM, LangOptions()); - - return getRewrittenData(Replacements, Rewrites, Result); -} - -/// \brief Apply code formatting to all places where replacements were made. -/// -/// \pre !Replacements.empty(). -/// \pre Replacements[i].getFilePath() == Replacements[i+1].getFilePath(). -/// \pre Replacements[i].getOffset() <= Replacements[i+1].getOffset(). -/// -/// \param[in] Replacements Replacements that were made to the file. Provided -/// to indicate where changes were made. -/// \param[in] FileData The contents of the file \b after \c Replacements have -/// been applied. -/// \param[out] FormattedFileData The contents of the file after reformatting. -/// \param[in] FormatStyle Style to apply. -/// \param[in] Diagnostics For diagnostic output. -/// -/// \returns \parblock -/// \li true if reformatting replacements were all successfully -/// applied. -/// \li false if at least one reformatting replacement failed to apply. -static bool -applyFormatting(const std::vector &Replacements, - const StringRef FileData, std::string &FormattedFileData, - const format::FormatStyle &FormatStyle, - DiagnosticsEngine &Diagnostics) { - assert(!Replacements.empty() && "Need at least one replacement"); - - RangeVector Ranges = calculateChangedRanges(Replacements); - - StringRef FileName = Replacements.begin()->getFilePath(); - tooling::Replacements R = - format::reformat(FormatStyle, FileData, Ranges, FileName); - - // FIXME: Remove this copy when tooling::Replacements is implemented as a - // vector instead of a set. - std::vector FormattingReplacements; - std::copy(R.begin(), R.end(), back_inserter(FormattingReplacements)); - - if (FormattingReplacements.empty()) { - FormattedFileData = FileData; - return true; - } - - FileManager Files((FileSystemOptions())); - SourceManager SM(Diagnostics, Files); - SM.overrideFileContents(Files.getFile(FileName), - llvm::MemoryBuffer::getMemBufferCopy(FileData)); - Rewriter Rewrites(SM, LangOptions()); - - return getRewrittenData(FormattingReplacements, Rewrites, FormattedFileData); -} - int main(int argc, char **argv) { cl::HideUnrelatedOptions(makeArrayRef(VisibleCategories)); @@ -244,46 +134,19 @@ FileManager Files((FileSystemOptions())); SourceManager SM(Diagnostics, Files); - FileToReplacementsMap GroupedReplacements; - if (!mergeAndDeduplicate(TURs, GroupedReplacements, SM)) - return 1; - if (!mergeAndDeduplicate(TUDs, GroupedReplacements, SM)) + FileToChangesMap Changes; + if (!mergeAndDeduplicate(TURs, TUDs, Changes, SM)) return 1; - Rewriter ReplacementsRewriter(SM, LangOptions()); - - for (const auto &FileAndReplacements : GroupedReplacements) { - // This shouldn't happen but if a file somehow has no replacements skip to - // next file. - if (FileAndReplacements.second.empty()) - continue; - - std::string NewFileData; - StringRef FileName = FileAndReplacements.first->getName(); - if (!applyReplacements(FileAndReplacements.second, NewFileData, - Diagnostics)) { - errs() << "Failed to apply replacements to " << FileName << "\n"; - continue; - } - - // Apply formatting if requested. - if (DoFormat && - !applyFormatting(FileAndReplacements.second, NewFileData, NewFileData, - FormatStyle, Diagnostics)) { - errs() << "Failed to apply reformatting replacements for " << FileName - << "\n"; - continue; - } - - // Write new file to disk - std::error_code EC; - llvm::raw_fd_ostream FileStream(FileName, EC, llvm::sys::fs::F_None); - if (EC) { - llvm::errs() << "Could not open " << FileName << " for writing\n"; - continue; - } + tooling::ApplyChangesSpec Spec; + Spec.Cleanup = true; + Spec.Style = FormatStyle; + Spec.Format = DoFormat ? tooling::ApplyChangesSpec::kAll + : tooling::ApplyChangesSpec::kNone; - FileStream << NewFileData; + for (const auto &FileChange : Changes) { + const FileEntry* Entry = FileChange.first; + applyChanges(Entry->getName(), FileChange.second, SM, Spec); } return 0; Index: test/clang-apply-replacements/Inputs/basic/basic.h =================================================================== --- test/clang-apply-replacements/Inputs/basic/basic.h +++ test/clang-apply-replacements/Inputs/basic/basic.h @@ -10,7 +10,7 @@ class Derived : public Parent { public: virtual void func() {} - // CHECK: virtual void func() override {} + // CHECK: virtual void func() noexcept override {} }; extern void ext(int (&)[5], const Parent &); Index: test/clang-apply-replacements/Inputs/basic/file1.yaml =================================================================== --- test/clang-apply-replacements/Inputs/basic/file1.yaml +++ test/clang-apply-replacements/Inputs/basic/file1.yaml @@ -21,5 +21,5 @@ - FilePath: $(path)/../basic/basic.h Offset: 148 Length: 0 - ReplacementText: 'override ' + ReplacementText: 'noexcept ' ... Index: test/clang-apply-replacements/Inputs/basic/file2.yaml =================================================================== --- test/clang-apply-replacements/Inputs/basic/file2.yaml +++ test/clang-apply-replacements/Inputs/basic/file2.yaml @@ -10,4 +10,8 @@ Offset: 148 Length: 0 ReplacementText: 'override ' + - FilePath: $(path)/../basic/basic.h + Offset: 298 + Length: 1 + ReplacementText: elem ... Index: test/clang-apply-replacements/Inputs/conflict/expected.txt =================================================================== --- test/clang-apply-replacements/Inputs/conflict/expected.txt +++ test/clang-apply-replacements/Inputs/conflict/expected.txt @@ -3,9 +3,11 @@ Replace 8:8-8:33 with "auto & i : ints" Replace 8:8-8:33 with "int & elem : ints" The following changes conflict: - Replace 9:5-9:11 with "elem" Replace 9:5-9:11 with "i" + Replace 9:5-9:11 with "elem" The following changes conflict: Remove 12:3-12:14 - Insert at 12:12 (int*) Replace 12:12-12:12 with "nullptr" +The following changes conflict: + Remove 12:3-12:14 + Insert at 12:12 (int*) Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp =================================================================== --- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp +++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp @@ -9,6 +9,7 @@ //===----------------------------------------------------------------------===// #include "clang-apply-replacements/Tooling/ApplyReplacements.h" +#include "clang/Format/Format.h" #include "gtest/gtest.h" using namespace clang::replace; @@ -41,11 +42,12 @@ IntrusiveRefCntPtr(new DiagnosticIDs()), DiagOpts.get()); FileManager Files((FileSystemOptions())); SourceManager SM(Diagnostics, Files); + TUReplacements TURs; TUDiagnostics TUs = makeTUDiagnostics("path/to/source.cpp", "diagnostic", {}, {}, "path/to"); - FileToReplacementsMap ReplacementsMap; + FileToChangesMap ReplacementsMap; - EXPECT_TRUE(mergeAndDeduplicate(TUs, ReplacementsMap, SM)); + EXPECT_TRUE(mergeAndDeduplicate(TURs, TUs, ReplacementsMap, SM)); EXPECT_TRUE(ReplacementsMap.empty()); } Index: unittests/clang-apply-replacements/CMakeLists.txt =================================================================== --- unittests/clang-apply-replacements/CMakeLists.txt +++ unittests/clang-apply-replacements/CMakeLists.txt @@ -9,7 +9,6 @@ add_extra_unittest(ClangApplyReplacementsTests ApplyReplacementsTest.cpp - ReformattingTest.cpp ) target_link_libraries(ClangApplyReplacementsTests @@ -17,4 +16,5 @@ clangApplyReplacements clangBasic clangToolingCore + clangToolingRefactor ) Index: unittests/clang-apply-replacements/ReformattingTest.cpp =================================================================== --- unittests/clang-apply-replacements/ReformattingTest.cpp +++ /dev/null @@ -1,67 +0,0 @@ -//===- clang-apply-replacements/ReformattingTest.cpp ----------------------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "clang-apply-replacements/Tooling/ApplyReplacements.h" -#include "common/VirtualFileHelper.h" -#include "clang/Format/Format.h" -#include "clang/Tooling/Refactoring.h" -#include "gtest/gtest.h" - -using namespace clang; -using namespace clang::tooling; -using namespace clang::replace; - -typedef std::vector ReplacementsVec; - -static Replacement makeReplacement(unsigned Offset, unsigned Length, - unsigned ReplacementLength, - llvm::StringRef FilePath = "") { - return Replacement(FilePath, Offset, Length, - std::string(ReplacementLength, '~')); -} - -// generate a set of replacements containing one element -static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length, - unsigned ReplacementLength, - llvm::StringRef FilePath = "~") { - ReplacementsVec Replaces; - Replaces.push_back( - makeReplacement(Offset, Length, ReplacementLength, FilePath)); - return Replaces; -} - -// Put these functions in the clang::tooling namespace so arg-dependent name -// lookup finds these functions for the EXPECT_EQ macros below. -namespace clang { -namespace tooling { - -std::ostream &operator<<(std::ostream &os, const Range &R) { - return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")"; -} - -} // namespace tooling -} // namespace clang - -// Ensure zero-length ranges are produced. Even lines where things are deleted -// need reformatting. -TEST(CalculateChangedRangesTest, producesZeroLengthRange) { - RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0)); - EXPECT_EQ(Range(0, 0), Changes.front()); -} - -// Basic test to ensure replacements turn into ranges properly. -TEST(CalculateChangedRangesTest, calculatesRanges) { - ReplacementsVec R; - R.push_back(makeReplacement(2, 0, 3)); - R.push_back(makeReplacement(5, 2, 4)); - RangeVector Changes = calculateChangedRanges(R); - - Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) }; - EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges)); -}