[clang] a8105b3 - [clang-format] Add case aware include sorting.
Marek Kurdej via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 2 06:12:49 PST 2021
Author: Kent Sommer
Date: 2021-02-02T15:12:27+01:00
New Revision: a8105b3766e4195ca2390cd0714e07406bc8a4a5
URL: https://github.com/llvm/llvm-project/commit/a8105b3766e4195ca2390cd0714e07406bc8a4a5
DIFF: https://github.com/llvm/llvm-project/commit/a8105b3766e4195ca2390cd0714e07406bc8a4a5.diff
LOG: [clang-format] Add case aware include sorting.
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.
Reviewed By: MyDeveloperDay, curdeius, HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D95017
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/tools/clang-format/ClangFormat.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/SortImportsTestJava.cpp
clang/unittests/Format/SortIncludesTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index a28efa5dad04..d55c0d59b36a 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3004,14 +3004,43 @@ the configuration (without a prefix: ``Auto``).
/* 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``.
- .. code-block:: c++
+ Possible Values:
- false: true:
- #include "b.h" vs. #include "a.h"
- #include "a.h" #include "b.h"
+ * ``SI_Never`` (in configuration ``Never``)
+ Includes are never sorted.
+
+ .. code-block:: c++
+
+ #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
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6c5c993f98b1..d1a5156f0d00 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,6 +159,35 @@ 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
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 269707fedaac..7cedbfb80610 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2613,13 +2613,44 @@ struct FormatStyle {
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 {
@@ -3161,6 +3192,7 @@ struct FormatStyle {
R.PenaltyBreakTemplateDeclaration &&
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
+ SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 16a42f2d97c4..fbef7c5148a2 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -415,6 +415,18 @@ struct ScalarEnumerationTraits<FormatStyle::BitFieldColonSpacingStyle> {
}
};
+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,
@@ -1030,7 +1042,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
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");
@@ -1233,7 +1245,7 @@ FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
"java",
"javax",
};
- ChromiumStyle.SortIncludes = true;
+ ChromiumStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
} else if (Language == FormatStyle::LK_JavaScript) {
ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
ChromiumStyle.AllowShortLoopsOnASingleLine = false;
@@ -1347,7 +1359,7 @@ FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
FormatStyle getNoStyle() {
FormatStyle NoStyle = getLLVMStyle();
NoStyle.DisableFormat = true;
- NoStyle.SortIncludes = false;
+ NoStyle.SortIncludes = FormatStyle::SI_Never;
NoStyle.SortUsingDeclarations = false;
return NoStyle;
}
@@ -2226,10 +2238,23 @@ static void sortCppIncludes(const FormatStyle &Style,
for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
Indices.push_back(i);
}
- 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);
- });
+
+ 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;
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index 64f0e2badf33..e59ba663f7f0 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -402,8 +402,12 @@ static bool format(StringRef FileName) {
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);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 1090f0ca8ee2..da267b159f88 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -15463,7 +15463,6 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
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);
@@ -15959,6 +15958,16 @@ TEST_F(FormatTest, ParsesConfiguration) {
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 = {
{
@@ -17970,7 +17979,7 @@ TEST_F(ReplacementTest, SortIncludesAfterReplacement) {
"#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";
diff --git a/clang/unittests/Format/SortImportsTestJava.cpp b/clang/unittests/Format/SortImportsTestJava.cpp
index 4a3e18abd453..5a1d0de8b422 100644
--- a/clang/unittests/Format/SortImportsTestJava.cpp
+++ b/clang/unittests/Format/SortImportsTestJava.cpp
@@ -32,7 +32,7 @@ class SortImportsTestJava : public ::testing::Test {
SortImportsTestJava() {
FmtStyle = getGoogleStyle(FormatStyle::LK_Java);
FmtStyle.JavaImportGroups = {"com.test", "org", "com"};
- FmtStyle.SortIncludes = true;
+ FmtStyle.SortIncludes = FormatStyle::SI_CaseInsensitive;
}
};
diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp
index 41ff7afad10e..240757d9aaea 100644
--- a/clang/unittests/Format/SortIncludesTest.cpp
+++ b/clang/unittests/Format/SortIncludesTest.cpp
@@ -269,7 +269,7 @@ TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) {
}
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",
@@ -598,6 +598,49 @@ TEST_F(SortIncludesTest, MainHeaderIsSeparatedWhenRegroupping) {
"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))?$";
More information about the cfe-commits
mailing list