[llvm-branch-commits] [clang-tools-extra] 52557b9 - [clang-tidy] Fix LLVM include order check policy

[Tom Stellard via llvm-branch-commits]

Author: Kadir Cetinkaya
Date: 2022-02-07T13:31:54-08:00
New Revision: 52557b918b1dbb50f5ba52644db3be87b7934d9a

URL: https://github.com/llvm/llvm-project/commit/52557b918b1dbb50f5ba52644db3be87b7934d9a
DIFF: https://github.com/llvm/llvm-project/commit/52557b918b1dbb50f5ba52644db3be87b7934d9a.diff

LOG: [clang-tidy] Fix LLVM include order check policy

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.

Differential Revision: https://reviews.llvm.org/D118913

(cherry picked from commit 0447ec2fb050eb37ed1f5991a1562dea6e228f9e)

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
    clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
index c962fb3bc25b2..ab75e3100d85d 100644
--- a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -66,11 +66,15 @@ static int getPriority(StringRef Filename, bool IsAngled, bool IsMainModule) {
       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;
 }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
index 6dd3d6ace4581..d4da826ced386 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp
@@ -6,6 +6,7 @@
 #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
@@ -19,6 +20,7 @@
 // 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"

diff  --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
index 2a3fae05f585f..46a1600e2bd6d 100644
--- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -1,4 +1,7 @@
+#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"
@@ -9,11 +12,15 @@ 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)
@@ -22,27 +29,36 @@ static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
   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"
@@ -270,6 +286,23 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
 #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