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,35 @@ 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. -/// -/// \param[in] GroupedReplacements Deduplicated and conflict free Replacements -/// to apply. -/// \param[out] Rewrites The results of applying replacements will be applied -/// to this Rewriter. -/// -/// \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(). +/// \brief Apply \c AtomicChange on File and rewrite it. /// -/// \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] File Path of the file where to apply AtomicChange. +/// \param[in] Changes to apply. +/// \param[in] Spec For code cleanup and formatting. +/// \param[in] Diagnostics DiagnosticsEngine used for error output. /// -/// \param[in] Rewrites Rewriter containing written files to write to disk. -bool writeFiles(const clang::Rewriter &Rewrites); +/// \returns The changed code if all changes are applied successfully; +/// otherwise, an llvm::Error carrying llvm::StringError or an error_code. +llvm::Expected +applyChanges(StringRef File, const std::vector &Changes, + const tooling::ApplyChangesSpec &Spec, + DiagnosticsEngine &Diagnostics); /// \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,259 +124,108 @@ return ErrorCode; } -/// \brief Dumps information for a sequence of conflicting Replacements. +/// \brief Extract replacements from collected TranslationUnitReplacements and +/// TranslationUnitDiagnostics and group them per file. /// -/// \param[in] File FileEntry for the file the conflicting Replacements are -/// for. -/// \param[in] ConflictingReplacements List of conflicting Replacements. -/// \param[in] SM SourceManager used for reporting. -static void reportConflict( - const FileEntry *File, - const llvm::ArrayRef ConflictingReplacements, - 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) { - if (R.getLength() == 0) { - errs() << " Insert at " << SM.getLineNumber(FID, R.getOffset()) << ":" - << SM.getColumnNumber(FID, R.getOffset()) << " " - << R.getReplacementText() << "\n"; - } else { - if (R.getReplacementText().empty()) - errs() << " Remove "; - else - errs() << " Replace "; - - errs() << SM.getLineNumber(FID, R.getOffset()) << ":" - << SM.getColumnNumber(FID, R.getOffset()) << "-" - << SM.getLineNumber(FID, R.getOffset() + R.getLength() - 1) << ":" - << SM.getColumnNumber(FID, R.getOffset() + R.getLength() - 1); - - if (R.getReplacementText().empty()) - errs() << "\n"; - 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; +/// \param[in] TUs Collection of all found and deserialized +/// TranslationUnitReplacements. +/// \param[in] TUDs Collection of all found and deserialized +/// TranslationUnitDiagnostics. +/// \param[in] SM Used to deduplicate paths. +/// +/// \returns A map mapping FileEntry to a set of Replacement targeting that +/// file. +static llvm::DenseMap> +groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, + const clang::SourceManager &SM) { + std::set Warned; + llvm::DenseMap> + GroupedReplacements; + + auto AddToGroup = [&](const tooling::Replacement &R) { + // Use the file manager to deduplicate paths. FileEntries are + // automatically canonicalized. + 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"; } - } - 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)); -} - -/// \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. -/// -/// \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; + for (const auto &TU : TUs) + for (const tooling::Replacement &R : TU.Replacements) + AddToGroup(R); - errs() << "There are conflicting changes to " << Entry->getName() << ":\n"; + for (const auto &TU : TUDs) + for (const auto &D : TU.Diagnostics) + for (const auto &Fix : D.Fix) + for (const tooling::Replacement &R : Fix.second) + AddToGroup(R); - for (const tooling::Range &Conflict : Conflicts) { - auto ConflictingReplacements = llvm::makeArrayRef( - &Replacements[Conflict.getOffset()], Conflict.getLength()); - reportConflict(Entry, ConflictingReplacements, SM); - } - } - - return conflictsFound; + return GroupedReplacements; } -bool mergeAndDeduplicate(const TUReplacements &TUs, - FileToReplacementsMap &GroupedReplacements, +bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs, + FileToChangesMap &FileChanges, clang::SourceManager &SM) { - // Group all replacements by target file. - std::set Warned; - 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())) { - GroupedReplacements[Entry].push_back(R); - } else if (Warned.insert(R.getFilePath()).second) { - errs() << "Described file '" << R.getFilePath() - << "' doesn't exist. Ignoring...\n"; - } - } - } + llvm::DenseMap> + GroupedReplacements = groupReplacements(TUs, TUDs, SM); - // Ask clang to deduplicate and report conflicts. - return !deduplicateAndDetectConflicts(GroupedReplacements, SM); -} - -bool mergeAndDeduplicate(const TUDiagnostics &TUs, - FileToReplacementsMap &GroupedReplacements, - clang::SourceManager &SM) { + bool ConflictDetected = false; - // Group all replacements by target file. - std::set Warned; - for (const auto &TU : TUs) { - 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())) { - GroupedReplacements[Entry].push_back(R); - } else if (Warned.insert(R.getFilePath()).second) { - errs() << "Described file '" << R.getFilePath() - << "' doesn't exist. Ignoring...\n"; - } - } + // To keep current clang-apply-replacement behavior, report conflicting + // replacements skip file containing conflicts, all replacements are stored + // into 1 big AtomicChange. + for (const auto &FileAndReplacements : GroupedReplacements) { + const FileEntry *Entry = FileAndReplacements.first; + const SourceLocation BeginLoc = + SM.getLocForStartOfFile(SM.getOrCreateFileID(Entry, SrcMgr::C_User)); + tooling::AtomicChange FileChange(Entry->getName(), Entry->getName()); + for (const auto &R : FileAndReplacements.second) { + llvm::Error Err = + FileChange.replace(SM, BeginLoc.getLocWithOffset(R.getOffset()), + R.getLength(), R.getReplacementText()); + if (Err) { + // FIXME: This will report conflicts by pair using a file+offset format + // which is not so much human readable. + // A first improvement could be to translate offset to line+col. For + // this and without loosing error message some modifications arround + // `tooling::ReplacementError` are need (access to + // `getReplacementErrString`). + // A better strategy could be to add a pretty printer methods for + // conflict reporting. Methods that could be parameterized to report a + // conflict in different format, file+offset, file+line+col, or even + // more human readable using VCS conflict markers. + // For now, printing directly the error reported by `AtomicChange` is + // the easiest solution. + errs() << llvm::toString(std::move(Err)) << "\n"; + ConflictDetected = true; } } + FileChanges.try_emplace(Entry, + std::vector{FileChange}); } - // Ask clang to deduplicate and report conflicts. - return !deduplicateAndDetectConflicts(GroupedReplacements, SM); + return !ConflictDetected; } -bool applyReplacements(const FileToReplacementsMap &GroupedReplacements, - clang::Rewriter &Rewrites) { +llvm::Expected +applyChanges(StringRef File, const std::vector &Changes, + const tooling::ApplyChangesSpec &Spec, + DiagnosticsEngine &Diagnostics) { - // 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; - } - - return true; -} + FileManager Files((FileSystemOptions())); + SourceManager SM(Diagnostics, Files); -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)); - } - - return ChangedRanges; -} - -bool writeFiles(const clang::Rewriter &Rewrites) { - - for (auto BufferI = Rewrites.buffer_begin(), BufferE = Rewrites.buffer_end(); - BufferI != BufferE; ++BufferI) { - StringRef FileName = - Rewrites.getSourceMgr().getFileEntryForID(BufferI->first)->getName(); - - 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"; - continue; - } - BufferI->second.write(FileStream); - } + llvm::ErrorOr> Buffer = + SM.getFileManager().getBufferForFile(File); + if (!Buffer) + return errorCodeToError(Buffer.getError()); - return true; + StringRef Code = Buffer.get()->getBuffer(); + return tooling::applyAtomicChanges(File, Code, Changes, Spec); } bool deleteReplacementFiles(const TUReplacementFiles &Files, 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 @@ -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,34 +134,23 @@ 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"; + tooling::ApplyChangesSpec Spec; + Spec.Cleanup = true; + Spec.Style = FormatStyle; + Spec.Format = DoFormat ? tooling::ApplyChangesSpec::kAll + : tooling::ApplyChangesSpec::kNone; + + for (const auto &FileChange : Changes) { + const FileEntry *Entry = FileChange.first; + StringRef FileName = Entry->getName(); + llvm::Expected NewFileData = + applyChanges(FileName, FileChange.second, Spec, Diagnostics); + if (!NewFileData) { + errs() << llvm::toString(NewFileData.takeError()) << "\n"; continue; } @@ -282,8 +161,7 @@ llvm::errs() << "Could not open " << FileName << " for writing\n"; continue; } - - FileStream << NewFileData; + FileStream << *NewFileData; } return 0; 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 @@ -6,8 +6,8 @@ FilePath: $(path)/basic.h FileOffset: 148 Replacements: - - FilePath: $(path)/basic.h - 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 @@ -1,11 +1,12 @@ -There are conflicting changes to $(path)/common.h: -The following changes conflict: - 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" -The following changes conflict: - Remove 12:3-12:14 - Insert at 12:12 (int*) - Replace 12:12-12:12 with "nullptr" +The new replacement overlaps with an existing replacement. +New replacement: $(path)/common.h: 106:+26:"int & elem : ints" +Existing replacement: $(path)/common.h: 106:+26:"auto & i : ints" +The new replacement overlaps with an existing replacement. +New replacement: $(path)/common.h: 140:+7:"elem" +Existing replacement: $(path)/common.h: 140:+7:"i" +The new replacement overlaps with an existing replacement. +New replacement: $(path)/common.h: 169:+1:"nullptr" +Existing replacement: $(path)/common.h: 160:+12:"" +The new replacement overlaps with an existing replacement. +New replacement: $(path)/common.h: 169:+0:"(int*)" +Existing replacement: $(path)/common.h: 160:+12:"" Index: test/clang-apply-replacements/Inputs/identical/file1.yaml =================================================================== --- /dev/null +++ test/clang-apply-replacements/Inputs/identical/file1.yaml @@ -0,0 +1,18 @@ +--- +MainSourceFile: identical.cpp +Diagnostics: + - DiagnosticName: test-identical-insertion + Message: Fix + FilePath: $(path)/identical.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/identical.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' + - FilePath: $(path)/identical.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' +... + Index: test/clang-apply-replacements/Inputs/identical/identical.cpp =================================================================== --- /dev/null +++ test/clang-apply-replacements/Inputs/identical/identical.cpp @@ -0,0 +1,2 @@ +class MyType {}; +// CHECK: class MyType00 {}; Index: test/clang-apply-replacements/Inputs/order-dependent/expected.txt =================================================================== --- /dev/null +++ test/clang-apply-replacements/Inputs/order-dependent/expected.txt @@ -0,0 +1,3 @@ +The new insertion has the same insert location as an existing replacement. +New replacement: $(path)/order-dependent.cpp: 12:+0:"1" +Existing replacement: $(path)/order-dependent.cpp: 12:+0:"0" Index: test/clang-apply-replacements/Inputs/order-dependent/file1.yaml =================================================================== --- /dev/null +++ test/clang-apply-replacements/Inputs/order-dependent/file1.yaml @@ -0,0 +1,13 @@ +--- +MainSourceFile: order-dependent.cpp +Diagnostics: + - DiagnosticName: test-order-dependent-insertion + Message: Fix + FilePath: $(path)/order-dependent.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/order-dependent.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' +... Index: test/clang-apply-replacements/Inputs/order-dependent/file2.yaml =================================================================== --- /dev/null +++ test/clang-apply-replacements/Inputs/order-dependent/file2.yaml @@ -0,0 +1,13 @@ +--- +MainSourceFile: order-dependent.cpp +Diagnostics: + - DiagnosticName: test-order-dependent-insertion + Message: Fix + FilePath: $(path)/order-dependent.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/order-dependent.cpp + Offset: 12 + Length: 0 + ReplacementText: '1' +... Index: test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp =================================================================== --- /dev/null +++ test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp @@ -0,0 +1 @@ +class MyType {}; Index: test/clang-apply-replacements/identical.cpp =================================================================== --- /dev/null +++ test/clang-apply-replacements/identical.cpp @@ -0,0 +1,5 @@ +// RUN: mkdir -p %T/Inputs/identical +// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/identical/identical.cpp > %T/Inputs/identical/identical.cpp +// RUN: sed "s#\$(path)#%/T/Inputs/identical#" %S/Inputs/identical/file1.yaml > %T/Inputs/identical/file1.yaml +// RUN: clang-apply-replacements %T/Inputs/identical +// RUN: FileCheck -input-file=%T/Inputs/identical/identical.cpp %S/Inputs/identical/identical.cpp Index: test/clang-apply-replacements/order-dependent.cpp =================================================================== --- /dev/null +++ test/clang-apply-replacements/order-dependent.cpp @@ -0,0 +1,7 @@ +// RUN: mkdir -p %T/Inputs/order-dependent +// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/order-dependent/order-dependent.cpp > %T/Inputs/order-dependent/order-dependent.cpp +// RUN: sed "s#\$(path)#%/T/Inputs/order-dependent#" %S/Inputs/order-dependent/file1.yaml > %T/Inputs/order-dependent/file1.yaml +// RUN: sed "s#\$(path)#%/T/Inputs/order-dependent#" %S/Inputs/order-dependent/file2.yaml > %T/Inputs/order-dependent/file2.yaml +// RUN: sed "s#\$(path)#%/T/Inputs/order-dependent#" %S/Inputs/order-dependent/expected.txt > %T/Inputs/order-dependent/expected.txt +// RUN: not clang-apply-replacements %T/Inputs/order-dependent > %T/Inputs/order-dependent/output.txt 2>&1 +// RUN: diff -b %T/Inputs/order-dependent/output.txt %T/Inputs/order-dependent/expected.txt 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)); -}