KT-6798 J2K: Needless use of run {} function after converting for-loop

#KT-6798 Fixed

j2k/src/org/jetbrains/kotlin/j2k/ForConverter.kt

@@ -18,7 +18,10 @@ package org.jetbrains.kotlin.j2k
 
 import com.intellij.psi.*
 import com.intellij.psi.tree.IElementType
+import com.intellij.util.IncorrectOperationException
 import org.jetbrains.kotlin.j2k.ast.*
+import org.jetbrains.kotlin.psi.psiUtil.parents
+import org.jetbrains.kotlin.psi.psiUtil.siblings
 
 class ForConverter(
         private val statement: PsiForStatement,
@@ -101,23 +104,44 @@ class ForConverter(
                 statement.isInSingleLine()).assignNoPrototype()
         if (initializationConverted.isEmpty) return whileStatement
 
-        //TODO: we could omit "run { ... }" when it won't cause any name conflicts
-        return RunBlockWithLoopStatement(initializationConverted, whileStatement)
+        val kind = if (statement.parents(withItself = false).filter { it !is PsiLabeledStatement }.first() !is PsiCodeBlock) {
+            WhileWithInitializationPseudoStatement.Kind.WITH_BLOCK
+        }
+        else if (hasNameConflict())
+            WhileWithInitializationPseudoStatement.Kind.WITH_RUN_BLOCK
+        else
+            WhileWithInitializationPseudoStatement.Kind.SIMPLE
+        return WhileWithInitializationPseudoStatement(initializationConverted, whileStatement, kind)
     }
 
-    public class RunBlockWithLoopStatement(
+    public class WhileWithInitializationPseudoStatement(
             public val initialization: Statement,
-            public val loop: Statement
+            public val loop: Statement,
+            public val kind: WhileWithInitializationPseudoStatement.Kind
     ) : 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))
+        public enum class Kind {
+            SIMPLE
+            WITH_BLOCK
+            WITH_RUN_BLOCK
         }
 
+        private val statements = listOf(initialization, loop)
+
         override fun generateCode(builder: CodeBuilder) {
-            methodCall.generateCode(builder)
+            if (kind == Kind.SIMPLE) {
+                builder.append(statements, "\n")
+            }
+            else {
+                val block = Block(statements, LBrace().assignNoPrototype(), RBrace().assignNoPrototype()).assignNoPrototype()
+                if (kind == Kind.WITH_BLOCK) {
+                    block.generateCode(builder)
+                }
+                else {
+                    val call = MethodCallExpression.build(null, "run", listOf(), listOf(), false, LambdaExpression(null, block))
+                    call.generateCode(builder)
+                }
+            }
         }
     }
 
@@ -260,4 +284,42 @@ class ForConverter(
             else -> null
         }
     }
+
+    private fun hasNameConflict(): Boolean {
+        val names = statement.getInitialization()?.declaredVariableNames() ?: return false
+        if (names.isEmpty()) return false
+
+        val factory = PsiElementFactory.SERVICE.getInstance(project)
+        for (name in names) {
+            val refExpr = try {
+                factory.createExpressionFromText(name, statement) as? PsiReferenceExpression ?: return true
+            }
+            catch(e: IncorrectOperationException) {
+                return true
+            }
+            if (refExpr.resolve() != null) return true
+        }
+
+        var hasConflict = false
+        for (laterStatement in statement.siblings(forward = true, withItself = false)) {
+            laterStatement.accept(object: JavaRecursiveElementVisitor() {
+                override fun visitDeclarationStatement(statement: PsiDeclarationStatement) {
+                    super.visitDeclarationStatement(statement)
+
+                    if (statement.declaredVariableNames().any { it in names }) {
+                        hasConflict = true
+                    }
+                }
+            })
+        }
+
+        return hasConflict
+    }
+
+    private fun PsiStatement.declaredVariableNames(): Collection<String> {
+        val declarationStatement = this as? PsiDeclarationStatement ?: return listOf()
+        return declarationStatement.getDeclaredElements()
+                .filterIsInstance<PsiVariable>()
+                .map { it.getName() }
+    }
 }

j2k/src/org/jetbrains/kotlin/j2k/StatementConverter.kt

@@ -144,12 +144,11 @@ class DefaultStatementConverter : JavaElementVisitor(), StatementConverter {
     }
 
     override fun visitLabeledStatement(statement: PsiLabeledStatement) {
-        //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
+        if (statementConverted is ForConverter.WhileWithInitializationPseudoStatement) { // special case - if our loop gets converted to while with initialization we should move the label to the loop
             val labeledLoop = LabeledStatement(identifier, statementConverted.loop).assignPrototype(statement)
-            result = ForConverter.RunBlockWithLoopStatement(statementConverted.initialization, labeledLoop)
+            result = ForConverter.WhileWithInitializationPseudoStatement(statementConverted.initialization, labeledLoop, statementConverted.kind)
         }
         else {
             result = LabeledStatement(identifier, statementConverted)

j2k/testData/fileOrElement/for/commonCaseForTest.kt

@@ -1,7 +1,5 @@
-run {
-    init()
-    while (condition()) {
-        body()
-        update()
-    }
+init()
+while (condition()) {
+    body()
+    update()
 }

j2k/testData/fileOrElement/for/falseForRange.kt

@@ -1,7 +1,5 @@
-run {
-    var i = 0
-    while (i * 2 <= 10) {
-        foo(i)
-        i++
-    }
+var i = 0
+while (i * 2 <= 10) {
+    foo(i)
+    i++
 }

j2k/testData/fileOrElement/for/forWithBlock.kt

@@ -1,9 +1,7 @@
-run {
-    var i = 0
-    while (i < 10) {
-        System.out.println(i)
-        System.out.println(j)
-        j++
-        i++
-    }
+var i = 0
+while (i < 10) {
+    System.out.println(i)
+    System.out.println(j)
+    j++
+    i++
 }

j2k/testData/fileOrElement/for/forWithBlockAndDoubleUpdate.kt

@@ -1,11 +1,9 @@
-run {
-    var i = 0
-    while (i < 0) {
-        run {
-            var i = 1
-            i++
-        }
-        j++
+var i = 0
+while (i < 0) {
+    run {
+        var i = 1
         i++
     }
+    j++
+    i++
 }

j2k/testData/fileOrElement/for/forWithEmptyBlock.kt

@@ -1,7 +1,5 @@
-run {
-    var i = 0
-    while (i < 0) {
-        j++
-        i++
-    }
+var i = 0
+while (i < 0) {
+    j++
+    i++
 }

j2k/testData/fileOrElement/for/forWithNullCondition.kt

@@ -1,7 +1,5 @@
-run {
-    init()
-    while (true) {
-        body()
-        update()
-    }
+init()
+while (true) {
+    body()
+    update()
 }

j2k/testData/fileOrElement/for/forWithNullUpdate.kt

@@ -1,4 +1,2 @@
-run {
-    init()
-    while (condition()) body()
-}
+init()
+while (condition()) body()

j2k/testData/fileOrElement/for/forWithReturn.kt

@@ -1,8 +1,6 @@
-run {
-    var i = 0
-    while (i < 0) {
-        return i
-        j++
-        i++
-    }
+var i = 0
+while (i < 0) {
+    return i
+    j++
+    i++
 }

j2k/testData/fileOrElement/for/nameConflict1.java

@@ -0,0 +1,11 @@
+class A {
+    void foo() {
+        for (int i = 1; i < 1000; i *= 2) {
+            System.out.println(i);
+        }
+
+        for (int i = 1; i < 2000; i *= 2) {
+            System.out.println(i);
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict1.kt

@@ -0,0 +1,17 @@
+class A {
+    fun foo() {
+        run {
+            var i = 1
+            while (i < 1000) {
+                System.out.println(i)
+                i *= 2
+            }
+        }
+
+        var i = 1
+        while (i < 2000) {
+            System.out.println(i)
+            i *= 2
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict2.java

@@ -0,0 +1,11 @@
+class A {
+    void foo() {
+        for (int i = 1, j = 0; i < 1000; i *= 2, j++) {
+            System.out.println(i);
+        }
+
+        for (int j = 1; j < 2000; j *= 2) {
+            System.out.println(j);
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict2.kt

@@ -0,0 +1,19 @@
+class A {
+    fun foo() {
+        run {
+            var i = 1
+            var j = 0
+            while (i < 1000) {
+                System.out.println(i)
+                i *= 2
+                j++
+            }
+        }
+
+        var j = 1
+        while (j < 2000) {
+            System.out.println(j)
+            j *= 2
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict3.java

@@ -0,0 +1,11 @@
+class A {
+    int i = 1;
+
+    void foo() {
+        for (int i = 1; i < 1000; i *= 2) {
+            System.out.println(i);
+        }
+
+        i = 10;
+    }
+}

j2k/testData/fileOrElement/for/nameConflict3.kt

@@ -0,0 +1,15 @@
+class A {
+    var i = 1
+
+    fun foo() {
+        run {
+            var i = 1
+            while (i < 1000) {
+                System.out.println(i)
+                i *= 2
+            }
+        }
+
+        i = 10
+    }
+}

j2k/testData/fileOrElement/for/nameConflict4.java

@@ -0,0 +1,11 @@
+class A {
+    void foo(boolean p) {
+        for (int i = 1; i < 1000; i *= 2) {
+            System.out.println(i);
+        }
+
+        if (p) {
+            int i = 10
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict4.kt

@@ -0,0 +1,15 @@
+class A {
+    fun foo(p: Boolean) {
+        run {
+            var i = 1
+            while (i < 1000) {
+                System.out.println(i)
+                i *= 2
+            }
+        }
+
+        if (p) {
+            val i = 10
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict5.java

@@ -0,0 +1,11 @@
+class A {
+    void foo(boolean p) {
+        if (p) {
+            int i = 10
+        }
+
+        for (int i = 1; i < 1000; i *= 2) {
+            System.out.println(i);
+        }
+    }
+}

j2k/testData/fileOrElement/for/nameConflict5.kt

@@ -0,0 +1,13 @@
+class A {
+    fun foo(p: Boolean) {
+        if (p) {
+            val i = 10
+        }
+
+        var i = 1
+        while (i < 1000) {
+            System.out.println(i)
+            i *= 2
+        }
+    }
+}

j2k/testData/fileOrElement/for/notInBlock.java

@@ -0,0 +1,8 @@
+class A {
+    void foo(boolean p) {
+        if (p)
+            for (int i = 1; i < 1000; i *= 2) {
+                System.out.println(i);
+            }
+    }
+}

j2k/testData/fileOrElement/for/notInBlock.kt

@@ -0,0 +1,11 @@
+class A {
+    fun foo(p: Boolean) {
+        if (p) {
+            var i = 1
+            while (i < 1000) {
+                System.out.println(i)
+                i *= 2
+            }
+        }
+    }
+}

j2k/testData/fileOrElement/for/withContinue1.kt

@@ -1,17 +1,15 @@
 public class TestClass {
     companion object {
         public fun main(args: Array<String>) {
-            run {
-                var i = 0
-                while (i < 10) {
-                    if (i == 4 || i == 8) {
-                        i++
-                        ++i
-                        continue
-                    }
-                    System.err.println(i)
+            var i = 0
+            while (i < 10) {
+                if (i == 4 || i == 8) {
+                    i++
                     ++i
+                    continue
                 }
+                System.err.println(i)
+                ++i
             }
         }
     }

j2k/testData/fileOrElement/for/withContinue2.kt

@@ -1,20 +1,18 @@
 public class TestClass {
     companion object {
         public fun main(args: Array<String>) {
-            run {
-                var i = 0
-                var j = 1
-                while (i < 10) {
-                    if (i == 4 || i == 8) {
-                        i++
-                        ++i
-                        j *= 2
-                        continue
-                    }
-                    System.err.println(j)
+            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
             }
         }
     }

j2k/testData/fileOrElement/for/withContinue3.kt

@@ -1,16 +1,14 @@
 public class TestClass {
     companion object {
         public fun main(args: Array<String>) {
-            run {
-                var i = 1
-                while (i < 1000) {
-                    if (i == 4 || i == 8) {
-                        i *= 2
-                        continue
-                    }
-                    System.err.println(i)
+            var i = 1
+            while (i < 1000) {
+                if (i == 4 || i == 8) {
                     i *= 2
+                    continue
                 }
+                System.err.println(i)
+                i *= 2
             }
         }
     }

j2k/testData/fileOrElement/for/withContinueAndLabels.kt

@@ -2,30 +2,26 @@
 public class TestClass {
     companion object {
         public fun main(args: Array<String>) {
-            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
-                        }
+            var i = 1
+            @OuterLoop1 @OuterLoop2 while (i < 1000) {
+                var j = 1
+                @InnerLoop while (j < 100) {
+                    if (j == 3) {
+                        j *= 3
+                        continue@InnerLoop
                     }
-                    i *= 2
+                    if (i == j) {
+                        i *= 2
+                        continue@OuterLoop1
+                    }
+                    System.err.println(j)
+                    if (j == 9) {
+                        j *= 3
+                        continue
+                    }
+                    j *= 3
                 }
+                i *= 2
             }
         }
     }

j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java

@@ -2002,6 +2002,42 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo
             doTest(fileName);
         }
 
+        @TestMetadata("nameConflict1.java")
+        public void testNameConflict1() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict1.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict2.java")
+        public void testNameConflict2() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict2.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict3.java")
+        public void testNameConflict3() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict3.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict4.java")
+        public void testNameConflict4() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict4.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict5.java")
+        public void testNameConflict5() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict5.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("notInBlock.java")
+        public void testNotInBlock() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/notInBlock.java");
+            doTest(fileName);
+        }
+
         @TestMetadata("withContinue1.java")
         public void testWithContinue1() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue1.java");

j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java

@@ -2002,6 +2002,42 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo
             doTest(fileName);
         }
 
+        @TestMetadata("nameConflict1.java")
+        public void testNameConflict1() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict1.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict2.java")
+        public void testNameConflict2() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict2.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict3.java")
+        public void testNameConflict3() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict3.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict4.java")
+        public void testNameConflict4() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict4.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("nameConflict5.java")
+        public void testNameConflict5() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/nameConflict5.java");
+            doTest(fileName);
+        }
+
+        @TestMetadata("notInBlock.java")
+        public void testNotInBlock() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/notInBlock.java");
+            doTest(fileName);
+        }
+
         @TestMetadata("withContinue1.java")
         public void testWithContinue1() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/for/withContinue1.java");