diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/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)); } } } diff --git a/clang-tools-extra/test/clang-tidy/alternative-fixes.cpp b/clang-tools-extra/test/clang-tidy/alternative-fixes.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/alternative-fixes.cpp @@ -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)) { + } +} diff --git a/clang/include/clang/Tooling/Core/Diagnostic.h b/clang/include/clang/Tooling/Core/Diagnostic.h --- a/clang/include/clang/Tooling/Core/Diagnostic.h +++ b/clang/include/clang/Tooling/Core/Diagnostic.h @@ -93,8 +93,8 @@ std::vector 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 *selectFirstFix(const Diagnostic& D); } // end namespace tooling