[clang-format] Remove braces of else blocks that embody an if block

Fixes #55663.

Differential Revision: https://reviews.llvm.org/D127260
This commit is contained in:
owenca 2022-06-06 03:03:27 -07:00
parent 0e9a01dcac
commit 5ead1f13a2
3 changed files with 170 additions and 38 deletions

View File

@ -469,18 +469,21 @@ bool UnwrappedLineParser::precededByCommentOrPPDirective() const {
/// \param CanContainBracedList If the content can contain (at any level) a /// \param CanContainBracedList If the content can contain (at any level) a
/// braced list. /// braced list.
/// \param NextLBracesType The type for left brace found in this level. /// \param NextLBracesType The type for left brace found in this level.
/// \param IfKind The if statement kind in the level. /// \param IfKind The \p if statement kind in the level.
/// \param IfLeftBrace The left brace of the \p if block in the level.
/// \returns true if a simple block of if/else/for/while, or false otherwise. /// \returns true if a simple block of if/else/for/while, or false otherwise.
/// (A simple block has a single statement.) /// (A simple block has a single statement.)
bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace, bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
bool CanContainBracedList, bool CanContainBracedList,
TokenType NextLBracesType, TokenType NextLBracesType,
IfStmtKind *IfKind) { IfStmtKind *IfKind,
FormatToken **IfLeftBrace) {
auto NextLevelLBracesType = NextLBracesType == TT_CompoundRequirementLBrace auto NextLevelLBracesType = NextLBracesType == TT_CompoundRequirementLBrace
? TT_BracedListLBrace ? TT_BracedListLBrace
: TT_Unknown; : TT_Unknown;
const bool IsPrecededByCommentOrPPDirective = const bool IsPrecededByCommentOrPPDirective =
!Style.RemoveBracesLLVM || precededByCommentOrPPDirective(); !Style.RemoveBracesLLVM || precededByCommentOrPPDirective();
FormatToken *IfLBrace = nullptr;
bool HasDoWhile = false; bool HasDoWhile = false;
bool HasLabel = false; bool HasLabel = false;
unsigned StatementCount = 0; unsigned StatementCount = 0;
@ -498,9 +501,9 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
kind = tok::r_brace; kind = tok::r_brace;
auto ParseDefault = [this, OpeningBrace, NextLevelLBracesType, IfKind, auto ParseDefault = [this, OpeningBrace, NextLevelLBracesType, IfKind,
&HasDoWhile, &HasLabel, &StatementCount] { &IfLBrace, &HasDoWhile, &HasLabel, &StatementCount] {
parseStructuralElement(!OpeningBrace, NextLevelLBracesType, IfKind, parseStructuralElement(!OpeningBrace, NextLevelLBracesType, IfKind,
HasDoWhile ? nullptr : &HasDoWhile, &IfLBrace, HasDoWhile ? nullptr : &HasDoWhile,
HasLabel ? nullptr : &HasLabel); HasLabel ? nullptr : &HasLabel);
++StatementCount; ++StatementCount;
assert(StatementCount > 0 && "StatementCount overflow!"); assert(StatementCount > 0 && "StatementCount overflow!");
@ -545,7 +548,11 @@ bool UnwrappedLineParser::parseLevel(const FormatToken *OpeningBrace,
return false; return false;
} }
const FormatToken *Next = Tokens->peekNextToken(); const FormatToken *Next = Tokens->peekNextToken();
return Next->isNot(tok::comment) || Next->NewlinesBefore > 0; if (Next->is(tok::comment) && Next->NewlinesBefore == 0)
return false;
if (IfLeftBrace)
*IfLeftBrace = IfLBrace;
return true;
} }
nextToken(); nextToken();
addUnwrappedLine(); addUnwrappedLine();
@ -818,12 +825,10 @@ bool UnwrappedLineParser::mightFitOnOneLine(
return Line.Level * Style.IndentWidth + Length <= ColumnLimit; return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
} }
void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels, FormatToken *UnwrappedLineParser::parseBlock(
bool MunchSemi, bool KeepBraces, bool MustBeDeclaration, unsigned AddLevels, bool MunchSemi, bool KeepBraces,
IfStmtKind *IfKind, IfStmtKind *IfKind, bool UnindentWhitesmithsBraces,
bool UnindentWhitesmithsBraces, bool CanContainBracedList, TokenType NextLBracesType) {
bool CanContainBracedList,
TokenType NextLBracesType) {
assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) && assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
"'{' or macro block token expected"); "'{' or macro block token expected");
FormatToken *Tok = FormatTok; FormatToken *Tok = FormatTok;
@ -844,7 +849,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
// Bail out if there are too many levels. Otherwise, the stack might overflow. // Bail out if there are too many levels. Otherwise, the stack might overflow.
if (Line->Level > 300) if (Line->Level > 300)
return; return nullptr;
if (MacroBlock && FormatTok->is(tok::l_paren)) if (MacroBlock && FormatTok->is(tok::l_paren))
parseParens(); parseParens();
@ -868,31 +873,37 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths) if (AddLevels > 0u && Style.BreakBeforeBraces != FormatStyle::BS_Whitesmiths)
Line->Level += AddLevels; Line->Level += AddLevels;
FormatToken *IfLBrace = nullptr;
const bool SimpleBlock = const bool SimpleBlock =
parseLevel(Tok, CanContainBracedList, NextLBracesType, IfKind); parseLevel(Tok, CanContainBracedList, NextLBracesType, IfKind, &IfLBrace);
if (eof()) if (eof())
return; return IfLBrace;
if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd) if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
: !FormatTok->is(tok::r_brace)) { : !FormatTok->is(tok::r_brace)) {
Line->Level = InitialLevel; Line->Level = InitialLevel;
FormatTok->setBlockKind(BK_Block); FormatTok->setBlockKind(BK_Block);
return; return IfLBrace;
} }
auto RemoveBraces = [=]() mutable { auto RemoveBraces = [=]() mutable {
if (KeepBraces || !SimpleBlock) if (!SimpleBlock)
return false; return false;
assert(Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace)); assert(Tok->isOneOf(TT_ControlStatementLBrace, TT_ElseLBrace));
assert(FormatTok->is(tok::r_brace)); assert(FormatTok->is(tok::r_brace));
const bool WrappedOpeningBrace = !Tok->Previous; const bool WrappedOpeningBrace = !Tok->Previous;
if (WrappedOpeningBrace && FollowedByComment) if (WrappedOpeningBrace && FollowedByComment)
return false; return false;
const bool HasRequiredIfBraces = IfLBrace && !IfLBrace->Optional;
if (KeepBraces && !HasRequiredIfBraces)
return false;
if (Tok->isNot(TT_ElseLBrace) || !HasRequiredIfBraces) {
const FormatToken *Previous = Tokens->getPreviousToken(); const FormatToken *Previous = Tokens->getPreviousToken();
assert(Previous); assert(Previous);
if (Previous->is(tok::r_brace) && !Previous->Optional) if (Previous->is(tok::r_brace) && !Previous->Optional)
return false; return false;
}
assert(!CurrentLines->empty()); assert(!CurrentLines->empty());
if (!mightFitOnOneLine(CurrentLines->back())) if (!mightFitOnOneLine(CurrentLines->back()))
return false; return false;
@ -943,6 +954,8 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels,
CurrentLines->size() - 1; CurrentLines->size() - 1;
} }
} }
return IfLBrace;
} }
static bool isGoogScope(const UnwrappedLine &Line) { static bool isGoogScope(const UnwrappedLine &Line) {
@ -1419,11 +1432,9 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() {
} }
} }
void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel, void UnwrappedLineParser::parseStructuralElement(
TokenType NextLBracesType, bool IsTopLevel, TokenType NextLBracesType, IfStmtKind *IfKind,
IfStmtKind *IfKind, FormatToken **IfLeftBrace, bool *HasDoWhile, bool *HasLabel) {
bool *HasDoWhile,
bool *HasLabel) {
if (Style.Language == FormatStyle::LK_TableGen && if (Style.Language == FormatStyle::LK_TableGen &&
FormatTok->is(tok::pp_include)) { FormatTok->is(tok::pp_include)) {
nextToken(); nextToken();
@ -1463,13 +1474,16 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel,
parseAccessSpecifier(); parseAccessSpecifier();
} }
return; return;
case tok::kw_if: case tok::kw_if: {
if (Style.isJavaScript() && Line->MustBeDeclaration) { if (Style.isJavaScript() && Line->MustBeDeclaration) {
// field/method declaration. // field/method declaration.
break; break;
} }
parseIfThenElse(IfKind); FormatToken *Tok = parseIfThenElse(IfKind);
if (IfLeftBrace)
*IfLeftBrace = Tok;
return; return;
}
case tok::kw_for: case tok::kw_for:
case tok::kw_while: case tok::kw_while:
if (Style.isJavaScript() && Line->MustBeDeclaration) { if (Style.isJavaScript() && Line->MustBeDeclaration) {
@ -2600,12 +2614,17 @@ FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
ElseLeftBrace = FormatTok; ElseLeftBrace = FormatTok;
CompoundStatementIndenter Indenter(this, Style, Line->Level); CompoundStatementIndenter Indenter(this, Style, Line->Level);
IfStmtKind ElseBlockKind = IfStmtKind::NotIf; IfStmtKind ElseBlockKind = IfStmtKind::NotIf;
FormatToken *IfLBrace =
parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u, parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
/*MunchSemi=*/true, KeepElseBraces, &ElseBlockKind); /*MunchSemi=*/true, KeepElseBraces, &ElseBlockKind);
if ((ElseBlockKind == IfStmtKind::IfOnly || if (FormatTok->is(tok::kw_else)) {
ElseBlockKind == IfStmtKind::IfElseIf) && KeepElseBraces = KeepElseBraces ||
FormatTok->is(tok::kw_else)) { ElseBlockKind == IfStmtKind::IfOnly ||
ElseBlockKind == IfStmtKind::IfElseIf;
} else if (IfLBrace && !IfLBrace->Optional) {
KeepElseBraces = true; KeepElseBraces = true;
assert(ElseLeftBrace->MatchingParen);
markOptionalBraces(ElseLeftBrace);
} }
addUnwrappedLine(); addUnwrappedLine();
} else if (FormatTok->is(tok::kw_if)) { } else if (FormatTok->is(tok::kw_if)) {

View File

@ -95,12 +95,13 @@ private:
bool parseLevel(const FormatToken *OpeningBrace = nullptr, bool parseLevel(const FormatToken *OpeningBrace = nullptr,
bool CanContainBracedList = true, bool CanContainBracedList = true,
TokenType NextLBracesType = TT_Unknown, TokenType NextLBracesType = TT_Unknown,
IfStmtKind *IfKind = nullptr); IfStmtKind *IfKind = nullptr,
FormatToken **IfLeftBrace = nullptr);
bool mightFitOnOneLine(UnwrappedLine &Line, bool mightFitOnOneLine(UnwrappedLine &Line,
const FormatToken *OpeningBrace = nullptr) const; const FormatToken *OpeningBrace = nullptr) const;
void parseBlock(bool MustBeDeclaration = false, unsigned AddLevels = 1u, FormatToken *parseBlock(bool MustBeDeclaration = false,
bool MunchSemi = true, bool KeepBraces = true, unsigned AddLevels = 1u, bool MunchSemi = true,
IfStmtKind *IfKind = nullptr, bool KeepBraces = true, IfStmtKind *IfKind = nullptr,
bool UnindentWhitesmithsBraces = false, bool UnindentWhitesmithsBraces = false,
bool CanContainBracedList = true, bool CanContainBracedList = true,
TokenType NextLBracesType = TT_Unknown); TokenType NextLBracesType = TT_Unknown);
@ -117,6 +118,7 @@ private:
void parseStructuralElement(bool IsTopLevel = false, void parseStructuralElement(bool IsTopLevel = false,
TokenType NextLBracesType = TT_Unknown, TokenType NextLBracesType = TT_Unknown,
IfStmtKind *IfKind = nullptr, IfStmtKind *IfKind = nullptr,
FormatToken **IfLeftBrace = nullptr,
bool *HasDoWhile = nullptr, bool *HasDoWhile = nullptr,
bool *HasLabel = nullptr); bool *HasLabel = nullptr);
bool tryToParseBracedList(); bool tryToParseBracedList();

View File

@ -25466,6 +25466,117 @@ TEST_F(FormatTest, RemoveBraces) {
" h;", " h;",
Style); Style);
verifyFormat("if (a) {\n"
" b;\n"
"} else if (c) {\n"
" d;\n"
" e;\n"
"}",
"if (a) {\n"
" b;\n"
"} else {\n"
" if (c) {\n"
" d;\n"
" e;\n"
" }\n"
"}",
Style);
verifyFormat("if (a) {\n"
" b;\n"
" c;\n"
"} else if (d) {\n"
" e;\n"
" f;\n"
"}",
"if (a) {\n"
" b;\n"
" c;\n"
"} else {\n"
" if (d) {\n"
" e;\n"
" f;\n"
" }\n"
"}",
Style);
verifyFormat("if (a) {\n"
" b;\n"
"} else if (c) {\n"
" d;\n"
"} else {\n"
" e;\n"
" f;\n"
"}",
"if (a) {\n"
" b;\n"
"} else {\n"
" if (c) {\n"
" d;\n"
" } else {\n"
" e;\n"
" f;\n"
" }\n"
"}",
Style);
verifyFormat("if (a) {\n"
" b;\n"
"} else if (c) {\n"
" d;\n"
"} else if (e) {\n"
" f;\n"
" g;\n"
"}",
"if (a) {\n"
" b;\n"
"} else {\n"
" if (c) {\n"
" d;\n"
" } else if (e) {\n"
" f;\n"
" g;\n"
" }\n"
"}",
Style);
verifyFormat("if (a) {\n"
" if (b)\n"
" c;\n"
" else if (d) {\n"
" e;\n"
" f;\n"
" }\n"
"} else {\n"
" g;\n"
"}",
"if (a) {\n"
" if (b)\n"
" c;\n"
" else {\n"
" if (d) {\n"
" e;\n"
" f;\n"
" }\n"
" }\n"
"} else {\n"
" g;\n"
"}",
Style);
verifyFormat("if (a)\n"
" if (b)\n"
" c;\n"
" else {\n"
" if (d) {\n"
" e;\n"
" f;\n"
" }\n"
" }\n"
"else\n"
" g;",
Style);
verifyFormat("if (a)\n" verifyFormat("if (a)\n"
" b;\n" " b;\n"
"else if (c)\n" "else if (c)\n"