[clang] [clang-format] Fix BreakAfterOpenBracketIf with unary operators (PR #215039)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: johnnyb2543

<details>
<summary>Changes</summary>

Issue:
In the lambda StartsSimpleOneargList, inside the member function ContinuationIndenter::addTokenOnCurrentLine, the parameter is a const FormatToken& TokAfterLParen. If that token was a unary operator then true was returned from the lambda. This lambda was then used in a negation of a condition for an if statement that would make sure there was no line break. This could cause an unnecessary line break.
Solution:
I moved the check for the unary operator after the logic that checks if the token relates to an if statement rather than an argument to a function. This ensures that false returns for the beginning of an if statement rather than true for code such as "if (!x, y)".
Fixes #<!-- -->212206

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


2 Files Affected:

- (modified) clang/lib/Format/ContinuationIndenter.cpp (+6-5) 
- (modified) clang/unittests/Format/FormatTest.cpp (+22-2) 


``````````diff
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 085b48fd1ed78..ce4d75e69b3b7 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -964,11 +964,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
     // - foo(::new Bar())
     if (Tok.is(tok::kw_new) || Tok.startsSequence(tok::coloncolon, tok::kw_new))
       return true;
-    if (Tok.is(TT_UnaryOperator) ||
-        (Style.isJavaScript() &&
-         Tok.isOneOf(tok::ellipsis, Keywords.kw_await))) {
-      return true;
-    }
+
     const auto *Previous = TokAfterLParen.Previous;
     assert(Previous); // IsOpeningBracket(Previous)
     if (Previous->Previous &&
@@ -976,6 +972,11 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
          Previous->Previous->is(tok::kw_switch))) {
       return false;
     }
+    if (Tok.is(TT_UnaryOperator) ||
+        (Style.isJavaScript() &&
+         Tok.isOneOf(tok::ellipsis, Keywords.kw_await))) {
+      return true;
+    }
     if (Previous->isNoneOf(TT_FunctionDeclarationLParen,
                            TT_LambdaDefinitionLParen) &&
         !IsFunctionCallParen(*Previous)) {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index b72a683ac1fff..706284c8f8914 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -16,7 +16,6 @@ namespace test {
 namespace {
 
 class FormatTest : public test::FormatTestBase {};
-
 TEST_F(FormatTest, MessUp) {
   EXPECT_EQ("1 2 3", messUp("1 2 3"));
   EXPECT_EQ("1 2 3", messUp("1\n2\n3"));
@@ -26616,7 +26615,28 @@ TEST_F(FormatTest, UnbalancedAngleBrackets) {
 TEST_F(FormatTest, LambdaArrowAsTrailingReturnArrow) {
   verifyNoCrash("void foo()([] consteval -> int {}())");
 }
-
+TEST_F(FormatTest, BreakBeforeCloseBracketIfIndependentOfOpenBracketBreak) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ContinuationIndentWidth = 2;
+  Style.PackArguments.BinPack = FormatStyle::BPAS_OnePerLine;
+  Style.BreakAfterOpenBracketIf = true;
+  Style.BreakBeforeCloseBracketIf = true;
+
+  verifyFormat("int main() {\n"
+               "  if (\n"
+               "    !printf(\"%s %s %s %s %s %s\",\n"
+               "            \"foobar\",\n"
+               "            \"foobar\",\n"
+               "            \"foobar\",\n"
+               "            \"foobar\",\n"
+               "            \"foobar\",\n"
+               "            \"foobar\")\n"
+               "  ) {\n"
+               "    return 1;\n"
+               "  }\n"
+               "}\n",
+               Style);
+}
 } // namespace
 } // namespace test
 } // namespace format

``````````

</details>


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