From f368d9761aa6048c26f02dc63f59dd62e8b50a1e Mon Sep 17 00:00:00 2001 From: Ilya Kirillov Date: Mon, 25 Nov 2019 14:01:03 +0300 Subject: [PATCH] Fix `for` loop conversion when multiple initializers are present #KT-35074 fixed --- .../kotlin/nj2k/JavaToJKTreeBuilder.kt | 18 ++++++++++-------- .../kotlin/nj2k/conversions/ForConversion.kt | 18 ++++++++++-------- .../conversions/LabeledStatementConversion.kt | 14 ++++++-------- .../kotlin/nj2k/printing/JKCodeBuilder.kt | 6 +++++- .../jetbrains/kotlin/nj2k/tree/statements.kt | 8 ++++---- .../testData/newJ2k/for/emptyInitializers.java | 7 +++++++ nj2k/testData/newJ2k/for/emptyInitializers.kt | 9 +++++++++ .../newJ2k/for/multipleInitializers.java | 7 +++++++ .../newJ2k/for/multipleInitializers.kt | 11 +++++++++++ ...leInitializersWithvariableDeclarations.java | 6 ++++++ ...ipleInitializersWithvariableDeclarations.kt | 9 +++++++++ ...KotlinConverterSingleFileTestGenerated.java | 15 +++++++++++++++ 12 files changed, 99 insertions(+), 29 deletions(-) create mode 100644 nj2k/testData/newJ2k/for/emptyInitializers.java create mode 100644 nj2k/testData/newJ2k/for/emptyInitializers.kt create mode 100644 nj2k/testData/newJ2k/for/multipleInitializers.java create mode 100644 nj2k/testData/newJ2k/for/multipleInitializers.kt create mode 100644 nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.java create mode 100644 nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.kt diff --git a/nj2k/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt b/nj2k/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt index 4201d6d5a7c..89d4439b8fa 100644 --- a/nj2k/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt +++ b/nj2k/src/org/jetbrains/kotlin/nj2k/JavaToJKTreeBuilder.kt @@ -885,6 +885,14 @@ class JavaToJKTreeBuilder constructor( } } + fun PsiStatement?.asJKStatementsList() = when (this) { + null -> emptyList() + is PsiExpressionListStatement -> expressionList.expressions.map { expression -> + JKExpressionStatement(with(expressionTreeMapper) { expression.toJK() }) + } + else -> listOf(toJK()) + } + fun PsiStatement?.toJK(): JKStatement { return when (this) { null -> JKExpressionStatement(JKStubExpression()) @@ -909,15 +917,9 @@ class JavaToJKTreeBuilder constructor( is PsiForStatement -> JKJavaForLoopStatement( - initialization.toJK(), + initialization.asJKStatementsList(), with(expressionTreeMapper) { condition.toJK() }, - when (update) { - is PsiExpressionListStatement -> - (update as PsiExpressionListStatement).expressionList.expressions.map { - JKExpressionStatement(with(expressionTreeMapper) { it.toJK() }) - } - else -> listOf(update.toJK()) - }, + update.asJKStatementsList(), body.toJK() ) is PsiForeachStatement -> diff --git a/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/ForConversion.kt b/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/ForConversion.kt index 91d2511fa9c..184e7948901 100644 --- a/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/ForConversion.kt +++ b/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/ForConversion.kt @@ -40,11 +40,13 @@ class ForConversion(context: NewJ2kConverterContext) : RecursiveApplicableConver else JKLiteralExpression("true", JKLiteralExpression.LiteralType.BOOLEAN) val whileStatement = JKWhileStatement(condition, whileBody) - if (loopStatement.initializer is JKEmptyStatement) return whileStatement + if (loopStatement.initializers.isEmpty() + || loopStatement.initializers.singleOrNull() is JKEmptyStatement + ) return whileStatement val convertedFromForLoopSyntheticWhileStatement = JKKtConvertedFromForLoopSyntheticWhileStatement( - loopStatement::initializer.detached(), + loopStatement::initializers.detached(), whileStatement ) @@ -81,8 +83,7 @@ class ForConversion(context: NewJ2kConverterContext) : RecursiveApplicableConver val body = continueStatementConverter.applyToElement(loopStatement::body.detached()) if (body is JKBlockStatement) { - val initializer = loopStatement.initializer - val hasNameConflict = + val hasNameConflict = loopStatement.initializers.any { initializer -> initializer is JKDeclarationStatement && initializer.declaredStatements.any { loopVar -> loopVar is JKLocalVariable && body.statements.any { statement -> statement is JKDeclarationStatement && statement.declaredStatements.any { @@ -90,6 +91,7 @@ class ForConversion(context: NewJ2kConverterContext) : RecursiveApplicableConver } } } + } val statements = if (hasNameConflict) { @@ -106,8 +108,8 @@ class ForConversion(context: NewJ2kConverterContext) : RecursiveApplicableConver } private fun convertToForeach(loopStatement: JKJavaForLoopStatement): JKForInStatement? { - val loopVar = - (loopStatement.initializer as? JKDeclarationStatement)?.declaredStatements?.singleOrNull() as? JKLocalVariable ?: return null + val initializer = loopStatement.initializers.singleOrNull() ?: return null + val loopVar = (initializer as? JKDeclarationStatement)?.declaredStatements?.singleOrNull() as? JKLocalVariable ?: return null val loopVarPsi = loopVar.psi() ?: return null val condition = loopStatement.condition as? JKBinaryExpression ?: return null if (!loopVarPsi.hasWriteAccesses(referenceSearcher, loopStatement.body.psi()) @@ -276,7 +278,7 @@ class ForConversion(context: NewJ2kConverterContext) : RecursiveApplicableConver } private fun JKJavaForLoopStatement.hasNameConflict(): Boolean { - val names = initializer.declaredVariableNames() + val names = initializers.flatMap { it.declaredVariableNames() } if (names.isEmpty()) return false val factory = PsiElementFactory.SERVICE.getInstance(context.project) @@ -301,7 +303,7 @@ class ForConversion(context: NewJ2kConverterContext) : RecursiveApplicableConver when (this) { is JKDeclarationStatement -> declaredStatements.filterIsInstance().map { it.name.value } - is JKJavaForLoopStatement -> initializer.declaredVariableNames() + is JKJavaForLoopStatement -> initializers.flatMap { it.declaredVariableNames() } else -> emptyList() } diff --git a/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/LabeledStatementConversion.kt b/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/LabeledStatementConversion.kt index 453e71e4ce6..5b2ad7f64ef 100644 --- a/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/LabeledStatementConversion.kt +++ b/nj2k/src/org/jetbrains/kotlin/nj2k/conversions/LabeledStatementConversion.kt @@ -13,7 +13,7 @@ import org.jetbrains.kotlin.nj2k.tree.* import org.jetbrains.kotlin.utils.addToStdlib.safeAs -class LabeledStatementConversion(context : NewJ2kConverterContext) : RecursiveApplicableConversionBase(context) { +class LabeledStatementConversion(context: NewJ2kConverterContext) : RecursiveApplicableConversionBase(context) { override fun applyToElement(element: JKTreeElement): JKTreeElement { if (element !is JKExpressionStatement) return recurse(element) val labeledStatement = element.expression as? JKLabeledExpression ?: return recurse(element) @@ -26,13 +26,11 @@ class LabeledStatementConversion(context : NewJ2kConverterContext) : RecursiveAp return recurse( JKBlockStatementWithoutBrackets( - listOf( - convertedFromForLoopSyntheticWhileStatement::variableDeclaration.detached(), - JKLabeledExpression( - convertedFromForLoopSyntheticWhileStatement::whileStatement.detached(), - labeledStatement::labels.detached() - ).asStatement() - ) + convertedFromForLoopSyntheticWhileStatement::variableDeclarations.detached() + + JKLabeledExpression( + convertedFromForLoopSyntheticWhileStatement::whileStatement.detached(), + labeledStatement::labels.detached() + ).asStatement() ) ) } diff --git a/nj2k/src/org/jetbrains/kotlin/nj2k/printing/JKCodeBuilder.kt b/nj2k/src/org/jetbrains/kotlin/nj2k/printing/JKCodeBuilder.kt index c4ea17291f3..f843fca1307 100644 --- a/nj2k/src/org/jetbrains/kotlin/nj2k/printing/JKCodeBuilder.kt +++ b/nj2k/src/org/jetbrains/kotlin/nj2k/printing/JKCodeBuilder.kt @@ -548,7 +548,11 @@ internal class JKCodeBuilder(context: NewJ2kConverterContext) { override fun visitKtConvertedFromForLoopSyntheticWhileStatementRaw( ktConvertedFromForLoopSyntheticWhileStatement: JKKtConvertedFromForLoopSyntheticWhileStatement ) { - ktConvertedFromForLoopSyntheticWhileStatement.variableDeclaration.accept(this) + printer.renderList( + ktConvertedFromForLoopSyntheticWhileStatement.variableDeclarations, + { printer.println() }) { + it.accept(this) + } printer.println() ktConvertedFromForLoopSyntheticWhileStatement.whileStatement.accept(this) } diff --git a/nj2k/src/org/jetbrains/kotlin/nj2k/tree/statements.kt b/nj2k/src/org/jetbrains/kotlin/nj2k/tree/statements.kt index 31a275387f1..e9ac2caf137 100644 --- a/nj2k/src/org/jetbrains/kotlin/nj2k/tree/statements.kt +++ b/nj2k/src/org/jetbrains/kotlin/nj2k/tree/statements.kt @@ -83,10 +83,10 @@ class JKKtWhenStatement( } class JKKtConvertedFromForLoopSyntheticWhileStatement( - variableDeclaration: JKStatement, + variableDeclarations: List, whileStatement: JKWhileStatement ) : JKStatement() { - var variableDeclaration: JKStatement by child(variableDeclaration) + var variableDeclarations: List by children(variableDeclarations) var whileStatement: JKWhileStatement by child(whileStatement) override fun accept(visitor: JKVisitor) = visitor.visitKtConvertedFromForLoopSyntheticWhileStatement(this) } @@ -154,7 +154,7 @@ class JKJavaAssertStatement(condition: JKExpression, description: JKExpression) } class JKJavaForLoopStatement( - initializer: JKStatement, + initializers: List, condition: JKExpression, updaters: List, body: JKStatement @@ -162,7 +162,7 @@ class JKJavaForLoopStatement( override var body by child(body) var updaters by children(updaters) var condition by child(condition) - var initializer by child(initializer) + var initializers by children(initializers) override fun accept(visitor: JKVisitor) = visitor.visitJavaForLoopStatement(this) } diff --git a/nj2k/testData/newJ2k/for/emptyInitializers.java b/nj2k/testData/newJ2k/for/emptyInitializers.java new file mode 100644 index 00000000000..559923a4971 --- /dev/null +++ b/nj2k/testData/newJ2k/for/emptyInitializers.java @@ -0,0 +1,7 @@ +public class SomeClass { + void doSomeFor() { + int i = 0, u = 0; + for (; i < 42; i++) { + } + } +} \ No newline at end of file diff --git a/nj2k/testData/newJ2k/for/emptyInitializers.kt b/nj2k/testData/newJ2k/for/emptyInitializers.kt new file mode 100644 index 00000000000..41ca7500acf --- /dev/null +++ b/nj2k/testData/newJ2k/for/emptyInitializers.kt @@ -0,0 +1,9 @@ +class SomeClass { + fun doSomeFor() { + var i = 0 + val u = 0 + while (i < 42) { + i++ + } + } +} diff --git a/nj2k/testData/newJ2k/for/multipleInitializers.java b/nj2k/testData/newJ2k/for/multipleInitializers.java new file mode 100644 index 00000000000..f2d32dd9c31 --- /dev/null +++ b/nj2k/testData/newJ2k/for/multipleInitializers.java @@ -0,0 +1,7 @@ +public class SomeClass { + void doSomeFor() { + int i, u; + for (i = 0, u = 0; i < 42; i++) { + } + } +} \ No newline at end of file diff --git a/nj2k/testData/newJ2k/for/multipleInitializers.kt b/nj2k/testData/newJ2k/for/multipleInitializers.kt new file mode 100644 index 00000000000..1bcd1f0fdf9 --- /dev/null +++ b/nj2k/testData/newJ2k/for/multipleInitializers.kt @@ -0,0 +1,11 @@ +class SomeClass { + fun doSomeFor() { + var i: Int + val u: Int + i = 0 + u = 0 + while (i < 42) { + i++ + } + } +} diff --git a/nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.java b/nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.java new file mode 100644 index 00000000000..bd1c2a48b41 --- /dev/null +++ b/nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.java @@ -0,0 +1,6 @@ +public class SomeClass { + void doSomeFor() { + for (int i = 0, u = 0; i < 42; i++) { + } + } +} \ No newline at end of file diff --git a/nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.kt b/nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.kt new file mode 100644 index 00000000000..848b2d55aff --- /dev/null +++ b/nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.kt @@ -0,0 +1,9 @@ +class SomeClass { + fun doSomeFor() { + var i = 0 + val u = 0 + while (i < 42) { + i++ + } + } +} \ No newline at end of file diff --git a/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java b/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java index ac1eeeec7fb..8ea52259f75 100644 --- a/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java +++ b/nj2k/tests/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverterSingleFileTestGenerated.java @@ -2023,6 +2023,11 @@ public class NewJavaToKotlinConverterSingleFileTestGenerated extends AbstractNew runTest("nj2k/testData/newJ2k/for/downTo4.java"); } + @TestMetadata("emptyInitializers.java") + public void testEmptyInitializers() throws Exception { + runTest("nj2k/testData/newJ2k/for/emptyInitializers.java"); + } + @TestMetadata("falseArrayIndicesReversed.java") public void testFalseArrayIndicesReversed() throws Exception { runTest("nj2k/testData/newJ2k/for/falseArrayIndicesReversed.java"); @@ -2153,6 +2158,16 @@ public class NewJavaToKotlinConverterSingleFileTestGenerated extends AbstractNew runTest("nj2k/testData/newJ2k/for/infiniteFor.java"); } + @TestMetadata("multipleInitializers.java") + public void testMultipleInitializers() throws Exception { + runTest("nj2k/testData/newJ2k/for/multipleInitializers.java"); + } + + @TestMetadata("multipleInitializersWithvariableDeclarations.java") + public void testMultipleInitializersWithvariableDeclarations() throws Exception { + runTest("nj2k/testData/newJ2k/for/multipleInitializersWithvariableDeclarations.java"); + } + @TestMetadata("nameConflict1.java") public void testNameConflict1() throws Exception { runTest("nj2k/testData/newJ2k/for/nameConflict1.java");