Skip to content

Commit e5fcbfa

Browse files
owencaowenca
authored
[clang-format] Add an option for editing enum trailing commas (llvm#133576)
Also refactor the code that removes/replaces a token.
1 parent 2796e41 commit e5fcbfa

File tree

6 files changed

+198
-36
lines changed

6 files changed

+198
-36
lines changed

‎clang/docs/ClangFormatStyleOptions.rst‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3976,6 +3976,47 @@ the configuration (without a prefix: ``Auto``).
39763976

39773977

39783978

3979+
.. _EnumTrailingComma:
3980+
3981+
**EnumTrailingComma** (``EnumTrailingCommaStyle``) :versionbadge:`clang-format 21` :ref:`<EnumTrailingComma>`
3982+
Insert a comma (if missing) or remove the comma at the end of an ``enum``
3983+
enumerator list.
3984+
3985+
.. warning::
3986+
3987+
Setting this option to any value other than ``Leave`` could lead to
3988+
incorrect code formatting due to clang-format's lack of complete semantic
3989+
information. As such, extra care should be taken to review code changes
3990+
made by this option.
3991+
3992+
Possible values:
3993+
3994+
* ``ETC_Leave`` (in configuration: ``Leave``)
3995+
Don't insert or remove trailing commas.
3996+
3997+
.. code-block:: c++
3998+
3999+
enum { a, b, c, };
4000+
enum Color { red, green, blue };
4001+
4002+
* ``ETC_Insert`` (in configuration: ``Insert``)
4003+
Insert trailing commas.
4004+
4005+
.. code-block:: c++
4006+
4007+
enum { a, b, c, };
4008+
enum Color { red, green, blue, };
4009+
4010+
* ``ETC_Remove`` (in configuration: ``Remove``)
4011+
Remove trailing commas.
4012+
4013+
.. code-block:: c++
4014+
4015+
enum { a, b, c };
4016+
enum Color { red, green, blue };
4017+
4018+
4019+
39794020
.. _ExperimentalAutoDetectBinPacking:
39804021

39814022
**ExperimentalAutoDetectBinPacking** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`<ExperimentalAutoDetectBinPacking>`

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,8 @@ clang-format
492492
- Allow specifying the language (C, C++, or Objective-C) for a ``.h`` file by
493493
adding a special comment (e.g. ``// clang-format Language: ObjC``) near the
494494
top of the file.
495+
- Add ``EnumTrailingComma`` option for inserting/removing commas at the end of
496+
``enum`` enumerator lists.
495497

496498
libclang
497499
--------

‎clang/include/clang/Format/Format.h‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,6 +2704,39 @@ struct FormatStyle {
27042704
/// \version 12
27052705
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
27062706

2707+
/// Styles for ``enum`` trailing commas.
2708+
enum EnumTrailingCommaStyle : int8_t {
2709+
/// Don't insert or remove trailing commas.
2710+
/// \code
2711+
/// enum { a, b, c, };
2712+
/// enum Color { red, green, blue };
2713+
/// \endcode
2714+
ETC_Leave,
2715+
/// Insert trailing commas.
2716+
/// \code
2717+
/// enum { a, b, c, };
2718+
/// enum Color { red, green, blue, };
2719+
/// \endcode
2720+
ETC_Insert,
2721+
/// Remove trailing commas.
2722+
/// \code
2723+
/// enum { a, b, c };
2724+
/// enum Color { red, green, blue };
2725+
/// \endcode
2726+
ETC_Remove,
2727+
};
2728+
2729+
/// Insert a comma (if missing) or remove the comma at the end of an ``enum``
2730+
/// enumerator list.
2731+
/// \warning
2732+
/// Setting this option to any value other than ``Leave`` could lead to
2733+
/// incorrect code formatting due to clang-format's lack of complete semantic
2734+
/// information. As such, extra care should be taken to review code changes
2735+
/// made by this option.
2736+
/// \endwarning
2737+
/// \version 21
2738+
EnumTrailingCommaStyle EnumTrailingComma;
2739+
27072740
/// If ``true``, clang-format detects whether function calls and
27082741
/// definitions are formatted with one parameter per line.
27092742
///
@@ -5323,6 +5356,7 @@ struct FormatStyle {
53235356
DisableFormat == R.DisableFormat &&
53245357
EmptyLineAfterAccessModifier == R.EmptyLineAfterAccessModifier &&
53255358
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
5359+
EnumTrailingComma == R.EnumTrailingComma &&
53265360
ExperimentalAutoDetectBinPacking ==
53275361
R.ExperimentalAutoDetectBinPacking &&
53285362
FixNamespaceComments == R.FixNamespaceComments &&

‎clang/lib/Format/Format.cpp‎

Lines changed: 81 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,15 @@ struct ScalarEnumerationTraits<
361361
}
362362
};
363363

364+
template <>
365+
struct ScalarEnumerationTraits<FormatStyle::EnumTrailingCommaStyle> {
366+
static void enumeration(IO &IO, FormatStyle::EnumTrailingCommaStyle &Value) {
367+
IO.enumCase(Value, "Leave", FormatStyle::ETC_Leave);
368+
IO.enumCase(Value, "Insert", FormatStyle::ETC_Insert);
369+
IO.enumCase(Value, "Remove", FormatStyle::ETC_Remove);
370+
}
371+
};
372+
364373
template <>
365374
struct ScalarEnumerationTraits<FormatStyle::IndentExternBlockStyle> {
366375
static void enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) {
@@ -1042,6 +1051,7 @@ template <> struct MappingTraits<FormatStyle> {
10421051
Style.EmptyLineAfterAccessModifier);
10431052
IO.mapOptional("EmptyLineBeforeAccessModifier",
10441053
Style.EmptyLineBeforeAccessModifier);
1054+
IO.mapOptional("EnumTrailingComma", Style.EnumTrailingComma);
10451055
IO.mapOptional("ExperimentalAutoDetectBinPacking",
10461056
Style.ExperimentalAutoDetectBinPacking);
10471057
IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
@@ -1558,6 +1568,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
15581568
LLVMStyle.DisableFormat = false;
15591569
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
15601570
LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
1571+
LLVMStyle.EnumTrailingComma = FormatStyle::ETC_Leave;
15611572
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
15621573
LLVMStyle.FixNamespaceComments = true;
15631574
LLVMStyle.ForEachMacros.push_back("foreach");
@@ -2203,6 +2214,21 @@ FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind Language) const {
22032214

22042215
namespace {
22052216

2217+
void replaceToken(const FormatToken &Token, FormatToken *Next,
2218+
const SourceManager &SourceMgr, tooling::Replacements &Result,
2219+
StringRef Text = "") {
2220+
const auto &Tok = Token.Tok;
2221+
SourceLocation Start;
2222+
if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
2223+
Start = Tok.getLocation();
2224+
Next->WhitespaceRange = Token.WhitespaceRange;
2225+
} else {
2226+
Start = Token.WhitespaceRange.getBegin();
2227+
}
2228+
const auto &Range = CharSourceRange::getCharRange(Start, Tok.getEndLoc());
2229+
cantFail(Result.add(tooling::Replacement(SourceMgr, Range, Text)));
2230+
}
2231+
22062232
class ParensRemover : public TokenAnalyzer {
22072233
public:
22082234
ParensRemover(const Environment &Env, const FormatStyle &Style)
@@ -2229,20 +2255,8 @@ class ParensRemover : public TokenAnalyzer {
22292255
continue;
22302256
for (const auto *Token = Line->First; Token && !Token->Finalized;
22312257
Token = Token->Next) {
2232-
if (!Token->Optional || !Token->isOneOf(tok::l_paren, tok::r_paren))
2233-
continue;
2234-
auto *Next = Token->Next;
2235-
assert(Next && Next->isNot(tok::eof));
2236-
SourceLocation Start;
2237-
if (Next->NewlinesBefore == 0) {
2238-
Start = Token->Tok.getLocation();
2239-
Next->WhitespaceRange = Token->WhitespaceRange;
2240-
} else {
2241-
Start = Token->WhitespaceRange.getBegin();
2242-
}
2243-
const auto &Range =
2244-
CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
2245-
cantFail(Result.add(tooling::Replacement(SourceMgr, Range, " ")));
2258+
if (Token->Optional && Token->isOneOf(tok::l_paren, tok::r_paren))
2259+
replaceToken(*Token, Token->Next, SourceMgr, Result, " ");
22462260
}
22472261
}
22482262
}
@@ -2331,24 +2345,13 @@ class BracesRemover : public TokenAnalyzer {
23312345
const auto *NextLine = I + 1 == End ? nullptr : I[1];
23322346
for (const auto *Token = Line->First; Token && !Token->Finalized;
23332347
Token = Token->Next) {
2334-
if (!Token->Optional)
2335-
continue;
2336-
if (!Token->isOneOf(tok::l_brace, tok::r_brace))
2348+
if (!Token->Optional || !Token->isOneOf(tok::l_brace, tok::r_brace))
23372349
continue;
23382350
auto *Next = Token->Next;
23392351
assert(Next || Token == Line->Last);
23402352
if (!Next && NextLine)
23412353
Next = NextLine->First;
2342-
SourceLocation Start;
2343-
if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
2344-
Start = Token->Tok.getLocation();
2345-
Next->WhitespaceRange = Token->WhitespaceRange;
2346-
} else {
2347-
Start = Token->WhitespaceRange.getBegin();
2348-
}
2349-
const auto &Range =
2350-
CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
2351-
cantFail(Result.add(tooling::Replacement(SourceMgr, Range, "")));
2354+
replaceToken(*Token, Next, SourceMgr, Result);
23522355
}
23532356
}
23542357
}
@@ -2400,16 +2403,51 @@ class SemiRemover : public TokenAnalyzer {
24002403
assert(Next || Token == Line->Last);
24012404
if (!Next && NextLine)
24022405
Next = NextLine->First;
2403-
SourceLocation Start;
2404-
if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
2405-
Start = Token->Tok.getLocation();
2406-
Next->WhitespaceRange = Token->WhitespaceRange;
2407-
} else {
2408-
Start = Token->WhitespaceRange.getBegin();
2406+
replaceToken(*Token, Next, SourceMgr, Result);
2407+
}
2408+
}
2409+
}
2410+
};
2411+
2412+
class EnumTrailingCommaEditor : public TokenAnalyzer {
2413+
public:
2414+
EnumTrailingCommaEditor(const Environment &Env, const FormatStyle &Style)
2415+
: TokenAnalyzer(Env, Style) {}
2416+
2417+
std::pair<tooling::Replacements, unsigned>
2418+
analyze(TokenAnnotator &Annotator,
2419+
SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
2420+
FormatTokenLexer &Tokens) override {
2421+
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
2422+
tooling::Replacements Result;
2423+
editEnumTrailingComma(AnnotatedLines, Result);
2424+
return {Result, 0};
2425+
}
2426+
2427+
private:
2428+
void editEnumTrailingComma(SmallVectorImpl<AnnotatedLine *> &Lines,
2429+
tooling::Replacements &Result) {
2430+
const auto &SourceMgr = Env.getSourceManager();
2431+
for (auto *Line : Lines) {
2432+
if (!Line->Children.empty())
2433+
editEnumTrailingComma(Line->Children, Result);
2434+
if (!Line->Affected)
2435+
continue;
2436+
for (const auto *Token = Line->First; Token && !Token->Finalized;
2437+
Token = Token->Next) {
2438+
if (Token->isNot(TT_EnumRBrace))
2439+
continue;
2440+
const auto *BeforeRBrace = Token->getPreviousNonComment();
2441+
assert(BeforeRBrace);
2442+
if (BeforeRBrace->is(TT_EnumLBrace)) // Empty braces.
2443+
continue;
2444+
if (BeforeRBrace->is(tok::comma)) {
2445+
if (Style.EnumTrailingComma == FormatStyle::ETC_Remove)
2446+
replaceToken(*BeforeRBrace, BeforeRBrace->Next, SourceMgr, Result);
2447+
} else if (Style.EnumTrailingComma == FormatStyle::ETC_Insert) {
2448+
cantFail(Result.add(tooling::Replacement(
2449+
SourceMgr, BeforeRBrace->Tok.getEndLoc(), 0, ",")));
24092450
}
2410-
const auto &Range =
2411-
CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
2412-
cantFail(Result.add(tooling::Replacement(SourceMgr, Range, "")));
24132451
}
24142452
}
24152453
}
@@ -3812,6 +3850,13 @@ reformat(const FormatStyle &Style, StringRef Code,
38123850
});
38133851
}
38143852

3853+
if (Style.EnumTrailingComma != FormatStyle::ETC_Leave) {
3854+
Passes.emplace_back([&](const Environment &Env) {
3855+
return EnumTrailingCommaEditor(Env, Expanded)
3856+
.process(/*SkipAnnotation=*/true);
3857+
});
3858+
}
3859+
38153860
if (Style.FixNamespaceComments) {
38163861
Passes.emplace_back([&](const Environment &Env) {
38173862
return NamespaceEndCommentsFixer(Env, Expanded).process();

‎clang/unittests/Format/ConfigParseTest.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,14 @@ TEST(ConfigParseTest, ParsesConfiguration) {
520520
CHECK_PARSE("EmptyLineBeforeAccessModifier: Always",
521521
EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Always);
522522

523+
Style.EnumTrailingComma = FormatStyle::ETC_Insert;
524+
CHECK_PARSE("EnumTrailingComma: Leave", EnumTrailingComma,
525+
FormatStyle::ETC_Leave);
526+
CHECK_PARSE("EnumTrailingComma: Insert", EnumTrailingComma,
527+
FormatStyle::ETC_Insert);
528+
CHECK_PARSE("EnumTrailingComma: Remove", EnumTrailingComma,
529+
FormatStyle::ETC_Remove);
530+
523531
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
524532
CHECK_PARSE("AlignAfterOpenBracket: Align", AlignAfterOpenBracket,
525533
FormatStyle::BAS_Align);

‎clang/unittests/Format/FormatTest.cpp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27902,6 +27902,38 @@ TEST_F(FormatTest, RemoveSemicolon) {
2790227902
verifyFormat("STRUCT(T, B) { int i; };", Style);
2790327903
}
2790427904

27905+
TEST_F(FormatTest, EnumTrailingComma) {
27906+
constexpr StringRef Code("enum : int { /**/ };\n"
27907+
"enum {\n"
27908+
" a,\n"
27909+
" b,\n"
27910+
" c, //\n"
27911+
"};\n"
27912+
"enum Color { red, green, blue /**/ };");
27913+
verifyFormat(Code);
27914+
27915+
auto Style = getLLVMStyle();
27916+
Style.EnumTrailingComma = FormatStyle::ETC_Insert;
27917+
verifyFormat("enum : int { /**/ };\n"
27918+
"enum {\n"
27919+
" a,\n"
27920+
" b,\n"
27921+
" c, //\n"
27922+
"};\n"
27923+
"enum Color { red, green, blue, /**/ };",
27924+
Code, Style);
27925+
27926+
Style.EnumTrailingComma = FormatStyle::ETC_Remove;
27927+
verifyFormat("enum : int { /**/ };\n"
27928+
"enum {\n"
27929+
" a,\n"
27930+
" b,\n"
27931+
" c //\n"
27932+
"};\n"
27933+
"enum Color { red, green, blue /**/ };",
27934+
Code, Style);
27935+
}
27936+
2790527937
TEST_F(FormatTest, BreakAfterAttributes) {
2790627938
constexpr StringRef Code("[[maybe_unused]] const int i;\n"
2790727939
"[[foo([[]])]] [[maybe_unused]]\n"

0 commit comments

Comments
 (0)