Java to Kotlin converter: no explicit type for private and internal fields when possible

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

@@ -271,7 +271,7 @@ public class Converter private(val project: Project, val settings: ConverterSett
         return Initializer(convertBlock(initializer.getBody()), convertModifiers(initializer)).assignPrototype(initializer)
     }
 
-    private fun convertField(field: PsiField): Field {
+    private fun convertField(field: PsiField): Member {
         val annotations = convertAnnotations(field)
         val modifiers = convertModifiers(field)
         val name = field.declarationIdentifier()
@@ -283,12 +283,20 @@ public class Converter private(val project: Project, val settings: ConverterSett
                          convertElement(field.getArgumentList()))
         }
         else {
+            val initializer = field.getInitializer()
+            val convertedType = typeConverter.convertVariableType(field)
+            val isVal = field.hasModifierProperty(PsiModifier.FINAL)
+            val omitType = !settings.specifyFieldTypeByDefault &&
+                    initializer != null &&
+                    (modifiers.isPrivate && (isVal || convertedType == typeConverter.convertExpressionType(initializer)) ||
+                            modifiers.isInternal && convertedType == typeConverter.convertExpressionType(initializer))
             Field(name,
                   annotations,
                   modifiers,
-                  typeConverter.convertVariableType(field),
+                  convertedType,
-                  convertExpression(field.getInitializer(), field.getType()),
+                  convertExpression(initializer, field.getType()),
-                  field.hasModifierProperty(PsiModifier.FINAL),
+                  isVal,
+                  !omitType,
                   field.hasWriteAccesses(field.getContainingClass()))
         }
         return converted.assignPrototype(field)
@@ -324,7 +332,7 @@ public class Converter private(val project: Project, val settings: ConverterSett
             if (settings.openByDefault) {
                 val isEffectivelyFinal = method.hasModifierProperty(PsiModifier.FINAL) ||
                         containingClass != null && (containingClass.hasModifierProperty(PsiModifier.FINAL) || containingClass.isEnum())
-                if (!isEffectivelyFinal && !modifiers.contains(Modifier.ABSTRACT) && !modifiers.contains(Modifier.PRIVATE)) {
+                if (!isEffectivelyFinal && !modifiers.contains(Modifier.ABSTRACT) && !modifiers.isPrivate) {
                     modifiers = modifiers.with(Modifier.OPEN)
                 }
             }

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

@@ -16,20 +16,19 @@
 
 package org.jetbrains.jet.j2k
 
-public class ConverterSettings(
+public data class ConverterSettings(
         var forceNotNullTypes: Boolean,
         var forceLocalVariableImmutability: Boolean,
         var specifyLocalVariableTypeByDefault: Boolean,
+        var specifyFieldTypeByDefault: Boolean,
         var openByDefault: Boolean) {
 
-    public fun copy(): ConverterSettings
-            = ConverterSettings(forceNotNullTypes, forceLocalVariableImmutability, specifyLocalVariableTypeByDefault, openByDefault)
-
     class object {
         public val defaultSettings: ConverterSettings = ConverterSettings(
                 forceNotNullTypes = true,
                 forceLocalVariableImmutability = true,
                 specifyLocalVariableTypeByDefault = false,
+                specifyFieldTypeByDefault = false,
                 openByDefault = false
         )
     }

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

@@ -88,7 +88,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver
         if (nullability == Nullability.Default) {
             val initializer = variable.getInitializer()
             if (initializer != null) {
-                val initializerNullability = initializer.nullability()
+                val initializerNullability = initializer.nullability(false)
                 if (variable.isEffectivelyFinal()) {
                     nullability = initializerNullability
                 }
@@ -129,7 +129,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver
                     for (call in findMethodCalls(method, scope)) {
                         val args = call.getArgumentList().getExpressions()
                         if (args.size == parameters.size) {
-                            if (args[parameterIndex].nullability() == Nullability.Nullable) {
+                            if (args[parameterIndex].nullability(false) == Nullability.Nullable) {
                                 nullability = Nullability.Nullable
                                 break
                             }
@@ -166,7 +166,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver
         if (nullability == Nullability.Default) {
             method.getBody()?.accept(object: JavaRecursiveElementVisitor() {
                 override fun visitReturnStatement(statement: PsiReturnStatement) {
-                    if (statement.getReturnValue()?.nullability() == Nullability.Nullable) {
+                    if (statement.getReturnValue()?.nullability(false) == Nullability.Nullable) {
                         nullability = Nullability.Nullable
                     }
                 }
@@ -189,6 +189,10 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver
         return nullability
     }
 
+    public fun convertExpressionType(expression: PsiExpression): Type {
+        return convertType(expression.getType(), expression.nullability(true))
+    }
+
     private fun searchScope(element: PsiElement): PsiElement? {
         return when(element) {
             is PsiParameter -> element.getDeclarationScope()
@@ -199,22 +203,39 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver
         }
     }
 
-    private fun PsiExpression.nullability(): Nullability {
+    private fun PsiExpression.nullability(useDeclarationsNullability: Boolean): Nullability {
         return when (this) {
             is PsiLiteralExpression -> if (getType() != PsiType.NULL) Nullability.NotNull else Nullability.Nullable
 
             is PsiNewExpression -> Nullability.NotNull
 
             is PsiConditionalExpression -> {
-                val nullability1 = getThenExpression()?.nullability()
+                val nullability1 = getThenExpression()?.nullability(useDeclarationsNullability)
                 if (nullability1 == Nullability.Nullable) return Nullability.Nullable
-                val nullability2 = getElseExpression()?.nullability()
+                val nullability2 = getElseExpression()?.nullability(useDeclarationsNullability)
                 if (nullability2 == Nullability.Nullable) return Nullability.Nullable
                 if (nullability1 == Nullability.NotNull && nullability2 == Nullability.NotNull) return Nullability.NotNull
                 Nullability.Default
             }
 
-            is PsiParenthesizedExpression -> getExpression()?.nullability() ?: Nullability.Default
+            is PsiParenthesizedExpression -> getExpression()?.nullability(useDeclarationsNullability) ?: Nullability.Default
+
+            is PsiMethodCallExpression -> if (useDeclarationsNullability) {
+                val method = resolveMethod()
+                if (method != null) methodNullability(method) else Nullability.Default
+            }
+            else {
+                Nullability.Default
+            }
+
+            is PsiReferenceExpression -> if (useDeclarationsNullability) {
+                val variable = resolve() as? PsiVariable
+                if (variable != null) variableNullability(variable) else Nullability.Default
+            }
+            else {
+                Nullability.Default
+            }
+
 
         //TODO: some other cases
 
@@ -225,7 +246,7 @@ class TypeConverter(val settings: ConverterSettings, val conversionScope: Conver
     private fun isNullableFromUsage(usage: PsiExpression): Boolean {
         val parent = usage.getParent() ?: return false
         if (parent is PsiAssignmentExpression && parent.getOperationTokenType() == JavaTokenType.EQ && usage == parent.getLExpression()) {
-            return parent.getRExpression()?.nullability() == Nullability.Nullable
+            return parent.getRExpression()?.nullability(false) == Nullability.Nullable
         }
         else if (parent is PsiBinaryExpression) {
             val operationType = parent.getOperationTokenType()

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

@@ -19,20 +19,20 @@ package org.jetbrains.jet.j2k.ast
 import org.jetbrains.jet.j2k.*
 
 class EnumConstant(
-        identifier: Identifier,
+        val identifier: Identifier,
         annotations: Annotations,
         modifiers: Modifiers,
-        `type`: Type,
+        val `type`: Type,
-        params: Element
+        val params: Element
-) : Field(identifier, annotations, modifiers, `type`.toNotNullType(), params, true, false) {
+) : Member(annotations, modifiers) {
 
     override fun generateCode(builder: CodeBuilder) {
-        if (initializer.isEmpty) {
+        if (params.isEmpty) {
             builder append annotations append identifier
             return
         }
 
-        builder append annotations append identifier append " : " append `type` append "(" append initializer append ")"
+        builder append annotations append identifier append " : " append `type` append "(" append params append ")"
     }
 
 

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

@@ -18,13 +18,14 @@ package org.jetbrains.jet.j2k.ast
 
 import org.jetbrains.jet.j2k.*
 
-open class Field(
+class Field(
         val identifier: Identifier,
         annotations: Annotations,
         modifiers: Modifiers,
         val `type`: Type,
         val initializer: Element,
         val isVal: Boolean,
+        val explicitType: Boolean,
         private val hasWriteAccesses: Boolean
 ) : Member(annotations, modifiers) {
 
@@ -33,8 +34,10 @@ open class Field(
                 .appendWithSpaceAfter(modifiers)
                 .append(if (isVal) "val " else "var ")
                 .append(identifier)
-                .append(" : ")
+
-                .append(`type`)
+        if (explicitType) {
+            builder append ":" append `type`
+        }
 
         var initializerToUse = initializer
         if (initializerToUse.isEmpty && !(isVal && hasWriteAccesses)) {

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

@@ -55,6 +55,11 @@ class Modifiers(val modifiers: Collection<Modifier>) : Element() {
 
     fun contains(modifier: Modifier): Boolean = modifiers.contains(modifier)
 
+    val isPublic: Boolean get() = contains(Modifier.PUBLIC)
+    val isPrivate: Boolean get() = contains(Modifier.PRIVATE)
+    val isProtected: Boolean get() = contains(Modifier.PROTECTED)
+    val isInternal: Boolean get() = !isPublic && !isPrivate && !isProtected
+
     class object {
         val Empty = Modifiers(listOf())
     }

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

@@ -48,12 +48,16 @@ abstract class Type() : Element() {
     abstract val isNullable: Boolean
 
     open fun toNotNullType(): Type {
-        if (isNullable) throw UnsupportedOperationException("toNotNullType must be defined")
+        if (isNullable) {
+            throw UnsupportedOperationException("toNotNullType must be defined")
+        }
         return this
     }
 
     open fun toNullableType(): Type {
-        if (!isNullable) throw UnsupportedOperationException("toNullableType must be defined")
+        if (!isNullable) {
+            throw UnsupportedOperationException("toNullableType must be defined")
+        }
         return this
     }
 
@@ -69,6 +73,14 @@ abstract class Type() : Element() {
         }
     }
 
+    object Null: Type() {
+        override val isNullable: Boolean = true
+
+        override fun generateCode(builder: CodeBuilder) {
+            builder.append("???")
+        }
+    }
+
     override fun equals(other: Any?): Boolean = other is Type && other.canonicalCode() == this.canonicalCode()
 
     override fun hashCode(): Int = canonicalCode().hashCode()

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

@@ -36,6 +36,9 @@ class TypeVisitor(private val converter: TypeConverter, private val importNames:
         else if (PRIMITIVE_TYPES_NAMES.contains(name)) {
             PrimitiveType(Identifier(StringUtil.capitalize(name)))
         }
+        else if (name == "null") {
+            Type.Null
+        }
         else {
             PrimitiveType(Identifier(name))
         }

j2k/tests/test/org/jetbrains/jet/j2k/test/AbstractJavaToKotlinConverterTest.kt

@@ -77,6 +77,7 @@ abstract class AbstractJavaToKotlinConverterTest() : LightIdeaTestCase() {
                 "forceNotNullTypes" -> settings.forceNotNullTypes = parseBoolean(value)
                 "forceLocalVariableImmutability" -> settings.forceLocalVariableImmutability = parseBoolean(value)
                 "specifyLocalVariableTypeByDefault" -> settings.specifyLocalVariableTypeByDefault = parseBoolean(value)
+                "specifyFieldTypeByDefault" -> settings.specifyFieldTypeByDefault = parseBoolean(value)
                 "openByDefault" -> settings.openByDefault = parseBoolean(value)
                 else -> throw IllegalArgumentException("Unknown option: $name")
             }

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

@@ -1094,6 +1094,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             doTest("j2k/tests/testData/ast/field/publicField.java");
         }
         
+        @TestMetadata("specifyType.java")
+        public void testSpecifyType() throws Exception {
+            doTest("j2k/tests/testData/ast/field/specifyType.java");
+        }
+        
         @TestMetadata("valWithInit.java")
         public void testValWithInit() throws Exception {
             doTest("j2k/tests/testData/ast/field/valWithInit.java");
@@ -2354,6 +2359,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             doTest("j2k/tests/testData/ast/settings/openByDefault.java");
         }
         
+        @TestMetadata("specifyFieldTypeByDefault.java")
+        public void testSpecifyFieldTypeByDefault() throws Exception {
+            doTest("j2k/tests/testData/ast/settings/specifyFieldTypeByDefault.java");
+        }
+        
         @TestMetadata("specifyLocalVariableTypeByDefault.java")
         public void testSpecifyLocalVariableTypeByDefault() throws Exception {
             doTest("j2k/tests/testData/ast/settings/specifyLocalVariableTypeByDefault.java");

j2k/tests/testData/ast/annotations/annotationUsages.kt

@@ -9,15 +9,15 @@ Anon6(array("x", "y"))
 Anon7(javaClass<String>(), javaClass<StringBuilder>())
 Anon8(classes = *array(javaClass<String>(), javaClass<StringBuilder>()))
 class C() {
-    Anon5(1) deprecated("") private var field1: Int = 0
+    Anon5(1) deprecated("") private var field1 = 0
 
     Anon5(1)
-    private var field2: Int = 0
+    private var field2 = 0
 
-    Anon5(1) var field3: Int = 0
+    Anon5(1) var field3 = 0
 
     Anon5(1)
-    var field4: Int = 0
+    var field4 = 0
 
     Anon6(array())
     fun foo(deprecated("") p1: Int, deprecated("") Anon5(2) p2: Char) {

j2k/tests/testData/ast/annotations/jetbrainsNotNull.kt

@@ -3,7 +3,7 @@
 package test
 
 public class Test(str: String) {
-    var myStr: String = "String2"
+    var myStr = "String2"
 
     public fun sout(str: String) {
         System.out!!.println(str)

j2k/tests/testData/ast/class/classWithFields.kt

@@ -1,4 +1,4 @@
 class T() {
-    var a: String = "abc"
+    var a = "abc"
-    var b: Int = 10
+    var b = 10
 }

j2k/tests/testData/ast/class/classWithMultiplyFields.kt

@@ -1,5 +1,5 @@
 class T() {
     var a: String = 0
     var b: String = 0
-    var c: String = "abc"
+    var c = "abc"
 }

j2k/tests/testData/ast/class/oneStaticFieldOneNonStatic.kt

@@ -4,6 +4,6 @@ class S() {
     }
 
     class object {
-        var myI: Int = 10
+        var myI = 10
     }
 }

j2k/tests/testData/ast/comments/comments2.kt

@@ -11,7 +11,7 @@ class A() {
         /* body is empty */
     }
 
-    private /*it's private*/ var field: Int = 0
+    private /*it's private*/ var field = 0
 
     public /*it's public*/ fun foo(): Char {
     }

j2k/tests/testData/ast/comments/fieldWithEndOfLineComment.kt

@@ -1,3 +1,3 @@
 class A() {
-    private var isOpen: Boolean = true // ideally should be atomic boolean
+    private var isOpen = true // ideally should be atomic boolean
 }

j2k/tests/testData/ast/constructors/genericIdentifier.kt

@@ -1,5 +1,5 @@
 public class Identifier<T> private(private val myName: T, private var myHasDollar: Boolean) {
-    private var myNullable: Boolean = true
+    private var myNullable = true
 
     public fun getName(): T {
         return myName

j2k/tests/testData/ast/constructors/identifier.kt

@@ -1,5 +1,5 @@
 public class Identifier private(private val myName: String, private var myHasDollar: Boolean) {
-    private var myNullable: Boolean = true
+    private var myNullable = true
 
     public fun getName(): String {
         return myName

j2k/tests/testData/ast/field/specifyType.java

@@ -0,0 +1,28 @@
+//file
+import org.jetbrains.annotations.Nullable;
+import java.util.*;
+
+class A {
+    private final List<String> field1 = new ArrayList<String>();
+    final List<String> field2 = new ArrayList<String>();
+    public final int field3 = 0;
+    protected final int field4 = 0;
+
+    private List<String> field5 = new ArrayList<String>();
+    List<String> field6 = new ArrayList<String>();
+
+    private int field7 = 0;
+    int field8 = 0;
+
+    @Nullable private String field9 = "a"
+    @Nullable private String field10 = foo();
+
+    String foo() { return "x"; }
+
+    void bar() {
+        field7++;
+        field8++;
+        field9 = null;
+        field10 = null;
+    }
+}

j2k/tests/testData/ast/field/specifyType.kt

@@ -0,0 +1,29 @@
+import java.util.*
+import kotlin.List
+
+class A() {
+    private val field1 = ArrayList<String>()
+    val field2: List<String> = ArrayList<String>()
+    public val field3: Int = 0
+    protected val field4: Int = 0
+
+    private var field5: List<String> = ArrayList<String>()
+    var field6: List<String> = ArrayList<String>()
+
+    private var field7 = 0
+    var field8 = 0
+
+    private var field9: String? = "a"
+    private var field10: String? = foo()
+
+    fun foo(): String {
+        return "x"
+    }
+
+    fun bar() {
+        field7++
+        field8++
+        field9 = null
+        field10 = null
+    }
+}

j2k/tests/testData/ast/field/valWithInit.kt

@@ -1,3 +1,3 @@
 class C() {
-    val f: Foo = Foo(1, 2)
+    val f = Foo(1, 2)
 }

j2k/tests/testData/ast/field/varWithInit.kt

@@ -1,3 +1,3 @@
 class C() {
-    var f: Foo = Foo(1, 2)
+    var f = Foo(1, 2)
 }

j2k/tests/testData/ast/formatting/nonStaticMembers.kt

@@ -5,7 +5,7 @@ class F() {
 
     fun f2() {
     }
-    var i: Int = 0
+    var i = 0
 
     fun f3() {
     }

j2k/tests/testData/ast/formatting/nonStaticMembersWithComments.kt

@@ -16,7 +16,7 @@ class F() {
 
     fun f2() {
     }
-    var i: Int = 0
+    var i = 0
 
     fun f3() {
     }

j2k/tests/testData/ast/formatting/staticAndNonStaticMembersWithComments.kt

@@ -24,7 +24,7 @@ class F() {
 
         fun f1() {
         }
-        var i: Int = 0
+        var i = 0
 
         //c5
 

j2k/tests/testData/ast/formatting/staticMembersWithComments.kt

@@ -17,7 +17,7 @@ class F() {
 
         fun f2() {
         }
-        var i: Int = 0
+        var i = 0
 
         fun f3() {
         }

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

@@ -1,5 +1,5 @@
 public class Identifier<T> private(private val myName: T, private var myHasDollar: Boolean) {
-    private var myNullable: Boolean = true
+    private var myNullable = true
 
     public fun getName(): T {
         return myName

j2k/tests/testData/ast/issues/kt-809-string.kt

@@ -1,12 +1,12 @@
 package demo
 
 class Container() {
-    var myString: String = "1"
+    var myString = "1"
 }
 
 class One() {
     class object {
-        var myContainer: Container = Container()
+        var myContainer = Container()
     }
 }
 

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

@@ -1,12 +1,12 @@
 package demo
 
 class Container() {
-    var myInt: Int = 1
+    var myInt = 1
 }
 
 class One() {
     class object {
-        var myContainer: Container = Container()
+        var myContainer = Container()
     }
 }
 

j2k/tests/testData/ast/issues/kt-820-field.kt

@@ -1,12 +1,12 @@
 package demo
 
 class Container() {
-    var myInt: Int = 1
+    var myInt = 1
 }
 
 class One() {
     class object {
-        var myContainer: Container = Container()
+        var myContainer = Container()
     }
 }
 

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

@@ -1,12 +1,12 @@
 package demo
 
 class Container() {
-    var myInt: Int = 1
+    var myInt = 1
 }
 
 class One() {
     class object {
-        var myContainer: Container = Container()
+        var myContainer = Container()
     }
 }
 

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

@@ -1,12 +1,12 @@
 package demo
 
 class Container() {
-    var myBoolean: Boolean = true
+    var myBoolean = true
 }
 
 class One() {
     class object {
-        var myContainer: Container = Container()
+        var myContainer = Container()
     }
 }
 

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

@@ -11,6 +11,6 @@ public class Language(protected var code: String) : Serializable {
     class object {
         public var ENGLISH: Language = Language("en")
         public var SWEDISH: Language = Language("sv")
-        private val serialVersionUID: Long = -2442762969929206780
+        private val serialVersionUID = -2442762969929206780
     }
 }

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

@@ -1,7 +1,7 @@
 package demo
 
 class C(a: Int) {
-    var abc: Int = 0
+    var abc = 0
 
     {
         abc = a * 2

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

@@ -1,5 +1,5 @@
 class C() {
-    private val string: String? = getString()
+    private val string = getString()
 
     class object {
 

j2k/tests/testData/ast/settings/specifyFieldTypeByDefault.java

@@ -0,0 +1,17 @@
+//file
+// !specifyFieldTypeByDefault: true
+import org.jetbrains.annotations.Nullable;
+import java.util.*;
+
+class A {
+    private final List<String> field1 = new ArrayList<String>();
+    final List<String> field2 = new ArrayList<String>();
+    public final int field3 = 0;
+    protected final int field4 = 0;
+
+    private List<String> field5 = new ArrayList<String>();
+    List<String> field6 = new ArrayList<String>();
+
+    private int field7 = 0;
+    int field8 = 0;
+}

j2k/tests/testData/ast/settings/specifyFieldTypeByDefault.kt

@@ -0,0 +1,16 @@
+// !specifyFieldTypeByDefault: true
+import java.util.*
+import kotlin.List
+
+class A() {
+    private val field1: List<String> = ArrayList<String>()
+    val field2: List<String> = ArrayList<String>()
+    public val field3: Int = 0
+    protected val field4: Int = 0
+
+    private var field5: List<String> = ArrayList<String>()
+    var field6: List<String> = ArrayList<String>()
+
+    private var field7: Int = 0
+    var field8: Int = 0
+}

j2k/tests/testData/ast/staticMembers/PrivateStaticMembers.kt

@@ -6,7 +6,7 @@ class A() {
     }
 
     class object {
-        private val s: String = "abc"
+        private val s = "abc"
 
         private fun privateStatic1() {
         }