65400 #140661
Closed
65400 #140661
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows adding other suboptions e.g. IgnoreExtension in llvm#137840.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-format @llvm/pr-subscribers-clang Author: Owen Pan (owenca) ChangesFull diff: https://github.com/llvm/llvm-project/pull/140661.diff 9 Files Affected:
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..2c5742d7dc3c7 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;
@@ -3799,8 +3809,10 @@ reformat(const FormatStyle &Style, StringRef Code,
tooling::Replacements Replaces =
Formatter(*Env, Style, Status).process().first;
// add a replacement to remove the "x = " from the result.
- Replaces = Replaces.merge(
- tooling::Replacements(tooling::Replacement(FileName, 0, 4, "")));
+ if (Code.starts_with("x = ")) {
+ Replaces = Replaces.merge(
+ tooling::Replacements(tooling::Replacement(FileName, 0, 4, "")));
+ }
// apply the reformatting changes and the removal of "x = ".
if (applyAllReplacements(Code, Replaces))
return {Replaces, 0};
diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp
index c45e3a2c28327..e40e04eac5472 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,
@@ -492,8 +491,8 @@ static bool format(StringRef FileName, bool ErrorOnIncompleteFormat = false) {
// To format JSON insert a variable to trick the code into thinking its
// JavaScript.
if (IsJson && !FormatStyle->DisableFormat) {
- auto Err = Replaces.add(tooling::Replacement(
- tooling::Replacement(AssumedFileName, 0, 0, "x = ")));
+ auto Err =
+ Replaces.add(tooling::Replacement(AssumedFileName, 0, 0, "x = "));
if (Err)
llvm::errs() << "Bad Json variable insertion\n";
}
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/FormatTestRawStrings.cpp b/clang/unittests/Format/FormatTestRawStrings.cpp
index 0615fb1fad4c5..7a7aa88665554 100644
--- a/clang/unittests/Format/FormatTestRawStrings.cpp
+++ b/clang/unittests/Format/FormatTestRawStrings.cpp
@@ -988,6 +988,28 @@ fffffffffffffffffffff("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
)pb");)test",
Style));
}
+
+TEST_F(FormatTestRawStrings, Json) {
+ auto Style = getLLVMStyle();
+ Style.RawStringFormats = {
+ {
+ /*Language=*/FormatStyle::LK_Json,
+ /*Delimiters=*/{"json"},
+ /*EnclosingFunctions=*/{},
+ /*CanonicalDelimiter=*/"",
+ /*BasedOnStyle=*/"llvm",
+ },
+ };
+
+ EXPECT_EQ(R"test(json = R"json({
+ "str": "test"
+ })json";)test",
+ format(R"test(json = R"json({
+ "str": "test"
+})json";)test",
+ Style));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
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"
|
owenca
removed
clang
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.