Skip to content

Commit

Permalink
[clang-format] Handle Java switch expressions (#91112)
Browse files Browse the repository at this point in the history
Also adds AllowShortCaseExpressionOnASingleLine option and
AlignCaseArrows suboption of AlignConsecutiveShortCaseStatements.

Fixes #55903.
  • Loading branch information
owenca committed May 7, 2024
1 parent aac83fc commit 236b3e1
Show file tree
Hide file tree
Showing 14 changed files with 332 additions and 21 deletions.
36 changes: 35 additions & 1 deletion clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,8 @@ the configuration (without a prefix: ``Auto``).

**AlignConsecutiveShortCaseStatements** (``ShortCaseStatementsAlignmentStyle``) :versionbadge:`clang-format 17` :ref:`<AlignConsecutiveShortCaseStatements>`
Style of aligning consecutive short case labels.
Only applies if ``AllowShortCaseLabelsOnASingleLine`` is ``true``.
Only applies if ``AllowShortCaseExpressionOnASingleLine`` or
``AllowShortCaseLabelsOnASingleLine`` is ``true``.


.. code-block:: yaml
Expand Down Expand Up @@ -935,6 +936,24 @@ the configuration (without a prefix: ``Auto``).
default: return "";
}
* ``bool AlignCaseArrows`` Whether to align the case arrows when aligning short case expressions.

.. code-block:: java
true:
i = switch (day) {
case THURSDAY, SATURDAY -> 8;
case WEDNESDAY -> 9;
default -> 0;
};
false:
i = switch (day) {
case THURSDAY, SATURDAY -> 8;
case WEDNESDAY -> 9;
default -> 0;
};
* ``bool AlignCaseColons`` Whether aligned case labels are aligned on the colon, or on the tokens
after the colon.

Expand Down Expand Up @@ -1692,6 +1711,21 @@ the configuration (without a prefix: ``Auto``).



.. _AllowShortCaseExpressionOnASingleLine:

**AllowShortCaseExpressionOnASingleLine** (``Boolean``) :versionbadge:`clang-format 19` :ref:`<AllowShortCaseExpressionOnASingleLine>`
Whether to merge a short switch labeled rule into a single line.

.. code-block:: java
true: false:
switch (a) { vs. switch (a) {
case 1 -> 1; case 1 ->
default -> 0; 1;
}; default ->
0;
};
.. _AllowShortCaseLabelsOnASingleLine:

**AllowShortCaseLabelsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 3.6` :ref:`<AllowShortCaseLabelsOnASingleLine>`
Expand Down
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,9 @@ clang-format
``BreakTemplateDeclarations``.
- ``AlwaysBreakAfterReturnType`` is deprecated and renamed to
``BreakAfterReturnType``.
- Handles Java ``switch`` expressions.
- Adds ``AllowShortCaseExpressionOnASingleLine`` option.
- Adds ``AlignCaseArrows`` suboption to ``AlignConsecutiveShortCaseStatements``.

libclang
--------
Expand Down
36 changes: 35 additions & 1 deletion clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,23 @@ struct FormatStyle {
/// }
/// \endcode
bool AcrossComments;
/// Whether to align the case arrows when aligning short case expressions.
/// \code{.java}
/// true:
/// i = switch (day) {
/// case THURSDAY, SATURDAY -> 8;
/// case WEDNESDAY -> 9;
/// default -> 0;
/// };
///
/// false:
/// i = switch (day) {
/// case THURSDAY, SATURDAY -> 8;
/// case WEDNESDAY -> 9;
/// default -> 0;
/// };
/// \endcode
bool AlignCaseArrows;
/// Whether aligned case labels are aligned on the colon, or on the tokens
/// after the colon.
/// \code
Expand All @@ -396,12 +413,14 @@ struct FormatStyle {
bool operator==(const ShortCaseStatementsAlignmentStyle &R) const {
return Enabled == R.Enabled && AcrossEmptyLines == R.AcrossEmptyLines &&
AcrossComments == R.AcrossComments &&
AlignCaseArrows == R.AlignCaseArrows &&
AlignCaseColons == R.AlignCaseColons;
}
};

/// Style of aligning consecutive short case labels.
/// Only applies if ``AllowShortCaseLabelsOnASingleLine`` is ``true``.
/// Only applies if ``AllowShortCaseExpressionOnASingleLine`` or
/// ``AllowShortCaseLabelsOnASingleLine`` is ``true``.
///
/// \code{.yaml}
/// # Example of usage:
Expand Down Expand Up @@ -724,6 +743,19 @@ struct FormatStyle {
/// \version 3.5
ShortBlockStyle AllowShortBlocksOnASingleLine;

/// Whether to merge a short switch labeled rule into a single line.
/// \code{.java}
/// true: false:
/// switch (a) { vs. switch (a) {
/// case 1 -> 1; case 1 ->
/// default -> 0; 1;
/// }; default ->
/// 0;
/// };
/// \endcode
/// \version 19
bool AllowShortCaseExpressionOnASingleLine;

/// If ``true``, short case labels will be contracted to a single line.
/// \code
/// true: false:
Expand Down Expand Up @@ -4923,6 +4955,8 @@ struct FormatStyle {
AllowBreakBeforeNoexceptSpecifier ==
R.AllowBreakBeforeNoexceptSpecifier &&
AllowShortBlocksOnASingleLine == R.AllowShortBlocksOnASingleLine &&
AllowShortCaseExpressionOnASingleLine ==
R.AllowShortCaseExpressionOnASingleLine &&
AllowShortCaseLabelsOnASingleLine ==
R.AllowShortCaseLabelsOnASingleLine &&
AllowShortCompoundRequirementOnASingleLine ==
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ struct MappingTraits<FormatStyle::ShortCaseStatementsAlignmentStyle> {
IO.mapOptional("Enabled", Value.Enabled);
IO.mapOptional("AcrossEmptyLines", Value.AcrossEmptyLines);
IO.mapOptional("AcrossComments", Value.AcrossComments);
IO.mapOptional("AlignCaseArrows", Value.AlignCaseArrows);
IO.mapOptional("AlignCaseColons", Value.AlignCaseColons);
}
};
Expand Down Expand Up @@ -911,6 +912,8 @@ template <> struct MappingTraits<FormatStyle> {
Style.AllowBreakBeforeNoexceptSpecifier);
IO.mapOptional("AllowShortBlocksOnASingleLine",
Style.AllowShortBlocksOnASingleLine);
IO.mapOptional("AllowShortCaseExpressionOnASingleLine",
Style.AllowShortCaseExpressionOnASingleLine);
IO.mapOptional("AllowShortCaseLabelsOnASingleLine",
Style.AllowShortCaseLabelsOnASingleLine);
IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
Expand Down Expand Up @@ -1423,6 +1426,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
LLVMStyle.AllowBreakBeforeNoexceptSpecifier = FormatStyle::BBNSS_Never;
LLVMStyle.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
LLVMStyle.AllowShortCaseExpressionOnASingleLine = true;
LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
LLVMStyle.AllowShortEnumsOnASingleLine = true;
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Format/FormatToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ namespace format {
/* l_brace of a block that is not the body of a (e.g. loop) statement. */ \
TYPE(BlockLBrace) \
TYPE(BracedListLBrace) \
TYPE(CaseLabelArrow) \
/* The colon at the end of a case label. */ \
TYPE(CaseLabelColon) \
TYPE(CastRParen) \
Expand Down Expand Up @@ -148,6 +149,8 @@ namespace format {
TYPE(StructLBrace) \
TYPE(StructRBrace) \
TYPE(StructuredBindingLSquare) \
TYPE(SwitchExpressionLabel) \
TYPE(SwitchExpressionLBrace) \
TYPE(TableGenBangOperator) \
TYPE(TableGenCondOperator) \
TYPE(TableGenCondOperatorColon) \
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5051,6 +5051,8 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
return true; // "x! as string", "x! in y"
}
} else if (Style.Language == FormatStyle::LK_Java) {
if (Left.is(TT_CaseLabelArrow) || Right.is(TT_CaseLabelArrow))
return true;
if (Left.is(tok::r_square) && Right.is(tok::l_brace))
return true;
// spaces inside square brackets.
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,12 @@ class LineJoiner {
}
}

if (TheLine->First->is(TT_SwitchExpressionLabel)) {
return Style.AllowShortCaseExpressionOnASingleLine
? tryMergeShortCaseLabels(I, E, Limit)
: 0;
}

if (TheLine->Last->is(tok::l_brace)) {
bool ShouldMerge = false;
// Try to merge records.
Expand Down
46 changes: 37 additions & 9 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,9 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
unsigned StoredPosition = Tokens->getPosition();
auto *Next = Tokens->getNextNonComment();
FormatTok = Tokens->setPosition(StoredPosition);
if (Next->isNot(tok::colon)) {
// default not followed by ':' is not a case label; treat it like
// an identifier.
if (!Next->isOneOf(tok::colon, tok::arrow)) {
// default not followed by `:` or `->` is not a case label; treat it
// like an identifier.
parseStructuralElement();
break;
}
Expand All @@ -451,6 +451,7 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
}
if (!SwitchLabelEncountered &&
(Style.IndentCaseLabels ||
(OpeningBrace && OpeningBrace->is(TT_SwitchExpressionLBrace)) ||
(Line->InPPDirective && Line->Level == 1))) {
++Line->Level;
}
Expand Down Expand Up @@ -1519,24 +1520,32 @@ void UnwrappedLineParser::parseStructuralElement(
// 'switch: string' field declaration.
break;
}
parseSwitch();
parseSwitch(/*IsExpr=*/false);
return;
case tok::kw_default:
case tok::kw_default: {
// In Verilog default along with other labels are handled in the next loop.
if (Style.isVerilog())
break;
if (Style.isJavaScript() && Line->MustBeDeclaration) {
// 'default: string' field declaration.
break;
}
auto *Default = FormatTok;
nextToken();
if (FormatTok->is(tok::colon)) {
FormatTok->setFinalizedType(TT_CaseLabelColon);
parseLabel();
return;
}
if (FormatTok->is(tok::arrow)) {
FormatTok->setFinalizedType(TT_CaseLabelArrow);
Default->setFinalizedType(TT_SwitchExpressionLabel);
parseLabel();
return;
}
// e.g. "default void f() {}" in a Java interface.
break;
}
case tok::kw_case:
// Proto: there are no switch/case statements.
if (Style.Language == FormatStyle::LK_Proto) {
Expand Down Expand Up @@ -2062,6 +2071,11 @@ void UnwrappedLineParser::parseStructuralElement(
case tok::kw_new:
parseNew();
break;
case tok::kw_switch:
if (Style.Language == FormatStyle::LK_Java)
parseSwitch(/*IsExpr=*/true);
nextToken();
break;
case tok::kw_case:
// Proto: there are no switch/case statements.
if (Style.Language == FormatStyle::LK_Proto) {
Expand Down Expand Up @@ -2589,6 +2603,9 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
else
nextToken();
break;
case tok::kw_switch:
parseSwitch(/*IsExpr=*/true);
break;
case tok::kw_requires: {
auto RequiresToken = FormatTok;
nextToken();
Expand Down Expand Up @@ -3246,6 +3263,7 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {

void UnwrappedLineParser::parseCaseLabel() {
assert(FormatTok->is(tok::kw_case) && "'case' expected");
auto *Case = FormatTok;

// FIXME: fix handling of complex expressions here.
do {
Expand All @@ -3254,11 +3272,16 @@ void UnwrappedLineParser::parseCaseLabel() {
FormatTok->setFinalizedType(TT_CaseLabelColon);
break;
}
if (Style.Language == FormatStyle::LK_Java && FormatTok->is(tok::arrow)) {
FormatTok->setFinalizedType(TT_CaseLabelArrow);
Case->setFinalizedType(TT_SwitchExpressionLabel);
break;
}
} while (!eof());
parseLabel();
}

void UnwrappedLineParser::parseSwitch() {
void UnwrappedLineParser::parseSwitch(bool IsExpr) {
assert(FormatTok->is(tok::kw_switch) && "'switch' expected");
nextToken();
if (FormatTok->is(tok::l_paren))
Expand All @@ -3268,10 +3291,15 @@ void UnwrappedLineParser::parseSwitch() {

if (FormatTok->is(tok::l_brace)) {
CompoundStatementIndenter Indenter(this, Style, Line->Level);
FormatTok->setFinalizedType(TT_ControlStatementLBrace);
parseBlock();
FormatTok->setFinalizedType(IsExpr ? TT_SwitchExpressionLBrace
: TT_ControlStatementLBrace);
if (IsExpr)
parseChildBlock();
else
parseBlock();
setPreviousRBraceType(TT_ControlStatementRBrace);
addUnwrappedLine();
if (!IsExpr)
addUnwrappedLine();
} else {
addUnwrappedLine();
++Line->Level;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Format/UnwrappedLineParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class UnwrappedLineParser {
void parseDoWhile();
void parseLabel(bool LeftAlignLabel = false);
void parseCaseLabel();
void parseSwitch();
void parseSwitch(bool IsExpr);
void parseNamespace();
bool parseModuleImport();
void parseNew();
Expand Down
22 changes: 14 additions & 8 deletions clang/lib/Format/WhitespaceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ const tooling::Replacements &WhitespaceManager::generateReplacements() {
llvm::sort(Changes, Change::IsBeforeInFile(SourceMgr));
calculateLineBreakInformation();
alignConsecutiveMacros();
alignConsecutiveShortCaseStatements();
alignConsecutiveShortCaseStatements(/*IsExpr=*/true);
alignConsecutiveShortCaseStatements(/*IsExpr=*/false);
alignConsecutiveDeclarations();
alignConsecutiveBitFields();
alignConsecutiveAssignments();
Expand Down Expand Up @@ -878,22 +879,27 @@ void WhitespaceManager::alignConsecutiveColons(
Changes, /*StartAt=*/0, AlignStyle);
}

void WhitespaceManager::alignConsecutiveShortCaseStatements() {
void WhitespaceManager::alignConsecutiveShortCaseStatements(bool IsExpr) {
if (!Style.AlignConsecutiveShortCaseStatements.Enabled ||
!Style.AllowShortCaseLabelsOnASingleLine) {
!(IsExpr ? Style.AllowShortCaseExpressionOnASingleLine
: Style.AllowShortCaseLabelsOnASingleLine)) {
return;
}

const auto Type = IsExpr ? TT_CaseLabelArrow : TT_CaseLabelColon;
const auto &Option = Style.AlignConsecutiveShortCaseStatements;
const bool AlignArrowOrColon =
IsExpr ? Option.AlignCaseArrows : Option.AlignCaseColons;

auto Matches = [&](const Change &C) {
if (Style.AlignConsecutiveShortCaseStatements.AlignCaseColons)
return C.Tok->is(TT_CaseLabelColon);
if (AlignArrowOrColon)
return C.Tok->is(Type);

// Ignore 'IsInsideToken' to allow matching trailing comments which
// need to be reflowed as that causes the token to appear in two
// different changes, which will cause incorrect alignment as we'll
// reflow early due to detecting multiple aligning tokens per line.
return !C.IsInsideToken && C.Tok->Previous &&
C.Tok->Previous->is(TT_CaseLabelColon);
return !C.IsInsideToken && C.Tok->Previous && C.Tok->Previous->is(Type);
};

unsigned MinColumn = 0;
Expand Down Expand Up @@ -944,7 +950,7 @@ void WhitespaceManager::alignConsecutiveShortCaseStatements() {
if (Changes[I].Tok->isNot(tok::comment))
LineIsComment = false;

if (Changes[I].Tok->is(TT_CaseLabelColon)) {
if (Changes[I].Tok->is(Type)) {
LineIsEmptyCase =
!Changes[I].Tok->Next || Changes[I].Tok->Next->isTrailingComment();

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Format/WhitespaceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ class WhitespaceManager {
void alignChainedConditionals();

/// Align consecutive short case statements over all \c Changes.
void alignConsecutiveShortCaseStatements();
void alignConsecutiveShortCaseStatements(bool IsExpr);

/// Align consecutive TableGen DAGArg colon over all \c Changes.
void alignConsecutiveTableGenBreakingDAGArgColons();
Expand Down
Loading

0 comments on commit 236b3e1

Please sign in to comment.