From 3f3a235aa2e610b5ba393228a666d55a8135ef4a Mon Sep 17 00:00:00 2001 From: Sockke Date: Mon, 30 May 2022 13:02:25 +0800 Subject: [PATCH] [clang-apply-replacements] Added an option to ignore insert conflict. If two different texts are inserted at the same offset, clang-apply-replacements prints the conflict error and discards all fixes. This patch adds support for adjusting conflict offset and keeps running to continually fix them. https://godbolt.org/z/P938EGoxj doesn't have any fixes when I run run-clang-tidy.py to generate a YAML file with clang-tidy and fix them with clang-apply-replacements. The YAML file has two different header texts insertions at the same offset, unlike clang-tidy with '-fix', clang-apply-replacements does not adjust for this conflict. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D123924 --- .../Tooling/ApplyReplacements.h | 3 ++- .../lib/Tooling/ApplyReplacements.cpp | 21 +++++++++++++++++-- .../tool/ClangApplyReplacementsMain.cpp | 7 ++++++- .../clang-tidy/tool/run-clang-tidy.py | 1 + .../Inputs/ignore-conflict/file1.yaml | 24 ++++++++++++++++++++++ .../Inputs/ignore-conflict/ignore-conflict.cpp | 4 ++++ .../clang-apply-replacements/ignore-conflict.cpp | 5 +++++ .../clang/Tooling/Refactoring/AtomicChange.h | 2 ++ 8 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/file1.yaml create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/ignore-conflict.cpp create mode 100644 clang-tools-extra/test/clang-apply-replacements/ignore-conflict.cpp diff --git a/clang-tools-extra/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h b/clang-tools-extra/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h index e377682..b0cf731 100644 --- a/clang-tools-extra/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h +++ b/clang-tools-extra/clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h @@ -87,7 +87,8 @@ std::error_code collectReplacementsFromDirectory( /// \li false If there were conflicts. bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs, FileToChangesMap &FileChanges, - clang::SourceManager &SM); + clang::SourceManager &SM, + bool IgnoreInsertConflict = false); /// Apply \c AtomicChange on File and rewrite it. /// diff --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp index b13b83c..e25c8e2 100644 --- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -202,7 +202,7 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs, FileToChangesMap &FileChanges, - clang::SourceManager &SM) { + clang::SourceManager &SM, bool IgnoreInsertConflict) { auto GroupedReplacements = groupReplacements(TUs, TUDs, SM); bool ConflictDetected = false; @@ -231,7 +231,24 @@ bool mergeAndDeduplicate(const TUReplacements &TUs, const TUDiagnostics &TUDs, // For now, printing directly the error reported by `AtomicChange` is // the easiest solution. errs() << llvm::toString(std::move(Err)) << "\n"; - ConflictDetected = true; + if (IgnoreInsertConflict) { + tooling::Replacements &Replacements = FileChange.getReplacements(); + unsigned NewOffset = + Replacements.getShiftedCodePosition(R.getOffset()); + unsigned NewLength = Replacements.getShiftedCodePosition( + R.getOffset() + R.getLength()) - + NewOffset; + if (NewLength == R.getLength()) { + tooling::Replacement RR = tooling::Replacement( + R.getFilePath(), NewOffset, NewLength, R.getReplacementText()); + Replacements = Replacements.merge(tooling::Replacements(RR)); + } else { + llvm::errs() + << "Can't resolve conflict, skipping the replacement.\n"; + ConflictDetected = true; + } + } else + ConflictDetected = true; } } FileChanges.try_emplace(Entry, diff --git a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp index 52ef725..68e7fea 100644 --- a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp @@ -42,6 +42,11 @@ static cl::opt RemoveTUReplacementFiles( "merging/replacing."), cl::init(false), cl::cat(ReplacementCategory)); +static cl::opt IgnoreInsertConflict( + "ignore-insert-conflict", + cl::desc("Ignore insert conflict and keep running to fix."), + cl::init(false), cl::cat(ReplacementCategory)); + static cl::opt DoFormat( "format", cl::desc("Enable formatting of code changed by applying replacements.\n" @@ -131,7 +136,7 @@ int main(int argc, char **argv) { SourceManager SM(Diagnostics, Files); FileToChangesMap Changes; - if (!mergeAndDeduplicate(TURs, TUDs, Changes, SM)) + if (!mergeAndDeduplicate(TURs, TUDs, Changes, SM, IgnoreInsertConflict)) return 1; tooling::ApplyChangesSpec Spec; diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index fdfe5a0..821b941 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -180,6 +180,7 @@ def find_binary(arg, name, build_path): def apply_fixes(args, clang_apply_replacements_binary, tmpdir): """Calls clang-apply-fixes on a given directory.""" invocation = [clang_apply_replacements_binary] + invocation.append('-ignore-insert-conflict') if args.format: invocation.append('-format') if args.style: diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/file1.yaml new file mode 100644 index 0000000..a80cf82 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/file1.yaml @@ -0,0 +1,24 @@ +--- +MainSourceFile: ignore-conflict.cpp +Diagnostics: + - DiagnosticName: test-ignore-conflict-insertion + DiagnosticMessage: + Message: Fix + FilePath: $(path)/ignore-conflict.cpp + FileOffset: 0 + Replacements: + - FilePath: $(path)/ignore-conflict.cpp + Offset: 0 + Length: 0 + ReplacementText: "#include \n" + - DiagnosticName: test-ignore-conflict-insertion + DiagnosticMessage: + Message: Fix + FilePath: $(path)/ignore-conflict.cpp + FileOffset: 0 + Replacements: + - FilePath: $(path)/ignore-conflict.cpp + Offset: 0 + Length: 0 + ReplacementText: "#include \n" +... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/ignore-conflict.cpp b/clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/ignore-conflict.cpp new file mode 100644 index 0000000..8791dd9 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/ignore-conflict/ignore-conflict.cpp @@ -0,0 +1,4 @@ +class MyType {}; +// CHECK: #include +// CHECK-NEXT: #include +// CEHCK-NEXT: class MyType {}; diff --git a/clang-tools-extra/test/clang-apply-replacements/ignore-conflict.cpp b/clang-tools-extra/test/clang-apply-replacements/ignore-conflict.cpp new file mode 100644 index 0000000..4e681dd --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/ignore-conflict.cpp @@ -0,0 +1,5 @@ +// RUN: mkdir -p %T/Inputs/ignore-conflict +// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/ignore-conflict/ignore-conflict.cpp > %T/Inputs/ignore-conflict/ignore-conflict.cpp +// RUN: sed "s#\$(path)#%/T/Inputs/ignore-conflict#" %S/Inputs/ignore-conflict/file1.yaml > %T/Inputs/ignore-conflict/file1.yaml +// RUN: clang-apply-replacements --ignore-insert-conflict %T/Inputs/ignore-conflict +// RUN: FileCheck -input-file=%T/Inputs/ignore-conflict/ignore-conflict.cpp %S/Inputs/ignore-conflict/ignore-conflict.cpp diff --git a/clang/include/clang/Tooling/Refactoring/AtomicChange.h b/clang/include/clang/Tooling/Refactoring/AtomicChange.h index 3945a7c..92f322e 100644 --- a/clang/include/clang/Tooling/Refactoring/AtomicChange.h +++ b/clang/include/clang/Tooling/Refactoring/AtomicChange.h @@ -116,6 +116,8 @@ public: /// Returns a const reference to existing replacements. const Replacements &getReplacements() const { return Replaces; } + Replacements &getReplacements() { return Replaces; } + llvm::ArrayRef getInsertedHeaders() const { return InsertedHeaders; } -- 2.7.4