KT-17379: Fix J2K removal of parentheses in multiline expressions

When there is multiline polyadic expression with some operators J2K should keep surrounding parentheses, otherwise operators will be dangling due resolved to prefix variant #KT-17379 fixed
2017-07-30 04:49:55 +03:00
parent 0920b2574c
commit f0035a7be0
11 changed files with 147 additions and 53 deletions
@@ -530,6 +530,10 @@ public class KtPsiUtil {
            return false;
        }

+        if (innerExpression instanceof KtBinaryExpression && isKeepBinaryExpressionParenthesized((KtBinaryExpression) innerExpression)) {
+            return true;
+        }
+
        int innerPriority = getPriority(innerExpression);
        int parentPriority = getPriority((KtExpression) parentElement);

@@ -551,6 +555,21 @@ public class KtPsiUtil {
        return innerPriority < parentPriority;
    }

+    private static boolean isKeepBinaryExpressionParenthesized(KtBinaryExpression expression) {
+        PsiElement expr = expression.getFirstChild();
+        while (expr != null) {
+            if (expr instanceof PsiWhiteSpace && expr.textContains('\n')) {
+                return true;
+            }
+            if (expr instanceof KtOperationReferenceExpression) {
+                break;
+            }
+            expr = expr.getNextSibling();
+        }
+        return (expression.getRight() instanceof KtBinaryExpression && isKeepBinaryExpressionParenthesized((KtBinaryExpression) expression.getRight())) ||
+               (expression.getLeft() instanceof KtBinaryExpression && isKeepBinaryExpressionParenthesized((KtBinaryExpression) expression.getLeft()));
+    }
+
    public static boolean isAssignment(@NotNull PsiElement element) {
        return element instanceof KtBinaryExpression &&
               KtTokens.ALL_ASSIGNMENTS.contains(((KtBinaryExpression) element).getOperationToken());
@@ -49,7 +49,7 @@ interface SpecialExpressionConverter {
 }

 fun ExpressionConverter.withSpecialConverter(specialConverter: SpecialExpressionConverter): ExpressionConverter {
-    return object: ExpressionConverter {
+    return object : ExpressionConverter {
        override fun convertExpression(expression: PsiExpression, codeConverter: CodeConverter)
                = specialConverter.convertExpression(expression, codeConverter) ?: this@withSpecialConverter.convertExpression(expression, codeConverter)
    }
@@ -94,7 +94,7 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
        val lhs = codeConverter.convertExpression(expression.lExpression)
        val rhs = codeConverter.convertExpression(expression.rExpression!!, expression.lExpression.type)

-        val secondOp = when(tokenType) {
+        val secondOp = when (tokenType) {
            JavaTokenType.GTGTEQ, JavaTokenType.LTLTEQ, JavaTokenType.GTGTGTEQ,
            JavaTokenType.XOREQ, JavaTokenType.OREQ,
            JavaTokenType.ANDEQ -> true
@@ -111,6 +111,7 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
    }

    override fun visitBinaryExpression(expression: PsiBinaryExpression) {
+
        val left = expression.lOperand
        val right = expression.rOperand

@@ -140,6 +141,9 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
                }
            }
            result = BinaryExpression(leftConverted, rightConverted, operator.assignPrototype(expression.operationSign))
+            if (!expression.isInSingleLine()) {
+                result = ParenthesizedExpression(result.assignNoPrototype())
+            }
        }
    }

@@ -159,29 +163,29 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
    }

    infix operator fun PsiPrimitiveType.compareTo(other: PsiPrimitiveType): Int {
-        return when(this) {
+        return when (this) {
            other -> 0
-            PsiType.BYTE -> when(other) {
+            PsiType.BYTE -> when (other) {
                PsiType.CHAR -> 1
                else -> -1
            }
-            PsiType.SHORT -> when(other) {
+            PsiType.SHORT -> when (other) {
                PsiType.CHAR,
                PsiType.BYTE -> 1
                else -> -1
            }
-            PsiType.INT -> when(other) {
+            PsiType.INT -> when (other) {
                PsiType.BYTE,
                PsiType.SHORT,
                PsiType.CHAR -> 1
                else -> -1
            }
-            PsiType.LONG -> when(other) {
+            PsiType.LONG -> when (other) {
                PsiType.DOUBLE,
                PsiType.FLOAT -> -1
                else -> 1
            }
-            PsiType.FLOAT -> when(other) {
+            PsiType.FLOAT -> when (other) {
                PsiType.DOUBLE -> -1
                else -> 1
            }
@@ -244,7 +248,8 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
        }
        val name = if (primitiveType != null) {
            "javaPrimitiveType"
-        } else {
+        }
+        else {
            "java"
        }
        result = QualifiedExpression(ClassLiteralExpression(type).assignNoPrototype(), Identifier.withNoPrototype(name), null)
@@ -378,7 +383,7 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
                            propertyName
                        }

-                        when(if (isExtension) parameterCount - 1 else parameterCount) {
+                        when (if (isExtension) parameterCount - 1 else parameterCount) {
                            0 /* getter */ -> {
                                result = propertyAccess
                                return
@@ -691,7 +696,7 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
            var op: Operator = operators.first()
            var index = 0
            operators.forEachIndexed { i, operator ->
-                if(operator.precedence >= op.precedence) {
+                if (operator.precedence >= op.precedence) {
                    op = operator
                    index = i
                }
@@ -708,13 +713,12 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
            codeConverter.convertExpression(it, expression.type).assignPrototype(it, CommentsAndSpacesInheritance.LINE_BREAKS)
        }
        val operators = expression.operands.mapNotNull {
-            expression.getTokenBeforeOperand(it)?.let {
-                val operator = Operator(it.tokenType)
-                val commentsAndSpacesInheritance = if (operator.acceptLineBreakBefore()) CommentsAndSpacesInheritance.LINE_BREAKS else CommentsAndSpacesInheritance.NO_SPACES
-                operator.assignPrototype(it, commentsAndSpacesInheritance)
-            }
+            expression.getTokenBeforeOperand(it)?.let { Operator(it.tokenType).assignPrototype(it, CommentsAndSpacesInheritance.LINE_BREAKS) }
+        }
+        result = polyadicExpressionToBinaryExpressions(args, operators)
+        if (!expression.isInSingleLine()) {
+            result = ParenthesizedExpression(result.assignNoPrototype())
        }
-        result = polyadicExpressionToBinaryExpressions(args, operators).assignPrototype(expression)
    }

    private fun convertArguments(expression: PsiCallExpression, isExtension: Boolean = false): ArgumentList {
@@ -765,7 +769,7 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
                result = LambdaExpression(convertedParameters, Block.of(convertedBody).assignNoPrototype())
            }
            is PsiCodeBlock -> {
-                val convertedBlock = codeConverter.withSpecialStatementConverter(object: SpecialStatementConverter {
+                val convertedBlock = codeConverter.withSpecialStatementConverter(object : SpecialStatementConverter {
                    override fun convertStatement(statement: PsiStatement, codeConverter: CodeConverter): Statement? {
                        if (statement !is PsiReturnStatement) return null

@@ -877,7 +881,8 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
                if (parameters.size == 1 && !isKotlinFunctionType) {
                    // for lambdas all parameters with types should be present
                    emptyList()
-                } else {
+                }
+                else {
                    parameters.map { LambdaParameter(it.first, it.second).assignNoPrototype() }
                },
                lPar = null,
@@ -903,7 +908,7 @@ class DefaultExpressionConverter : JavaElementVisitor(), ExpressionConverter {
    private fun isFunctionType(functionalType: PsiType?) = functionalType?.canonicalText?.startsWith("kotlin.jvm.functions.Function") ?: false

    private fun convertMethodReferenceQualifier(qualifier: PsiElement): String {
-        return when(qualifier) {
+        return when (qualifier) {
            is PsiExpression -> codeConverter.convertExpression(qualifier).canonicalCode()
            is PsiTypeElement -> converter.convertTypeElement(qualifier, Nullability.NotNull).canonicalCode()
            else -> qualifier.text
@@ -1,26 +1,47 @@
 object A {
    @JvmStatic
    fun main(args: Array<String>) {
-        val ANDAND = true
+        val ANDAND = (true
                && true
-                && true
-        val OROR = true
+                && true)
+        val OROR = (true
                || true
-                || true
-        val PLUS = 1
-        +2
-        +3
-        val MINUS = 1
-        -2
-        -3
-        val ASTERISK = 1 * 2 * 3
-        val DIV = 1 / 2 / 3
-        val PERC = 1 % 2 % 3
-        val GTGT = 1 shl 2 shl 3
-        val LTLT = 1 shr 2 shr 3
-        val XOR = 1 xor 2 xor 3
-        val AND = 1 and 2 and 3
-        val OR = 1 % 2 % 3
-        val GTGTGT = 1 ushr 2 ushr 3
+                || true)
+        val PLUS = (1
+                + 2
+                + 3)
+        val MINUS = (1
+                - 2
+                - 3)
+        val ASTERISK = (1
+                * 2
+                * 3)
+        val DIV = (1
+
+                / 2
+
+
+                / 3)
+        val PERC = (1
+                % 2
+                % 3)
+        val GTGT = (1
+                shl 2
+                shl 3)
+        val LTLT = (1
+                shr 2
+                shr 3)
+        val XOR = (1
+                xor 2
+                xor 3)
+        val AND = (1
+                and 2
+                and 3)
+        val OR = (1
+                % 2
+                % 3)
+        val GTGTGT = (1
+                ushr 2
+                ushr 3)
    }
 }
@@ -0,0 +1,19 @@
+//file
+package demo;
+
+class Test {
+  void test() {
+    int a = 0;
+    int b = 1;
+    int c = 2;
+    int d = 4;
+    int y = a // polyadic expression case
+            + b // x2
+            + c // x3
+            + d; // x4
+    int z = a // binary expression case
+            + b; // x4
+    int j = b +
+               c;
+  }
+}
@@ -0,0 +1,20 @@
+package demo
+
+internal class Test {
+    fun test() {
+        val a = 0
+        val b = 1
+        val c = 2
+        val d = 4
+        val y = (a // polyadic expression case
+
+                + b // x2
+
+                + c // x3
+
+                + d) // x4
+        val z = (a // binary expression case
+                + b) // x4
+        val j = b + c
+    }
+}
@@ -1,7 +1,6 @@
 //file
 import kotlinApi.*;

-//TODO: Formatter works incorrectly
 class A {
    int foo(KotlinClass c) {
        return c.getNullableProperty().length()
@@ -1,15 +1,14 @@
 import kotlinApi.*

-//TODO: Formatter works incorrectly
 internal class A {
    fun foo(c: KotlinClass): Int {
-        return c.nullableProperty!!.length
-        +c.property.length
-        +KotlinClass.nullableStaticVar!!
-        +KotlinClass.staticVar
-        +KotlinClass.nullableStaticFun(1)!!
-        +KotlinClass.staticFun(1)
-        +nullableGlobalFunction("")!!.length
-        +globalFunction("").length
+        return (c.nullableProperty!!.length
+                + c.property.length
+                + KotlinClass.nullableStaticVar!!
+                + KotlinClass.staticVar
+                + KotlinClass.nullableStaticFun(1)!!
+                + KotlinClass.staticFun(1)
+                + nullableGlobalFunction("")!!.length
+                + globalFunction("").length)
    }
 }
@@ -8,4 +8,4 @@ internal class C {
                KotlinObject.property1 +
                KotlinObject.property2
    }
-}
+}
@@ -4,7 +4,7 @@ object A {
            "text3"


-    val TEXT2 = "text1\n"
-    + "text2\n"
-    + "text3"
+    val TEXT2 = ("text1\n"
+            + "text2\n"
+            + "text3")
 }
@@ -2945,6 +2945,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo
            doTest(fileName);
        }

+        @TestMetadata("kt-17379.java")
+        public void testKt_17379() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/issues/kt-17379.java");
+            doTest(fileName);
+        }
+
        @TestMetadata("kt-5294.java")
        public void testKt_5294() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/issues/kt-5294.java");
@@ -2945,6 +2945,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo
            doTest(fileName);
        }

+        @TestMetadata("kt-17379.java")
+        public void testKt_17379() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/issues/kt-17379.java");
+            doTest(fileName);
+        }
+
        @TestMetadata("kt-5294.java")
        public void testKt_5294() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/issues/kt-5294.java");