[clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

[owenpan]

Summary

Account for an r_brace that precedes an "else if" statement when calculating whether the line might fit on one line if the r_brace is removed.

Fixes https://github.com/llvm/llvm-project/issues/59778.

[MyDeveloperDay]

LGTM

[HazardyKnusperkeks]

On second thought, shouldn't we test for removing the braces?

[owenpan]

In D140835#4023080, @HazardyKnusperkeks wrote:

On second thought, shouldn't we test for removing the braces?

Hmm. This patch including the added test case is for removing braces.

[HazardyKnusperkeks]

In D140835#4023132, @owenpan wrote:

In D140835#4023080, @HazardyKnusperkeks wrote:

On second thought, shouldn't we test for removing the braces?

Hmm. This patch including the added test case is for removing braces.

Then replace test with check.
I mean if we don't want to remove braces and the line than becomes to long, because we keep the brace. As far as I can see that could now happen right?

[owenpan]

In D140835#4028257, @HazardyKnusperkeks wrote:

In D140835#4023132, @owenpan wrote:

In D140835#4023080, @HazardyKnusperkeks wrote:

On second thought, shouldn't we test for removing the braces?

Hmm. This patch including the added test case is for removing braces.

Then replace test with check.

There is no test in my patch. What's check?

I mean if we don't want to remove braces and the line than becomes to long, because we keep the brace. As far as I can see that could now happen right?

If RemoveBracesLLVM is false, the line is too long and will wrap. If RemoveBracesLLVM is true, the line will fit on a single line after the braces are removed, However, without this patch, the braces would be kept and the line would wrap. (See the linked issue in the summary.)

[HazardyKnusperkeks]

In D140835#4028274, @owenpan wrote:

If RemoveBracesLLVM is false, the line is too long and will wrap.

That's what I fail to see, but if you say so.

[owenpan]

In D140835#4028479, @HazardyKnusperkeks wrote:

In D140835#4028274, @owenpan wrote:

If RemoveBracesLLVM is false, the line is too long and will wrap.

That's what I fail to see, but if you say so.

The line is 24 characters long, but ColumnLimit is set to 20 on line 731.

clang/lib/Format/UnwrappedLineParser.cpp

if (OpeningBrace) {
     assert(OpeningBrace != Tokens.front().Tok);
     if (auto Prev = OpeningBrace->Previous;
         Prev && Prev->TotalLength + ColumnLimit == OpeningBrace->TotalLength) {
       Length -= ColumnLimit;
     }
     Length -= OpeningBrace->TokenText.size() + 1;
   }
 
+  if (const auto *FirstToken = Line.First; FirstToken->is(tok::r_brace)) {
+    assert(!OpeningBrace || OpeningBrace->is(TT_ControlStatementLBrace));
+    Length -= FirstToken->TokenText.size() + 1;
+  }
+
   Index = 0;
   for (auto &Token : Tokens) {
     const auto &SavedToken = SavedTokens[Index++];
     Token.Tok->copyFrom(*SavedToken.Tok);
     Token.Children = std::move(SavedToken.Children);
     delete SavedToken.Tok;
   }
 

clang/unittests/Format/BracesRemoverTest.cpp

verifyFormat("if (a)\n"
                "  if (c) {\n"
                "    d;\n"
                "    e;\n"
                "  } else\n"
                "    f = g(foo, bar, baz);\n"
                "}",
                Style);
 
+  verifyFormat("if (foo)\n"
+               "  f();\n"
+               "else if (bar || baz)\n"
+               "  g();",
+               "if (foo) {\n"
+               "  f();\n"
+               "} else if (bar || baz) {\n"
+               "  g();\n"
+               "}",
+               Style);
+
   Style.ColumnLimit = 0;
   verifyFormat("if (a)\n"
                "  b234567890223456789032345678904234567890 = "
                "c234567890223456789032345678904234567890;",
                "if (a) {\n"
                "  b234567890223456789032345678904234567890 = "
                "c234567890223456789032345678904234567890;\n"
                "}",