[clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

[chandlerc]

Summary

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.

[ioeric]

Lgtm.

lib/Format/Format.cpp

1406

Also add tests for these non-main categories?

[chandlerc]

Add test coverage for the case-insensitive category logic.

[chandlerc]

Added more tests, PTAL?

[ioeric]

unittests/Format/SortIncludesTest.cpp

269

I think this TEST is only for main header sorting. Could you split tests for other categories into a separate TEST?

[chandlerc]

Update based on review comments.

[chandlerc]

Tried to address the test comment -- let me know if something else was intended.

[djasper]

include/clang/Format/Format.h

941

Do we really need a flag here? Shouldn't we just always do this?

[chandlerc]

include/clang/Format/Format.h

941

I have no opinion one way or the other. I opted to not change behavior for existing styles purely in the spirit of making as targeted of a change as I could..

Feel free to tell me how you want this to work / be tested.

[djasper]

Just make clang-format always do this. I don't think anyone is relying on the current behavior.

[chandlerc]

Update based an Daniel's feedback.

[chandlerc]

In D33932#793994, @djasper wrote:

Just make clang-format always do this. I don't think anyone is relying on the current behavior.

Done, PTAL.

[djasper]

Looks good. Thank you.

[mteodor]

In D33932#793994, @djasper wrote:

Just make clang-format always do this. I don't think anyone is relying on the current behavior.

Well, someone did rely on case sensitive match. I thought case insensitive by default was just a Windows thing. Not anymore.

- Regex:           '^"I[A-Z]+[a-z].*\.h"'
  Priority:        1

This rule became too greedy on latest revision. Now not only interfaces are prioritized but also all headers that start with [Ii].

[adzenith]

I just came across this case-insensitive behavior when trying to regroup include blocks and it was surprising. I'm trying to put system headers after other angle-bracket includes (like for Qt), and I was trying:

- Regex:           '^<[a-z_]*>'
  Priority:        5
- Regex:           '^<[A-Za-z_]*>'
  Priority:        4

Unfortunately, #include <QWidget> matches the first regex and so Qt headers get sorted in with system headers. Would it be possible to add the option to do case-sensitive matching after all? Should I open a new issue?

For anyone else who wants case-sensitive matching, I just did discover that (?-i) seems to do the trick. I thought it wouldn't because the docs mention extended regex, but I guess it's more extended than I thought! Using '(?-i)^<[a-z_]*>' does the trick for me.

[adzenith]

It appears I was mistaken - the (?-i) caused the regex match to fail entirely, which made the system header move all the way to the end. It didn't actually solve my problem.

include/clang/Format/Format.h

struct FormatStyle {
   /// can also assign negative priorities if you have certain headers that
   /// always need to be first.
   ///
   /// To configure this in the .clang-format file, use:
   /// \code{.yaml}
   ///   IncludeCategories:
   ///     - Regex:           '^"(llvm|llvm-c|clang|clang-c)/'
   ///       Priority:        2
-  ///     - Regex:           '^(<|"(gtest|isl|json)/)'
+  ///     - Regex:           '^(<|"(gtest|gmock|isl|json)/)'
   ///       Priority:        3
   ///     - Regex:           '.*'
   ///       Priority:        1
   /// \endcode
   std::vector<IncludeCategory> IncludeCategories;
 
   /// \brief Specify a regular expression of suffixes that are allowed in the
   /// file-to-main-include mapping.
   ///
   /// When guessing whether a #include is the "main" include (to assign
   /// category 0, see above), use this regex of allowed suffixes to the header
   /// stem. A partial match is done, so that:
   /// - "" means "arbitrary suffix"
   /// - "$" means "no suffix"
   ///
   /// For example, if configured to "(_test)?$", then a header a.h would be seen
   /// 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.
   /// Switch statement body is always indented one level more than case labels.
   /// \code
   ///    false:                                 true:
   ///    switch (fool) {                vs.     switch (fool) {
   ///    case 1:                                  case 1:

lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
     IO.mapOptional("DisableFormat", Style.DisableFormat);
     IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
     IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
     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",
                    Style.IndentWrappedFunctionNames);
     IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
     IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
FormatStyle getLLVMStyle() {
   LLVMStyle.Cpp11BracedListStyle = true;
   LLVMStyle.DerivePointerAlignment = false;
   LLVMStyle.ExperimentalAutoDetectBinPacking = false;
   LLVMStyle.FixNamespaceComments = true;
   LLVMStyle.ForEachMacros.push_back("foreach");
   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;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;

 // This class manages priorities of #include categories and calculates
 // priorities for headers.
 class IncludeCategoryManager {
 public:
   IncludeCategoryManager(const FormatStyle &Style, StringRef FileName)
       : 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") ||
                  FileName.endswith(".mm");
   }
 
   // Returns the priority of the category which \p IncludeName belongs to.
   // If \p CheckMainHeader is true and \p IncludeName is a main header, returns
public:
   }
 
 private:
   bool isMainHeader(StringRef IncludeName) const {
     if (!IncludeName.startswith("\""))
       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;
     }
     return false;
   }
 
   const FormatStyle &Style;
   bool IsMainFile;

unittests/Format/SortIncludesTest.cpp

EXPECT_EQ("#include \"a.h\"\n"
             "#include \"b.h\"\n"
             "#include \"c.h\"\n"
             "#include \"llvm/a.h\"\n",
             sort("#include \"b.h\"\n"
                  "#include \"a.h\"\n"
                  "#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) {
   Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
   EXPECT_EQ("#include \"important_os_header.h\"\n"
             "#include \"c_main.h\"\n"
             "#include \"a_other.h\"\n",
             sort("#include \"c_main.h\"\n"