[clang-format] Add case aware include sorting.

[kentsommer]

Summary

Adds an option to [clang-format] which sorts headers in an alphabetical manner using case only for tie-breakers. The options is off by default in favor of the current ASCIIbetical sorting style.

[kentsommer]

I'll work on fixing the unit tests. Thought they ran with check-clang-format but I was obviously wrong.

[kentsommer]

Fix both failing unit tests

[HazardyKnusperkeks]

Please expand the test case to different options like grouping. Otherwise looks good.

[kentsommer]

Expanded unit test

[kentsommer]

@HazardyKnusperkeks let me know if the additions to the unit test are along the lines you were hoping for or not, I think this captures what you had in mind.

[HazardyKnusperkeks]

LGTM.

[kentsommer]

I do not have commit access.

Full Name: Kent Sommer
Email: [e-mail]

[HazardyKnusperkeks]

I can push that, but will wait a bit longer so the others have time to object.

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

2354

The name is somewhat awkward IMO (in the sense it doesn't read nicely), but it's nice to be (alphabetically) closer to other Include-related options in the documentation.
I have no better name though. Just noticing.

2360

I'd write this in the same manner as other options, so sth like "When false: <code>... When true: <code>".

Otherwise, it's necessary to read up to the end of the description "This option is off by default" and conclude that off=false and that was the first snippet that corresponded to false.

clang/lib/Format/Format.cpp

2241

Maybe it would be better for CodeGen to get the if outside of sort? It would mean some code duplication, but maybe it could be mitigated with a lambda.

clang/unittests/Format/SortIncludesTest.cpp

602

You should test that this option is false for LLVM style.
BTW, shouldn't you set it somewhere so that the style is explicit?

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

2354

I agree how about IncludeSortCaseSensitive

[kentsommer]

Addressed comments

[kentsommer]

clang/docs/ClangFormatStyleOptions.rst

2354

I went with IncludeSortAlphabetically for now. To me, this reads better and seems to more clearly indicate the effect of turning this on. The downside is it no longer captures the fact that case does still play a part in the sort.

What do you both think?

2360

Good call, I definitely agree. I've rewritten this to better match the bool flag instances.

clang/lib/Format/Format.cpp

2241

This is a good point. I moved the if outside of the call to stable_sort. I don't feel there is actually much code duplication caused by this (other than the call signature).

[kentsommer]

Undo some accidental comment formatting

[curdeius]

LGTM. Thanks for being prompt at addressing the comments. I'll merge it today for you if @MyDeveloperDay has no more remarks.

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

2355

Are you sure IncludeSortAlphabetically expresses what you mean? Once these things get released they cannot be changed easily.

If you were not sure of a new option, in my view we should slow down and make sure we have the correct design, for example you could have used a enum and it might have given you more possibility for greater flexibility

enum IncludeSort
{
       CaseInsensitive
       CaseSensitive
}

Please give people time to re-review your changes before we commit, especially if they've taken the time to look at your review in the first place. Just saying.

[kentsommer]

clang/docs/ClangFormatStyleOptions.rst

2355

Hi, @MyDeveloperDay I definitely agree. It was not my intention to rush through the review. I was simply trying to follow the process outlined in https://llvm.org/docs/Contributing.html#how-to-submit-a-patch which mentions giving sufficient information to allow for a commit on your behalf when you don't have access after an LGTM (which is all that I did). As you can see from the lack of additional comments from my end, I was happy to let this sit and be reviewed.

Per the discussion about the option itself, I do believe IncludeSortAlphabetically currently expresses what I mean as the behavior with this off is indeed not an alphabetical sort as case takes precedence over the alphabetical ordering. However, looking at the enum and realizing that others probably will have additional styles they prefer (maybe they want alphabetical but lower case first, etc.) I do believe it might have been a better way to go as it leaves more flexibility and room for additional ordering styles. Given that this just landed, I would be happy to open a patch to turn this into an enum as I do see benefits to doing so. What do you think?

[curdeius]

Should we then revert ASAP and rework it later? @MyDeveloperDay

[kentsommer]

In D95017#2522063, @curdeius wrote:

Should we then revert ASAP and rework it later? @MyDeveloperDay

Reverting would also be fine with me. I am happy to drive towards a feature (and flags) that everyone is comfortable with in whichever way makes the most sense.

[curdeius]

Ok, reverted. If there's a doubt, then I guess it's the best solution.

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

2355

Hmmm, and how about using the existing option SortIncludes and change its type from bool to some enum?
We could then, for backward-compatibility, map false to (tentative) Never and true to ASCII/CaseInsensitive, and add CaseSensitive.

This will have the advantage of not adding additional style options.
... and it will prove once again that using bools for style options is not a good idea.

[curdeius]

Requesting changes so that it appears correctly in the review queue.

[MyDeveloperDay]

clang/docs/ClangFormatStyleOptions.rst

2355

I think that is an excellent idea @curdeius

[HazardyKnusperkeks]

clang/docs/ClangFormatStyleOptions.rst

2355

I also fully support that! (Although I would not say a bool is per se bad.)

[kentsommer]

Turn SortIncludes into an enum

[kentsommer]

clang/docs/ClangFormatStyleOptions.rst

2355

@curdeius @MyDeveloperDay @HazardyKnusperkeks this is now done.

However... The command-line option (--sort-includes) is not in a place that I like at the moment but I am having trouble thinking of any really good options.

The issue as it stands is that there are a lot of usages of the flag that assume it is a bool and therefore sometimes do not pass any value. These of course could be updated along with the flag to accept a std::string value, however, this breaks backward capability for anyone relying on that flag not requiring a value. As I have it now, backward compatibility is maintained but the command line flag is rather severely limited compared to the configuration option. Thoughts on which path to take? A third option I have not considered?

[kentsommer]

Remove improper enum mapping

[curdeius]

clang/lib/Format/Format.cpp

426

Is this needed?

[curdeius]

clang/lib/Format/Format.cpp

426

Ok. You just fixed it.

[curdeius]

Concerning the --sort-includes command-line flag. I'd rather keep it as is and, if need be, work on accepting an optional string argument.
Please update release notes.

[curdeius]

clang/docs/ClangFormatStyleOptions.rst

2355

I also fully support that! (Although I would not say a bool is per se bad.)

@HazardyKnusperkeks, I was of course a bit exaggerating :).

[kentsommer]

In D95017#2527512, @curdeius wrote:

Concerning the --sort-includes command-line flag. I'd rather keep it as is and, if need be, work on accepting an optional string argument.
Please update release notes.

@curdeius Just to confirm, are you asking for the "commit" message to be updated or something else?

[curdeius]

I was thinking of https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst#clang-format. Look at the history to see how it was written before.
Also, you may change the revision title if you want (it will become the commit message).

[kentsommer]

Added release notes, updated commit message and summary

[kentsommer]

Fixed release notes typo

[kentsommer]

NFC Rebase

[curdeius]

LGTM.

[MyDeveloperDay]

I like this much better LGTM

[kentsommer]

NFC rebase to fix CI

[PragmaNull]

I find the naming of the case sensitive options confusing here.
When I read "CaseSensitive" I think of the behavior of strcmp() which sorts according to "ASCIIbetical" order. But here "CaseSensitive" throws away case by comparing the result of "Filename.lower()" which I would consider case-insensitive, or the same behavior as a call to stricmp().

[curdeius]

Do you have an idea for better names?
I see that e.g. MS documentation uses ascending order and case-sensitive order.

[PragmaNull]

In D95017#2572238, @curdeius wrote:

Do you have an idea for better names?
I see that e.g. MS documentation uses ascending order and case-sensitive order.

I'm okay with the names, it just seems to me that they are reversed, where CaseSensitive should sort using ASCIIbetic order, and CaseInsensitive should ignore case entirely in the same way that most case-insensitive string compares would.

[HazardyKnusperkeks]

In D95017#2572578, @PragmaNull wrote:

In D95017#2572238, @curdeius wrote:

Do you have an idea for better names?
I see that e.g. MS documentation uses ascending order and case-sensitive order.

I'm okay with the names, it just seems to me that they are reversed, where CaseSensitive should sort using ASCIIbetic order, and CaseInsensitive should ignore case entirely in the same way that most case-insensitive string compares would.

After initially reading it I was not sure about it, but now I have looked into the wikipedia:

In computers, case sensitivity defines whether uppercase and lowercase letters are treated as distinct (case-sensitive) or equivalent (case-insensitive).

So I would support a name switch. I would wait a few days for @kentsommer to say something, if he doesn't respond just make a diff and add me as reviewer. :)

[HazardyKnusperkeks]

In D95017#2572578, @PragmaNull wrote:

In D95017#2572238, @curdeius wrote:

Do you have an idea for better names?
I see that e.g. MS documentation uses ascending order and case-sensitive order.

I'm okay with the names, it just seems to me that they are reversed, where CaseSensitive should sort using ASCIIbetic order, and CaseInsensitive should ignore case entirely in the same way that most case-insensitive string compares would.

Do you make a change? Otherwise I will do, but that will take some time, because I'm rather busy.

[PragmaNull]

In D95017#2595230, @HazardyKnusperkeks wrote:

Do you make a change? Otherwise I will do, but that will take some time, because I'm rather busy.

Sorry, I have not; unfortunately I've been rather busy myself and haven't had a moment to get on it.

clang/docs/ClangFormatStyleOptions.rst

**IncludeIsMainSourceRegex** (``std::string``)
   For example, if this option is configured to ``(Impl\.hpp)$``,
   then a file ``ClassImpl.hpp`` is considered "main" (in addition to
   ``Class.c``, ``Class.cc``, ``Class.cpp`` and so on) and "main
   include file" logic will be executed (with *IncludeIsMainRegex* setting
   also being respected in later phase). Without this option set,
   ``ClassImpl.hpp`` would not have the main include file put on top
   before any other include.
 
 **IndentCaseBlocks** (``bool``)
   Indent case label blocks one level from the case label.
 
   When ``false``, the block following the case label uses the same
   indentation level as for the case label, treating the case label the same
   as an if-statement.
   When ``true``, the block gets indented as a scope block.
 
   .. code-block:: c++
 
      false:                                 true:
      switch (fool) {                vs.     switch (fool) {
      case 1: {                              case 1:
        bar();                                 {
      } break;                                   bar();
.. code-block:: c++
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
 
      true:
      // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
      // information
      /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
       * information */
 
-**SortIncludes** (``bool``)
-  If ``true``, clang-format will sort ``#includes``.
+**SortIncludes** (``SortIncludesOptions``)
+  Controls if and how clang-format will sort ``#includes``.
+
+  Possible Values:
+
+  * ``SI_Never`` (in configuration ``Never``)
+    Includes are never sorted.
 
     .. code-block:: c++
 
-     false:                                 true:
-     #include "b.h"                 vs.     #include "a.h"
-     #include "a.h"                         #include "b.h"
+      #include "B/A.h"
+      #include "A/B.h"
+      #include "a/b.h"
+      #include "A/b.h"
+      #include "B/a.h"
+
+  * ``SI_CaseInsensitive`` (in configuration ``CaseInsensitive``)
+    Includes are sorted in an ASCIIbetical or case insensitive fashion.
+
+    .. code-block:: c++
+
+      #include "A/B.h"
+      #include "A/b.h"
+      #include "B/A.h"
+      #include "B/a.h"
+      #include "a/b.h"
+
+  * ``SI_CaseSensitive`` (in configuration ``CaseSensitive``)
+    Includes are sorted in an alphabetical or case sensitive fashion.
+
+    .. code-block:: c++
+
+      #include "A/B.h"
+      #include "A/b.h"
+      #include "a/b.h"
+      #include "B/A.h"
+      #include "B/a.h"
 
 **SortJavaStaticImport** (``SortJavaStaticImportOptions``)
   When sorting Java imports, by default static imports are placed before
   non-static imports. If ``JavaStaticImportAfterImport`` is ``After``,
   static imports are placed after non-static imports.
 
   Possible values:
 

clang/docs/ReleaseNotes.rst

 - ...
 
 clang-format
 ------------
 
 - Option ``SpacesInLineCommentPrefix`` has been added to control the
   number of spaces in a line comments prefix.
 
+- Option ``SortIncludes`` has been updated from a ``bool`` to an
+  ``enum`` with backwards compatibility. In addition to the previous
+  ``true``/``false`` states (now ``CaseInsensitive``/``Never``), a third
+  state has been added (``CaseSensitive``) which causes an alphabetical sort
+  with case used as a tie-breaker.
+
+  .. code-block:: c++
+
+    // Never (previously false)
+    #include "B/A.h"
+    #include "A/B.h"
+    #include "a/b.h"
+    #include "A/b.h"
+    #include "B/a.h"
+
+    // CaseInsensitive (previously true)
+    #include "A/B.h"
+    #include "A/b.h"
+    #include "B/A.h"
+    #include "B/a.h"
+    #include "a/b.h"
+
+    // CaseSensitive
+    #include "A/B.h"
+    #include "A/b.h"
+    #include "a/b.h"
+    #include "B/A.h"
+    #include "B/a.h"
+
 libclang
 --------
 
 - ...
 
 Static Analyzer
 ---------------
 

clang/include/clang/Format/Format.h

   ///    // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
   ///    // information
   ///    /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
   ///     * information */
   /// \endcode
   bool ReflowComments;
   // clang-format on
 
-  /// If ``true``, clang-format will sort ``#includes``.
-  /// \code
-  ///    false:                                 true:
-  ///    #include "b.h"                 vs.     #include "a.h"
-  ///    #include "a.h"                         #include "b.h"
-  /// \endcode
-  bool SortIncludes;
+  /// Include sorting options.
+  enum SortIncludesOptions : unsigned char {
+    /// Includes are never sorted.
+    /// \code
+    ///    #include "B/A.h"
+    ///    #include "A/B.h"
+    ///    #include "a/b.h"
+    ///    #include "A/b.h"
+    ///    #include "B/a.h"
+    /// \endcode
+    SI_Never,
+    /// Includes are sorted in an ASCIIbetical or case insensitive fashion.
+    /// \code
+    ///    #include "A/B.h"
+    ///    #include "A/b.h"
+    ///    #include "B/A.h"
+    ///    #include "B/a.h"
+    ///    #include "a/b.h"
+    /// \endcode
+    SI_CaseInsensitive,
+    /// Includes are sorted in an alphabetical or case sensitive fashion.
+    /// \code
+    ///    #include "A/B.h"
+    ///    #include "A/b.h"
+    ///    #include "a/b.h"
+    ///    #include "B/A.h"
+    ///    #include "B/a.h"
+    /// \endcode
+    SI_CaseSensitive,
+  };
+
+  /// Controls if and how clang-format will sort ``#includes``.
+  /// If ``Never``, includes are never sorted.
+  /// If ``CaseInsensitive``, includes are sorted in an ASCIIbetical or case
+  /// insensitive fashion.
+  /// If ``CaseSensitive``, includes are sorted in an alphabetical or case
+  /// sensitive fashion.
+  SortIncludesOptions SortIncludes;
 
   /// Position for Java Static imports.
   enum SortJavaStaticImportOptions : unsigned char {
     /// Static imports are placed before non-static imports.
     /// \code{.java}
     ///   import static org.example.function1;
     ///
     ///   import org.example.ClassA;
return AccessModifierOffset == R.AccessModifierOffset &&
            PenaltyBreakFirstLessLess == R.PenaltyBreakFirstLessLess &&
            PenaltyBreakString == R.PenaltyBreakString &&
            PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
            PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
            PenaltyBreakTemplateDeclaration ==
                R.PenaltyBreakTemplateDeclaration &&
            PointerAlignment == R.PointerAlignment &&
            RawStringFormats == R.RawStringFormats &&
+           SortIncludes == R.SortIncludes &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
            SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
            SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
            SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
            SpaceBeforeCaseColon == R.SpaceBeforeCaseColon &&
            SpaceBeforeCpp11BracedList == R.SpaceBeforeCpp11BracedList &&
            SpaceBeforeCtorInitializerColon ==

clang/lib/Format/Format.cpp

static void enumeration(IO &IO,
                           FormatStyle::BitFieldColonSpacingStyle &Value) {
     IO.enumCase(Value, "Both", FormatStyle::BFCS_Both);
     IO.enumCase(Value, "None", FormatStyle::BFCS_None);
     IO.enumCase(Value, "Before", FormatStyle::BFCS_Before);
     IO.enumCase(Value, "After", FormatStyle::BFCS_After);
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::SortIncludesOptions> {
+  static void enumeration(IO &IO, FormatStyle::SortIncludesOptions &Value) {
+    IO.enumCase(Value, "Never", FormatStyle::SI_Never);
+    IO.enumCase(Value, "CaseInsensitive", FormatStyle::SI_CaseInsensitive);
+    IO.enumCase(Value, "CaseSensitive", FormatStyle::SI_CaseSensitive);
+
+    // For backward compatibility.
+    IO.enumCase(Value, "false", FormatStyle::SI_Never);
+    IO.enumCase(Value, "true", FormatStyle::SI_CaseInsensitive);
+  }
+};
+
 template <>
 struct ScalarEnumerationTraits<FormatStyle::SortJavaStaticImportOptions> {
   static void enumeration(IO &IO,
                           FormatStyle::SortJavaStaticImportOptions &Value) {
     IO.enumCase(Value, "Before", FormatStyle::SJSIO_Before);
     IO.enumCase(Value, "After", FormatStyle::SJSIO_After);
   }
 };
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.PenaltyBreakString = 1000;
   LLVMStyle.PenaltyExcessCharacter = 1000000;
   LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 60;
   LLVMStyle.PenaltyBreakBeforeFirstCallParameter = 19;
   LLVMStyle.PenaltyBreakTemplateDeclaration = prec::Relational;
   LLVMStyle.PenaltyIndentedWhitespace = 0;
 
   LLVMStyle.DisableFormat = false;
-  LLVMStyle.SortIncludes = true;
+  LLVMStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
   LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = true;
   LLVMStyle.StatementAttributeLikeMacros.push_back("Q_EMIT");
   LLVMStyle.StatementMacros.push_back("Q_UNUSED");
   LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("PP_STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("BOOST_PP_STRINGIZE");
ChromiumStyle.JavaImportGroups = {
         "dalvik",
         "junit",
         "org",
         "com.google.android.apps.chrome",
         "org.chromium",
         "java",
         "javax",
     };
-    ChromiumStyle.SortIncludes = true;
+    ChromiumStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
   } else if (Language == FormatStyle::LK_JavaScript) {
     ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
     ChromiumStyle.AllowShortLoopsOnASingleLine = false;
   } else {
     ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
     ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
     ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
     ChromiumStyle.AllowShortLoopsOnASingleLine = false;
FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   return Style;
 }
 
 FormatStyle getNoStyle() {
   FormatStyle NoStyle = getLLVMStyle();
   NoStyle.DisableFormat = true;
-  NoStyle.SortIncludes = false;
+  NoStyle.SortIncludes = FormatStyle::SI_Never;
   NoStyle.SortUsingDeclarations = false;
   return NoStyle;
 }
 
 bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language,
                         FormatStyle *Style) {
   if (Name.equals_lower("llvm")) {
     *Style = getLLVMStyle(Language);
unsigned IncludesEndOffset =
       Includes.back().Offset + Includes.back().Text.size();
   unsigned IncludesBlockSize = IncludesEndOffset - IncludesBeginOffset;
   if (!affectsRange(Ranges, IncludesBeginOffset, IncludesEndOffset))
     return;
   SmallVector<unsigned, 16> Indices;
   for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
     Indices.push_back(i);
   }
+
+  if (Style.SortIncludes == FormatStyle::SI_CaseSensitive) {
+    llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
+      const auto LHSFilenameLower = Includes[LHSI].Filename.lower();
+      const auto RHSFilenameLower = Includes[RHSI].Filename.lower();
+      return std::tie(Includes[LHSI].Priority, LHSFilenameLower,
+                      Includes[LHSI].Filename) <
+             std::tie(Includes[RHSI].Priority, RHSFilenameLower,
+                      Includes[RHSI].Filename);
+    });
+  } else {
     llvm::stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) {
       return std::tie(Includes[LHSI].Priority, Includes[LHSI].Filename) <
              std::tie(Includes[RHSI].Priority, Includes[RHSI].Filename);
     });
+  }
+
   // The index of the include on which the cursor will be put after
   // sorting/deduplicating.
   unsigned CursorIndex;
   // The offset from cursor to the end of line.
   unsigned CursorToEOLOffset;
   if (Cursor)
     std::tie(CursorIndex, CursorToEOLOffset) =
         FindCursorIndex(Includes, Indices, *Cursor);

clang/tools/clang-format/ClangFormat.cpp

static bool format(StringRef FileName) {
   llvm::Expected<FormatStyle> FormatStyle =
       getStyle(Style, AssumedFileName, FallbackStyle, Code->getBuffer(),
                nullptr, WNoErrorList.isSet(WNoError::Unknown));
   if (!FormatStyle) {
     llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
     return true;
   }
 
-  if (SortIncludes.getNumOccurrences() != 0)
-    FormatStyle->SortIncludes = SortIncludes;
+  if (SortIncludes.getNumOccurrences() != 0) {
+    if (SortIncludes)
+      FormatStyle->SortIncludes = FormatStyle::SI_CaseInsensitive;
+    else
+      FormatStyle->SortIncludes = FormatStyle::SI_Never;
+  }
   unsigned CursorPosition = Cursor;
   Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges,
                                        AssumedFileName, &CursorPosition);
   auto ChangedCode = tooling::applyAllReplacements(Code->getBuffer(), Replaces);
   if (!ChangedCode) {
     llvm::errs() << llvm::toString(ChangedCode.takeError()) << "\n";
     return true;
   }

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentRequires);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
   CHECK_PARSE_BOOL(Cpp11BracedListStyle);
   CHECK_PARSE_BOOL(ReflowComments);
-  CHECK_PARSE_BOOL(SortIncludes);
   CHECK_PARSE_BOOL(SortUsingDeclarations);
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
   CHECK_PARSE_BOOL(SpacesInConditionalStatement);
   CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
CHECK_PARSE("IncludeCategories:\n"
               "    Priority: 1\n"
               "    CaseSensitive: true\n",
               IncludeStyle.IncludeCategories, ExpectedCategories);
   CHECK_PARSE("IncludeIsMainRegex: 'abc$'", IncludeStyle.IncludeIsMainRegex,
               "abc$");
   CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'",
               IncludeStyle.IncludeIsMainSourceRegex, "abc$");
 
+  Style.SortIncludes = FormatStyle::SI_Never;
+  CHECK_PARSE("SortIncludes: true", SortIncludes,
+              FormatStyle::SI_CaseInsensitive);
+  CHECK_PARSE("SortIncludes: false", SortIncludes, FormatStyle::SI_Never);
+  CHECK_PARSE("SortIncludes: CaseInsensitive", SortIncludes,
+              FormatStyle::SI_CaseInsensitive);
+  CHECK_PARSE("SortIncludes: CaseSensitive", SortIncludes,
+              FormatStyle::SI_CaseSensitive);
+  CHECK_PARSE("SortIncludes: Never", SortIncludes, FormatStyle::SI_Never);
+
   Style.RawStringFormats.clear();
   std::vector<FormatStyle::RawStringFormat> ExpectedRawStringFormats = {
       {
           FormatStyle::LK_TextProto,
           {"pb", "proto"},
           {"PARSE_TEXT_PROTO"},
           /*CanonicalDelimiter=*/"",
           "llvm",
std::string Expected = "#include \"a.h\"\n"
                          "  return 0;\n"
                          "}";
   FileID ID = Context.createInMemoryFile("fix.cpp", Code);
   tooling::Replacements Replaces = toReplacements(
       {tooling::Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 0,
                             "#include \"b.h\"\n")});
 
   format::FormatStyle Style = format::getLLVMStyle();
-  Style.SortIncludes = true;
+  Style.SortIncludes = FormatStyle::SI_CaseInsensitive;
   auto FormattedReplaces = formatReplacements(Code, Replaces, Style);
   EXPECT_TRUE(static_cast<bool>(FormattedReplaces))
       << llvm::toString(FormattedReplaces.takeError()) << "\n";
   auto Result = applyAllReplacements(Code, *FormattedReplaces);
   EXPECT_TRUE(static_cast<bool>(Result));
   EXPECT_EQ(Expected, *Result);
 }
 

clang/unittests/Format/SortImportsTestJava.cpp

protected:
   std::string sort(StringRef Code) { return sort(Code, GetCodeRange(Code)); }
 
   FormatStyle FmtStyle;
 
 public:
   SortImportsTestJava() {
     FmtStyle = getGoogleStyle(FormatStyle::LK_Java);
     FmtStyle.JavaImportGroups = {"com.test", "org", "com"};
-    FmtStyle.SortIncludes = true;
+    FmtStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
   }
 };
 
 TEST_F(SortImportsTestJava, StaticSplitFromNormal) {
   EXPECT_EQ("import static org.b;\n"
             "\n"
             "import org.a;\n",
             sort("import org.a;\n"

clang/unittests/Format/SortIncludesTest.cpp

EXPECT_EQ("#include <a>\n"
                  "#include <b>\n"
                  "#include <a>\n"
                  "#include <c>\n"
                  "/* clang-format onwards */\n",
                  "input.h", 2));
 }
 
 TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {
-  FmtStyle.SortIncludes = false;
+  FmtStyle.SortIncludes = FormatStyle::SI_Never;
   EXPECT_EQ("#include \"a.h\"\n"
             "#include \"c.h\"\n"
             "#include \"b.h\"\n",
             sort("#include \"a.h\"\n"
                  "#include \"c.h\"\n"
                  "#include \"b.h\"\n",
                  "input.h", 0));
 }
EXPECT_EQ("#include \"a.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, SupportOptionalCaseSensitiveSorting) {
+  EXPECT_FALSE(FmtStyle.SortIncludes == FormatStyle::SI_CaseSensitive);
+
+  FmtStyle.SortIncludes = FormatStyle::SI_CaseSensitive;
+
+  EXPECT_EQ("#include \"A/B.h\"\n"
+            "#include \"A/b.h\"\n"
+            "#include \"a/b.h\"\n"
+            "#include \"B/A.h\"\n"
+            "#include \"B/a.h\"\n",
+            sort("#include \"B/a.h\"\n"
+                 "#include \"B/A.h\"\n"
+                 "#include \"A/B.h\"\n"
+                 "#include \"a/b.h\"\n"
+                 "#include \"A/b.h\"\n",
+                 "a.h"));
+
+  Style.IncludeBlocks = clang::tooling::IncludeStyle::IBS_Regroup;
+  Style.IncludeCategories = {
+      {"^\"", 1, 0, false}, {"^<.*\\.h>$", 2, 0, false}, {"^<", 3, 0, false}};
+
+  StringRef UnsortedCode = "#include \"qt.h\"\n"
+                           "#include <algorithm>\n"
+                           "#include <qtwhatever.h>\n"
+                           "#include <Qtwhatever.h>\n"
+                           "#include <Algorithm>\n"
+                           "#include \"vlib.h\"\n"
+                           "#include \"Vlib.h\"\n"
+                           "#include \"AST.h\"\n";
+
+  EXPECT_EQ("#include \"AST.h\"\n"
+            "#include \"qt.h\"\n"
+            "#include \"Vlib.h\"\n"
+            "#include \"vlib.h\"\n"
+            "\n"
+            "#include <Qtwhatever.h>\n"
+            "#include <qtwhatever.h>\n"
+            "\n"
+            "#include <Algorithm>\n"
+            "#include <algorithm>\n",
+            sort(UnsortedCode));
+}
+
 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"