[clang-tidy] Address post-commit comments

Summary:
Also add a test to verify clang-tidy only apply the first alternative
fix.

Reviewers: alexfh

Subscribers: xazax.hun, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D60857

llvm-svn: 358666
This commit is contained in:
Haojian Wu 2019-04-18 14:18:14 +00:00
parent b8f82ca1b2
commit 8bbbd31cdd
3 changed files with 41 additions and 33 deletions

View File

@ -133,41 +133,40 @@ public:
for (const auto &Repl : FileAndReplacements.second) {
++TotalFixes;
bool CanBeApplied = false;
if (Repl.isApplicable()) {
SourceLocation FixLoc;
SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
Files.makeAbsolutePath(FixAbsoluteFilePath);
tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(),
Repl.getLength(),
Repl.getReplacementText());
Replacements &Replacements = FileReplacements[R.getFilePath()];
llvm::Error Err = Replacements.add(R);
if (Err) {
// FIXME: Implement better conflict handling.
llvm::errs() << "Trying to resolve conflict: "
<< llvm::toString(std::move(Err)) << "\n";
unsigned NewOffset =
Replacements.getShiftedCodePosition(R.getOffset());
unsigned NewLength = Replacements.getShiftedCodePosition(
R.getOffset() + R.getLength()) -
NewOffset;
if (NewLength == R.getLength()) {
R = Replacement(R.getFilePath(), NewOffset, NewLength,
R.getReplacementText());
Replacements = Replacements.merge(tooling::Replacements(R));
CanBeApplied = true;
++AppliedFixes;
} else {
llvm::errs()
<< "Can't resolve conflict, skipping the replacement.\n";
}
} else {
if (!Repl.isApplicable())
continue;
SourceLocation FixLoc;
SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
Files.makeAbsolutePath(FixAbsoluteFilePath);
tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(),
Repl.getLength(), Repl.getReplacementText());
Replacements &Replacements = FileReplacements[R.getFilePath()];
llvm::Error Err = Replacements.add(R);
if (Err) {
// FIXME: Implement better conflict handling.
llvm::errs() << "Trying to resolve conflict: "
<< llvm::toString(std::move(Err)) << "\n";
unsigned NewOffset =
Replacements.getShiftedCodePosition(R.getOffset());
unsigned NewLength = Replacements.getShiftedCodePosition(
R.getOffset() + R.getLength()) -
NewOffset;
if (NewLength == R.getLength()) {
R = Replacement(R.getFilePath(), NewOffset, NewLength,
R.getReplacementText());
Replacements = Replacements.merge(tooling::Replacements(R));
CanBeApplied = true;
++AppliedFixes;
} else {
llvm::errs()
<< "Can't resolve conflict, skipping the replacement.\n";
}
FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
} else {
CanBeApplied = true;
++AppliedFixes;
}
FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
}
}
}

View File

@ -0,0 +1,9 @@
// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t
void foo(int a) {
if (a = 1) {
// CHECK-NOTES: [[@LINE-1]]:9: warning: using the result of an assignment as a condition without parentheses [clang-diagnostic-parentheses]
// CHECK-NOTES: [[@LINE-2]]:9: note: place parentheses around the assignment to silence this warning
// CHECK-NOTES: [[@LINE-3]]:9: note: use '==' to turn this assignment into an equality comparison
// CHECK-FIXES: if ((a = 1)) {
}
}

View File

@ -93,8 +93,8 @@ struct TranslationUnitDiagnostics {
std::vector<Diagnostic> Diagnostics;
};
// Get the first fix to apply for this diagnostic.
// Return nullptr if no fixes attached to the diagnostic.
/// Get the first fix to apply for this diagnostic.
/// \returns nullptr if no fixes are attached to the diagnostic.
const llvm::StringMap<Replacements> *selectFirstFix(const Diagnostic& D);
} // end namespace tooling