From e31648a4b6ee457ec970d733f02b4c7521a29ce7 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Wed, 25 Mar 2015 22:20:29 +0300 Subject: [PATCH] KT-6476 Kotlin J2K converter fails with for-loops with continues #KT-6476 Fixed --- .../org/jetbrains/kotlin/j2k/CodeConverter.kt | 3 + .../org/jetbrains/kotlin/j2k/ForConverter.kt | 121 ++++++++++-------- .../kotlin/j2k/StatementConverter.kt | 23 +++- .../jetbrains/kotlin/j2k/ast/Statements.kt | 2 +- .../fileOrElement/for/withContinue1.java | 11 ++ .../fileOrElement/for/withContinue1.kt | 20 +++ .../fileOrElement/for/withContinue2.java | 11 ++ .../fileOrElement/for/withContinue2.kt | 23 ++++ .../fileOrElement/for/withContinue3.java | 8 ++ .../fileOrElement/for/withContinue3.kt | 19 +++ .../for/withContinueAndLabels.java | 15 +++ .../for/withContinueAndLabels.kt | 34 +++++ ...otlinConverterForWebDemoTestGenerated.java | 24 ++++ ...otlinConverterSingleFileTestGenerated.java | 24 ++++ 14 files changed, 280 insertions(+), 58 deletions(-) create mode 100644 j2k/testData/fileOrElement/for/withContinue1.java create mode 100644 j2k/testData/fileOrElement/for/withContinue1.kt create mode 100644 j2k/testData/fileOrElement/for/withContinue2.java create mode 100644 j2k/testData/fileOrElement/for/withContinue2.kt create mode 100644 j2k/testData/fileOrElement/for/withContinue3.java create mode 100644 j2k/testData/fileOrElement/for/withContinue3.kt create mode 100644 j2k/testData/fileOrElement/for/withContinueAndLabels.java create mode 100644 j2k/testData/fileOrElement/for/withContinueAndLabels.kt diff --git a/j2k/src/org/jetbrains/kotlin/j2k/CodeConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/CodeConverter.kt index 58774588fd0..2162803fbbb 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/CodeConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/CodeConverter.kt @@ -63,6 +63,9 @@ class CodeConverter( public fun withSpecialExpressionConverter(specialConverter: SpecialExpressionConverter): CodeConverter = CodeConverter(converter, expressionConverter.withSpecialConverter(specialConverter), statementConverter, methodReturnType) + public fun withSpecialStatementConverter(specialConverter: SpecialStatementConverter): CodeConverter + = CodeConverter(converter, expressionConverter, statementConverter.withSpecialConverter(specialConverter), methodReturnType) + public fun withMethodReturnType(methodReturnType: PsiType?): CodeConverter = CodeConverter(converter, expressionConverter, statementConverter, methodReturnType) diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ForConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/ForConverter.kt index 5b39a4660b1..5b95b266c04 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ForConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ForConverter.kt @@ -16,44 +16,9 @@ package org.jetbrains.kotlin.j2k -import com.intellij.psi.PsiForStatement -import org.jetbrains.kotlin.j2k.ast.ForeachStatement -import com.intellij.psi.PsiDeclarationStatement -import com.intellij.psi.PsiLocalVariable -import com.intellij.psi.PsiBinaryExpression -import com.intellij.psi.JavaTokenType -import com.intellij.psi.PsiReferenceExpression -import com.intellij.psi.PsiExpressionStatement -import org.jetbrains.kotlin.j2k.ast.PrimitiveType -import org.jetbrains.kotlin.j2k.ast.Identifier -import org.jetbrains.kotlin.j2k.ast.assignNoPrototype -import org.jetbrains.kotlin.j2k.ast.declarationIdentifier -import com.intellij.psi.PsiElement -import com.intellij.psi.PsiVariable -import com.intellij.psi.PsiPostfixExpression -import com.intellij.psi.PsiPrefixExpression -import com.intellij.psi.PsiExpression +import com.intellij.psi.* import com.intellij.psi.tree.IElementType -import org.jetbrains.kotlin.j2k.ast.Expression -import com.intellij.psi.PsiLiteralExpression -import com.intellij.psi.PsiMethodCallExpression -import com.intellij.psi.PsiElementFactory -import com.intellij.psi.CommonClassNames -import org.jetbrains.kotlin.j2k.ast.QualifiedExpression -import com.intellij.psi.PsiArrayType -import org.jetbrains.kotlin.j2k.ast.BinaryExpression -import org.jetbrains.kotlin.j2k.ast.LiteralExpression -import org.jetbrains.kotlin.j2k.ast.RangeExpression -import com.intellij.psi.PsiBlockStatement -import com.intellij.psi.PsiNamedElement -import org.jetbrains.kotlin.j2k.ast.Block -import org.jetbrains.kotlin.j2k.ast.LBrace -import org.jetbrains.kotlin.j2k.ast.RBrace -import org.jetbrains.kotlin.j2k.ast.assignPrototypesFrom -import org.jetbrains.kotlin.j2k.ast.WhileStatement -import org.jetbrains.kotlin.j2k.ast.MethodCallExpression -import org.jetbrains.kotlin.j2k.ast.LambdaExpression -import org.jetbrains.kotlin.j2k.ast.Statement +import org.jetbrains.kotlin.j2k.ast.* class ForConverter( private val statement: PsiForStatement, @@ -83,28 +48,52 @@ class ForConverter( val whileBody = if (updateConverted.isEmpty) { codeConverter.convertStatementOrBlock(body) } - else if (body is PsiBlockStatement) { - val nameConflict = initialization is PsiDeclarationStatement && initialization.getDeclaredElements().any { loopVar -> - loopVar is PsiNamedElement && body.getCodeBlock().getStatements().any { statement -> - statement is PsiDeclarationStatement && statement.getDeclaredElements().any { - it is PsiNamedElement && it.getName() == loopVar.getName() + else { + // we should process all continue-statements because we need to add update statement(s) before them + val codeConverterToUse = codeConverter.withSpecialStatementConverter(object : SpecialStatementConverter { + override fun convertStatement(statement: PsiStatement, codeConverter: CodeConverter): Statement? { + if (statement !is PsiContinueStatement) return null + if (statement.findContinuedStatement()?.toContinuedLoop() != this@ForConverter.statement) return null + + val continueConverted = this@ForConverter.codeConverter.convertStatement(statement) + val statements = listOf(updateConverted, continueConverted) + if (statement.getParent() is PsiCodeBlock) { + // generate fictive statement which will generate multiple statements + return object : Statement() { + override fun generateCode(builder: CodeBuilder) { + builder.append(statements, "\n") + } + } + } + else { + return Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()) } } - } + }) - if (nameConflict) { - val statements = listOf(codeConverter.convertStatement(body), updateConverted) - Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype(), true).assignNoPrototype() + if (body is PsiBlockStatement) { + val nameConflict = initialization is PsiDeclarationStatement && initialization.getDeclaredElements().any { loopVar -> + loopVar is PsiNamedElement && body.getCodeBlock().getStatements().any { statement -> + statement is PsiDeclarationStatement && statement.getDeclaredElements().any { + it is PsiNamedElement && it.getName() == loopVar.getName() + } + } + } + + if (nameConflict) { + val statements = listOf(codeConverterToUse.convertStatement(body), updateConverted) + Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype(), true).assignNoPrototype() + } + else { + val block = codeConverterToUse.convertBlock(body.getCodeBlock(), true) + Block(block.statements + listOf(updateConverted), block.lBrace, block.rBrace, true).assignPrototypesFrom(block) + } } else { - val block = codeConverter.convertBlock(body.getCodeBlock(), true) - Block(block.statements + listOf(updateConverted), block.lBrace, block.rBrace, true).assignPrototypesFrom(block) + val statements = listOf(codeConverterToUse.convertStatement(body), updateConverted) + Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype(), true).assignNoPrototype() } } - else { - val statements = listOf(codeConverter.convertStatement(body), updateConverted) - Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype(), true).assignNoPrototype() - } val whileStatement = WhileStatement( if (condition != null) codeConverter.convertExpression(condition) else LiteralExpression("true").assignNoPrototype(), @@ -112,9 +101,24 @@ class ForConverter( statement.isInSingleLine()).assignNoPrototype() if (initializationConverted.isEmpty) return whileStatement - val statements = listOf(initializationConverted, whileStatement) - val block = Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() - return MethodCallExpression.build(null, "run", listOf(), listOf(), false, LambdaExpression(null, block)) + //TODO: we could omit "run { ... }" when it won't cause any name conflicts + return RunBlockWithLoopStatement(initializationConverted, whileStatement) + } + + public class RunBlockWithLoopStatement( + public val initialization: Statement, + public val loop: Statement + ) : Statement() { + + private val methodCall = run { + val statements = listOf(initialization, loop) + val block = Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype() + MethodCallExpression.build(null, "run", listOf(), listOf(), false, LambdaExpression(null, block)) + } + + override fun generateCode(builder: CodeBuilder) { + methodCall.generateCode(builder) + } } private fun convertToForeach(): ForeachStatement? { @@ -194,4 +198,11 @@ class ForConverter( return RangeExpression(codeConverter.convertExpression(start), endExpression) } + private fun PsiStatement.toContinuedLoop(): PsiLoopStatement? { + return when (this) { + is PsiLoopStatement -> this + is PsiLabeledStatement -> this.getStatement()?.toContinuedLoop() + else -> null + } + } } diff --git a/j2k/src/org/jetbrains/kotlin/j2k/StatementConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/StatementConverter.kt index 52acd24ebec..80a4013baba 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/StatementConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/StatementConverter.kt @@ -24,6 +24,17 @@ trait StatementConverter { fun convertStatement(statement: PsiStatement, codeConverter: CodeConverter): Statement } +trait SpecialStatementConverter { + fun convertStatement(statement: PsiStatement, codeConverter: CodeConverter): Statement? +} + +fun StatementConverter.withSpecialConverter(specialConverter: SpecialStatementConverter): StatementConverter { + return object: StatementConverter { + override fun convertStatement(statement: PsiStatement, codeConverter: CodeConverter): Statement + = specialConverter.convertStatement(statement, codeConverter) ?: this@withSpecialConverter.convertStatement(statement, codeConverter) + } +} + class DefaultStatementConverter : JavaElementVisitor(), StatementConverter { private var _codeConverter: CodeConverter? = null private var result: Statement = Statement.Empty @@ -133,8 +144,16 @@ class DefaultStatementConverter : JavaElementVisitor(), StatementConverter { } override fun visitLabeledStatement(statement: PsiLabeledStatement) { - result = LabelStatement(converter.convertIdentifier(statement.getLabelIdentifier()), - codeConverter.convertStatement(statement.getStatement())) + //TODO: multiple labels + val statementConverted = codeConverter.convertStatement(statement.getStatement()) + val identifier = converter.convertIdentifier(statement.getLabelIdentifier()) + if (statementConverted is ForConverter.RunBlockWithLoopStatement) { // special case - if our loop gets converted to run { ... } we should move label inside + val labeledLoop = LabeledStatement(identifier, statementConverted.loop).assignPrototype(statement) + result = ForConverter.RunBlockWithLoopStatement(statementConverted.initialization, labeledLoop) + } + else { + result = LabeledStatement(identifier, statementConverted) + } } override fun visitSwitchLabelStatement(statement: PsiSwitchLabelStatement) { diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ast/Statements.kt b/j2k/src/org/jetbrains/kotlin/j2k/ast/Statements.kt index d52dd5b7036..f21af3925ca 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ast/Statements.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ast/Statements.kt @@ -37,7 +37,7 @@ class ExpressionListStatement(val expressions: List) : Expression() } } -class LabelStatement(val name: Identifier, val statement: Element) : Statement() { +class LabeledStatement(val name: Identifier, val statement: Element) : Statement() { override fun generateCode(builder: CodeBuilder) { builder append "@" append name append " " append statement } diff --git a/j2k/testData/fileOrElement/for/withContinue1.java b/j2k/testData/fileOrElement/for/withContinue1.java new file mode 100644 index 00000000000..32c5812ae1b --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinue1.java @@ -0,0 +1,11 @@ +public class TestClass { + public static void main(String[] args) { + for (int i = 0; i < 10; ++i) { + if (i == 4 || i == 8) { + i++; + continue; + } + System.err.println(i); + } + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/for/withContinue1.kt b/j2k/testData/fileOrElement/for/withContinue1.kt new file mode 100644 index 00000000000..51488ee59b2 --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinue1.kt @@ -0,0 +1,20 @@ +public class TestClass { + companion object { + public fun main(args: Array) { + run { + var i = 0 + while (i < 10) { + if (i == 4 || i == 8) { + i++ + ++i + continue + } + System.err.println(i) + ++i + } + } + } + } +} + +fun main(args: Array) = TestClass.main(args) diff --git a/j2k/testData/fileOrElement/for/withContinue2.java b/j2k/testData/fileOrElement/for/withContinue2.java new file mode 100644 index 00000000000..0661559b2b3 --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinue2.java @@ -0,0 +1,11 @@ +public class TestClass { + public static void main(String[] args) { + for (int i = 0, j = 1; i < 10; ++i, j *= 2) { + if (i == 4 || i == 8) { + i++; + continue; + } + System.err.println(j); + } + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/for/withContinue2.kt b/j2k/testData/fileOrElement/for/withContinue2.kt new file mode 100644 index 00000000000..43f07c7e810 --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinue2.kt @@ -0,0 +1,23 @@ +public class TestClass { + companion object { + public fun main(args: Array) { + run { + var i = 0 + var j = 1 + while (i < 10) { + if (i == 4 || i == 8) { + i++ + ++i + j *= 2 + continue + } + System.err.println(j) + ++i + j *= 2 + } + } + } + } +} + +fun main(args: Array) = TestClass.main(args) diff --git a/j2k/testData/fileOrElement/for/withContinue3.java b/j2k/testData/fileOrElement/for/withContinue3.java new file mode 100644 index 00000000000..7abd2efa576 --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinue3.java @@ -0,0 +1,8 @@ +public class TestClass { + public static void main(String[] args) { + for (int i = 1; i < 1000; i *= 2) { + if (i == 4 || i == 8) continue; + System.err.println(i); + } + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/for/withContinue3.kt b/j2k/testData/fileOrElement/for/withContinue3.kt new file mode 100644 index 00000000000..28a9f3a1fcd --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinue3.kt @@ -0,0 +1,19 @@ +public class TestClass { + companion object { + public fun main(args: Array) { + run { + var i = 1 + while (i < 1000) { + if (i == 4 || i == 8) { + i *= 2 + continue + } + System.err.println(i) + i *= 2 + } + } + } + } +} + +fun main(args: Array) = TestClass.main(args) diff --git a/j2k/testData/fileOrElement/for/withContinueAndLabels.java b/j2k/testData/fileOrElement/for/withContinueAndLabels.java new file mode 100644 index 00000000000..625c8012ed5 --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinueAndLabels.java @@ -0,0 +1,15 @@ +public class TestClass { + public static void main(String[] args) { + OuterLoop1: + OuterLoop2: + for (int i = 1; i < 1000; i *= 2) { + InnerLoop: + for (int j = 1; j < 100; j *= 3) { + if (j == 3) continue InnerLoop; + if (i == j) continue OuterLoop1; + System.err.println(j); + if (j == 9) continue; + } + } + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/for/withContinueAndLabels.kt b/j2k/testData/fileOrElement/for/withContinueAndLabels.kt new file mode 100644 index 00000000000..5aa34412c08 --- /dev/null +++ b/j2k/testData/fileOrElement/for/withContinueAndLabels.kt @@ -0,0 +1,34 @@ +// ERROR: The label '@OuterLoop1' does not denote a loop +public class TestClass { + companion object { + public fun main(args: Array) { + run { + var i = 1 + @OuterLoop1 @OuterLoop2 while (i < 1000) { + run { + var j = 1 + @InnerLoop while (j < 100) { + if (j == 3) { + j *= 3 + continue@InnerLoop + } + if (i == j) { + i *= 2 + continue@OuterLoop1 + } + System.err.println(j) + if (j == 9) { + j *= 3 + continue + } + j *= 3 + } + } + i *= 2 + } + } + } + } +} + +fun main(args: Array) = TestClass.main(args) diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java index 40866525c98..95d9ba2f2b0 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java @@ -1935,6 +1935,30 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/infiniteFor.java"); doTest(fileName); } + + @TestMetadata("withContinue1.java") + public void testWithContinue1() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue1.java"); + doTest(fileName); + } + + @TestMetadata("withContinue2.java") + public void testWithContinue2() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue2.java"); + doTest(fileName); + } + + @TestMetadata("withContinue3.java") + public void testWithContinue3() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue3.java"); + doTest(fileName); + } + + @TestMetadata("withContinueAndLabels.java") + public void testWithContinueAndLabels() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinueAndLabels.java"); + doTest(fileName); + } } @TestMetadata("j2k/testData/fileOrElement/foreachStatement") diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java index 2ef7638e339..e9afe4d670a 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java @@ -1935,6 +1935,30 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/infiniteFor.java"); doTest(fileName); } + + @TestMetadata("withContinue1.java") + public void testWithContinue1() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue1.java"); + doTest(fileName); + } + + @TestMetadata("withContinue2.java") + public void testWithContinue2() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue2.java"); + doTest(fileName); + } + + @TestMetadata("withContinue3.java") + public void testWithContinue3() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue3.java"); + doTest(fileName); + } + + @TestMetadata("withContinueAndLabels.java") + public void testWithContinueAndLabels() throws Exception { + String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinueAndLabels.java"); + doTest(fileName); + } } @TestMetadata("j2k/testData/fileOrElement/foreachStatement")