[clang-format] Add option to group multiple #include blocks when sorting includes

[KrzysztofKapusta]

Summary

This patch allows grouping multiple #include blocks together and sort all includes as one big block.
Additionally, sorted includes can be regrouped after sorting based on configured categories.

[krasimir]

Looks good! Just a few nits:

lib/Format/Format.cpp

1570 ↗

(On Diff #123735)

Please replace this with two explicit checks for the expected styles. Plus, an else if with an empty body is super awkward.

unittests/Format/SortIncludesTest.cpp

357 ↗

(On Diff #123735)

What is this testing?

[KrzysztofKapusta]

Addressed all comments. Changed one testcase to better reflect the desired effect.

[KrzysztofKapusta]

unittests/Format/SortIncludesTest.cpp

357 ↗

(On Diff #123735)

I changed this to better reflect what I was trying to test. Now the expected order is not sorted from a..c

[krasimir]

Thank you! Do you have commit access?

[KrzysztofKapusta]

@krasimir thanks for the review! I don't have commit rights. Would you push this for me?

cfe/trunk/docs/ClangFormatStyleOptions.rst

**ForEachMacros** (``std::vector<std::string>``)
   In the .clang-format configuration file, this can be configured like:
 
   .. code-block:: yaml
 
     ForEachMacros: ['RANGES_FOR', 'FOREACH']
 
   For example: BOOST_FOREACH.
 
+**IncludeBlocks** (``IncludeBlocksStyle``)
+  Dependent on the value, multiple ``#include`` blocks can be sorted
+  as one and divided based on category.
+
+  Possible values:
+
+  * ``IBS_Preserve`` (in configuration: ``Preserve``)
+    Sort each ``#include`` block separately.
+
+    .. code-block:: c++
+
+       #include "b.h"               into      #include "b.h"
+
+       #include <lib/main.h>                  #include "a.h"
+       #include "a.h"                         #include <lib/main.h>
+
+  * ``IBS_Merge`` (in configuration: ``Merge``)
+    Merge multiple ``#include`` blocks together and sort as one.
+
+    .. code-block:: c++
+
+       #include "b.h"               into      #include "a.h"
+                                              #include "b.h"
+       #include <lib/main.h>                  #include <lib/main.h>
+       #include "a.h"
+
+  * ``IBS_Regroup`` (in configuration: ``Regroup``)
+    Merge multiple ``#include`` blocks together and sort as one.
+    Then split into groups based on category priority. See ``IncludeCategories``.
+
+    .. code-block:: c++
+
+       #include "b.h"               into      #include "a.h"
+                                              #include "b.h"
+       #include <lib/main.h>
+       #include "a.h"                         #include <lib/main.h>
+
+
+
 **IncludeCategories** (``std::vector<IncludeCategory>``)
   Regular expressions denoting the different ``#include`` categories
   used for ordering ``#includes``.
 
   These regular expressions are matched against the filename of an include
   (including the <> or "") in order. The value belonging to the first
   matching regular expression is assigned and ``#includes`` are sorted first
   according to increasing category number and then alphabetically within

cfe/trunk/include/clang/Format/Format.h

struct FormatStyle {
   /// In the .clang-format configuration file, this can be configured like:
   /// \code{.yaml}
   ///   ForEachMacros: ['RANGES_FOR', 'FOREACH']
   /// \endcode
   ///
   /// For example: BOOST_FOREACH.
   std::vector<std::string> ForEachMacros;
 
+  /// \brief Styles for sorting multiple ``#include`` blocks.
+  enum IncludeBlocksStyle {
+    /// \brief Sort each ``#include`` block separately.
+    /// \code
+    ///    #include "b.h"               into      #include "b.h"
+    ///
+    ///    #include <lib/main.h>                  #include "a.h"
+    ///    #include "a.h"                         #include <lib/main.h>
+    /// \endcode
+    IBS_Preserve,
+    /// \brief Merge multiple ``#include`` blocks together and sort as one.
+    /// \code
+    ///    #include "b.h"               into      #include "a.h"
+    ///                                           #include "b.h"
+    ///    #include <lib/main.h>                  #include <lib/main.h>
+    ///    #include "a.h"
+    /// \endcode
+    IBS_Merge,
+    /// \brief Merge multiple ``#include`` blocks together and sort as one.
+    /// Then split into groups based on category priority. See
+    /// ``IncludeCategories``.
+    /// \code
+    ///    #include "b.h"               into      #include "a.h"
+    ///                                           #include "b.h"
+    ///    #include <lib/main.h>
+    ///    #include "a.h"                         #include <lib/main.h>
+    /// \endcode
+    IBS_Regroup,
+  };
+
+  /// \brief Dependent on the value, multiple ``#include`` blocks can be sorted
+  /// as one and divided based on category.
+  IncludeBlocksStyle IncludeBlocks;
+
   /// \brief See documentation of ``IncludeCategories``.
   struct IncludeCategory {
     /// \brief The regular expression that this category matches.
     std::string Regex;
     /// \brief The priority to assign to this category.
     int Priority;
     bool operator==(const IncludeCategory &Other) const {
       return Regex == Other.Regex && Priority == Other.Priority;
return AccessModifierOffset == R.AccessModifierOffset &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
            Cpp11BracedListStyle == R.Cpp11BracedListStyle &&
            DerivePointerAlignment == R.DerivePointerAlignment &&
            DisableFormat == R.DisableFormat &&
            ExperimentalAutoDetectBinPacking ==
                R.ExperimentalAutoDetectBinPacking &&
            FixNamespaceComments == R.FixNamespaceComments &&
            ForEachMacros == R.ForEachMacros &&
+           IncludeBlocks == R.IncludeBlocks &&
            IncludeCategories == R.IncludeCategories &&
            IndentCaseLabels == R.IndentCaseLabels &&
            IndentPPDirectives == R.IndentPPDirectives &&
            IndentWidth == R.IndentWidth && Language == R.Language &&
            IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
            JavaScriptQuotes == R.JavaScriptQuotes &&
            JavaScriptWrapImports == R.JavaScriptWrapImports &&
            KeepEmptyLinesAtTheStartOfBlocks ==

cfe/trunk/lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
     IO.mapOptional("Cpp11BracedListStyle", Style.Cpp11BracedListStyle);
     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("IncludeBlocks", Style.IncludeBlocks);
     IO.mapOptional("IncludeCategories", Style.IncludeCategories);
     IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
     IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
     IO.mapOptional("IndentWidth", Style.IndentWidth);
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
     IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);

 
 template <> struct MappingTraits<FormatStyle::IncludeCategory> {
   static void mapping(IO &IO, FormatStyle::IncludeCategory &Category) {
     IO.mapOptional("Regex", Category.Regex);
     IO.mapOptional("Priority", Category.Priority);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::IncludeBlocksStyle> {
+  static void enumeration(IO &IO, FormatStyle::IncludeBlocksStyle &Value) {
+    IO.enumCase(Value, "Preserve", FormatStyle::IBS_Preserve);
+    IO.enumCase(Value, "Merge", FormatStyle::IBS_Merge);
+    IO.enumCase(Value, "Regroup", FormatStyle::IBS_Regroup);
+  }
+};
+
 template <> struct MappingTraits<FormatStyle::RawStringFormat> {
   static void mapping(IO &IO, FormatStyle::RawStringFormat &Format) {
     IO.mapOptional("Delimiter", Format.Delimiter);
     IO.mapOptional("Language", Format.Language);
     IO.mapOptional("BasedOnStyle", Format.BasedOnStyle);
   }
 };
 
FormatStyle getLLVMStyle() {
   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|gmock|isl|json)/)", 3},
                                  {".*", 1}};
   LLVMStyle.IncludeIsMainRegex = "(Test)?$";
+  LLVMStyle.IncludeBlocks = FormatStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.TabWidth = 8;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
static void sortCppIncludes(const FormatStyle &Style,
 
   // Deduplicate #includes.
   Indices.erase(std::unique(Indices.begin(), Indices.end(),
                             [&](unsigned LHSI, unsigned RHSI) {
                               return Includes[LHSI].Text == Includes[RHSI].Text;
                             }),
                 Indices.end());
 
+  int CurrentCategory = Includes.front().Category;
+
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
   if (Indices.size() == Includes.size() &&
-      std::is_sorted(Indices.begin(), Indices.end()))
+      std::is_sorted(Indices.begin(), Indices.end()) &&
+      Style.IncludeBlocks == FormatStyle::IBS_Preserve)
     return;
 
   std::string result;
   for (unsigned Index : Indices) {
-    if (!result.empty())
+    if (!result.empty()) {
       result += "\n";
+      if (Style.IncludeBlocks == FormatStyle::IBS_Regroup &&
+          CurrentCategory != Includes[Index].Category)
+        result += "\n";
+    }
     result += Includes[Index].Text;
     if (Cursor && CursorIndex == Index)
       *Cursor = IncludesBeginOffset + result.size() - CursorToEOLOffset;
+    CurrentCategory = Includes[Index].Category;
   }
 
   auto Err = Replaces.add(tooling::Replacement(
       FileName, Includes.front().Offset, IncludesBlockSize, result));
   // FIXME: better error handling. For now, just skip the replacement for the
   // release version.
   if (Err) {
     llvm::errs() << llvm::toString(std::move(Err)) << "\n";
StringRef Line =
         Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
 
     StringRef Trimmed = Line.trim();
     if (Trimmed == "// clang-format off")
       FormattingOff = true;
     else if (Trimmed == "// clang-format on")
       FormattingOff = false;
 
+    const bool EmptyLineSkipped =
+        Trimmed.empty() && (Style.IncludeBlocks == FormatStyle::IBS_Merge ||
+                            Style.IncludeBlocks == FormatStyle::IBS_Regroup);
+
     if (!FormattingOff && !Line.endswith("\\")) {
       if (IncludeRegex.match(Line, &Matches)) {
         StringRef IncludeName = Matches[2];
         int Category = Categories.getIncludePriority(
             IncludeName,
             /*CheckMainHeader=*/!MainIncludeFound && FirstIncludeBlock);
         if (Category == 0)
           MainIncludeFound = true;
         IncludesInBlock.push_back({IncludeName, Line, Prev, Category});
-      } else if (!IncludesInBlock.empty()) {
+      } else if (!IncludesInBlock.empty() && !EmptyLineSkipped) {
         sortCppIncludes(Style, IncludesInBlock, Ranges, FileName, Replaces,
                         Cursor);
         IncludesInBlock.clear();
         FirstIncludeBlock = false;
       }
       Prev = Pos + 1;
     }
     if (Pos == StringRef::npos || Pos + 1 == Code.size())

cfe/trunk/unittests/Format/SortIncludesTest.cpp

std::string Code = "#include <a>\n"
                      "#include <b>\n"
                      "#include <c>\n"
                      "#include <d>\n"
                      "#include <e>\n"
                      "#include <f>\n";
   EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty());
 }
 
+TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"b.h\"\n"
+            "#include \"c.h\"\n",
+            sort("#include \"a.h\"\n"
+                 "#include \"c.h\"\n"
+                 "\n"
+                 "\n"
+                 "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"b.h\"\n"
+            "#include \"c.h\"\n",
+            sort("#include \"a.h\"\n"
+                 "#include \"c.h\"\n"
+                 "\n"
+                 "\n"
+                 "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, SupportClangFormatOff) {
   EXPECT_EQ("#include <a>\n"
             "#include <b>\n"
             "#include <c>\n"
             "// clang-format off\n"
             "#include <b>\n"
             "#include <a>\n"
             "#include <c>\n"
EXPECT_EQ("#include \"a.h\"\n"
             "\n"
             "#include \"b.h\"\n",
             sort("#include \"a.h\"\n"
                  "#include \"c.h\"\n"
                  "\n"
                  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SortsAllBlocksWhenMerging) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"b.h\"\n"
+            "#include \"c.h\"\n",
+            sort("#include \"a.h\"\n"
+                 "#include \"c.h\"\n"
+                 "\n"
+                 "#include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, CommentsAlwaysSeparateGroups) {
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"c.h\"\n"
+            "// comment\n"
+            "#include \"b.h\"\n",
+            sort("#include \"c.h\"\n"
+                 "#include \"a.h\"\n"
+                 "// comment\n"
+                 "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"c.h\"\n"
+            "// comment\n"
+            "#include \"b.h\"\n",
+            sort("#include \"c.h\"\n"
+                 "#include \"a.h\"\n"
+                 "// comment\n"
+                 "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"c.h\"\n"
+            "// comment\n"
+            "#include \"b.h\"\n",
+            sort("#include \"c.h\"\n"
+                 "#include \"a.h\"\n"
+                 "// comment\n"
+                 "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
             "#include \"c.h\"\n"
             "#include <b.h>\n"
             "#include <d.h>\n",
             sort("#include <d.h>\n"
                  "#include <b.h>\n"
                  "#include \"c.h\"\n"
                  "#include \"a.h\"\n"));
 
   Style = getGoogleStyle(FormatStyle::LK_Cpp);
   EXPECT_EQ("#include <b.h>\n"
             "#include <d.h>\n"
             "#include \"a.h\"\n"
             "#include \"c.h\"\n",
             sort("#include <d.h>\n"
                  "#include <b.h>\n"
                  "#include \"c.h\"\n"
                  "#include \"a.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, RegroupsAngledIncludesInSeparateBlocks) {
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"c.h\"\n"
+            "\n"
+            "#include <b.h>\n"
+            "#include <d.h>\n",
+            sort("#include <d.h>\n"
+                 "#include <b.h>\n"
+                 "#include \"c.h\"\n"
+                 "#include \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesMultilineIncludes) {
   EXPECT_EQ("#include \"a.h\"\n"
             "#include \"b.h\"\n"
             "#include \"c.h\"\n",
             sort("#include \"a.h\"\n"
                  "#include \\\n"
                  "\"c.h\"\n"
                  "#include \"b.h\"\n"));
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, RecognizeMainHeaderInAllGroups) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+
+  EXPECT_EQ("#include \"c.h\"\n"
+            "#include \"a.h\"\n"
+            "#include \"b.h\"\n",
+            sort("#include \"b.h\"\n"
+                 "\n"
+                 "#include \"a.h\"\n"
+                 "#include \"c.h\"\n",
+                 "c.cc"));
+}
+
+TEST_F(SortIncludesTest, MainHeaderIsSeparatedWhenRegroupping) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+
+  EXPECT_EQ("#include \"a.h\"\n"
+            "\n"
+            "#include \"b.h\"\n"
+            "#include \"c.h\"\n",
+            sort("#include \"b.h\"\n"
+                 "\n"
+                 "#include \"a.h\"\n"
+                 "#include \"c.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"
EXPECT_EQ("#include \"important_os_header.h\"\n"
             "#include \"c_main.h\"\n"
             "#include \"a_other.h\"\n",
             sort("#include \"important_os_header.h\"\n"
                  "#include \"c_main.h\"\n"
                  "#include \"a_other.h\"\n",
                  "c_main.cc"));
 }
 
+TEST_F(SortIncludesTest, PriorityGroupsAreSeparatedWhenRegroupping) {
+  Style.IncludeCategories = {{".*important_os_header.*", -1}, {".*", 1}};
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+
+  EXPECT_EQ("#include \"important_os_header.h\"\n"
+            "\n"
+            "#include \"c_main.h\"\n"
+            "\n"
+            "#include \"a_other.h\"\n",
+            sort("#include \"c_main.h\"\n"
+                 "#include \"a_other.h\"\n"
+                 "#include \"important_os_header.h\"\n",
+                 "c_main.cc"));
+
+  // check stable when re-run
+  EXPECT_EQ("#include \"important_os_header.h\"\n"
+            "\n"
+            "#include \"c_main.h\"\n"
+            "\n"
+            "#include \"a_other.h\"\n",
+            sort("#include \"important_os_header.h\"\n"
+                 "\n"
+                 "#include \"c_main.h\"\n"
+                 "\n"
+                 "#include \"a_other.h\"\n",
+                 "c_main.cc"));
+}
+
 TEST_F(SortIncludesTest, CalculatesCorrectCursorPosition) {
   std::string Code = "#include <ccc>\n"    // Start of line: 0
                      "#include <bbbbbb>\n" // Start of line: 15
                      "#include <a>\n";     // Start of line: 33
   EXPECT_EQ(31u, newCursor(Code, 0));
   EXPECT_EQ(13u, newCursor(Code, 15));
   EXPECT_EQ(0u, newCursor(Code, 33));
 
   EXPECT_EQ(41u, newCursor(Code, 10));
   EXPECT_EQ(23u, newCursor(Code, 25));
   EXPECT_EQ(10u, newCursor(Code, 43));
 }
 
 TEST_F(SortIncludesTest, DeduplicateIncludes) {
   EXPECT_EQ("#include <a>\n"
             "#include <b>\n"
             "#include <c>\n",
             sort("#include <a>\n"
                  "#include <b>\n"
                  "#include <b>\n"
                  "#include <b>\n"
                  "#include <b>\n"
                  "#include <c>\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include <a>\n"
+            "#include <b>\n"
+            "#include <c>\n",
+            sort("#include <a>\n"
+                 "#include <b>\n"
+                 "\n"
+                 "#include <b>\n"
+                 "\n"
+                 "#include <b>\n"
+                 "#include <c>\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include <a>\n"
+            "#include <b>\n"
+            "#include <c>\n",
+            sort("#include <a>\n"
+                 "#include <b>\n"
+                 "\n"
+                 "#include <b>\n"
+                 "\n"
+                 "#include <b>\n"
+                 "#include <c>\n"));
 }
 
 TEST_F(SortIncludesTest, SortAndDeduplicateIncludes) {
   EXPECT_EQ("#include <a>\n"
             "#include <b>\n"
             "#include <c>\n",
             sort("#include <b>\n"
                  "#include <a>\n"
                  "#include <b>\n"
                  "#include <b>\n"
                  "#include <c>\n"
                  "#include <b>\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include <a>\n"
+            "#include <b>\n"
+            "#include <c>\n",
+            sort("#include <b>\n"
+                 "#include <a>\n"
+                 "\n"
+                 "#include <b>\n"
+                 "\n"
+                 "#include <c>\n"
+                 "#include <b>\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include <a>\n"
+            "#include <b>\n"
+            "#include <c>\n",
+            sort("#include <b>\n"
+                 "#include <a>\n"
+                 "\n"
+                 "#include <b>\n"
+                 "\n"
+                 "#include <c>\n"
+                 "#include <b>\n"));
 }
 
 TEST_F(SortIncludesTest, CalculatesCorrectCursorPositionAfterDeduplicate) {
   std::string Code = "#include <b>\n"      // Start of line: 0
                      "#include <a>\n"      // Start of line: 13
                      "#include <b>\n"      // Start of line: 26
                      "#include <b>\n"      // Start of line: 39
                      "#include <c>\n"      // Start of line: 52