[clang] [clang-format]: Fix formatting of if statements with BlockIndent (PR #77699)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Gedare Bloom (gedare)

<details>
<summary>Changes</summary>

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
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) {

``````````

</details>


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