Java to Kotlin converter: better formatting preserving from original code

#KT-4801 Fixed

j2k/src/org/jetbrains/jet/j2k/Converter.kt

@@ -348,7 +348,7 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
             }
             dispatcher.expressionVisitor = ExpressionVisitor(this, typeConverter, usageReplacementMap)
             try {
-                Block(convertStatements(body.getStatements().filter{ !statementsToRemove.contains(it) }), false)
+                convertBlock(body, false, { !statementsToRemove.contains(it) })
             }
             finally {
                 dispatcher.expressionVisitor = ExpressionVisitor(this, typeConverter, mapOf())
@@ -404,14 +404,12 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
         return null
     }
 
-    public fun convertBlock(block: PsiCodeBlock?): Block {
+    public fun convertBlock(block: PsiCodeBlock?, notEmpty: Boolean = true, statementFilter: (PsiStatement) -> Boolean = {true}): Block {
         if (block == null) return Block.Empty
 
-        return Block(convertStatements(block.getChildren().toList()), true)
+        val filteredChildren = block.getChildren().filter { it !is PsiStatement || statementFilter(it) }
-    }
+        val statementList = StatementList(filteredChildren.map { if (it is PsiStatement) convertStatement(it) else convertElement(it) })
-
+        return Block(statementList, notEmpty)
-    public fun convertStatements(statements: List<PsiElement>): StatementList {
-        return StatementList(statements.map { if (it is PsiStatement) convertStatement(it) else convertElement(it) })
     }
 
     public fun convertStatement(statement: PsiStatement?): Statement {

j2k/src/org/jetbrains/jet/j2k/Utils.kt

@@ -78,3 +78,17 @@ fun isQualifierEmptyOrThis(ref: PsiReferenceExpression): Boolean {
     val qualifier = ref.getQualifierExpression()
     return qualifier == null || (qualifier is PsiThisExpression && qualifier.getQualifier() == null)
 }
+
+fun PsiElement.isInSingleLine(): Boolean {
+    if (this is PsiWhiteSpace) {
+        val text = getText()!!
+        return text.indexOf('\n') < 0 && text.indexOf('\r') < 0
+    }
+
+    var child = getFirstChild()
+    while (child != null) {
+        if (!child!!.isInSingleLine()) return false
+        child = child!!.getNextSibling()
+    }
+    return true
+}

j2k/src/org/jetbrains/jet/j2k/ast/Block.kt

@@ -27,7 +27,6 @@ fun Block(statements: List<Statement>, notEmpty: Boolean = false): Block {
 }
 
 class Block(val statementList: StatementList, val notEmpty: Boolean = false) : Statement() {
-
     val statements: List<Statement> = statementList.statements
 
 
@@ -43,6 +42,6 @@ class Block(val statementList: StatementList, val notEmpty: Boolean = false) : S
     }
 
     class object {
-        val Empty = Block(StatementList(ArrayList()))
+        val Empty = Block(StatementList(listOf()))
     }
 }

j2k/src/org/jetbrains/jet/j2k/ast/Statements.kt

@@ -46,48 +46,61 @@ class ReturnStatement(val expression: Expression) : Statement() {
 class IfStatement(
         val condition: Expression,
         val thenStatement: Element,
-        val elseStatement: Element
+        val elseStatement: Element,
+        singleLine: Boolean
 ) : Expression() {
-    override fun toKotlin(): String {
+    private val br = if (singleLine) " " else "\n"
-        val result: String = "if (" + condition.toKotlin() + ")\n" + thenStatement.toKotlin()
-        if (!elseStatement.isEmpty) {
-            return result + "\nelse\n" + elseStatement.toKotlin()
-        }
 
+    override fun toKotlin(): String {
+        val result = "if (" + condition.toKotlin() + ")$br" + thenStatement.toKotlin()
+        if (!elseStatement.isEmpty) {
+            return "$result${br}else$br${elseStatement.toKotlin()}"
+        }
         return result
     }
 }
 
 // Loops --------------------------------------------------------------------------------------------------
 
-open class WhileStatement(val condition: Expression, val body: Element) : Statement() {
+class WhileStatement(val condition: Expression, val body: Element, singleLine: Boolean) : Statement() {
-    override fun toKotlin() = "while (" + condition.toKotlin() + ")\n" + body.toKotlin()
+    private val br = if (singleLine) " " else "\n"
+
+    override fun toKotlin() = "while (" + condition.toKotlin() + ")$br" + body.toKotlin()
 }
 
-class DoWhileStatement(condition: Expression, body: Element) : WhileStatement(condition, body) {
+class DoWhileStatement(val condition: Expression, val body: Element, singleLine: Boolean) : Statement() {
-    override fun toKotlin() = "do\n" + body.toKotlin() + "\nwhile (" + condition.toKotlin() + ")"
+    private val br = if (singleLine) " " else "\n"
+
+    override fun toKotlin() = "do$br" + body.toKotlin() + "${br}while (" + condition.toKotlin() + ")"
 }
 
 class ForeachStatement(
         val variable: Parameter,
         val expression: Expression,
-        val body: Element
+        val body: Element,
+        singleLine: Boolean
 ) : Statement() {
-    override fun toKotlin() = "for (" + variable.identifier.toKotlin() + " in " + expression.toKotlin() + ")\n" + body.toKotlin()
+
+    private val br = if (singleLine) " " else "\n"
+
+    override fun toKotlin() = "for (" + variable.identifier.toKotlin() + " in " + expression.toKotlin() + ")$br" + body.toKotlin()
 }
 
 class ForeachWithRangeStatement(val identifier: Identifier,
-                                     val start: Expression,
+                                val start: Expression,
-                                     val end: Expression,
+                                val end: Expression,
-                                     val body: Element) : Statement() {
+                                val body: Element,
-    override fun toKotlin() = "for (" + identifier.toKotlin() + " in " + start.toKotlin() + ".." + end.toKotlin() + ") " + body.toKotlin()
+                                singleLine: Boolean) : Statement() {
+    private val br = if (singleLine) " " else "\n"
+
+    override fun toKotlin() = "for (" + identifier.toKotlin() + " in " + start.toKotlin() + ".." + end.toKotlin() + ")$br" + body.toKotlin()
 }
 
-open class BreakStatement(val label: Identifier = Identifier.Empty) : Statement() {
+class BreakStatement(val label: Identifier = Identifier.Empty) : Statement() {
     override fun toKotlin() = "break" + label.withPrefix("@")
 }
 
-open class ContinueStatement(val label: Identifier = Identifier.Empty) : Statement() {
+class ContinueStatement(val label: Identifier = Identifier.Empty) : Statement() {
     override fun toKotlin() = "continue" + label.withPrefix("@")
 }
 
@@ -142,7 +155,8 @@ class SynchronizedStatement(val expression: Expression, val block: Block) : Stat
     override fun toKotlin() = "synchronized (" + expression.toKotlin() + ") " + block.toKotlin()
 }
 
-class StatementList(elements: List<Element>) : WhiteSpaceSeparatedElementList(elements, WhiteSpace.NewLine) {
+class StatementList(elements: List<Element>)
+  : WhiteSpaceSeparatedElementList(elements, WhiteSpace.NewLine) {
     val statements: List<Statement>
         get() = elements.filterIsInstance(javaClass<Statement>())
 }

j2k/src/org/jetbrains/jet/j2k/visitors/ExpressionVisitor.kt

@@ -103,7 +103,8 @@ class ExpressionVisitor(private val converter: Converter,
             converter.convertExpression(condition)
         result = IfStatement(expr,
                              converter.convertExpression(expression.getThenExpression()),
-                             converter.convertExpression(expression.getElseExpression()))
+                             converter.convertExpression(expression.getElseExpression()),
+                             expression.isInSingleLine())
     }
 
     override fun visitExpressionList(list: PsiExpressionList) {

j2k/src/org/jetbrains/jet/j2k/visitors/StatementVisitor.kt

@@ -22,6 +22,7 @@ import org.jetbrains.jet.j2k.ast.*
 import org.jetbrains.jet.j2k.countWriteAccesses
 import java.util.ArrayList
 import org.jetbrains.jet.j2k.hasWriteAccesses
+import org.jetbrains.jet.j2k.isInSingleLine
 
 class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
     public var result: Statement = Statement.Empty
@@ -64,7 +65,7 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
             converter.convertExpression(condition, condition.getType())
         else
             converter.convertExpression(condition)
-        result = DoWhileStatement(expression, converter.convertStatement(statement.getBody()))
+        result = DoWhileStatement(expression, converter.convertStatement(statement.getBody()), statement.isInSingleLine())
     }
 
     override fun visitExpressionStatement(statement: PsiExpressionStatement) {
@@ -106,7 +107,8 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
             result = ForeachWithRangeStatement(Identifier(loopVar.getName()!!),
                                                  converter.convertExpression(loopVar.getInitializer()),
                                                  endExpression,
-                                                 converter.convertStatement(body))
+                                                 converter.convertStatement(body),
+                                                 statement.isInSingleLine())
         }
         else {
             var forStatements = ArrayList<Statement>()
@@ -114,11 +116,9 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
             val bodyAndUpdate = listOf(converter.convertStatement(body),
                                        Block(listOf(converter.convertStatement(update))))
             forStatements.add(WhileStatement(
-                    if (condition == null)
+                    if (condition == null) LiteralExpression("true") else converter.convertExpression(condition),
-                        LiteralExpression("true")
+                    Block(bodyAndUpdate),
-                    else
+                    statement.isInSingleLine()))
-                        converter.convertExpression(condition),
-                    Block(bodyAndUpdate)))
             result = Block(forStatements)
         }
     }
@@ -133,15 +133,17 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
         }
         result = ForeachStatement(converter.convertParameter(statement.getIterationParameter()),
                                   iterator,
-                                  converter.convertStatement(statement.getBody()))
+                                  converter.convertStatement(statement.getBody()),
+                                  statement.isInSingleLine())
     }
 
     override fun visitIfStatement(statement: PsiIfStatement) {
         val condition = statement.getCondition()
         val expression = converter.convertExpression(condition, PsiType.BOOLEAN)
         result = IfStatement(expression,
-                               converter.convertStatement(statement.getThenBranch()),
+                             converter.convertStatement(statement.getThenBranch()),
-                               converter.convertStatement(statement.getElseBranch()))
+                             converter.convertStatement(statement.getElseBranch()),
+                             statement.isInSingleLine())
     }
 
     override fun visitLabeledStatement(statement: PsiLabeledStatement) {
@@ -172,7 +174,6 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
         var i = 0
         var hasDefaultCase: Boolean = false
         for (ls in cases) {
-            // TODO assert {(ls?.size()).sure() > 0}
             if (ls.size() > 0) {
                 var label = ls[0] as PsiSwitchLabelStatement
                 hasDefaultCase = hasDefaultCase || label.isDefaultCase()
@@ -181,14 +182,18 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
                 if (ls.size() > 1) {
                     pendingLabels.add(converter.convertStatement(label))
                     val slice: List<PsiElement> = ls.subList(1, (ls.size()))
+
+                    fun convertStatements(elements: List<PsiElement>): List<Statement>
+                            = elements.map { if (it is PsiStatement) converter.convertStatement(it) else null }.filterNotNull()
+
                     if (!containsBreak(slice)) {
-                        val statements = ArrayList(converter.convertStatements(slice).statements)
+                        val statements = ArrayList(convertStatements(slice))
-                        statements.addAll(converter.convertStatements(getAllToNextBreak(allSwitchStatements, i + ls.size())).statements)
+                        statements.addAll(convertStatements(getAllToNextBreak(allSwitchStatements, i + ls.size())))
                         result.add(CaseContainer(pendingLabels, statements))
                         pendingLabels = ArrayList()
                     }
                     else {
-                        result.add(CaseContainer(pendingLabels, converter.convertStatements(slice).statements))
+                        result.add(CaseContainer(pendingLabels, convertStatements(slice)))
                         pendingLabels = ArrayList()
                     }
                 }
@@ -225,12 +230,12 @@ class StatementVisitor(public val converter: Converter) : JavaElementVisitor() {
     }
 
     override fun visitWhileStatement(statement: PsiWhileStatement) {
-        var condition: PsiExpression? = statement.getCondition()
+        val condition = statement.getCondition()
-        val expression: Expression = (if (condition != null && condition?.getType() != null)
+        val expression = if (condition?.getType() != null)
-            converter.convertExpression(condition, condition?.getType())
+            converter.convertExpression(condition, condition!!.getType())
         else
-            converter.convertExpression(condition))
+            converter.convertExpression(condition)
-        result = WhileStatement(expression, converter.convertStatement(statement.getBody()))
+        result = WhileStatement(expression, converter.convertStatement(statement.getBody()), statement.isInSingleLine())
     }
 
     override fun visitReturnStatement(statement: PsiReturnStatement) {

j2k/tests/test/org/jetbrains/jet/j2k/test/JavaToKotlinConverterTestGenerated.java

@@ -712,9 +712,14 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("j2k/tests/testData/ast/conditionalExpression"), Pattern.compile("^(.+)\\.java$"), true);
         }
         
-        @TestMetadata("simpleConditionalExpression.java")
+        @TestMetadata("multiline.java")
-        public void testSimpleConditionalExpression() throws Exception {
+        public void testMultiline() throws Exception {
-            doTest("j2k/tests/testData/ast/conditionalExpression/simpleConditionalExpression.java");
+            doTest("j2k/tests/testData/ast/conditionalExpression/multiline.java");
+        }
+        
+        @TestMetadata("simple.java")
+        public void testSimple() throws Exception {
+            doTest("j2k/tests/testData/ast/conditionalExpression/simple.java");
         }
         
     }
@@ -1316,24 +1321,29 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("j2k/tests/testData/ast/ifStatement"), Pattern.compile("^(.+)\\.java$"), true);
         }
         
-        @TestMetadata("ifStatementWithEmptyBlocks.java")
+        @TestMetadata("multiLine.java")
-        public void testIfStatementWithEmptyBlocks() throws Exception {
+        public void testMultiLine() throws Exception {
-            doTest("j2k/tests/testData/ast/ifStatement/ifStatementWithEmptyBlocks.java");
+            doTest("j2k/tests/testData/ast/ifStatement/multiLine.java");
         }
         
-        @TestMetadata("ifStatementWithMultilineBlocks.java")
+        @TestMetadata("singleLine.java")
-        public void testIfStatementWithMultilineBlocks() throws Exception {
+        public void testSingleLine() throws Exception {
-            doTest("j2k/tests/testData/ast/ifStatement/ifStatementWithMultilineBlocks.java");
+            doTest("j2k/tests/testData/ast/ifStatement/singleLine.java");
         }
         
-        @TestMetadata("ifStatementWithOneLineBlocks.java")
+        @TestMetadata("withBlocks.java")
-        public void testIfStatementWithOneLineBlocks() throws Exception {
+        public void testWithBlocks() throws Exception {
-            doTest("j2k/tests/testData/ast/ifStatement/ifStatementWithOneLineBlocks.java");
+            doTest("j2k/tests/testData/ast/ifStatement/withBlocks.java");
         }
         
-        @TestMetadata("ifStatementWithoutElse.java")
+        @TestMetadata("withEmptyBlocks.java")
-        public void testIfStatementWithoutElse() throws Exception {
+        public void testWithEmptyBlocks() throws Exception {
-            doTest("j2k/tests/testData/ast/ifStatement/ifStatementWithoutElse.java");
+            doTest("j2k/tests/testData/ast/ifStatement/withEmptyBlocks.java");
+        }
+        
+        @TestMetadata("withoutElse.java")
+        public void testWithoutElse() throws Exception {
+            doTest("j2k/tests/testData/ast/ifStatement/withoutElse.java");
         }
         
     }

j2k/tests/testData/ast/conditionalExpression/multiline.java

@@ -0,0 +1,7 @@
+//method
+void foo(boolean b) {
+  if (b)
+      System.out.println("true")
+  else
+      System.out.println("false")
+}

j2k/tests/testData/ast/conditionalExpression/multiline.kt

@@ -0,0 +1,6 @@
+fun foo(b: Boolean) {
+    if (b)
+        System.out.println("true")
+    else
+        System.out.println("false")
+}

j2k/tests/testData/ast/conditionalExpression/simple.kt

@@ -0,0 +1 @@
+if (a.isEmpty()) 0 else 1

j2k/tests/testData/ast/conditionalExpression/simpleConditionalExpression.kt

@@ -1,4 +0,0 @@
-if (a.isEmpty())
-    0
-else
-    1

j2k/tests/testData/ast/doWhileStatement/whileWithExpression.kt

@@ -1,3 +1 @@
-do
+do i = i + 1 while (true)
-    i = i + 1
-while (true)

j2k/tests/testData/ast/doWhileStatement/whileWithReturn.kt

@@ -1,3 +1 @@
-do
+do return 1 while (true)
-    return 1
-while (true)

j2k/tests/testData/ast/foreachStatement/enhancedForWithExpression.kt

@@ -1,2 +1 @@
-for (n in list)
+for (n in list) i++
-    i++

j2k/tests/testData/ast/foreachStatement/enhancedForWithReturn.kt

@@ -1,2 +1 @@
-for (n in list)
+for (n in list) return n
-    return n

j2k/tests/testData/ast/ifStatement/ifStatementWithOneLineBlocks.kt

@@ -1,4 +0,0 @@
-if (true)
-    return 1
-else
-    return 0

j2k/tests/testData/ast/ifStatement/multiLine.java

@@ -0,0 +1,3 @@
+//statement
+if (true)
+    return 1;

j2k/tests/testData/ast/ifStatement/multiLine.kt

@@ -0,0 +1,2 @@
+if (true)
+    return 1

j2k/tests/testData/ast/ifStatement/singleLine.kt

@@ -0,0 +1 @@
+if (true) return 1 else return 0

j2k/tests/testData/ast/issues/kt-824.kt

@@ -15,10 +15,7 @@ class Test() {
         if (One.myContainer.myBoolean)
             System.out.println("Ok")
 
-        val s = if (One.myContainer.myBoolean)
+        val s = if (One.myContainer.myBoolean) "YES" else "NO"
-            "YES"
-        else
-            "NO"
 
         while (One.myContainer.myBoolean)
             System.out.println("Ok")

j2k/tests/testData/ast/labelStatement/complicatedExampleFromJavaTutorial.kt

@@ -10,7 +10,4 @@
     foundIt = true
     break@test
 }
-System.out.println(if (foundIt)
+System.out.println(if (foundIt) "Found it" else "Didn't find it")
-    "Found it"
-else
-    "Didn't find it")

j2k/tests/testData/ast/nullability/MethodInvokedWithTernaryNullArg.kt

@@ -3,9 +3,6 @@ class C() {
     }
 
     fun bar(b: Boolean) {
-        foo(if (b)
+        foo(if (b) "a" else null)
-            "a"
-        else
-            null)
     }
 }

j2k/tests/testData/ast/nullability/MethodResultComparedWithNull4.kt

@@ -5,8 +5,7 @@ trait I {
 class C() {
     fun foo(i: I, b: Boolean) {
         val result = i.getString()
-        if (b)
+        if (b) result = null
-            result = null
         if (result != null) {
             print(result)
         }

j2k/tests/testData/ast/nullability/MethodReturnsTernaryNull.kt

@@ -1,8 +1,5 @@
 class C() {
     fun foo(b: Boolean): String? {
-        return if (b)
+        return if (b) "abc" else null
-            "abc"
-        else
-            null
     }
 }

j2k/tests/testData/ast/nullability/NullableMethodDotAccess.kt

@@ -1,9 +1,6 @@
 class C() {
     fun getString(b: Boolean): String? {
-        return if (b)
+        return if (b) "a" else null
-            "a"
-        else
-            null
     }
 
     fun foo(): Int {

j2k/tests/testData/ast/nullability/NullableVariableDotAccess.kt

@@ -1,7 +1,5 @@
 fun foo(s: String?, b: Boolean): Int {
-    if (s == null)
+    if (s == null) System.out.println("null")
-        System.out.println("null")
+    if (b) return s?.length()!!
-    if (b)
-        return s?.length()!!
     return 10
 }

j2k/tests/testData/ast/nullability/VariableInitializedWithTernaryNull.kt

@@ -1,7 +1,4 @@
 // !specifyLocalVariableTypeByDefault: true
 fun foo(b: Boolean) {
-    val s: String? = (if (b)
+    val s: String? = (if (b) "abc" else null)
-        "abc"
-    else
-        null)
 }

j2k/tests/testData/ast/objectLiteral/MyFrame.kt

@@ -11,7 +11,9 @@ public class Client() : Frame() {
             override fun windowClosing() {
             }
         }
+
         addWindowListener(a)
+
         addWindowListener(object : WindowAdapter() {
             override fun windowClosing() {
             }

j2k/tests/testData/ast/switch/comlicatedFallDown.kt

@@ -84,7 +84,8 @@ public class SwitchDemo() {
         }
 
         public fun main(args: Array<String>) {
-            for (i in 1..12) test(i)
+            for (i in 1..12)
+                test(i)
         }
     }
 }

j2k/tests/testData/ast/whileStatement/whileWithExpression.kt

@@ -1,2 +1 @@
-while (true)
+while (true) i = i + 1
-    i = i + 1

j2k/tests/testData/ast/whileStatement/whileWithReturn.kt

@@ -1,2 +1 @@
-while (true)
+while (true) return 1
-    return 1