Fix formatting for multiline binary expressions

#KT-4797 Fixed

idea/src/org/jetbrains/jet/plugin/formatter/JetBlock.java

@@ -22,7 +22,6 @@ import com.intellij.lang.ASTNode;
 import com.intellij.psi.TokenType;
 import com.intellij.psi.codeStyle.CodeStyleSettings;
 import com.intellij.psi.codeStyle.CommonCodeStyleSettings;
-import com.intellij.psi.formatter.FormatterUtil;
 import com.intellij.psi.formatter.common.AbstractBlock;
 import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.tree.TokenSet;
@@ -111,19 +110,13 @@ public class JetBlock extends AbstractBlock {
         if (child.getElementType() == OPERATION_REFERENCE) {
             ASTNode operationNode = child.getFirstChildNode();
             if (operationNode != null) {
-                return new JetBlock(operationNode, alignmentStrategy, Indent.getNoneIndent(), null, mySettings, mySpacingBuilder);
+                return new JetBlock(operationNode, alignmentStrategy, createChildIndent(child), null, mySettings, mySpacingBuilder);
             }
         }
 
         return new JetBlock(child, alignmentStrategy, createChildIndent(child), null, mySettings, mySpacingBuilder);
     }
 
-    private static Indent indentIfNotBrace(@NotNull ASTNode child) {
-        return child.getElementType() == RBRACE || child.getElementType() == LBRACE
-               ? Indent.getNoneIndent()
-               : Indent.getNormalIndent();
-    }
-
     private static ASTNode getPrevWithoutWhitespace(ASTNode node) {
         node = node.getTreePrev();
         while (node != null && node.getElementType() == TokenType.WHITE_SPACE) {
@@ -171,6 +164,10 @@ public class JetBlock extends AbstractBlock {
             return new ChildAttributes(Indent.getSpaceIndent(KDOC_COMMENT_INDENT), null);
         }
 
+        if (type == PARENTHESIZED) {
+            return super.getChildAttributes(newChildIndex);
+        }
+
         if (isIncomplete()) {
             return super.getChildAttributes(newChildIndex);
         }
@@ -184,28 +181,52 @@ public class JetBlock extends AbstractBlock {
     }
 
     private NodeAlignmentStrategy getChildrenAlignmentStrategy() {
-        CommonCodeStyleSettings jetCommonSettings = mySettings.getCommonSettings(JetLanguage.INSTANCE);
+        final CommonCodeStyleSettings jetCommonSettings = mySettings.getCommonSettings(JetLanguage.INSTANCE);
         JetCodeStyleSettings jetSettings = mySettings.getCustomSettings(JetCodeStyleSettings.class);
 
-        // Prepare default null strategy
-        NodeAlignmentStrategy strategy = myAlignmentStrategy;
-
         // Redefine list of strategies for some special elements
         IElementType parentType = myNode.getElementType();
         if (parentType == VALUE_PARAMETER_LIST) {
-            strategy = getAlignmentForChildInParenthesis(
+            return getAlignmentForChildInParenthesis(
                     jetCommonSettings.ALIGN_MULTILINE_PARAMETERS, VALUE_PARAMETER, COMMA,
                     jetCommonSettings.ALIGN_MULTILINE_METHOD_BRACKETS, LPAR, RPAR);
         }
         else if (parentType == VALUE_ARGUMENT_LIST) {
-            strategy = getAlignmentForChildInParenthesis(
+            return getAlignmentForChildInParenthesis(
                     jetCommonSettings.ALIGN_MULTILINE_PARAMETERS_IN_CALLS, VALUE_ARGUMENT, COMMA,
                     jetCommonSettings.ALIGN_MULTILINE_METHOD_BRACKETS, LPAR, RPAR);
         }
         else if (parentType == WHEN) {
-            strategy = getAlignmentForCaseBranch(jetSettings.ALIGN_IN_COLUMNS_CASE_BRANCH);
+            return getAlignmentForCaseBranch(jetSettings.ALIGN_IN_COLUMNS_CASE_BRANCH);
         }
-        return strategy;
+        else if (parentType == BINARY_EXPRESSION) {
+            return NodeAlignmentStrategy.fromTypes(AlignmentStrategy.wrap(
+                    createAlignment(jetCommonSettings.ALIGN_MULTILINE_BINARY_OPERATION, getAlignment())));
+        }
+        else if (parentType == PARENTHESIZED) {
+            return new NodeAlignmentStrategy() {
+                Alignment bracketsAlignment = jetCommonSettings.ALIGN_MULTILINE_BINARY_OPERATION ? Alignment.createAlignment() : null;
+
+                @Nullable
+                @Override
+                public Alignment getAlignment(@NotNull ASTNode childNode) {
+                    IElementType childNodeType = childNode.getElementType();
+                    ASTNode prev = getPrevWithoutWhitespace(childNode);
+
+                    if ((prev != null && prev.getElementType() == TokenType.ERROR_ELEMENT) || childNodeType == TokenType.ERROR_ELEMENT) {
+                        return bracketsAlignment;
+                    }
+
+                    if (childNodeType == LPAR || childNodeType == RPAR) {
+                        return bracketsAlignment;
+                    }
+
+                    return null;
+                }
+            };
+        }
+
+        return myAlignmentStrategy;
     }
 
     private static NodeAlignmentStrategy getAlignmentForChildInParenthesis(
@@ -288,6 +309,14 @@ public class JetBlock extends AbstractBlock {
                     .in(DOT_QUALIFIED_EXPRESSION, SAFE_ACCESS_EXPRESSION)
                     .set(Indent.getContinuationWithoutFirstIndent(true)),
 
+            ASTIndentStrategy.forNode("Binary expressions")
+                    .in(BINARY_EXPRESSION)
+                    .set(Indent.getContinuationWithoutFirstIndent(false)),
+
+            ASTIndentStrategy.forNode("Parenthesized expression")
+                    .in(PARENTHESIZED)
+                    .set(Indent.getContinuationWithoutFirstIndent(false)),
+
             ASTIndentStrategy.forNode("KDoc comment indent")
                     .in(DOC_COMMENT)
                     .forType(KDocTokens.LEADING_ASTERISK, KDocTokens.END)
@@ -338,4 +367,17 @@ public class JetBlock extends AbstractBlock {
 
         return Indent.getNoneIndent();
     }
+
+    @Nullable
+    private static Alignment createAlignment(boolean alignOption, @Nullable Alignment defaultAlignment) {
+        return alignOption ? createAlignmentOrDefault(null, defaultAlignment) : defaultAlignment;
+    }
+
+    @Nullable
+    private static Alignment createAlignmentOrDefault(@Nullable Alignment base, @Nullable Alignment defaultAlignment) {
+        if (defaultAlignment == null) {
+            return base == null ? Alignment.createAlignment() : Alignment.createChildAlignment(base);
+        }
+        return defaultAlignment;
+    }
 }

idea/src/org/jetbrains/jet/plugin/formatter/JetLanguageCodeStyleSettingsProvider.java

@@ -155,6 +155,7 @@ public class JetLanguageCodeStyleSettingsProvider extends LanguageCodeStyleSetti
                         "ALIGN_MULTILINE_PARAMETERS",
                         "ALIGN_MULTILINE_PARAMETERS_IN_CALLS",
                         "ALIGN_MULTILINE_METHOD_BRACKETS",
+                        "ALIGN_MULTILINE_BINARY_OPERATION",
                         "ELSE_ON_NEW_LINE",
                         "WHILE_ON_NEW_LINE",
                         "CATCH_ON_NEW_LINE",

idea/testData/codeInsight/moveUpDown/expressions/insideExpression1.kt

@@ -2,7 +2,7 @@
 fun foo() {
     fun bar1() {}
     val x = 1 +
-    <caret>2 +
+            <caret>2 +
-    3
+            3
     fun bar2() {}
 }

idea/testData/codeInsight/moveUpDown/expressions/insideExpression1.kt.after

@@ -1,8 +1,8 @@
 // MOVE: up
 fun foo() {
     val x = 1 +
-    <caret>2 +
+            <caret>2 +
-    3
+            3
     fun bar1() {}
     fun bar2() {}
 }

idea/testData/codeInsight/moveUpDown/expressions/insideExpression2.kt.after

@@ -3,6 +3,6 @@ fun foo() {
     fun bar1() {}
     fun bar2() {}
     val x = 1 +
-    <caret>2 +
+            <caret>2 +
-    3
+            3
 }

idea/testData/codeInsight/moveUpDown/expressions/insideExpression3.kt

@@ -2,7 +2,7 @@
 fun foo() {
     fun bar1() {}
     val x = 1 +
-    (2 +
+            (2 +
-    <caret>3)
+                    <caret>3)
     fun bar2() {}
 }

idea/testData/codeInsight/moveUpDown/expressions/insideExpression3.kt.after

@@ -1,8 +1,8 @@
 // MOVE: up
 fun foo() {
     val x = 1 +
-    (2 +
+            (2 +
-    <caret>3)
+                    <caret>3)
     fun bar1() {}
     fun bar2() {}
 }

idea/testData/codeInsight/moveUpDown/expressions/insideExpression4.kt.after

@@ -3,6 +3,6 @@ fun foo() {
     fun bar1() {}
     fun bar2() {}
     val x = 1 +
-    (2 +
+            (2 +
-    <caret>3)
+                    <caret>3)
 }

idea/testData/formatter/BinaryExpressions.after.inv.kt

@@ -0,0 +1,15 @@
+fun test() {
+    val somelong = 1 + 2 +
+            3 - 4 -
+            5 * 6 *
+                    7 / 8 /
+                    9 % 10 %
+                    11
+
+    val withBrackets = 3 +
+            4 - (5 +
+            4 * 5)
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION
+// Strage behaviour for disabled alignment is same to Java

idea/testData/formatter/BinaryExpressions.after.kt

@@ -0,0 +1,15 @@
+fun test() {
+    val somelong = 1 + 2 +
+                   3 - 4 -
+                   5 * 6 *
+                   7 / 8 /
+                   9 % 10 %
+                   11
+
+    val withBrackets = 3 +
+                       4 - (5 +
+                            4 * 5)
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION
+// Strage behaviour for disabled alignment is same to Java

idea/testData/formatter/BinaryExpressions.kt

@@ -0,0 +1,15 @@
+fun test() {
+    val somelong = 1 + 2 +
+    3 - 4 -
+    5 * 6 *
+    7 / 8 /
+    9 % 10 %
+    11
+
+    val withBrackets = 3 +
+    4 - (5 +
+    4 * 5)
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION
+// Strage behaviour for disabled alignment is same to Java

idea/testData/formatter/BinaryExpressionsBoolean.after.inv.kt

@@ -0,0 +1,32 @@
+fun test() {
+    if (true
+            || false
+            || true) {
+
+    }
+
+    if (true || false
+            || true) {
+
+    }
+
+    if (true
+            || false
+            || true) {
+
+    }
+
+    if (true ||
+            false ||
+            true) {
+
+    }
+
+    true
+            || true
+
+    false
+            && false
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/formatter/BinaryExpressionsBoolean.after.kt

@@ -0,0 +1,32 @@
+fun test() {
+    if (true
+        || false
+        || true) {
+
+    }
+
+    if (true || false
+        || true) {
+
+    }
+
+    if (true
+        || false
+        || true) {
+
+    }
+
+    if (true ||
+        false ||
+        true) {
+
+    }
+
+    true
+    || true
+
+    false
+    && false
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/formatter/BinaryExpressionsBoolean.kt

@@ -0,0 +1,32 @@
+fun test() {
+    if (true
+    || false
+    || true) {
+
+    }
+
+    if (true || false
+    || true) {
+
+    }
+
+    if (true
+    || false
+    || true) {
+
+    }
+
+    if (true ||
+    false ||
+    true) {
+
+    }
+
+    true
+    || true
+
+    false
+    && false
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionInMiddle.after.inv.kt

@@ -0,0 +1,6 @@
+fun some() {
+    val test = 3 +
+            <caret>4
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionInMiddle.after.kt

@@ -0,0 +1,6 @@
+fun some() {
+    val test = 3 +
+               <caret>4
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionInMiddle.kt

@@ -0,0 +1,5 @@
+fun some() {
+    val test = 3 + <caret>4
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionUnfinished.after.inv.kt

@@ -0,0 +1,6 @@
+fun some() {
+    val test = 3 +
+            <caret>
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionUnfinished.after.kt

@@ -0,0 +1,6 @@
+fun some() {
+    val test = 3 +
+               <caret>
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionUnfinished.kt

@@ -0,0 +1,5 @@
+fun some() {
+    val test = 3 + <caret>
+}
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionsBeforeCloseParenthesis.after.inv.kt

@@ -0,0 +1,4 @@
+val somelong = 3 + 4 - (3 +
+        <caret>)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionsBeforeCloseParenthesis.after.kt

@@ -0,0 +1,4 @@
+val somelong = 3 + 4 - (3 +
+                       <caret>)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InBinaryExpressionsBeforeCloseParenthesis.kt

@@ -0,0 +1,3 @@
+val somelong = 3 + 4 - (3 + <caret>)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InExpressionsParentheses.after.inv.kt

@@ -0,0 +1,5 @@
+val somelong = 3 + 4 - (
+        <caret>
+        )
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InExpressionsParentheses.after.kt

@@ -0,0 +1,5 @@
+val somelong = 3 + 4 - (
+                       <caret>
+                       )
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InExpressionsParentheses.kt

@@ -0,0 +1,3 @@
+val somelong = 3 + 4 - (<caret>)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InExpressionsParenthesesBeforeOperand.after.inv.kt

@@ -0,0 +1,4 @@
+val somelong = 3 + (3 *
+        <caret>3)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InExpressionsParenthesesBeforeOperand.after.kt

@@ -0,0 +1,4 @@
+val somelong = 3 + (3 *
+                    <caret>3)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/testData/indentationOnNewline/InExpressionsParenthesesBeforeOperand.kt

@@ -0,0 +1,3 @@
+val somelong = 3 + (3 * <caret>3)
+
+// SET_TRUE: ALIGN_MULTILINE_BINARY_OPERATION

idea/tests/org/jetbrains/jet/formatter/JetFormatterTestGenerated.java

@@ -36,7 +36,19 @@ public class JetFormatterTestGenerated extends AbstractJetFormatterTest {
     @InnerTestClasses({Formatter.ModifierList.class})
     public static class Formatter extends AbstractJetFormatterTest {
         public void testAllFilesPresentInFormatter() throws Exception {
-            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/formatter"), Pattern.compile("^([^\\.]+)\\.after\\.kt.*$"), true);
+            JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage",
+                                                         new File("idea/testData/formatter"), Pattern.compile("^([^\\.]+)\\.after\\.kt.*$"),
+                                                         true);
+        }
+        
+        @TestMetadata("BinaryExpressions.after.kt")
+        public void testBinaryExpressions() throws Exception {
+            doTest("idea/testData/formatter/BinaryExpressions.after.kt");
+        }
+        
+        @TestMetadata("BinaryExpressionsBoolean.after.kt")
+        public void testBinaryExpressionsBoolean() throws Exception {
+            doTest("idea/testData/formatter/BinaryExpressionsBoolean.after.kt");
         }
         
         @TestMetadata("BlockFor.after.kt")
@@ -407,6 +419,16 @@ public class JetFormatterTestGenerated extends AbstractJetFormatterTest {
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/formatter"), Pattern.compile("^([^\\.]+)\\.after\\.inv\\.kt.*$"), true);
         }
         
+        @TestMetadata("BinaryExpressions.after.inv.kt")
+        public void testBinaryExpressions() throws Exception {
+            doTestInverted("idea/testData/formatter/BinaryExpressions.after.inv.kt");
+        }
+        
+        @TestMetadata("BinaryExpressionsBoolean.after.inv.kt")
+        public void testBinaryExpressionsBoolean() throws Exception {
+            doTestInverted("idea/testData/formatter/BinaryExpressionsBoolean.after.inv.kt");
+        }
+        
         @TestMetadata("CatchFinallyOnNewLine.after.inv.kt")
         public void testCatchFinallyOnNewLine() throws Exception {
             doTestInverted("idea/testData/formatter/CatchFinallyOnNewLine.after.inv.kt");

idea/tests/org/jetbrains/jet/formatter/JetTypingIndentationTestBaseGenerated.java

@@ -106,6 +106,31 @@ public class JetTypingIndentationTestBaseGenerated extends AbstractJetTypingInde
             doNewlineTest("idea/testData/indentationOnNewline/If.after.kt");
         }
         
+        @TestMetadata("InBinaryExpressionInMiddle.after.kt")
+        public void testInBinaryExpressionInMiddle() throws Exception {
+            doNewlineTest("idea/testData/indentationOnNewline/InBinaryExpressionInMiddle.after.kt");
+        }
+        
+        @TestMetadata("InBinaryExpressionUnfinished.after.kt")
+        public void testInBinaryExpressionUnfinished() throws Exception {
+            doNewlineTest("idea/testData/indentationOnNewline/InBinaryExpressionUnfinished.after.kt");
+        }
+        
+        @TestMetadata("InBinaryExpressionsBeforeCloseParenthesis.after.kt")
+        public void testInBinaryExpressionsBeforeCloseParenthesis() throws Exception {
+            doNewlineTest("idea/testData/indentationOnNewline/InBinaryExpressionsBeforeCloseParenthesis.after.kt");
+        }
+        
+        @TestMetadata("InExpressionsParentheses.after.kt")
+        public void testInExpressionsParentheses() throws Exception {
+            doNewlineTest("idea/testData/indentationOnNewline/InExpressionsParentheses.after.kt");
+        }
+        
+        @TestMetadata("InExpressionsParenthesesBeforeOperand.after.kt")
+        public void testInExpressionsParenthesesBeforeOperand() throws Exception {
+            doNewlineTest("idea/testData/indentationOnNewline/InExpressionsParenthesesBeforeOperand.after.kt");
+        }
+        
         @TestMetadata("NotFirstParameter.after.kt")
         public void testNotFirstParameter() throws Exception {
             doNewlineTest("idea/testData/indentationOnNewline/NotFirstParameter.after.kt");
@@ -171,6 +196,31 @@ public class JetTypingIndentationTestBaseGenerated extends AbstractJetTypingInde
                                                          Pattern.compile("^([^\\.]+)\\.after\\.inv\\.kt.*$"), true);
         }
         
+        @TestMetadata("InBinaryExpressionInMiddle.after.inv.kt")
+        public void testInBinaryExpressionInMiddle() throws Exception {
+            doNewlineTestWithInvert("idea/testData/indentationOnNewline/InBinaryExpressionInMiddle.after.inv.kt");
+        }
+        
+        @TestMetadata("InBinaryExpressionUnfinished.after.inv.kt")
+        public void testInBinaryExpressionUnfinished() throws Exception {
+            doNewlineTestWithInvert("idea/testData/indentationOnNewline/InBinaryExpressionUnfinished.after.inv.kt");
+        }
+        
+        @TestMetadata("InBinaryExpressionsBeforeCloseParenthesis.after.inv.kt")
+        public void testInBinaryExpressionsBeforeCloseParenthesis() throws Exception {
+            doNewlineTestWithInvert("idea/testData/indentationOnNewline/InBinaryExpressionsBeforeCloseParenthesis.after.inv.kt");
+        }
+        
+        @TestMetadata("InExpressionsParentheses.after.inv.kt")
+        public void testInExpressionsParentheses() throws Exception {
+            doNewlineTestWithInvert("idea/testData/indentationOnNewline/InExpressionsParentheses.after.inv.kt");
+        }
+        
+        @TestMetadata("InExpressionsParenthesesBeforeOperand.after.inv.kt")
+        public void testInExpressionsParenthesesBeforeOperand() throws Exception {
+            doNewlineTestWithInvert("idea/testData/indentationOnNewline/InExpressionsParenthesesBeforeOperand.after.inv.kt");
+        }
+        
         @TestMetadata("SettingAlignMultilineParametersInCalls.after.inv.kt")
         public void testSettingAlignMultilineParametersInCalls() throws Exception {
             doNewlineTestWithInvert("idea/testData/indentationOnNewline/SettingAlignMultilineParametersInCalls.after.inv.kt");