[clang-tidy] Address post-commit comments

[hokein]

Summary

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

[hokein]

Cleanup.

[alexfh]

clang-tools-extra/test/clang-tidy/alternative-fixes.cpp

2–8

Why can't we use the check_clang_tidy script?

[hokein]

Use check_clang_tidy.

[hokein]

clang-tools-extra/test/clang-tidy/alternative-fixes.cpp

2–8

Yes, we do. This was blindly copied from other diagnostic tests.

[alexfh]

There's one more nit. Otherwise good to go.

Thanks!

clang/include/clang/Tooling/Core/Diagnostic.h

97

This should be Doxygen's \returns keyword.

[hokein]

fix doxygen comment.

clang-tools-extra/clang-tidy/ClangTidy.cpp

SmallVector<std::pair<SourceLocation, bool>, 4> FixLocations;
                   << Message.Message << Name;
       // FIXME: explore options to support interactive fix selection.
       const llvm::StringMap<Replacements> *ChosenFix = selectFirstFix(Error);
       if (ApplyFixes && ChosenFix) {
         for (const auto &FileAndReplacements : *ChosenFix) {
           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 {
-                CanBeApplied = true;
-                ++AppliedFixes;
-              }
-              FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
-              FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
-            }
-          }
-        }
-      }
+            if (!Repl.isApplicable())
+              continue;
+                                   Repl.getLength(), Repl.getReplacementText());
       reportFix(Diag, Error.Message.Fix);
     }
     for (auto Fix : FixLocations) {
       Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied
                                          : diag::note_fixit_failed);
     }
     for (const auto &Note : Error.Notes)
       reportNote(Note);

clang-tools-extra/test/clang-tidy/alternative-fixes.cpp

+// RUN: clang-tidy -checks='-*,llvm-namespace-comment,clang-diagnostic-*' %s -- \
+// RUN: | FileCheck -implicit-check-not='{{warning|error|note}}:' %s
+
+// Verify clang-tidy only apply 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)) {
+  }
+}
 No newline at end of file

clang/include/clang/Tooling/Core/Diagnostic.h

 
 /// Collection of Diagnostics generated from a single translation unit.
 struct TranslationUnitDiagnostics {
   /// Name of the main source for the translation unit.
   std::string MainSourceFile;
   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
 } // end namespace clang
 #endif // LLVM_CLANG_TOOLING_CORE_DIAGNOSTIC_H