Author: Owen Pan Date: 2025-03-30T16:02:49-07:00 New Revision: e5fcbfa2aa8291a57e5eb03cd458935b458c73c0
URL: https://github.com/llvm/llvm-project/commit/e5fcbfa2aa8291a57e5eb03cd458935b458c73c0 DIFF: https://github.com/llvm/llvm-project/commit/e5fcbfa2aa8291a57e5eb03cd458935b458c73c0.diff LOG: [clang-format] Add an option for editing enum trailing commas (#133576) Also refactor the code that removes/replaces a token. Added: Modified: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/unittests/Format/ConfigParseTest.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index 9ecac68ae72bf..3f8a5f49313b2 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -3976,6 +3976,47 @@ the configuration (without a prefix: ``Auto``). +.. _EnumTrailingComma: + +**EnumTrailingComma** (``EnumTrailingCommaStyle``) :versionbadge:`clang-format 21` :ref:`¶ <EnumTrailingComma>` + Insert a comma (if missing) or remove the comma at the end of an ``enum`` + enumerator list. + + .. warning:: + + Setting this option to any value other than ``Leave`` could lead to + incorrect code formatting due to clang-format's lack of complete semantic + information. As such, extra care should be taken to review code changes + made by this option. + + Possible values: + + * ``ETC_Leave`` (in configuration: ``Leave``) + Don't insert or remove trailing commas. + + .. code-block:: c++ + + enum { a, b, c, }; + enum Color { red, green, blue }; + + * ``ETC_Insert`` (in configuration: ``Insert``) + Insert trailing commas. + + .. code-block:: c++ + + enum { a, b, c, }; + enum Color { red, green, blue, }; + + * ``ETC_Remove`` (in configuration: ``Remove``) + Remove trailing commas. + + .. code-block:: c++ + + enum { a, b, c }; + enum Color { red, green, blue }; + + + .. _ExperimentalAutoDetectBinPacking: **ExperimentalAutoDetectBinPacking** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <ExperimentalAutoDetectBinPacking>` diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e409f206f6eae..d72beb3a479b0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -492,6 +492,8 @@ clang-format - Allow specifying the language (C, C++, or Objective-C) for a ``.h`` file by adding a special comment (e.g. ``// clang-format Language: ObjC``) near the top of the file. +- Add ``EnumTrailingComma`` option for inserting/removing commas at the end of + ``enum`` enumerator lists. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index fec47a248abb4..cea5e257659d6 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -2704,6 +2704,39 @@ struct FormatStyle { /// \version 12 EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier; + /// Styles for ``enum`` trailing commas. + enum EnumTrailingCommaStyle : int8_t { + /// Don't insert or remove trailing commas. + /// \code + /// enum { a, b, c, }; + /// enum Color { red, green, blue }; + /// \endcode + ETC_Leave, + /// Insert trailing commas. + /// \code + /// enum { a, b, c, }; + /// enum Color { red, green, blue, }; + /// \endcode + ETC_Insert, + /// Remove trailing commas. + /// \code + /// enum { a, b, c }; + /// enum Color { red, green, blue }; + /// \endcode + ETC_Remove, + }; + + /// Insert a comma (if missing) or remove the comma at the end of an ``enum`` + /// enumerator list. + /// \warning + /// Setting this option to any value other than ``Leave`` could lead to + /// incorrect code formatting due to clang-format's lack of complete semantic + /// information. As such, extra care should be taken to review code changes + /// made by this option. + /// \endwarning + /// \version 21 + EnumTrailingCommaStyle EnumTrailingComma; + /// If ``true``, clang-format detects whether function calls and /// definitions are formatted with one parameter per line. /// @@ -5323,6 +5356,7 @@ struct FormatStyle { DisableFormat == R.DisableFormat && EmptyLineAfterAccessModifier == R.EmptyLineAfterAccessModifier && EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier && + EnumTrailingComma == R.EnumTrailingComma && ExperimentalAutoDetectBinPacking == R.ExperimentalAutoDetectBinPacking && FixNamespaceComments == R.FixNamespaceComments && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 28aea86139e0d..b74a8631efe0f 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -361,6 +361,15 @@ struct ScalarEnumerationTraits< } }; +template <> +struct ScalarEnumerationTraits<FormatStyle::EnumTrailingCommaStyle> { + static void enumeration(IO &IO, FormatStyle::EnumTrailingCommaStyle &Value) { + IO.enumCase(Value, "Leave", FormatStyle::ETC_Leave); + IO.enumCase(Value, "Insert", FormatStyle::ETC_Insert); + IO.enumCase(Value, "Remove", FormatStyle::ETC_Remove); + } +}; + template <> struct ScalarEnumerationTraits<FormatStyle::IndentExternBlockStyle> { static void enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) { @@ -1042,6 +1051,7 @@ template <> struct MappingTraits<FormatStyle> { Style.EmptyLineAfterAccessModifier); IO.mapOptional("EmptyLineBeforeAccessModifier", Style.EmptyLineBeforeAccessModifier); + IO.mapOptional("EnumTrailingComma", Style.EnumTrailingComma); IO.mapOptional("ExperimentalAutoDetectBinPacking", Style.ExperimentalAutoDetectBinPacking); IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments); @@ -1558,6 +1568,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.DisableFormat = false; LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never; LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; + LLVMStyle.EnumTrailingComma = FormatStyle::ETC_Leave; LLVMStyle.ExperimentalAutoDetectBinPacking = false; LLVMStyle.FixNamespaceComments = true; LLVMStyle.ForEachMacros.push_back("foreach"); @@ -2203,6 +2214,21 @@ FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind Language) const { namespace { +void replaceToken(const FormatToken &Token, FormatToken *Next, + const SourceManager &SourceMgr, tooling::Replacements &Result, + StringRef Text = "") { + const auto &Tok = Token.Tok; + SourceLocation Start; + if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) { + Start = Tok.getLocation(); + Next->WhitespaceRange = Token.WhitespaceRange; + } else { + Start = Token.WhitespaceRange.getBegin(); + } + const auto &Range = CharSourceRange::getCharRange(Start, Tok.getEndLoc()); + cantFail(Result.add(tooling::Replacement(SourceMgr, Range, Text))); +} + class ParensRemover : public TokenAnalyzer { public: ParensRemover(const Environment &Env, const FormatStyle &Style) @@ -2229,20 +2255,8 @@ class ParensRemover : public TokenAnalyzer { continue; for (const auto *Token = Line->First; Token && !Token->Finalized; Token = Token->Next) { - if (!Token->Optional || !Token->isOneOf(tok::l_paren, tok::r_paren)) - continue; - auto *Next = Token->Next; - assert(Next && Next->isNot(tok::eof)); - SourceLocation Start; - if (Next->NewlinesBefore == 0) { - Start = Token->Tok.getLocation(); - Next->WhitespaceRange = Token->WhitespaceRange; - } else { - Start = Token->WhitespaceRange.getBegin(); - } - const auto &Range = - CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc()); - cantFail(Result.add(tooling::Replacement(SourceMgr, Range, " "))); + if (Token->Optional && Token->isOneOf(tok::l_paren, tok::r_paren)) + replaceToken(*Token, Token->Next, SourceMgr, Result, " "); } } } @@ -2331,24 +2345,13 @@ class BracesRemover : public TokenAnalyzer { const auto *NextLine = I + 1 == End ? nullptr : I[1]; for (const auto *Token = Line->First; Token && !Token->Finalized; Token = Token->Next) { - if (!Token->Optional) - continue; - if (!Token->isOneOf(tok::l_brace, tok::r_brace)) + if (!Token->Optional || !Token->isOneOf(tok::l_brace, tok::r_brace)) continue; auto *Next = Token->Next; assert(Next || Token == Line->Last); if (!Next && NextLine) Next = NextLine->First; - SourceLocation Start; - if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) { - Start = Token->Tok.getLocation(); - Next->WhitespaceRange = Token->WhitespaceRange; - } else { - Start = Token->WhitespaceRange.getBegin(); - } - const auto &Range = - CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc()); - cantFail(Result.add(tooling::Replacement(SourceMgr, Range, ""))); + replaceToken(*Token, Next, SourceMgr, Result); } } } @@ -2400,16 +2403,51 @@ class SemiRemover : public TokenAnalyzer { assert(Next || Token == Line->Last); if (!Next && NextLine) Next = NextLine->First; - SourceLocation Start; - if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) { - Start = Token->Tok.getLocation(); - Next->WhitespaceRange = Token->WhitespaceRange; - } else { - Start = Token->WhitespaceRange.getBegin(); + replaceToken(*Token, Next, SourceMgr, Result); + } + } + } +}; + +class EnumTrailingCommaEditor : public TokenAnalyzer { +public: + EnumTrailingCommaEditor(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + + std::pair<tooling::Replacements, unsigned> + analyze(TokenAnnotator &Annotator, + SmallVectorImpl<AnnotatedLine *> &AnnotatedLines, + FormatTokenLexer &Tokens) override { + AffectedRangeMgr.computeAffectedLines(AnnotatedLines); + tooling::Replacements Result; + editEnumTrailingComma(AnnotatedLines, Result); + return {Result, 0}; + } + +private: + void editEnumTrailingComma(SmallVectorImpl<AnnotatedLine *> &Lines, + tooling::Replacements &Result) { + const auto &SourceMgr = Env.getSourceManager(); + for (auto *Line : Lines) { + if (!Line->Children.empty()) + editEnumTrailingComma(Line->Children, Result); + if (!Line->Affected) + continue; + for (const auto *Token = Line->First; Token && !Token->Finalized; + Token = Token->Next) { + if (Token->isNot(TT_EnumRBrace)) + continue; + const auto *BeforeRBrace = Token->getPreviousNonComment(); + assert(BeforeRBrace); + if (BeforeRBrace->is(TT_EnumLBrace)) // Empty braces. + continue; + if (BeforeRBrace->is(tok::comma)) { + if (Style.EnumTrailingComma == FormatStyle::ETC_Remove) + replaceToken(*BeforeRBrace, BeforeRBrace->Next, SourceMgr, Result); + } else if (Style.EnumTrailingComma == FormatStyle::ETC_Insert) { + cantFail(Result.add(tooling::Replacement( + SourceMgr, BeforeRBrace->Tok.getEndLoc(), 0, ","))); } - const auto &Range = - CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc()); - cantFail(Result.add(tooling::Replacement(SourceMgr, Range, ""))); } } } @@ -3812,6 +3850,13 @@ reformat(const FormatStyle &Style, StringRef Code, }); } + if (Style.EnumTrailingComma != FormatStyle::ETC_Leave) { + Passes.emplace_back([&](const Environment &Env) { + return EnumTrailingCommaEditor(Env, Expanded) + .process(/*SkipAnnotation=*/true); + }); + } + if (Style.FixNamespaceComments) { Passes.emplace_back([&](const Environment &Env) { return NamespaceEndCommentsFixer(Env, Expanded).process(); diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 287191d04d885..2b08b794792e9 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -520,6 +520,14 @@ TEST(ConfigParseTest, ParsesConfiguration) { CHECK_PARSE("EmptyLineBeforeAccessModifier: Always", EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Always); + Style.EnumTrailingComma = FormatStyle::ETC_Insert; + CHECK_PARSE("EnumTrailingComma: Leave", EnumTrailingComma, + FormatStyle::ETC_Leave); + CHECK_PARSE("EnumTrailingComma: Insert", EnumTrailingComma, + FormatStyle::ETC_Insert); + CHECK_PARSE("EnumTrailingComma: Remove", EnumTrailingComma, + FormatStyle::ETC_Remove); + Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak; CHECK_PARSE("AlignAfterOpenBracket: Align", AlignAfterOpenBracket, FormatStyle::BAS_Align); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0b90bd360b758..4dfa135120605 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -27902,6 +27902,38 @@ TEST_F(FormatTest, RemoveSemicolon) { verifyFormat("STRUCT(T, B) { int i; };", Style); } +TEST_F(FormatTest, EnumTrailingComma) { + constexpr StringRef Code("enum : int { /**/ };\n" + "enum {\n" + " a,\n" + " b,\n" + " c, //\n" + "};\n" + "enum Color { red, green, blue /**/ };"); + verifyFormat(Code); + + auto Style = getLLVMStyle(); + Style.EnumTrailingComma = FormatStyle::ETC_Insert; + verifyFormat("enum : int { /**/ };\n" + "enum {\n" + " a,\n" + " b,\n" + " c, //\n" + "};\n" + "enum Color { red, green, blue, /**/ };", + Code, Style); + + Style.EnumTrailingComma = FormatStyle::ETC_Remove; + verifyFormat("enum : int { /**/ };\n" + "enum {\n" + " a,\n" + " b,\n" + " c //\n" + "};\n" + "enum Color { red, green, blue /**/ };", + Code, Style); +} + TEST_F(FormatTest, BreakAfterAttributes) { constexpr StringRef Code("[[maybe_unused]] const int i;\n" "[[foo([[]])]] [[maybe_unused]]\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
