chandlerc created this revision.
Herald added subscribers: mcrosier, sanjoy, klimek.
This really is a collection of improvements to the rules for LLVM
include sorting:
- We have gmock headers now, so it adds support for those to one of the
categories.
- LLVM does use 'FooTest.cpp' files to test 'Foo.h' so it adds that suffix for
finding a main header.
- At times the test file's case may not match the header file's case, so adds
support for case-insensitive regex matching of these things.
With this set of changes, I can't spot any misbehaviors when re-sorting
all of LLVM's unittest '#include' lines.
Note that I don't know the first thing about testing clang-format, so please
feel free to tell me what all I need to be doing here.
https://reviews.llvm.org/D33932
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
unittests/Format/SortIncludesTest.cpp
Index: unittests/Format/SortIncludesTest.cpp
===================================================================
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -264,6 +264,24 @@
"#include \"c.h\"\n"
"#include \"llvm/a.h\"\n",
"a.cc"));
+
+ // Support case-sensitive and case insensitive matching.
+ Style.IncludeRegexCaseInsensitive = false;
+ EXPECT_EQ("#include \"b.h\"\n"
+ "#include \"c.h\"\n"
+ "#include \"llvm/A.h\"\n",
+ sort("#include \"c.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"llvm/A.h\"\n",
+ "a_TEST.cc"));
+ Style.IncludeRegexCaseInsensitive = true;
+ EXPECT_EQ("#include \"llvm/A.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"c.h\"\n",
+ sort("#include \"c.h\"\n"
+ "#include \"b.h\"\n"
+ "#include \"llvm/A.h\"\n",
+ "a_TEST.cc"));
}
TEST_F(SortIncludesTest, NegativePriorities) {
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -346,6 +346,8 @@
IO.mapOptional("ForEachMacros", Style.ForEachMacros);
IO.mapOptional("IncludeCategories", Style.IncludeCategories);
IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
+ IO.mapOptional("IncludeRegexCaseInsensitive",
+ Style.IncludeRegexCaseInsensitive);
IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
IO.mapOptional("IndentWidth", Style.IndentWidth);
IO.mapOptional("IndentWrappedFunctionNames",
@@ -572,9 +574,10 @@
LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH");
LLVMStyle.IncludeCategories = {{"^\"(llvm|llvm-c|clang|clang-c)/", 2},
- {"^(<|\"(gtest|isl|json)/)", 3},
+ {"^(<|\"(gtest|gmock|isl|json)/)", 3},
{".*", 1}};
- LLVMStyle.IncludeIsMainRegex = "$";
+ LLVMStyle.IncludeIsMainRegex = "(Test)?$";
+ LLVMStyle.IncludeRegexCaseInsensitive = true;
LLVMStyle.IndentCaseLabels = false;
LLVMStyle.IndentWrappedFunctionNames = false;
LLVMStyle.IndentWidth = 2;
@@ -1400,7 +1403,10 @@
: Style(Style), FileName(FileName) {
FileStem = llvm::sys::path::stem(FileName);
for (const auto &Category : Style.IncludeCategories)
- CategoryRegexs.emplace_back(Category.Regex);
+ CategoryRegexs.emplace_back(Category.Regex,
+ Style.IncludeRegexCaseInsensitive
+ ? llvm::Regex::IgnoreCase
+ : llvm::Regex::NoFlags);
IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
FileName.endswith(".cpp") || FileName.endswith(".c++") ||
FileName.endswith(".cxx") || FileName.endswith(".m") ||
@@ -1428,9 +1434,13 @@
return false;
StringRef HeaderStem =
llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(1));
- if (FileStem.startswith(HeaderStem)) {
+ if (FileStem.startswith(HeaderStem) ||
+ (Style.IncludeRegexCaseInsensitive &&
+ FileStem.startswith_lower(HeaderStem))) {
llvm::Regex MainIncludeRegex(
- (HeaderStem + Style.IncludeIsMainRegex).str());
+ (HeaderStem + Style.IncludeIsMainRegex).str(),
+ Style.IncludeRegexCaseInsensitive ? llvm::Regex::IgnoreCase
+ : llvm::Regex::NoFlags);
if (MainIncludeRegex.match(FileStem))
return true;
}
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -914,7 +914,7 @@
/// IncludeCategories:
/// - Regex: '^"(llvm|llvm-c|clang|clang-c)/'
/// Priority: 2
- /// - Regex: '^(<|"(gtest|isl|json)/)'
+ /// - Regex: '^(<|"(gtest|gmock|isl|json)/)'
/// Priority: 3
/// - Regex: '.*'
/// Priority: 1
@@ -934,6 +934,12 @@
/// as the "main" include in both a.cc and a_test.cc.
std::string IncludeIsMainRegex;
+ /// \brief Match include regular expressions in a case insensitive manner.
+ ///
+ /// This applies both to ``IncludeIsMainRegex`` and the regular expressions
+ /// inside ``IncludeCategories``.
+ bool IncludeRegexCaseInsensitive;
+
/// \brief Indent case labels one level from the switch statement.
///
/// When ``false``, use the same indentation level as for the switch statement.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
