[clang-format] Fix regressions in BAS_AlwaysBreak

[owenca]

Fixes #107401.
Fixes #107574.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #107401.

---

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

2 Files Affected:

• (modified) clang/lib/Format/ContinuationIndenter.cpp (+1-1)
• (modified) clang/unittests/Format/FormatTestJS.cpp (+8)

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 5843571718b3a2..f65c1640a8765a 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -861,7 +861,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
       //  or
       //  caaaaaaaaaaaaaaaaaaaaal(
       //       new SomethingElseeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee());
-      !IsSimpleFunction(Current)) {
+      Current.isNot(tok::comment) && !IsSimpleFunction(Current)) {
     CurrentState.NoLineBreak = true;
   }
 
diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp
index 4b29ba720f6823..a0b663170c7b33 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -2850,5 +2850,13 @@ TEST_F(FormatTestJS, DontBreakFieldsAsGoToLabels) {
                "};");
 }
 
+TEST_F(FormatTestJS, BreakAfterOpenBracket) {
+  auto Style = getGoogleStyle(FormatStyle::LK_JavaScript);
+  EXPECT_EQ(Style.AlignAfterOpenBracket, FormatStyle::BAS_AlwaysBreak);
+  verifyFormat("ctrl.onCopy(/** @type {!WizEvent}*/ (\n"
+               "    {event, targetElement: {el: () => selectedElement}}));",
+               Style);
+}
+
 } // namespace format
 } // end namespace clang

[kadircet]

would it make sense to introduce this into IsSimpleFunction ? similar to handling of new keyword in there? possibly just skipping over all of the comments. because we can also have foo(/*comments*/ new X()), and i think we still want that inner expression to be treated simple

[owenca]

would it make sense to introduce this into IsSimpleFunction ? similar to handling of new keyword in there?

I did that at first but thought it would be confusing to say that a comment is a simple function (or function call).

[kadircet]

i've also found 3 more cases that point back to same patch in #107574, in case you want to address all of them with this change

[owenca]

i've also found 3 more cases that point back to same patch in #107574, in case you want to address all of them with this change

I'd rather make incremental changes. Maybe you or @gedare can give them a try.

[gedare]

LGTM, I like the renaming of the lambda. I think this better captures the original author's intent about "simple functions."