From f698ca69d7473967a275c4eab18fc038e469679a Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Thu, 26 Jun 2014 18:51:41 +0400 Subject: [PATCH] Java to Kotlin converter: more correct detection of primary constructor + constructor chains are supported (not completely correct code generated yet) --- .../jetbrains/jet/j2k/ConstructorConverter.kt | 125 ++++++++++-------- .../org/jetbrains/jet/j2k/ConstructorUtils.kt | 53 ++++---- .../jet/j2k/visitors/ExpressionVisitor.kt | 2 +- .../JavaToKotlinConverterTestGenerated.java | 10 ++ .../testData/ast/constructors/chain.java | 23 ++++ j2k/tests/testData/ast/constructors/chain.kt | 26 ++++ .../testData/ast/constructors/noPrimary.java | 22 +++ .../testData/ast/constructors/noPrimary.kt | 29 ++++ 8 files changed, 206 insertions(+), 84 deletions(-) create mode 100644 j2k/tests/testData/ast/constructors/chain.java create mode 100644 j2k/tests/testData/ast/constructors/chain.kt create mode 100644 j2k/tests/testData/ast/constructors/noPrimary.java create mode 100644 j2k/tests/testData/ast/constructors/noPrimary.kt diff --git a/j2k/src/org/jetbrains/jet/j2k/ConstructorConverter.kt b/j2k/src/org/jetbrains/jet/j2k/ConstructorConverter.kt index 9af0ab11c9c..ed0ddc59d95 100644 --- a/j2k/src/org/jetbrains/jet/j2k/ConstructorConverter.kt +++ b/j2k/src/org/jetbrains/jet/j2k/ConstructorConverter.kt @@ -28,10 +28,10 @@ class ConstructorConverter(private val converter: Converter) { private val typeConverter = converter.typeConverter public fun convertConstructor(constructor: PsiMethod, - annotations: Annotations, - modifiers: Modifiers, - membersToRemove: MutableSet, - postProcessBody: (Block) -> Block): Member { + annotations: Annotations, + modifiers: Modifiers, + membersToRemove: MutableSet, + postProcessBody: (Block) -> Block): Member { if (constructor.isPrimaryConstructor()) { return convertPrimaryConstructor(constructor, annotations, modifiers, membersToRemove, postProcessBody) } @@ -164,64 +164,24 @@ class ConstructorConverter(private val converter: Converter) { public fun postProcessConstructors(classBody: ClassBody, psiClass: PsiClass): ClassBody { if (psiClass.getPrimaryConstructor() == null && psiClass.getConstructors().size > 1) { - return generateArtificialPrimaryConstructor(psiClass.declarationIdentifier(), classBody) + return generateArtificialPrimaryConstructor(psiClass.getName()!!, classBody) } else { - correctFactoryFunctions(classBody, psiClass.getName()!!) + replaceConstructorCallsInFactoryFunctions(classBody, psiClass.getName()!!) return classBody } } - private fun generateArtificialPrimaryConstructor(className: Identifier, classBody: ClassBody): ClassBody { + private fun generateArtificialPrimaryConstructor(className: String, classBody: ClassBody): ClassBody { assert(classBody.primaryConstructorSignature == null) val fieldsToInitialize = classBody.members.filterIsInstance(javaClass()).filter { it.isVal } - val initializers = HashMap() for (factoryFunction in classBody.factoryFunctions()) { - for (field in fieldsToInitialize) { - initializers.put(field, getDefaultInitializer(field)) - } - - val statements = ArrayList() - for (statement in factoryFunction.body!!.statements) { - var keepStatement = true - if (statement is AssignmentExpression) { - val assignee = statement.left - if (assignee is QualifiedExpression && (assignee.qualifier as? Identifier)?.name == tempValName) { - val name = (assignee.identifier as Identifier).name - for (field in fieldsToInitialize) { - if (name == field.identifier.name) { - initializers.put(field, statement.right) - keepStatement = false - } - - } - } - - } - - if (keepStatement) { - statements.add(statement) - } - } - - val arguments = fieldsToInitialize.map { initializers[it] ?: LiteralExpression("null").assignNoPrototype() } - val initializer = MethodCallExpression.buildNotNull(null, className.name, arguments).assignNoPrototype() - if (statements.isNotEmpty()) { - val localVar = LocalVariable(tempValIdentifier(), - Annotations.Empty, - Modifiers.Empty, - null, - initializer, - true).assignNoPrototype() - statements.add(0, DeclarationStatement(listOf(localVar)).assignNoPrototype()) - statements.add(ReturnStatement(tempValIdentifier()).assignNoPrototype()) - } - else { - statements.add(ReturnStatement(initializer).assignNoPrototype()) - } - - factoryFunction.body = Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() + val body = factoryFunction.body!! + // 2 cases: secondary constructor either calls another constructor or does not call any + val newStatements = replaceConstructorCallInFactoryFunction(body, className) ?: + insertCallToArtificialPrimary(body, className, fieldsToInitialize) + factoryFunction.body = Block(newStatements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() } val parameters = fieldsToInitialize.map { field -> @@ -236,15 +196,17 @@ class ConstructorConverter(private val converter: Converter) { return ClassBody(constructorSignature, updatedMembers, classBody.classObjectMembers, classBody.lBrace, classBody.rBrace) } - private fun correctFactoryFunctions(classBody: ClassBody, className: String) { + private fun replaceConstructorCallsInFactoryFunctions(classBody: ClassBody, className: String) { for (factoryFunction in classBody.factoryFunctions()) { val body = factoryFunction.body!! - val statements = correctFactoryFunctionStatements(body, className) - factoryFunction.body = Block(statements, body.lBrace, body.rBrace).assignPrototypesFrom(body) + val statements = replaceConstructorCallInFactoryFunction(body, className) + if (statements != null) { + factoryFunction.body = Block(statements, body.lBrace, body.rBrace).assignPrototypesFrom(body) + } } } - private fun correctFactoryFunctionStatements(body: Block, className: String): List { + private fun replaceConstructorCallInFactoryFunction(body: Block, className: String): List? { val statements = ArrayList(body.statements) // searching for other constructor call in form "this(...)" @@ -260,12 +222,59 @@ class ConstructorConverter(private val converter: Converter) { } val localVar = LocalVariable(tempValIdentifier(), Annotations.Empty, Modifiers.Empty, null, constructorCall, true).assignNoPrototype() statements[i] = DeclarationStatement(listOf(localVar)).assignNoPrototype() - break + statements.add(ReturnStatement(tempValIdentifier()).assignNoPrototype()) + return statements } } } - statements.add(ReturnStatement(tempValIdentifier()).assignNoPrototype()) + return null + } + + private fun insertCallToArtificialPrimary(body: Block, className: String, fieldsToInitialize: Collection): List { + val initializers = HashMap() + for (field in fieldsToInitialize) { + initializers.put(field, getDefaultInitializer(field)) + } + + val statements = ArrayList() + for (statement in body.statements) { + var keepStatement = true + if (statement is AssignmentExpression) { + val assignee = statement.left + if (assignee is QualifiedExpression && (assignee.qualifier as? Identifier)?.name == tempValName) { + val name = (assignee.identifier as Identifier).name + for (field in fieldsToInitialize) { + if (name == field.identifier.name) { + initializers.put(field, statement.right) + keepStatement = false + } + + } + } + + } + + if (keepStatement) { + statements.add(statement) + } + } + + val arguments = fieldsToInitialize.map { initializers[it] ?: LiteralExpression("null").assignNoPrototype() } + val initializer = MethodCallExpression.buildNotNull(null, className, arguments).assignNoPrototype() + if (statements.isNotEmpty()) { + val localVar = LocalVariable(tempValIdentifier(), + Annotations.Empty, + Modifiers.Empty, + null, + initializer, + true).assignNoPrototype() + statements.add(0, DeclarationStatement(listOf(localVar)).assignNoPrototype()) + statements.add(ReturnStatement(tempValIdentifier()).assignNoPrototype()) + } + else { + statements.add(ReturnStatement(initializer).assignNoPrototype()) + } return statements } diff --git a/j2k/src/org/jetbrains/jet/j2k/ConstructorUtils.kt b/j2k/src/org/jetbrains/jet/j2k/ConstructorUtils.kt index b35d369e1b3..d373f0f1b83 100644 --- a/j2k/src/org/jetbrains/jet/j2k/ConstructorUtils.kt +++ b/j2k/src/org/jetbrains/jet/j2k/ConstructorUtils.kt @@ -17,7 +17,7 @@ package org.jetbrains.jet.j2k import com.intellij.psi.* -import java.util.LinkedHashSet +import java.util.HashMap fun PsiMethod.isPrimaryConstructor(): Boolean { if (!isConstructor()) return false @@ -28,33 +28,39 @@ fun PsiMethod.isPrimaryConstructor(): Boolean { fun PsiClass.getPrimaryConstructor(): PsiMethod? { val constructors = getConstructors() - return when (constructors.size) { - 0 -> null + when (constructors.size) { + 0 -> return null - 1 -> constructors.single() + 1 -> return constructors.single() else -> { - // if there is more than one constructor then choose one invoked by all others - //TODO: logic is incorrect - there can be a constructor which does not call any other - class Visitor() : JavaRecursiveElementVisitor() { - //TODO: skip all non-constructor members (optimization) - private val invokedConstructors = LinkedHashSet() - - override fun visitReferenceExpression(expression: PsiReferenceExpression) { - expression.getReferences() - .filter { it.getCanonicalText() == "this" } - .map { it.resolve() } - .filterIsInstance(javaClass()) - .filterTo(invokedConstructors) { it.isConstructor() } + val toTargetConstructorMap = HashMap() + for (constructor in constructors) { + val firstStatement = constructor.getBody()?.getStatements()?.firstOrNull() + val refExpr = ((firstStatement as? PsiExpressionStatement) + ?.getExpression() as? PsiMethodCallExpression) + ?.getMethodExpression() + if (refExpr != null && refExpr.getCanonicalText() == "this") { + val target = refExpr.resolve() as? PsiMethod + if (target != null && target.isConstructor()) { + val finalTarget = toTargetConstructorMap[target] ?: target!!/*TODO: see KT-5335*/ + toTargetConstructorMap[constructor] = finalTarget + for (entry in toTargetConstructorMap.entrySet()) { + if (entry.getValue() == constructor) { + entry.setValue(finalTarget) + } + } + } } - - val primaryConstructor: PsiMethod? - get() = if (invokedConstructors.size == 1) invokedConstructors.single() else null } - val visitor = Visitor() - accept(visitor) - visitor.primaryConstructor + val candidates = constructors.filter { it !in toTargetConstructorMap } + if (candidates.size != 1) return null // there should be only one constructor which does not call other constructor + val candidate = candidates.single() + return if (toTargetConstructorMap.values().all { it == candidate } /* all other constructors call our candidate (directly or indirectly)*/) + candidate + else + null } } } @@ -82,7 +88,4 @@ fun PsiMethodCallExpression.isSuperConstructorCall(): Boolean { return false } -fun PsiReferenceExpression.isThisConstructorCall(): Boolean - = getReferences().filter { it.getCanonicalText() == "this" }.map { it.resolve() }.any { it is PsiMethod && it.isConstructor() } - fun PsiElement.isConstructor(): Boolean = this is PsiMethod && this.isConstructor() diff --git a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt index e7d607abaec..5d78057af27 100644 --- a/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt +++ b/j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt @@ -267,7 +267,7 @@ open class ExpressionVisitor(private val converter: Converter, var arguments = expression.getArgumentList()?.getExpressions() ?: array() val constructor = expression.resolveMethod() - if (constructor != null && !constructor.isPrimaryConstructor() && converter.conversionScope.contains(constructor)) { + if (constructor != null && converter.conversionScope.contains(constructor) && !constructor.isPrimaryConstructor()) { //TODO: handle anonymous class! // non-primary constructor converted to factory method in class object val reference = expression.getClassReference() 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 7952ba0dfda..eb1f76672b6 100644 --- a/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java +++ b/j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java @@ -811,6 +811,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("j2k/tests/testData/ast/constructors"), Pattern.compile("^(.+)\\.java$"), true); } + @TestMetadata("chain.java") + public void testChain() throws Exception { + doTest("j2k/tests/testData/ast/constructors/chain.java"); + } + @TestMetadata("customerBuilder.java") public void testCustomerBuilder() throws Exception { doTest("j2k/tests/testData/ast/constructors/customerBuilder.java"); @@ -866,6 +871,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv doTest("j2k/tests/testData/ast/constructors/methodCallInFactoryFun.java"); } + @TestMetadata("noPrimary.java") + public void testNoPrimary() throws Exception { + doTest("j2k/tests/testData/ast/constructors/noPrimary.java"); + } + @TestMetadata("parameterModification.java") public void testParameterModification() throws Exception { doTest("j2k/tests/testData/ast/constructors/parameterModification.java"); diff --git a/j2k/tests/testData/ast/constructors/chain.java b/j2k/tests/testData/ast/constructors/chain.java new file mode 100644 index 00000000000..26c7e2920f2 --- /dev/null +++ b/j2k/tests/testData/ast/constructors/chain.java @@ -0,0 +1,23 @@ +//file +class C { + C(int arg1, int arg2, int arg3) { + } + + C(int arg1, int arg2) { + this(arg1, arg2, 0); + System.out.println(); + } + + C(int arg1) { + this(arg1, 0); + System.out.println(); + } +} + +public class User { + public static void main() { + C c1 = new C(1, 2, 3); + C c2 = new C(5, 6); + C c3 = new C(7); + } +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/constructors/chain.kt b/j2k/tests/testData/ast/constructors/chain.kt new file mode 100644 index 00000000000..73db6c12498 --- /dev/null +++ b/j2k/tests/testData/ast/constructors/chain.kt @@ -0,0 +1,26 @@ +class C(arg1: Int, arg2: Int, arg3: Int) { + class object { + + fun create(arg1: Int, arg2: Int): C { + val __ = C(arg1, arg2, 0) + System.out.println() + return __ + } + + fun create(arg1: Int): C { + val __ = C(arg1, 0) + System.out.println() + return __ + } + } +} + +public class User { + class object { + public fun main() { + val c1 = C(1, 2, 3) + val c2 = C.create(5, 6) + val c3 = C.create(7) + } + } +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/constructors/noPrimary.java b/j2k/tests/testData/ast/constructors/noPrimary.java new file mode 100644 index 00000000000..5ec7ea277f3 --- /dev/null +++ b/j2k/tests/testData/ast/constructors/noPrimary.java @@ -0,0 +1,22 @@ +//file +class C { + C(int arg1, int arg2, int arg3) { + } + + C(int arg1, int arg2) { + this(arg1, arg2, 0); + System.out.println(); + } + + C(int arg) { + System.out.println(arg); + } +} + +public class User { + public static void main() { + C c1 = new C(1, 2, 3); + C c2 = new C(5, 6); + C c3 = new C(7); + } +} \ No newline at end of file diff --git a/j2k/tests/testData/ast/constructors/noPrimary.kt b/j2k/tests/testData/ast/constructors/noPrimary.kt new file mode 100644 index 00000000000..d597d5cbd43 --- /dev/null +++ b/j2k/tests/testData/ast/constructors/noPrimary.kt @@ -0,0 +1,29 @@ +class C private() { + class object { + fun create(arg1: Int, arg2: Int, arg3: Int): C { + return C() + } + + fun create(arg1: Int, arg2: Int): C { + val __ = C(arg1, arg2, 0) + System.out.println() + return __ + } + + fun create(arg: Int): C { + val __ = C() + System.out.println(arg) + return __ + } + } +} + +public class User { + class object { + public fun main() { + val c1 = C.create(1, 2, 3) + val c2 = C.create(5, 6) + val c3 = C.create(7) + } + } +} \ No newline at end of file