https://github.com/owenca created https://github.com/llvm/llvm-project/pull/140497
This allows adding other suboptions e.g. IgnoreExtension in #137840. >From 186ac09017c78cbff5a34a5b8768b6df5b8d0546 Mon Sep 17 00:00:00 2001 From: Owen Pan <owenpi...@gmail.com> Date: Sun, 18 May 2025 23:23:04 -0700 Subject: [PATCH] [clang-format][NFC] Upgrade SortIncludes option to a struct This allows adding other suboptions e.g. IgnoreExtension in #137840. --- clang/docs/ClangFormatStyleOptions.rst | 42 +++++---------- clang/include/clang/Format/Format.h | 52 ++++++++----------- clang/lib/Format/Format.cpp | 34 +++++++----- clang/tools/clang-format/ClangFormat.cpp | 5 +- clang/unittests/Format/ConfigParseTest.cpp | 19 ++++--- .../Format/FormatReplacementTest.cpp | 1 - .../unittests/Format/SortImportsTestJava.cpp | 3 +- clang/unittests/Format/SortIncludesTest.cpp | 6 +-- 8 files changed, 77 insertions(+), 85 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index a4c381bf583b6..83716cc049ee3 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -5990,41 +5990,25 @@ the configuration (without a prefix: ``Auto``). **SortIncludes** (``SortIncludesOptions``) :versionbadge:`clang-format 3.8` :ref:`¶ <SortIncludes>` Controls if and how clang-format will sort ``#includes``. - Possible values: - - * ``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_CaseSensitive`` (in configuration: ``CaseSensitive``) - Includes are sorted in an ASCIIbetical or case sensitive fashion. + Nested configuration flags: - .. code-block:: c++ + Includes sorting options. - #include "A/B.h" - #include "A/b.h" - #include "B/A.h" - #include "B/a.h" - #include "a/b.h" + * ``bool Enabled`` If ``true``, includes are sorted based on the other suboptions below. + (``Never`` is deprecated by ``Enabled: false``.) - * ``SI_CaseInsensitive`` (in configuration: ``CaseInsensitive``) - Includes are sorted in an alphabetical or case insensitive fashion. + * ``bool IgnoreCase`` Whether or not includes are sorted in a case-insensitive fashion. + (``CaseSensitive`` and ``CaseInsensitive`` are deprecated by + ``IgnoreCase: false`` and ``IgnoreCase: true``, respectively.) .. code-block:: c++ - #include "A/B.h" - #include "A/b.h" - #include "a/b.h" - #include "B/A.h" - #include "B/a.h" - + true: false: + #include "A/B.h" vs. #include "A/B.h" + #include "A/b.h" #include "A/b.h" + #include "a/b.h" #include "B/A.h" + #include "B/A.h" #include "B/a.h" + #include "B/a.h" #include "a/b.h" .. _SortJavaStaticImport: diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index b86c4bd00eb91..3ac4318824ac0 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -4365,35 +4365,29 @@ struct FormatStyle { /// \version 18 bool SkipMacroDefinitionBody; - /// Include sorting options. - enum SortIncludesOptions : int8_t { - /// 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 sensitive fashion. - /// \code - /// #include "A/B.h" - /// #include "A/b.h" - /// #include "B/A.h" - /// #include "B/a.h" - /// #include "a/b.h" - /// \endcode - SI_CaseSensitive, - /// Includes are sorted in an alphabetical or case insensitive fashion. - /// \code - /// #include "A/B.h" - /// #include "A/b.h" - /// #include "a/b.h" - /// #include "B/A.h" - /// #include "B/a.h" - /// \endcode - SI_CaseInsensitive, + /// Includes sorting options. + struct SortIncludesOptions { + /// If ``true``, includes are sorted based on the other suboptions below. + /// (``Never`` is deprecated by ``Enabled: false``.) + bool Enabled; + /// Whether or not includes are sorted in a case-insensitive fashion. + /// (``CaseSensitive`` and ``CaseInsensitive`` are deprecated by + /// ``IgnoreCase: false`` and ``IgnoreCase: true``, respectively.) + /// \code + /// true: false: + /// #include "A/B.h" vs. #include "A/B.h" + /// #include "A/b.h" #include "A/b.h" + /// #include "a/b.h" #include "B/A.h" + /// #include "B/A.h" #include "B/a.h" + /// #include "B/a.h" #include "a/b.h" + /// \endcode + bool IgnoreCase; + bool operator==(const SortIncludesOptions &R) const { + return Enabled == R.Enabled && IgnoreCase == R.IgnoreCase; + } + bool operator!=(const SortIncludesOptions &R) const { + return !(*this == R); + } }; /// Controls if and how clang-format will sort ``#includes``. diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 20b5352b83a9e..b0da0f17809af 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -659,15 +659,26 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ShortLambdaStyle> { } }; -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); +template <> struct MappingTraits<FormatStyle::SortIncludesOptions> { + static void enumInput(IO &IO, FormatStyle::SortIncludesOptions &Value) { + IO.enumCase(Value, "Never", FormatStyle::SortIncludesOptions({})); + IO.enumCase(Value, "CaseInsensitive", + FormatStyle::SortIncludesOptions({/*Enabled=*/true, + /*IgnoreCase=*/true})); + IO.enumCase(Value, "CaseSensitive", + FormatStyle::SortIncludesOptions({/*Enabled=*/true, + /*IgnoreCase=*/false})); // For backward compatibility. - IO.enumCase(Value, "false", FormatStyle::SI_Never); - IO.enumCase(Value, "true", FormatStyle::SI_CaseSensitive); + IO.enumCase(Value, "false", FormatStyle::SortIncludesOptions({})); + IO.enumCase(Value, "true", + FormatStyle::SortIncludesOptions({/*Enabled=*/true, + /*IgnoreCase=*/false})); + } + + static void mapping(IO &IO, FormatStyle::SortIncludesOptions &Value) { + IO.mapOptional("Enabled", Value.Enabled); + IO.mapOptional("IgnoreCase", Value.IgnoreCase); } }; @@ -1636,7 +1647,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; LLVMStyle.ShortNamespaceLines = 1; LLVMStyle.SkipMacroDefinitionBody = false; - LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive; + LLVMStyle.SortIncludes = {/*Enabled=*/true, /*IgnoreCase=*/false}; LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before; LLVMStyle.SortUsingDeclarations = FormatStyle::SUD_LexicographicNumeric; LLVMStyle.SpaceAfterCStyleCast = false; @@ -1901,7 +1912,6 @@ FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) { "java", "javax", }; - ChromiumStyle.SortIncludes = FormatStyle::SI_CaseSensitive; } else if (Language == FormatStyle::LK_JavaScript) { ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never; ChromiumStyle.AllowShortLoopsOnASingleLine = false; @@ -2029,7 +2039,7 @@ FormatStyle getClangFormatStyle() { FormatStyle getNoStyle() { FormatStyle NoStyle = getLLVMStyle(); NoStyle.DisableFormat = true; - NoStyle.SortIncludes = FormatStyle::SI_Never; + NoStyle.SortIncludes = {}; NoStyle.SortUsingDeclarations = FormatStyle::SUD_Never; return NoStyle; } @@ -3221,7 +3231,7 @@ static void sortCppIncludes(const FormatStyle &Style, SmallVector<unsigned, 16> Indices = llvm::to_vector<16>(llvm::seq<unsigned>(0, Includes.size())); - if (Style.SortIncludes == FormatStyle::SI_CaseInsensitive) { + if (Style.SortIncludes.Enabled && Style.SortIncludes.IgnoreCase) { stable_sort(Indices, [&](unsigned LHSI, unsigned RHSI) { const auto LHSFilenameLower = Includes[LHSI].Filename.lower(); const auto RHSFilenameLower = Includes[RHSI].Filename.lower(); @@ -3596,7 +3606,7 @@ tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, ArrayRef<tooling::Range> Ranges, StringRef FileName, unsigned *Cursor) { tooling::Replacements Replaces; - if (!Style.SortIncludes || Style.DisableFormat) + if (!Style.SortIncludes.Enabled || Style.DisableFormat) return Replaces; if (isLikelyXml(Code)) return Replaces; diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp index c45e3a2c28327..08fe95fbe9f03 100644 --- a/clang/tools/clang-format/ClangFormat.cpp +++ b/clang/tools/clang-format/ClangFormat.cpp @@ -478,10 +478,9 @@ static bool format(StringRef FileName, bool ErrorOnIncompleteFormat = false) { } if (SortIncludes.getNumOccurrences() != 0) { + FormatStyle->SortIncludes = {}; if (SortIncludes) - FormatStyle->SortIncludes = FormatStyle::SI_CaseSensitive; - else - FormatStyle->SortIncludes = FormatStyle::SI_Never; + FormatStyle->SortIncludes.Enabled = true; } unsigned CursorPosition = Cursor; Replacements Replaces = sortIncludes(*FormatStyle, Code->getBuffer(), Ranges, diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index bd27a9f60ffcc..5b9055d0a80be 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -257,6 +257,8 @@ TEST(ConfigParseTest, ParsesConfigurationBools) { CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InConditionalStatements); CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InEmptyParentheses); CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, Other); + CHECK_PARSE_NESTED_BOOL(SortIncludes, Enabled); + CHECK_PARSE_NESTED_BOOL(SortIncludes, IgnoreCase); } #undef CHECK_PARSE_BOOL @@ -976,15 +978,20 @@ TEST(ConfigParseTest, ParsesConfiguration) { CHECK_PARSE("IncludeIsMainSourceRegex: 'abc$'", IncludeStyle.IncludeIsMainSourceRegex, "abc$"); - Style.SortIncludes = FormatStyle::SI_Never; + Style.SortIncludes = {}; CHECK_PARSE("SortIncludes: true", SortIncludes, - FormatStyle::SI_CaseSensitive); - CHECK_PARSE("SortIncludes: false", SortIncludes, FormatStyle::SI_Never); + FormatStyle::SortIncludesOptions( + {/*Enabled=*/true, /*IgnoreCase=*/false})); + CHECK_PARSE("SortIncludes: false", SortIncludes, + FormatStyle::SortIncludesOptions({})); CHECK_PARSE("SortIncludes: CaseInsensitive", SortIncludes, - FormatStyle::SI_CaseInsensitive); + FormatStyle::SortIncludesOptions( + {/*Enabled=*/true, /*IgnoreCase=*/true})); CHECK_PARSE("SortIncludes: CaseSensitive", SortIncludes, - FormatStyle::SI_CaseSensitive); - CHECK_PARSE("SortIncludes: Never", SortIncludes, FormatStyle::SI_Never); + FormatStyle::SortIncludesOptions( + {/*Enabled=*/true, /*IgnoreCase=*/false})); + CHECK_PARSE("SortIncludes: Never", SortIncludes, + FormatStyle::SortIncludesOptions({})); Style.RawStringFormats.clear(); std::vector<FormatStyle::RawStringFormat> ExpectedRawStringFormats = { diff --git a/clang/unittests/Format/FormatReplacementTest.cpp b/clang/unittests/Format/FormatReplacementTest.cpp index e81c7e0371003..44896a7142e79 100644 --- a/clang/unittests/Format/FormatReplacementTest.cpp +++ b/clang/unittests/Format/FormatReplacementTest.cpp @@ -70,7 +70,6 @@ TEST_F(ReplacementTest, SortIncludesAfterReplacement) { "#include \"b.h\"\n")}); FormatStyle Style = getLLVMStyle(); - Style.SortIncludes = FormatStyle::SI_CaseSensitive; 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 d577efa34f86e..26674c75e97b1 100644 --- a/clang/unittests/Format/SortImportsTestJava.cpp +++ b/clang/unittests/Format/SortImportsTestJava.cpp @@ -31,8 +31,9 @@ class SortImportsTestJava : public testing::Test { public: SortImportsTestJava() { FmtStyle = getGoogleStyle(FormatStyle::LK_Java); + EXPECT_TRUE(FmtStyle.SortIncludes.Enabled); FmtStyle.JavaImportGroups = {"com.test", "org", "com"}; - FmtStyle.SortIncludes = FormatStyle::SI_CaseInsensitive; + FmtStyle.SortIncludes.IgnoreCase = true; } }; diff --git a/clang/unittests/Format/SortIncludesTest.cpp b/clang/unittests/Format/SortIncludesTest.cpp index f20862f5f7461..994227efdd4f8 100644 --- a/clang/unittests/Format/SortIncludesTest.cpp +++ b/clang/unittests/Format/SortIncludesTest.cpp @@ -284,7 +284,7 @@ TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) { } TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) { - FmtStyle.SortIncludes = FormatStyle::SI_Never; + FmtStyle.SortIncludes = {}; verifyFormat("#include \"a.h\"\n" "#include \"c.h\"\n" "#include \"b.h\"", @@ -628,9 +628,7 @@ TEST_F(SortIncludesTest, MainHeaderIsSeparatedWhenRegroupping) { } TEST_F(SortIncludesTest, SupportOptionalCaseSensitiveSorting) { - EXPECT_FALSE(FmtStyle.SortIncludes == FormatStyle::SI_CaseInsensitive); - - FmtStyle.SortIncludes = FormatStyle::SI_CaseInsensitive; + FmtStyle.SortIncludes.IgnoreCase = true; verifyFormat("#include \"A/B.h\"\n" "#include \"A/b.h\"\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits