[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
This commit is contained in:
Sockke 2022-05-30 13:02:25 +08:00
parent 88af539c0e
commit 3f3a235aa2
8 changed files with 63 additions and 4 deletions

View file

@ -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.
///

View file

@ -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,

View file

@ -42,6 +42,11 @@ static cl::opt<bool> RemoveTUReplacementFiles(
"merging/replacing."),
cl::init(false), cl::cat(ReplacementCategory));
static cl::opt<bool> IgnoreInsertConflict(
"ignore-insert-conflict",
cl::desc("Ignore insert conflict and keep running to fix."),
cl::init(false), cl::cat(ReplacementCategory));
static cl::opt<bool> 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;

View file

@ -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:

View file

@ -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 <a.h>\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 <b.h>\n"
...

View file

@ -0,0 +1,4 @@
class MyType {};
// CHECK: #include <a.h>
// CHECK-NEXT: #include <b.h>
// CEHCK-NEXT: class MyType {};

View file

@ -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

View file

@ -116,6 +116,8 @@ public:
/// Returns a const reference to existing replacements.
const Replacements &getReplacements() const { return Replaces; }
Replacements &getReplacements() { return Replaces; }
llvm::ArrayRef<std::string> getInsertedHeaders() const {
return InsertedHeaders;
}