From 63ebfe2c03108767ab311d7fae558a733831cb91 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Tue, 24 Jun 2014 19:48:31 +0400 Subject: [PATCH] Java to Kotlin converter: more correct logic for specifying field type --- j2k/src/org/jetbrains/jet/j2k/Converter.kt | 36 +++++++++++-------- .../org/jetbrains/jet/j2k/TypeConverter.kt | 36 +++++-------------- .../jetbrains/jet/j2k/ast/LocalVariable.kt | 3 +- .../jet/j2k/visitors/ElementVisitor.kt | 19 ++-------- .../JavaToKotlinConverterTestGenerated.java | 5 +++ .../jetbrainsNotNullChainExpr.java | 2 +- .../annotations/jetbrainsNotNullChainExpr.kt | 2 +- j2k/tests/testData/ast/field/conversion.java | 10 ++++++ j2k/tests/testData/ast/field/conversion.kt | 14 ++++++++ j2k/tests/testData/ast/issues/kt-820-field.kt | 2 +- .../forceLocalVariableImmutability.java | 1 + .../forceLocalVariableImmutability.kt | 1 + 12 files changed, 67 insertions(+), 64 deletions(-) create mode 100644 j2k/tests/testData/ast/field/conversion.java create mode 100644 j2k/tests/testData/ast/field/conversion.kt diff --git a/j2k/src/org/jetbrains/jet/j2k/Converter.kt b/j2k/src/org/jetbrains/jet/j2k/Converter.kt index 7d0af37c7ac..b96ed693e3f 100644 --- a/j2k/src/org/jetbrains/jet/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/Converter.kt @@ -251,8 +251,7 @@ public class Converter private(val project: Project, val settings: ConverterSett Modifiers.Empty, null, initializer, - true, - settings).assignNoPrototype() + true).assignNoPrototype() newStatements.add(0, DeclarationStatement(listOf(localVar)).assignNoPrototype()) constructor.block = Block(newStatements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() } @@ -286,30 +285,37 @@ public class Converter private(val project: Project, val settings: ConverterSett convertElement(field.getArgumentList())) } else { - val convertedType = typeConverter.convertVariableType(field) val isVal = isVal(field) + val typeToDeclare = variableTypeToDeclare(field, + settings.specifyFieldTypeByDefault || modifiers.isPublic || modifiers.isProtected, + isVal && modifiers.isPrivate) Field(name, annotations, modifiers, - convertedType, + typeToDeclare ?: typeConverter.convertVariableType(field), convertExpression(field.getInitializer(), field.getType()), isVal, - shouldDeclareType(field, isVal, convertedType), + typeToDeclare != null, field.hasWriteAccesses(field.getContainingClass())) } return converted.assignPrototype(field) } - private fun shouldDeclareType(field: PsiField, isVal: Boolean, convertedType: Type): Boolean { - if (settings.specifyFieldTypeByDefault) return true - val initializer = field.getInitializer() ?: return true - fun typeMatches() = convertedType == typeConverter.convertExpressionType(initializer) - return if (field.hasModifierProperty(PsiModifier.PRIVATE)) - !isVal && !typeMatches() - else if (field.hasModifierProperty(PsiModifier.PACKAGE_LOCAL)) - !typeMatches() - else - true + fun variableTypeToDeclare(variable: PsiVariable, specifyAlways: Boolean, canChangeType: Boolean): Type? { + fun convertType() = typeConverter.convertVariableType(variable) + + if (specifyAlways) return convertType() + + val initializer = variable.getInitializer() + if (initializer == null) return convertType() + if (initializer is PsiLiteralExpression && initializer.getType() == PsiType.NULL) return convertType() + + if (canChangeType) return null + + val convertedType = convertType() + var initializerType = 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 } private fun isVal(field: PsiField): Boolean { diff --git a/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt b/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt index 3c0a6b1f4fe..50029aa195b 100644 --- a/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/TypeConverter.kt @@ -92,7 +92,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver if (nullability == Nullability.Default) { val initializer = variable.getInitializer() if (initializer != null) { - val initializerNullability = initializer.nullability(false) + val initializerNullability = initializer.nullability() if (variable.isEffectivelyFinal()) { nullability = initializerNullability } @@ -133,7 +133,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver for (call in findMethodCalls(method, scope)) { val args = call.getArgumentList().getExpressions() if (args.size == parameters.size) { - if (args[parameterIndex].nullability(false) == Nullability.Nullable) { + if (args[parameterIndex].nullability() == Nullability.Nullable) { nullability = Nullability.Nullable break } @@ -172,7 +172,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver if (nullability == Nullability.Default) { method.getBody()?.accept(object: JavaRecursiveElementVisitor() { override fun visitReturnStatement(statement: PsiReturnStatement) { - if (statement.getReturnValue()?.nullability(false) == Nullability.Nullable) { + if (statement.getReturnValue()?.nullability() == Nullability.Nullable) { nullability = Nullability.Nullable } } @@ -195,10 +195,6 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver return nullability } - public fun convertExpressionType(expression: PsiExpression): Type { - return convertType(expression.getType(), expression.nullability(true)) - } - private fun searchScope(element: PsiElement): PsiElement? { return when(element) { is PsiParameter -> element.getDeclarationScope() @@ -209,38 +205,22 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver } } - private fun PsiExpression.nullability(useDeclarationsNullability: Boolean): Nullability { + private fun PsiExpression.nullability(): Nullability { return when (this) { is PsiLiteralExpression -> if (getType() != PsiType.NULL) Nullability.NotNull else Nullability.Nullable is PsiNewExpression -> Nullability.NotNull is PsiConditionalExpression -> { - val nullability1 = getThenExpression()?.nullability(useDeclarationsNullability) + val nullability1 = getThenExpression()?.nullability() if (nullability1 == Nullability.Nullable) return Nullability.Nullable - val nullability2 = getElseExpression()?.nullability(useDeclarationsNullability) + val nullability2 = getElseExpression()?.nullability() if (nullability2 == Nullability.Nullable) return Nullability.Nullable if (nullability1 == Nullability.NotNull && nullability2 == Nullability.NotNull) return Nullability.NotNull Nullability.Default } - is PsiParenthesizedExpression -> getExpression()?.nullability(useDeclarationsNullability) ?: Nullability.Default - - is PsiMethodCallExpression -> if (useDeclarationsNullability) { - val method = resolveMethod() - if (method != null) methodNullability(method) else Nullability.Default - } - else { - Nullability.Default - } - - is PsiReferenceExpression -> if (useDeclarationsNullability) { - val variable = resolve() as? PsiVariable - if (variable != null) variableNullability(variable) else Nullability.Default - } - else { - Nullability.Default - } + is PsiParenthesizedExpression -> getExpression()?.nullability() ?: Nullability.Default //TODO: some other cases @@ -252,7 +232,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver private fun isNullableFromUsage(usage: PsiExpression): Boolean { val parent = usage.getParent() ?: return false if (parent is PsiAssignmentExpression && parent.getOperationTokenType() == JavaTokenType.EQ && usage == parent.getLExpression()) { - return parent.getRExpression()?.nullability(false) == Nullability.Nullable + return parent.getRExpression()?.nullability() == Nullability.Nullable } else if (parent is PsiBinaryExpression) { val operationType = parent.getOperationTokenType() diff --git a/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt b/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt index a5e88774ffe..1568963dd2a 100644 --- a/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt +++ b/j2k/src/org/jetbrains/jet/j2k/ast/LocalVariable.kt @@ -26,8 +26,7 @@ class LocalVariable( private val modifiers: Modifiers, private val explicitType: Type?, private val initializer: Expression, - private val isVal: Boolean, - private val settings: ConverterSettings + private val isVal: Boolean ) : Element() { override fun generateCode(builder: CodeBuilder) { diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt index ae6da2c0d36..677bef64fc4 100644 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt +++ b/j2k/src/org/jetbrains/jet/j2k/visitors/ElementVisitor.kt @@ -27,26 +27,13 @@ class ElementVisitor(private val converter: Converter) : JavaElementVisitor() { protected set override fun visitLocalVariable(variable: PsiLocalVariable) { + val isVal = variable.hasModifierProperty(PsiModifier.FINAL) || !variable.hasWriteAccesses(variable.getContainingMethod()) result = LocalVariable(variable.declarationIdentifier(), converter.convertAnnotations(variable), converter.convertModifiers(variable), - explicitType(variable), + converter.variableTypeToDeclare(variable, converter.settings.specifyLocalVariableTypeByDefault, isVal), 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 + converter.settings.forceLocalVariableImmutability || isVal) } override fun visitExpressionList(list: PsiExpressionList) { 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 58d30e4fa69..03a81a6b95e 100644 --- a/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java +++ b/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java @@ -1074,6 +1074,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv doTest("j2k/tests/testData/ast/field/classChildExtendsBase.java"); } + @TestMetadata("conversion.java") + public void testConversion() throws Exception { + doTest("j2k/tests/testData/ast/field/conversion.java"); + } + @TestMetadata("internalField.java") public void testInternalField() throws Exception { doTest("j2k/tests/testData/ast/field/internalField.java"); diff --git a/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.java b/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.java index 588cea6e028..81ebf7b0368 100644 --- a/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.java +++ b/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.java @@ -11,7 +11,7 @@ class Foo { class Bar { @NotNull - Foo fooNotNull = Foo(); + Foo fooNotNull = new Foo(); Foo fooNullable = null; } diff --git a/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.kt b/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.kt index 8d6ab2860e1..bb3f3622c4f 100644 --- a/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.kt +++ b/j2k/tests/testData/ast/annotations/jetbrainsNotNullChainExpr.kt @@ -8,7 +8,7 @@ class Foo() { } class Bar() { - var fooNotNull: Foo = Foo() + var fooNotNull = Foo() var fooNullable: Foo? = null } diff --git a/j2k/tests/testData/ast/field/conversion.java b/j2k/tests/testData/ast/field/conversion.java new file mode 100644 index 00000000000..ca94e3434b5 --- /dev/null +++ b/j2k/tests/testData/ast/field/conversion.java @@ -0,0 +1,10 @@ +//file +class A { + private Integer i = getByte(); + + static byte getByte() { return 0; } + + void foo() { + i = 10; + } +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/field/conversion.kt b/j2k/tests/testData/ast/field/conversion.kt new file mode 100644 index 00000000000..e3eb44fc25a --- /dev/null +++ b/j2k/tests/testData/ast/field/conversion.kt @@ -0,0 +1,14 @@ +class A() { + private var i = getByte().toInt() + + fun foo() { + i = 10 + } + + class object { + + fun getByte(): Byte { + return 0 + } + } +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/issues/kt-820-field.kt b/j2k/tests/testData/ast/issues/kt-820-field.kt index 897f1a2ce91..b02d789a401 100644 --- a/j2k/tests/testData/ast/issues/kt-820-field.kt +++ b/j2k/tests/testData/ast/issues/kt-820-field.kt @@ -11,5 +11,5 @@ class One() { } class Test() { - var b: Byte = One.myContainer.myInt.toByte() + var b = One.myContainer.myInt.toByte() } \ No newline at end of file diff --git a/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.java b/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.java index 823fcf4b298..988ffb7d29f 100644 --- a/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.java +++ b/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.java @@ -2,6 +2,7 @@ // !forceLocalVariableImmutability: false public void foo() { int i = 1; + i++; String s = ""; s += "a"; } \ No newline at end of file diff --git a/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.kt b/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.kt index 7e3f8b9a73d..f7940a4b691 100644 --- a/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.kt +++ b/j2k/tests/testData/ast/settings/forceLocalVariableImmutability.kt @@ -1,6 +1,7 @@ // !forceLocalVariableImmutability: false public fun foo() { var i = 1 + i++ var s = "" s += "a" } \ No newline at end of file