Skip to content

Commit 0447ec2

Browse files
committed
[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 llvm#53525. Differential Revision: https://reviews.llvm.org/D118913
1 parent 2349fb0 commit 0447ec2

File tree

3 files changed

+56
-17
lines changed

3 files changed

+56
-17
lines changed

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

+7-3
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,15 @@ static int getPriority(StringRef Filename, bool IsAngled, bool IsMainModule) {
6666
Filename.startswith("clang/") || Filename.startswith("clang-c/"))
6767
return 2;
6868

69-
// System headers are sorted to the end.
70-
if (IsAngled || Filename.startswith("gtest/") ||
71-
Filename.startswith("gmock/"))
69+
// Put these between system and llvm headers to be consistent with LLVM
70+
// clang-format style.
71+
if (Filename.startswith("gtest/") || Filename.startswith("gmock/"))
7272
return 3;
7373

74+
// System headers are sorted to the end.
75+
if (IsAngled)
76+
return 4;
77+
7478
// Other headers are inserted between the main module header and LLVM headers.
7579
return 1;
7680
}

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

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "gmock/foo.h"
77
#include "i.h"
88
#include <s.h>
9+
#include <a.h>
910
#include "llvm/a.h"
1011
#include "clang/b.h"
1112
#include "clang-c/c.h" // hi
@@ -19,6 +20,7 @@
1920
// CHECK-FIXES-NEXT: #include "llvm/a.h"
2021
// CHECK-FIXES-NEXT: #include "gmock/foo.h"
2122
// CHECK-FIXES-NEXT: #include "gtest/foo.h"
23+
// CHECK-FIXES-NEXT: #include <a.h>
2224
// CHECK-FIXES-NEXT: #include <s.h>
2325

2426
#include "b.h"

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

+47-14
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
#include "ClangTidyOptions.h"
12
#include "ClangTidyTest.h"
3+
#include "llvm/ADT/ArrayRef.h"
4+
#include "llvm/ADT/StringRef.h"
25
#include "llvm/HeaderGuardCheck.h"
36
#include "llvm/IncludeOrderCheck.h"
47
#include "gtest/gtest.h"
@@ -9,11 +12,15 @@ namespace clang {
912
namespace tidy {
1013
namespace test {
1114

12-
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
13-
Optional<StringRef> ExpectedWarning) {
15+
template <typename T>
16+
static std::string runCheck(StringRef Code, const Twine &Filename,
17+
Optional<StringRef> ExpectedWarning,
18+
std::map<StringRef, StringRef> PathsToContent =
19+
std::map<StringRef, StringRef>()) {
1420
std::vector<ClangTidyError> Errors;
15-
std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
16-
Code, &Errors, Filename, std::string("-xc++-header"));
21+
std::string Result = test::runCheckOnCode<T>(
22+
Code, &Errors, Filename, std::string("-xc++-header"), ClangTidyOptions{},
23+
std::move(PathsToContent));
1724
if (Errors.size() != (size_t)ExpectedWarning.hasValue())
1825
return "invalid error count";
1926
if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
@@ -22,27 +29,36 @@ static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
2229
return Result;
2330
}
2431

32+
static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
33+
Optional<StringRef> ExpectedWarning) {
34+
return runCheck<LLVMHeaderGuardCheck>(Code, Filename,
35+
std::move(ExpectedWarning));
36+
}
37+
38+
static std::string
39+
runIncludeOrderCheck(StringRef Code, const Twine &Filename,
40+
Optional<StringRef> ExpectedWarning,
41+
llvm::ArrayRef<llvm::StringLiteral> Includes) {
42+
std::map<StringRef, StringRef> PathsToContent;
43+
for (auto Include : Includes)
44+
PathsToContent.emplace(Include, "");
45+
return runCheck<IncludeOrderCheck>(Code, Filename, std::move(ExpectedWarning),
46+
PathsToContent);
47+
}
48+
2549
namespace {
2650
struct WithEndifComment : public LLVMHeaderGuardCheck {
2751
WithEndifComment(StringRef Name, ClangTidyContext *Context)
2852
: LLVMHeaderGuardCheck(Name, Context) {}
2953
bool shouldSuggestEndifComment(StringRef Filename) override { return true; }
3054
};
31-
} // namespace
3255

3356
static std::string
3457
runHeaderGuardCheckWithEndif(StringRef Code, const Twine &Filename,
3558
Optional<StringRef> ExpectedWarning) {
36-
std::vector<ClangTidyError> Errors;
37-
std::string Result = test::runCheckOnCode<WithEndifComment>(
38-
Code, &Errors, Filename, std::string("-xc++-header"));
39-
if (Errors.size() != (size_t)ExpectedWarning.hasValue())
40-
return "invalid error count";
41-
if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
42-
return "expected: '" + ExpectedWarning->str() + "', saw: '" +
43-
Errors.back().Message.Message + "'";
44-
return Result;
59+
return runCheck<WithEndifComment>(Code, Filename, std::move(ExpectedWarning));
4560
}
61+
} // namespace
4662

4763
TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
4864
EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
@@ -270,6 +286,23 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
270286
#endif
271287
}
272288

289+
TEST(IncludeOrderCheck, GTestHeaders) {
290+
EXPECT_EQ(
291+
R"cpp(
292+
#include "foo.h"
293+
#include "llvm/foo.h"
294+
#include "gtest/foo.h"
295+
#include <algorithm>)cpp",
296+
runIncludeOrderCheck(
297+
R"cpp(
298+
#include "foo.h"
299+
#include "llvm/foo.h"
300+
#include <algorithm>
301+
#include "gtest/foo.h")cpp",
302+
"foo.cc", StringRef("#includes are not sorted properly"),
303+
{"foo.h", "algorithm", "gtest/foo.h", "llvm/foo.h"}));
304+
}
305+
273306
} // namespace test
274307
} // namespace tidy
275308
} // namespace clang

0 commit comments

Comments
 (0)