From 6e473506fcf8967d787895933fa86854b44fd43b Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Tue, 24 Jun 2014 19:04:03 +0400 Subject: [PATCH] Java to Kotlin converter: specify local variable type when necessary #KT-5277 Fixed --- j2k/src/org/jetbrains/jet/j2k/Converter.kt | 101 ++++++++++-------- .../jetbrains/jet/j2k/ast/LocalVariable.kt | 16 ++- .../jet/j2k/visitors/ElementVisitor.kt | 15 ++- .../jet/j2k/visitors/ExpressionVisitor.kt | 14 ++- .../JavaToKotlinConverterTestGenerated.java | 35 ++++++ j2k/tests/testData/ast/boxedType/Boxing.kt | 2 +- .../ast/localVariable/autoBangBang.java | 5 + .../ast/localVariable/autoBangBang.kt | 4 + .../ast/localVariable/conversion.java | 5 + .../testData/ast/localVariable/conversion.kt | 4 + .../ast/localVariable/literalConversion.java | 5 + .../ast/localVariable/literalConversion.kt | 4 + .../ast/localVariable/nullIInitializer.java | 2 + .../ast/localVariable/nullIInitializer.kt | 1 + .../testData/ast/localVariable/unboxing.java | 5 + .../testData/ast/localVariable/unboxing.kt | 4 + .../ast/localVariable/valTypeDoNotMatch.java | 4 + .../ast/localVariable/valTypeDoNotMatch.kt | 3 + .../ast/localVariable/varTypeDoNotMatch.java | 5 + .../ast/localVariable/varTypeDoNotMatch.kt | 4 + .../MethodResultComparedWithNull4.kt | 2 +- .../testData/ast/prefixOperator/nullableIf.kt | 2 +- 22 files changed, 182 insertions(+), 60 deletions(-) create mode 100644 j2k/tests/testData/ast/localVariable/autoBangBang.java create mode 100644 j2k/tests/testData/ast/localVariable/autoBangBang.kt create mode 100644 j2k/tests/testData/ast/localVariable/conversion.java create mode 100644 j2k/tests/testData/ast/localVariable/conversion.kt create mode 100644 j2k/tests/testData/ast/localVariable/literalConversion.java create mode 100644 j2k/tests/testData/ast/localVariable/literalConversion.kt create mode 100644 j2k/tests/testData/ast/localVariable/nullIInitializer.java create mode 100644 j2k/tests/testData/ast/localVariable/nullIInitializer.kt create mode 100644 j2k/tests/testData/ast/localVariable/unboxing.java create mode 100644 j2k/tests/testData/ast/localVariable/unboxing.kt create mode 100644 j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.java create mode 100644 j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.kt create mode 100644 j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.java create mode 100644 j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.kt diff --git a/j2k/src/org/jetbrains/jet/j2k/Converter.kt b/j2k/src/org/jetbrains/jet/j2k/Converter.kt index 05e91822095..7d0af37c7ac 100644 --- a/j2k/src/org/jetbrains/jet/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/Converter.kt @@ -249,7 +249,7 @@ public class Converter private(val project: Project, val settings: ConverterSett val localVar = LocalVariable(SecondaryConstructor.tempValIdentifier(), Annotations.Empty, Modifiers.Empty, - { ClassType(className, listOf(), Nullability.NotNull, settings) }, + null, initializer, true, settings).assignNoPrototype() @@ -554,34 +554,70 @@ public class Converter private(val project: Project, val settings: ConverterSett return Parameter(parameter.declarationIdentifier(), `type`, varValModifier, convertAnnotations(parameter), modifiers).assignPrototype(parameter) } - fun convertExpression(argument: PsiExpression?, expectedType: PsiType?): Expression { - if (argument == null) return Identifier.Empty + fun convertExpression(expression: PsiExpression?, expectedType: PsiType?): Expression { + if (expression == null) return Identifier.Empty - var expression = convertExpression(argument) - val actualType = argument.getType() - val isPrimitiveTypeOrNull = actualType == null || actualType is PsiPrimitiveType - if (isPrimitiveTypeOrNull && expression.isNullable) { - expression = BangBangExpression(expression) + var convertedExpression = convertExpression(expression) + if (expectedType == null || expectedType == PsiType.VOID) return convertedExpression + + val actualType = expression.getType() + if (actualType == null) return convertedExpression + + if (actualType is PsiPrimitiveType && convertedExpression.isNullable || + expectedType is PsiPrimitiveType && actualType is PsiClassType) { + convertedExpression = BangBangExpression(convertedExpression) } - else if (expectedType is PsiPrimitiveType && actualType is PsiClassType) { - if (PsiPrimitiveType.getUnboxedType(actualType) == expectedType) { - expression = BangBangExpression(expression) + + if (canConvertType(actualType, expectedType) && convertedExpression !is LiteralExpression) { + val conversion = PRIMITIVE_TYPE_CONVERSIONS[expectedType.getCanonicalText()] + if (conversion != null) { + convertedExpression = MethodCallExpression.buildNotNull(convertedExpression, conversion) } } - if (actualType != null) { - if (isConversionNeeded(actualType, expectedType) && expression !is LiteralExpression) { - val conversion = PRIMITIVE_TYPE_CONVERSIONS[expectedType?.getCanonicalText()] - if (conversion != null) { - expression = MethodCallExpression.buildNotNull(expression, conversion) - } - } - - } - - return expression.assignPrototype(argument) + return convertedExpression.assignPrototype(expression) } + fun convertedExpressionType(expression: PsiExpression, expectedType: PsiType): Type { + var convertedExpression = convertExpression(expression) + val actualType = expression.getType() + if (actualType == null) return ErrorType() + var resultType = typeConverter.convertType(actualType, if (convertedExpression.isNullable) Nullability.Nullable else Nullability.NotNull) + + if (actualType is PsiPrimitiveType && resultType.isNullable || + expectedType is PsiPrimitiveType && actualType is PsiClassType) { + resultType = resultType.toNotNullType() + } + + if (canConvertType(actualType, expectedType) && convertedExpression !is LiteralExpression) { + val conversion = PRIMITIVE_TYPE_CONVERSIONS[expectedType.getCanonicalText()] + if (conversion != null) { + resultType = typeConverter.convertType(expectedType, Nullability.NotNull) + } + } + + return resultType + } + + private fun canConvertType(actual: PsiType, expected: PsiType): Boolean { + val expectedStr = expected.getCanonicalText() + val actualStr = actual.getCanonicalText() + if (expectedStr == actualStr) return false + val o1 = expectedStr == typeConversionMap[actualStr] + val o2 = actualStr == typeConversionMap[expectedStr] + return o1 == o2 + } + + private val typeConversionMap: Map = mapOf( + JAVA_LANG_BYTE to "byte", + JAVA_LANG_SHORT to "short", + JAVA_LANG_INTEGER to "int", + JAVA_LANG_LONG to "long", + JAVA_LANG_FLOAT to "float", + JAVA_LANG_DOUBLE to "double", + JAVA_LANG_CHARACTER to "char" + ) + fun convertIdentifier(identifier: PsiIdentifier?): Identifier { if (identifier == null) return Identifier.Empty @@ -690,27 +726,6 @@ public class Converter private(val project: Project, val settings: ConverterSett val annotation = Annotation(Identifier("throws").assignNoPrototype(), arguments, false) return Annotations(listOf(annotation.assignPrototype(throwsList)), true).assignPrototype(throwsList) } - - private val TYPE_MAP: Map = mapOf( - JAVA_LANG_BYTE to "byte", - JAVA_LANG_SHORT to "short", - JAVA_LANG_INTEGER to "int", - JAVA_LANG_LONG to "long", - JAVA_LANG_FLOAT to "float", - JAVA_LANG_DOUBLE to "double", - JAVA_LANG_CHARACTER to "char" - ) - - private fun isConversionNeeded(actual: PsiType?, expected: PsiType?): Boolean { - if (actual == null || expected == null) return false - - val expectedStr = expected.getCanonicalText() - val actualStr = actual.getCanonicalText() - if (expectedStr == actualStr) return false - val o1 = expectedStr == TYPE_MAP[actualStr] - val o2 = actualStr == TYPE_MAP[expectedStr] - return o1 == o2 - } } val NOT_NULL_ANNOTATIONS: Set = setOf("org.jetbrains.annotations.NotNull", "com.sun.istack.internal.NotNull", "javax.annotation.Nonnull") diff --git a/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt b/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt index 68cb7538eea..a5e88774ffe 100644 --- a/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt +++ b/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt @@ -24,23 +24,19 @@ class LocalVariable( private val identifier: Identifier, private val annotations: Annotations, private val modifiers: Modifiers, - private val typeCalculator: () -> Type /* we use lazy type calculation for better performance */, + private val explicitType: Type?, private val initializer: Expression, private val isVal: Boolean, private val settings: ConverterSettings ) : Element() { override fun generateCode(builder: CodeBuilder) { - builder.append(annotations).append(if (isVal) "val " else "var ") - if (initializer.isEmpty) { - builder append identifier append ":" append typeCalculator() + builder append annotations append (if (isVal) "val " else "var ") append identifier + if (explicitType != null) { + builder append ":" append explicitType } - else { - val shouldSpecifyType = settings.specifyLocalVariableTypeByDefault - if (shouldSpecifyType) - builder append identifier append ":" append typeCalculator() append " = " append initializer - else - builder append identifier append " = " append initializer + if (!initializer.isEmpty) { + builder append " = " append initializer } } } diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt index 84c10c6ef77..ae6da2c0d36 100644 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt +++ b/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt @@ -30,12 +30,25 @@ class ElementVisitor(private val converter: Converter) : JavaElementVisitor() { result = LocalVariable(variable.declarationIdentifier(), converter.convertAnnotations(variable), converter.convertModifiers(variable), - { typeConverter.convertVariableType(variable) }, + explicitType(variable), converter.convertExpression(variable.getInitializer(), variable.getType()), converter.settings.forceLocalVariableImmutability || variable.hasModifierProperty(PsiModifier.FINAL), converter.settings) } + private fun explicitType(variable: PsiLocalVariable): Type? { + fun convertType() = typeConverter.convertVariableType(variable) + if (converter.settings.specifyLocalVariableTypeByDefault) return convertType() + val initializer = variable.getInitializer() + if (initializer == null) return convertType() + if (initializer is PsiLiteralExpression && initializer.getType() == PsiType.NULL) return convertType() + if (variable.hasModifierProperty(PsiModifier.FINAL) || !variable.hasWriteAccesses(variable.getContainingMethod())) return null + val convertedType = convertType() + var initializerType = converter.convertedExpressionType(initializer, variable.getType()) + if (initializerType is ErrorType) return null // do not add explicit type when initializer is not resolved, let user add it if really needed + return if (convertedType == initializerType) null else convertedType + } + override fun visitExpressionList(list: PsiExpressionList) { result = ExpressionList(converter.convertExpressions(list.getExpressions())) } diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt index 002007477df..ce69ce4eb05 100644 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt +++ b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt @@ -86,9 +86,17 @@ class ExpressionVisitor(private val converter: Converter, } override fun visitBinaryExpression(expression: PsiBinaryExpression) { - val lhs = converter.convertExpression(expression.getLOperand(), expression.getType()) - val rhs = converter.convertExpression(expression.getROperand(), expression.getType()) - if (expression.getOperationSign().getTokenType() == JavaTokenType.GTGTGT) { + val operandsExpectedType = when (expression.getOperationTokenType()) { + JavaTokenType.ANDAND, JavaTokenType.OROR -> PsiType.BOOLEAN + + JavaTokenType.PLUS, JavaTokenType.MINUS, JavaTokenType.ASTERISK, + JavaTokenType.DIV, JavaTokenType.PERC, JavaTokenType.LTLT, JavaTokenType.GTGT -> expression.getType() + + else -> null + } + val lhs = converter.convertExpression(expression.getLOperand(), operandsExpectedType) + val rhs = converter.convertExpression(expression.getROperand(), operandsExpectedType) + if (expression.getOperationTokenType() == JavaTokenType.GTGTGT) { result = MethodCallExpression.buildNotNull(lhs, "ushr", listOf(rhs)) } else { diff --git a/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java b/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java index e472a3a8820..58d30e4fa69 100644 --- a/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java +++ b/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java @@ -1852,11 +1852,46 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("j2k/tests/testData/ast/localVariable"), Pattern.compile("^(.+)\\.java$"), true); } + @TestMetadata("autoBangBang.java") + public void testAutoBangBang() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/autoBangBang.java"); + } + + @TestMetadata("conversion.java") + public void testConversion() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/conversion.java"); + } + + @TestMetadata("literalConversion.java") + public void testLiteralConversion() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/literalConversion.java"); + } + + @TestMetadata("nullIInitializer.java") + public void testNullIInitializer() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/nullIInitializer.java"); + } + @TestMetadata("object.java") public void testObject() throws Exception { doTest("j2k/tests/testData/ast/localVariable/object.java"); } + @TestMetadata("unboxing.java") + public void testUnboxing() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/unboxing.java"); + } + + @TestMetadata("valTypeDoNotMatch.java") + public void testValTypeDoNotMatch() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.java"); + } + + @TestMetadata("varTypeDoNotMatch.java") + public void testVarTypeDoNotMatch() throws Exception { + doTest("j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.java"); + } + } @TestMetadata("j2k/tests/testData/ast/methodCallExpression") diff --git a/j2k/tests/testData/ast/boxedType/Boxing.kt b/j2k/tests/testData/ast/boxedType/Boxing.kt index 3cbc28cefd1..8e6d73114ac 100644 --- a/j2k/tests/testData/ast/boxedType/Boxing.kt +++ b/j2k/tests/testData/ast/boxedType/Boxing.kt @@ -2,7 +2,7 @@ import java.util.ArrayList class Boxing() { fun test() { - val i = 0 + val i: Int? = 0 val n = 0.0.toFloat() i = 1 val j = i!! diff --git a/j2k/tests/testData/ast/localVariable/autoBangBang.java b/j2k/tests/testData/ast/localVariable/autoBangBang.java new file mode 100644 index 00000000000..d66521e26ad --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/autoBangBang.java @@ -0,0 +1,5 @@ +//method +void foo(Integer i) { + int i1 = i; + i1++; +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/autoBangBang.kt b/j2k/tests/testData/ast/localVariable/autoBangBang.kt new file mode 100644 index 00000000000..fd168ba8d8f --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/autoBangBang.kt @@ -0,0 +1,4 @@ +fun foo(i: Int) { + val i1 = i!! + i1++ +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/conversion.java b/j2k/tests/testData/ast/localVariable/conversion.java new file mode 100644 index 00000000000..5f4981bf7c6 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/conversion.java @@ -0,0 +1,5 @@ +//method +void foo(byte b) { + Integer i = b; + if (p) i = 10; +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/conversion.kt b/j2k/tests/testData/ast/localVariable/conversion.kt new file mode 100644 index 00000000000..f72d20e309d --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/conversion.kt @@ -0,0 +1,4 @@ +fun foo(b: Byte) { + val i = b.toInt() + if (p) i = 10 +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/literalConversion.java b/j2k/tests/testData/ast/localVariable/literalConversion.java new file mode 100644 index 00000000000..919852ec0d3 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/literalConversion.java @@ -0,0 +1,5 @@ +//method +void foo(p: Boolean) { + long i = 1; + if (p) i = 10; +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/literalConversion.kt b/j2k/tests/testData/ast/localVariable/literalConversion.kt new file mode 100644 index 00000000000..e99c21fc9d4 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/literalConversion.kt @@ -0,0 +1,4 @@ +fun foo() { + val i: Long = 1 + if (p) i = 10 +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/nullIInitializer.java b/j2k/tests/testData/ast/localVariable/nullIInitializer.java new file mode 100644 index 00000000000..6cab35d5ee4 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/nullIInitializer.java @@ -0,0 +1,2 @@ +//statement +String s = null; \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/nullIInitializer.kt b/j2k/tests/testData/ast/localVariable/nullIInitializer.kt new file mode 100644 index 00000000000..9dc9af1d165 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/nullIInitializer.kt @@ -0,0 +1 @@ +val s: String? = null \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/unboxing.java b/j2k/tests/testData/ast/localVariable/unboxing.java new file mode 100644 index 00000000000..4c555e648a4 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/unboxing.java @@ -0,0 +1,5 @@ +//method +void foo(p: Boolean) { + Integer i = 1; + if (p) i = null; +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/unboxing.kt b/j2k/tests/testData/ast/localVariable/unboxing.kt new file mode 100644 index 00000000000..6916bdb6417 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/unboxing.kt @@ -0,0 +1,4 @@ +fun foo() { + val i: Int? = 1 + if (p) i = null +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.java b/j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.java new file mode 100644 index 00000000000..16cd8ec51b7 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.java @@ -0,0 +1,4 @@ +//method +void foo(p: Boolean) { + Object o = ""; +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.kt b/j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.kt new file mode 100644 index 00000000000..50cdf727ad9 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/valTypeDoNotMatch.kt @@ -0,0 +1,3 @@ +fun foo() { + val o = "" +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.java b/j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.java new file mode 100644 index 00000000000..4a7fe0ef709 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.java @@ -0,0 +1,5 @@ +//method +void foo(p: Boolean) { + Object o = ""; + if (p) o = null; +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.kt b/j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.kt new file mode 100644 index 00000000000..c5183f88987 --- /dev/null +++ b/j2k/tests/testData/ast/localVariable/varTypeDoNotMatch.kt @@ -0,0 +1,4 @@ +fun foo() { + val o: Any? = "" + if (p) o = null +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/nullability/MethodResultComparedWithNull4.kt b/j2k/tests/testData/ast/nullability/MethodResultComparedWithNull4.kt index ec4eb61547b..7de4a8a0f0b 100644 --- a/j2k/tests/testData/ast/nullability/MethodResultComparedWithNull4.kt +++ b/j2k/tests/testData/ast/nullability/MethodResultComparedWithNull4.kt @@ -4,7 +4,7 @@ trait I { class C() { fun foo(i: I, b: Boolean) { - val result = i.getString() + val result: String? = i.getString() if (b) result = null if (result != null) { print(result) diff --git a/j2k/tests/testData/ast/prefixOperator/nullableIf.kt b/j2k/tests/testData/ast/prefixOperator/nullableIf.kt index d294f76c13d..484bcdc7b86 100644 --- a/j2k/tests/testData/ast/prefixOperator/nullableIf.kt +++ b/j2k/tests/testData/ast/prefixOperator/nullableIf.kt @@ -1,3 +1,3 @@ -val s = null +val s: String? = null if (!s!!.isEmpty()) { } \ No newline at end of file