hokein created this revision.
hokein added a reviewer: alexfh.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.
hokein updated this revision to Diff 195692.
hokein added a comment.
Cleanup.
Also add a test to verify clang-tidy only apply the first alternative
fix.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D60857
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/test/clang-tidy/alternative-fixes.cpp
clang/include/clang/Tooling/Core/Diagnostic.h
Index: clang/include/clang/Tooling/Core/Diagnostic.h
===================================================================
--- clang/include/clang/Tooling/Core/Diagnostic.h
+++ clang/include/clang/Tooling/Core/Diagnostic.h
@@ -93,8 +93,8 @@
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
Index: clang-tools-extra/test/clang-tidy/alternative-fixes.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/alternative-fixes.cpp
@@ -0,0 +1,15 @@
+// RUN: clang-tidy -checks='-*,llvm-namespace-comment,clang-diagnostic-*' %s -- \
+// RUN: | FileCheck -implicit-check-not='{{warning|error|note}}:' %s
+
+// Verify clang-tidy only applies the first alternative fix.
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy -checks='-*,llvm-namespace-comment,clang-diagnostic-*' -fix %t.cpp --
+// RUN: FileCheck -input-file=%t.cpp %s -check-prefix=CHECK-FIX
+void foo(int a) {
+ if (a = 1) {
+ // CHECK: [[@LINE-1]]:9: warning: using the result of an assignment as a condition without parentheses [clang-diagnostic-parentheses]
+ // CHECK: [[@LINE-2]]:9: note: place parentheses around the assignment to silence this warning
+ // CHECK: [[@LINE-3]]:9: note: use '==' to turn this assignment into an equality comparison
+ // CHECK-FIX: if ((a = 1)) {
+ }
+}
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -133,41 +133,40 @@
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));
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
