[clang-tools-extra] r358666 - [clang-tidy] Address post-commit comments

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Apr 18 07:18:14 2019
New Revision: 358666

URL: http://llvm.org/viewvc/llvm-project?rev=358666&view=rev
Log:
[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

Added:
    clang-tools-extra/trunk/test/clang-tidy/alternative-fixes.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=358666&r1=358665&r2=358666&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Thu Apr 18 07:18:14 2019
@@ -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));
           }
         }
       }

Added: clang-tools-extra/trunk/test/clang-tidy/alternative-fixes.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/alternative-fixes.cpp?rev=358666&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/alternative-fixes.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/alternative-fixes.cpp Thu Apr 18 07:18:14 2019
@@ -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)) {
+  }
+}