[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 ↗

(On Diff #101535)

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 ↗

(On Diff #101544)

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

993 ↗

(On Diff #103885)

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

[chandlerc]

include/clang/Format/Format.h

993 ↗

(On Diff #103885)

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.

cfe/trunk/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.

cfe/trunk/lib/Format/Format.cpp

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.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, llvm::Regex::IgnoreCase);
     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) ||
+        FileStem.startswith_lower(HeaderStem)) {
       llvm::Regex MainIncludeRegex(
-          (HeaderStem + Style.IncludeIsMainRegex).str());
+          (HeaderStem + Style.IncludeIsMainRegex).str(),
+          llvm::Regex::IgnoreCase);
       if (MainIncludeRegex.match(FileStem))
         return true;
     }
     return false;
   }
 
   const FormatStyle &Style;
   bool IsMainFile;

cfe/trunk/unittests/Format/SortIncludesTest.cpp

EXPECT_EQ("#include \"a.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"));
 }
 
+TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
+  // Setup an regex for main includes so we can cover those as well.
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+
+  // Ensure both main header detection and grouping work in a case insensitive
+  // manner.
+  EXPECT_EQ("#include \"llvm/A.h\"\n"
+            "#include \"b.h\"\n"
+            "#include \"c.h\"\n"
+            "#include \"LLVM/z.h\"\n"
+            "#include \"llvm/X.h\"\n"
+            "#include \"GTest/GTest.h\"\n"
+            "#include \"gmock/gmock.h\"\n",
+            sort("#include \"c.h\"\n"
+                 "#include \"b.h\"\n"
+                 "#include \"GTest/GTest.h\"\n"
+                 "#include \"llvm/A.h\"\n"
+                 "#include \"gmock/gmock.h\"\n"
+                 "#include \"llvm/X.h\"\n"
+                 "#include \"LLVM/z.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"
                  "#include \"a_other.h\"\n"
                  "#include \"important_os_header.h\"\n",