[clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[owenpan]

Summary

See bug report https://bugs.llvm.org/show_bug.cgi?id=38525 for more details.

[owenpan]

Test case:

BreakBeforeBinaryOperators.cpp307 BDownload

[JonasToth]

Please create the diffs with full context (git diff -U 9999999) and add an test-case.

[hiraditya]

In D50699#1198770, @owenpan wrote:

Test case:

BreakBeforeBinaryOperators.cpp307 BDownload

Please add a testcase in the source and add checks to verify the changes.

[owenpan]

Updated patch created by "svn diff --diff-cmd=diff -x -U999999"

[JonasToth]

In D50699#1198813, @owenpan wrote:

Updated patch created by "svn diff --diff-cmd=diff -x -U999999"

The patch seems not to be in the correct path (the lib/Format thing is missing). Could you check that again? If you plan to add more patches you could also have a look at arc which handles all of that for you :)

[owenpan]

Updated the patch to include lib/Format path prefix.

[owenpan]

Added a test case.

[owenpan]

Simplified the test case.

[JonasToth]

@owenpan shall i commit for you?

[owenpan]

@JonasToth Yes, please. Also https://reviews.llvm.org/D50697 :)
Thanks again for getting me started!

lib/Format/ContinuationIndenter.cpp

State.Stack.back().LastSpace = State.Column;
                                TT_CtorInitializerColon)) &&
              ((Previous.getPrecedence() != prec::Assignment &&
                (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
                 Previous.NextOperator)) ||
               Current.StartsBinaryExpression)) {
     // Indent relative to the RHS of the expression unless this is a simple
     // assignment without binary expression on the RHS. Also indent relative to
     // unary operators and the colons of constructor initializers.
+    if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
       State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
     State.Stack.back().Indent = State.Column;
     State.Stack.back().LastSpace = State.Column;
   } else if (Previous.opensScope()) {
     // If a function has a trailing call, indent all parameters from the
     // opening parenthesis. This avoids confusing indents like:
     //   OuterFunction(InnerFunctionCall( // break
     //       ParameterToInnerFunction))   // break

unittests/Format/FormatTest.cpp

verifyFormat("return boost::fusion::at_c<0>(iiii).second\n"
                "       == boost::fusion::at_c<1>(iiii).second;",
                Style);
 
   Style.ColumnLimit = 60;
   verifyFormat("zzzzzzzzzz\n"
                "    = bbbbbbbbbbbbbbbbb\n"
                "      >> aaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaa);",
                Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("{\n"
+            "\treturn someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+            "\t\t&& (someOtherLongishConditionPart1\n"
+            "\t\t\t|| someOtherEvenLongerNestedConditionPart2);\n"
+            "}",
+            format("{\n"
+                   "\treturn someVeryVeryLongConditionThatBarelyFitsOnALine && (someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);\n"
+                   "}",
+                   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
   // Here we'd like to wrap after the || operators, but a comment is forcing an
   // earlier wrap.
   verifyFormat("bool x = aaaaa //\n"
                "         || bbbbb\n"
                "         //\n"