[clang-tidy] Fix LLVM include order check policy

[kadircet]

Summary

Clang-format LLVM style has a custom include category for gtest/ and
gmock/ headers between regular includes and angled includes. Do the same here.

Fixes https://github.com/llvm/llvm-project/issues/53525.

[sammccall]

Thank you!
Seems strange to me that we have both forms of test here, but I'm sure there's a reason...

clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp

static int getPriority(StringRef Filename, bool IsAngled, bool IsMainModule) {
   if (IsMainModule)
     return 0;
 
   // LLVM and clang headers are in the penultimate position.
   if (Filename.startswith("llvm/") || Filename.startswith("llvm-c/") ||
       Filename.startswith("clang/") || Filename.startswith("clang-c/"))
     return 2;
 
-  // System headers are sorted to the end.
-  if (IsAngled || Filename.startswith("gtest/") ||
-      Filename.startswith("gmock/"))
+  // Put these between system and llvm headers to be consistent with LLVM
+  // clang-format style.
+  if (Filename.startswith("gtest/") || Filename.startswith("gmock/"))
     return 3;
 
+  // System headers are sorted to the end.
+  if (IsAngled)
+    return 4;
+
   // Other headers are inserted between the main module header and LLVM headers.
   return 1;
 }
 
 void IncludeOrderPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
     StringRef SearchPath, StringRef RelativePath, const Module *Imported,

clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp

 // RUN: %check_clang_tidy %s llvm-include-order %t -- -- -isystem %S/Inputs/Headers
 
 // CHECK-MESSAGES: [[@LINE+2]]:1: warning: #includes are not sorted properly
 #include "j.h"
 #include "gtest/foo.h"
 #include "gmock/foo.h"
 #include "i.h"
 #include <s.h>
+#include <a.h>
 #include "llvm/a.h"
 #include "clang/b.h"
 #include "clang-c/c.h" // hi
 #include "llvm-c/d.h" // -c
 
 // CHECK-FIXES: #include "j.h"
 // CHECK-FIXES-NEXT: #include "i.h"
 // CHECK-FIXES-NEXT: #include "clang-c/c.h" // hi
 // CHECK-FIXES-NEXT: #include "clang/b.h"
 // CHECK-FIXES-NEXT: #include "llvm-c/d.h" // -c
 // CHECK-FIXES-NEXT: #include "llvm/a.h"
 // CHECK-FIXES-NEXT: #include "gmock/foo.h"
 // CHECK-FIXES-NEXT: #include "gtest/foo.h"
+// CHECK-FIXES-NEXT: #include <a.h>
 // CHECK-FIXES-NEXT: #include <s.h>
 
 #include "b.h"
 #ifdef FOO
 #include "a.h"
 #endif
 
 // CHECK-FIXES: #include "b.h"

clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

+#include "ClangTidyOptions.h"
 #include "ClangTidyTest.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::llvm_check;
 
 namespace clang {
 namespace tidy {
 namespace test {
 
-static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
-                                       Optional<StringRef> ExpectedWarning) {
+template <typename T>
+static std::string runCheck(StringRef Code, const Twine &Filename,
+                            Optional<StringRef> ExpectedWarning,
+                            std::map<StringRef, StringRef> PathsToContent =
+                                std::map<StringRef, StringRef>()) {
   std::vector<ClangTidyError> Errors;
-  std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
-      Code, &Errors, Filename, std::string("-xc++-header"));
+  std::string Result = test::runCheckOnCode<T>(
+      Code, &Errors, Filename, std::string("-xc++-header"), ClangTidyOptions{},
+      std::move(PathsToContent));
   if (Errors.size() != (size_t)ExpectedWarning.hasValue())
     return "invalid error count";
   if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
     return "expected: '" + ExpectedWarning->str() + "', saw: '" +
            Errors.back().Message.Message + "'";
   return Result;
 }
 
+static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
+                                       Optional<StringRef> ExpectedWarning) {
+  return runCheck<LLVMHeaderGuardCheck>(Code, Filename,
+                                        std::move(ExpectedWarning));
+}
+
+static std::string
+runIncludeOrderCheck(StringRef Code, const Twine &Filename,
+                     Optional<StringRef> ExpectedWarning,
+                     llvm::ArrayRef<llvm::StringLiteral> Includes) {
+  std::map<StringRef, StringRef> PathsToContent;
+  for (auto Include : Includes)
+    PathsToContent.emplace(Include, "");
+  return runCheck<IncludeOrderCheck>(Code, Filename, std::move(ExpectedWarning),
+                                     PathsToContent);
+}
+
 namespace {
 struct WithEndifComment : public LLVMHeaderGuardCheck {
   WithEndifComment(StringRef Name, ClangTidyContext *Context)
       : LLVMHeaderGuardCheck(Name, Context) {}
   bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
 };
-} // namespace
 
 static std::string
 runHeaderGuardCheckWithEndif(StringRef Code, const Twine &Filename,
                              Optional<StringRef> ExpectedWarning) {
-  std::vector<ClangTidyError> Errors;
-  std::string Result = test::runCheckOnCode<WithEndifComment>(
-      Code, &Errors, Filename, std::string("-xc++-header"));
-  if (Errors.size() != (size_t)ExpectedWarning.hasValue())
-    return "invalid error count";
-  if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
-    return "expected: '" + ExpectedWarning->str() + "', saw: '" +
-           Errors.back().Message.Message + "'";
-  return Result;
+  return runCheck<WithEndifComment>(Code, Filename, std::move(ExpectedWarning));
 }
+} // namespace
 
 TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
             "#endif\n",
             runHeaderGuardCheck(
                 "#ifndef FOO\n"
                 "#define FOO\n"
EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FOO_H\n"
             "\n"
             "#endif\n",
             runHeaderGuardCheck(
                 "", "\\\\?\\C:\\llvm-project\\clang-tools-extra\\clangd\\foo.h",
                 StringRef("header is missing header guard")));
 #endif
 }
 
+TEST(IncludeOrderCheck, GTestHeaders) {
+  EXPECT_EQ(
+      R"cpp(
+  #include "foo.h"
+  #include "llvm/foo.h"
+  #include "gtest/foo.h"
+  #include <algorithm>)cpp",
+      runIncludeOrderCheck(
+          R"cpp(
+  #include "foo.h"
+  #include "llvm/foo.h"
+  #include <algorithm>
+  #include "gtest/foo.h")cpp",
+          "foo.cc", StringRef("#includes are not sorted properly"),
+          {"foo.h", "algorithm", "gtest/foo.h", "llvm/foo.h"}));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang