[clang-format]: Fix formatting of if statements with BlockIndent

[gedare]

A bug with BlockIndent prevents line breaks within if (and else if) clauses.

While fixing this bug, it appears that AlignAfterOpenBracket is not designed to work with loop and if statements, but AlwaysBreak works on if clauses. The documentation and tests are not clear on whether or not this behavior is intended.

This PR preserves the AlwaysBreak behavior on if clauses without supporting BlockIndent on if clauses to avoid regressions while fixing the bug.

It may be reasonable to create an explicit option for alignment of if (and loop) clauses intentionally for both AlwaysBreak and BlockIndent

Fixes #54663.

Migrated from Differential Revision: https://reviews.llvm.org/D154755
See more discussion there. Addressed last open comment from the rev about refactoring the complex conditional logic involved with the AlignAfterOpenBracket line break behavior.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Gedare Bloom (gedare)

Changes

A bug with BlockIndent prevents line breaks within if (and else if) clauses.

While fixing this bug, it appears that AlignAfterOpenBracket is not designed to work with loop and if statements, but AlwaysBreak works on if clauses. The documentation and tests are not clear on whether or not this behavior is intended.

This PR preserves the AlwaysBreak behavior on if clauses without supporting BlockIndent on if clauses to avoid regressions while fixing the bug.

It may be reasonable to create an explicit option for alignment of if (and loop) clauses intentionally for both AlwaysBreak and BlockIndent

Fixes #54663.

Migrated from Differential Revision: https://reviews.llvm.org/D154755
See more discussion there. Addressed last open comment from the rev about refactoring the complex conditional logic involved with the AlignAfterOpenBracket line break behavior.

---

Full diff: https://github.com/llvm/llvm-project/pull/77699.diff

2 Files Affected:

• (modified) clang/lib/Format/ContinuationIndenter.cpp (+16-7)
• (modified) clang/unittests/Format/FormatTest.cpp (+36-4)

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 102504182c4505..29426c225fec9e 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -768,15 +768,24 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
   // parenthesis by disallowing any further line breaks if there is no line
   // break after the opening parenthesis. Don't break if it doesn't conserve
   // columns.
+  auto isOpeningBracket = [&](const FormatToken &Tok) {
+    auto isStartOfBracedList = [&](const FormatToken &Tok) {
+      return Tok.is(tok::l_brace) && Tok.isNot(BK_Block) &&
+             Style.Cpp11BracedListStyle;
+    };
+    if (Tok.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
+        isStartOfBracedList(Tok)) {
+      if (!Tok.Previous)
+        return true;
+      if (Tok.Previous->isIf())
+        return Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak;
+      return !Tok.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
+                                    tok::kw_switch);
+    }
+  };
   if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
        Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
-      (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
-       (Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
-        Style.Cpp11BracedListStyle)) &&
-      State.Column > getNewLineColumn(State) &&
-      (!Previous.Previous ||
-       !Previous.Previous->isOneOf(TT_CastRParen, tok::kw_for, tok::kw_while,
-                                   tok::kw_switch)) &&
+      isOpeningBracket(Previous) && State.Column > getNewLineColumn(State) &&
       // Don't do this for simple (no expressions) one-argument function calls
       // as that feels like needlessly wasting whitespace, e.g.:
       //
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 25ef5c680af862..8d55e62e2558d3 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25889,8 +25889,8 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) {
                "}",
                Style);
 
-  verifyFormat("if (quitelongarg !=\n"
-               "    (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
+  verifyFormat("if (quiteLongArg !=\n"
+               "    (alsoLongArg - 1)) { // ABC is a very longgggggggggggg "
                "comment\n"
                "  return;\n"
                "}",
@@ -25903,12 +25903,44 @@ TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentIfStatement) {
                "}",
                Style);
 
-  verifyFormat("if (quitelongarg !=\n"
-               "    (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
+  verifyFormat("if (quiteLongArg !=\n"
+               "    (alsoLongArg - 1)) { // ABC is a very longgggggggggggg "
                "comment\n"
                "  return;\n"
                "}",
                Style);
+
+  verifyFormat("void foo() {\n"
+               "  if (camelCaseName < alsoLongName ||\n"
+               "      anotherEvenLongerName <=\n"
+               "          thisReallyReallyReallyReallyReallyReallyLongerName ||"
+               "\n"
+               "      otherName < thisLastName) {\n"
+               "    return;\n"
+               "  } else if (quiteLongName < alsoLongName ||\n"
+               "             anotherEvenLongerName <=\n"
+               "                 thisReallyReallyReallyReallyReallyReallyLonger"
+               "Name ||\n"
+               "             otherName < thisLastName) {\n"
+               "    return;\n"
+               "  }\n"
+               "}",
+               Style);
+
+  Style.ContinuationIndentWidth = 2;
+  verifyFormat("void foo() {\n"
+               "  if (ThisIsRatherALongIfClause && thatIExpectToBeBroken ||\n"
+               "      ontoMultipleLines && whenFormattedCorrectly) {\n"
+               "    if (false) {\n"
+               "      return;\n"
+               "    } else if (thisIsRatherALongIfClause && "
+               "thatIExpectToBeBroken ||\n"
+               "               ontoMultipleLines && whenFormattedCorrectly) {\n"
+               "      return;\n"
+               "    }\n"
+               "  }\n"
+               "}",
+               Style);
 }
 
 TEST_F(FormatTest, AlignAfterOpenBracketBlockIndentForStatement) {