diff options
author | Kadir Cetinkaya <kadircet@google.com> | 2022-02-03 17:26:41 +0100 |
---|---|---|
committer | Kadir Cetinkaya <kadircet@google.com> | 2022-02-03 17:32:43 +0100 |
commit | 0447ec2fb050eb37ed1f5991a1562dea6e228f9e (patch) | |
tree | 26a9388ba269ac98651574dffc07af58684323d3 | |
parent | 2349fb031270985d28d3056685d5f76dc3942c9d (diff) |
[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
3 files changed, 56 insertions, 17 deletions
diff --git a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp index c962fb3bc25b..ab75e3100d85 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 6dd3d6ace458..d4da826ced38 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 2a3fae05f585..46a1600e2bd6 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 |