From f0035a7be0f57761ae6af28f88d2db902791ec9a Mon Sep 17 00:00:00 2001 From: Dimach Date: Sun, 30 Jul 2017 04:49:55 +0300 Subject: [PATCH] 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 --- .../org/jetbrains/kotlin/psi/KtPsiUtil.java | 19 ++++++ .../kotlin/j2k/ExpressionConverter.kt | 45 +++++++------- .../binaryExpression/orWithNewLine.kt | 59 +++++++++++++------ .../fileOrElement/issues/kt-17379.java | 19 ++++++ j2k/testData/fileOrElement/issues/kt-17379.kt | 20 +++++++ .../CorrectNullabilityDetected.java | 1 - .../CorrectNullabilityDetected.kt | 17 +++--- .../kotlinApiAccess/ObjectMembers.kt | 2 +- .../fileOrElement/strings/formattedString.kt | 6 +- ...otlinConverterForWebDemoTestGenerated.java | 6 ++ ...otlinConverterSingleFileTestGenerated.java | 6 ++ 11 files changed, 147 insertions(+), 53 deletions(-) create mode 100644 j2k/testData/fileOrElement/issues/kt-17379.java create mode 100644 j2k/testData/fileOrElement/issues/kt-17379.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java index 4fdd15a7109..cd4d38896c8 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiUtil.java @@ -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()); diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ExpressionConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/ExpressionConverter.kt index d8c55712261..febaac2b6b5 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ExpressionConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ExpressionConverter.kt @@ -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 diff --git a/j2k/testData/fileOrElement/binaryExpression/orWithNewLine.kt b/j2k/testData/fileOrElement/binaryExpression/orWithNewLine.kt index cc1483f45ac..c20174c0565 100644 --- a/j2k/testData/fileOrElement/binaryExpression/orWithNewLine.kt +++ b/j2k/testData/fileOrElement/binaryExpression/orWithNewLine.kt @@ -1,26 +1,47 @@ object A { @JvmStatic fun main(args: Array) { - 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) } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/issues/kt-17379.java b/j2k/testData/fileOrElement/issues/kt-17379.java new file mode 100644 index 00000000000..e7e382c309f --- /dev/null +++ b/j2k/testData/fileOrElement/issues/kt-17379.java @@ -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; + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/issues/kt-17379.kt b/j2k/testData/fileOrElement/issues/kt-17379.kt new file mode 100644 index 00000000000..883df44de7d --- /dev/null +++ b/j2k/testData/fileOrElement/issues/kt-17379.kt @@ -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 + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.java b/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.java index 82d78102f45..4a0b0204c65 100644 --- a/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.java +++ b/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.java @@ -1,7 +1,6 @@ //file import kotlinApi.*; -//TODO: Formatter works incorrectly class A { int foo(KotlinClass c) { return c.getNullableProperty().length() diff --git a/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.kt b/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.kt index 822dcfdd419..aa33e0b63b1 100644 --- a/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.kt +++ b/j2k/testData/fileOrElement/kotlinApiAccess/CorrectNullabilityDetected.kt @@ -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) } } \ No newline at end of file diff --git a/j2k/testData/fileOrElement/kotlinApiAccess/ObjectMembers.kt b/j2k/testData/fileOrElement/kotlinApiAccess/ObjectMembers.kt index 54974c2ac5d..7b7799b9928 100644 --- a/j2k/testData/fileOrElement/kotlinApiAccess/ObjectMembers.kt +++ b/j2k/testData/fileOrElement/kotlinApiAccess/ObjectMembers.kt @@ -8,4 +8,4 @@ internal class C { KotlinObject.property1 + KotlinObject.property2 } -} \ No newline at end of file +} diff --git a/j2k/testData/fileOrElement/strings/formattedString.kt b/j2k/testData/fileOrElement/strings/formattedString.kt index d673aa4199d..9e492698b9a 100644 --- a/j2k/testData/fileOrElement/strings/formattedString.kt +++ b/j2k/testData/fileOrElement/strings/formattedString.kt @@ -4,7 +4,7 @@ object A { "text3" - val TEXT2 = "text1\n" - + "text2\n" - + "text3" + val TEXT2 = ("text1\n" + + "text2\n" + + "text3") } \ No newline at end of file diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java index d5e69444496..8a97a9221ed 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java @@ -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"); diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java index 56491ed7f30..03a3f5d0960 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.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");