Java to Kotlin conversion: fixed a few bugs related to modifiers

#KT-4424 Fixed

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

@@ -137,6 +137,10 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
                     }
                 }
 
+                if (settings.openByDefault && !modifiers.contains(Modifier.FINAL)) {
+                    modifiers.add(Modifier.OPEN)
+                }
+
                 return Class(this, name, getComments(psiClass), modifiers, typeParameters, extendsTypes, baseClassParams, implementsTypes, classBodyElements)
             }
         }
@@ -231,16 +235,17 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
             methodReturnType = method.getReturnType()
             val returnType = convertType(method.getReturnType(), method.isAnnotatedAsNotNull())
 
-            val modifiers = HashSet(convertModifierList(method.getModifierList()))
+            val modifiers = convertModifierList(method.getModifierList())
 
             val containingClass = method.getContainingClass()
-            if (containingClass != null && containingClass.isInterface()) {
-                modifiers.remove(Modifier.ABSTRACT)
-            }
             if (containingClass != null && (containingClass.hasModifierProperty(PsiModifier.FINAL) || containingClass.isEnum())) {
                 modifiers.add(Modifier.FINAL)
             }
 
+            if (isOverride(method)) {
+                modifiers.add(Modifier.OVERRIDE)
+            }
+
             if (settings.openByDefault &&
                     !modifiers.contains(Modifier.ABSTRACT) &&
                     !modifiers.contains(Modifier.FINAL) &&
@@ -248,11 +253,6 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
                 modifiers.add(Modifier.OPEN)
             }
 
-            if (isOverride(method)) {
-                modifiers.add(Modifier.OVERRIDE)
-                modifiers.remove(Modifier.OPEN)
-            }
-
             val comments = getComments(method)
 
             if (method.isConstructor()) {
@@ -268,7 +268,7 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
                 val params = convertParameterList(method.getParameterList())
                 val typeParameterList = convertTypeParameterList(method.getTypeParameterList())
                 val block = convertBlock(method.getBody())
-                return Function(this, Identifier(method.getName()), comments, modifiers, returnType, typeParameterList, params, block)
+                return Function(this, Identifier(method.getName()), comments, modifiers, returnType, typeParameterList, params, block, containingClass?.isInterface() ?: false)
             }
         }
         finally {
@@ -494,12 +494,13 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
         return Identifier(identifier.getText()!!)
     }
 
-    public fun convertModifierList(modifierList: PsiModifierList?): Set<Modifier> {
+    public fun convertModifierList(modifierList: PsiModifierList?): MutableSet<Modifier> {
-        if (modifierList == null) return setOf()
+        if (modifierList == null) return HashSet()
 
         val modifiersSet = HashSet<Modifier>()
 
-        if (modifierList.hasExplicitModifier(PsiModifier.ABSTRACT))
+        //TODO: map
+        if (modifierList.hasModifierProperty(PsiModifier.ABSTRACT))
             modifiersSet.add(Modifier.ABSTRACT)
 
         if (modifierList.hasModifierProperty(PsiModifier.FINAL))
@@ -508,16 +509,13 @@ public class Converter(val project: Project, val settings: ConverterSettings) {
         if (modifierList.hasModifierProperty(PsiModifier.STATIC))
             modifiersSet.add(Modifier.STATIC)
 
-        if (modifierList.hasExplicitModifier(PsiModifier.PUBLIC))
+        if (modifierList.hasModifierProperty(PsiModifier.PUBLIC))
             modifiersSet.add(Modifier.PUBLIC)
 
-        if (modifierList.hasExplicitModifier(PsiModifier.PROTECTED))
+        if (modifierList.hasModifierProperty(PsiModifier.PROTECTED))
             modifiersSet.add(Modifier.PROTECTED)
 
-        if (modifierList.hasExplicitModifier(PsiModifier.PACKAGE_LOCAL))
+        if (modifierList.hasModifierProperty(PsiModifier.PRIVATE))
-            modifiersSet.add(Modifier.INTERNAL)
-
-        if (modifierList.hasExplicitModifier(PsiModifier.PRIVATE))
             modifiersSet.add(Modifier.PRIVATE)
 
         return modifiersSet

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

@@ -17,7 +17,6 @@
 package org.jetbrains.jet.j2k.ast
 
 import org.jetbrains.jet.j2k.Converter
-import java.util.HashSet
 import java.util.ArrayList
 
 open class Class(
@@ -80,27 +79,21 @@ open class Class(
             " : " + allTypes.makeString(", ")
     }
 
-    protected fun modifiersToKotlin(): String {
+    protected open fun modifiersToKotlin(): String {
         val modifierList = ArrayList<Modifier>()
-        val modifier = modifiers.accessModifier()
+
-        if (modifier != null) {
+        modifiers.accessModifier()?.let { modifierList.add(it) }
-            modifierList.add(modifier)
+
-        }
+        if (modifiers.contains(Modifier.ABSTRACT)) {
-        if (isAbstract()) {
             modifierList.add(Modifier.ABSTRACT)
         }
-        else if (needsOpenModifier()) {
+        else if (modifiers.contains(Modifier.OPEN)) {
             modifierList.add(Modifier.OPEN)
         }
+
         return modifierList.toKotlin()
     }
 
-    protected open fun isDefinitelyFinal(): Boolean
-            = modifiers.contains(Modifier.FINAL)
-
-    protected open fun needsOpenModifier(): Boolean
-            = !isDefinitelyFinal() && converter.settings.openByDefault
-
     fun bodyToKotlin(): String {
         val innerBody = classMembers.nonStaticMembers.toKotlin() + primaryConstructorBodyToKotlin() + classObjectToKotlin()
         return if (innerBody.trim().isNotEmpty()) " {" + innerBody + "}" else ""

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

@@ -26,7 +26,7 @@ abstract class Constructor(
         modifiers: Set<Modifier>,
         parameterList: ParameterList,
         block: Block
-) : Function(converter, Identifier.Empty, comments, modifiers, Type.Empty, TypeParameterList.Empty, parameterList, block)
+) : Function(converter, Identifier.Empty, comments, modifiers, Type.Empty, TypeParameterList.Empty, parameterList, block, false)
 
 class PrimaryConstructor(converter: Converter,
                          comments: MemberComments,
@@ -61,6 +61,6 @@ class SecondaryConstructor(converter: Converter,
         typeParameters.addAll(containingClass.typeParameterList.parameters)
         return Function(converter, Identifier("init"), MemberComments.Empty, modifiers,
                         ClassType(containingClass.name, typeParameters, false, converter),
-                        TypeParameterList(typeParameters), parameterList, block)
+                        TypeParameterList(typeParameters), parameterList, block, false)
     }
 }

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

@@ -34,8 +34,6 @@ class Enum(
     override fun primaryConstructorSignatureToKotlin(): String
         = classMembers.primaryConstructor?.signatureToKotlin() ?: ""
 
-    override fun isDefinitelyFinal() = true
-
     override fun toKotlin(): String {
         return modifiersToKotlin() +
                 "enum class " + name.toKotlin() +

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

@@ -30,14 +30,11 @@ open class Field(
 
     fun modifiersToKotlin(): String {
         val modifierList = ArrayList<Modifier>()
-        if (isAbstract()) {
+        if (modifiers.contains(Modifier.ABSTRACT)) {
             modifierList.add(Modifier.ABSTRACT)
         }
 
-        val modifier = modifiers.accessModifier()
+        modifiers.accessModifier()?.let { modifierList.add(it) }
-        if (modifier != null) {
-            modifierList.add(modifier)
-        }
 
         return modifierList.toKotlin() + (if (isVal()) "val " else "var ")
     }

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

@@ -27,7 +27,8 @@ open class Function(
         val `type`: Type,
         val typeParameterList: TypeParameterList,
         val parameterList: ParameterList,
-        var block: Block?
+        var block: Block?,
+        val isInTrait: Boolean
 ) : Member(comments, modifiers) {
 
     private fun modifiersToKotlin(): String {
@@ -42,11 +43,11 @@ open class Function(
             resultingModifiers.add(accessModifier)
         }
 
-        if (isAbstract()) {
+        if (modifiers.contains(Modifier.ABSTRACT) && !isInTrait) {
             resultingModifiers.add(Modifier.ABSTRACT)
         }
 
-        if (modifiers.contains(Modifier.OPEN)) {
+        if (modifiers.contains(Modifier.OPEN) && !isOverride) {
             resultingModifiers.add(Modifier.OPEN)
         }
 

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

@@ -25,7 +25,6 @@ class MemberComments(elements: List<Element>) : WhiteSpaceSeparatedElementList(e
 }
 
 abstract class Member(val comments: MemberComments, val modifiers: Set<Modifier>) : Element {
-    fun isAbstract(): Boolean = modifiers.contains(Modifier.ABSTRACT)
     fun isStatic(): Boolean = modifiers.contains(Modifier.STATIC)
     fun commentsToKotlin(): String = comments.toKotlin()
 }

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

@@ -16,37 +16,26 @@
 
 package org.jetbrains.jet.j2k.ast
 
-import org.jetbrains.jet.j2k.ast.Modifier.PUBLIC
-import org.jetbrains.jet.j2k.ast.Modifier.PROTECTED
-import org.jetbrains.jet.j2k.ast.Modifier.PRIVATE
-import org.jetbrains.jet.j2k.ast.Modifier.INTERNAL
-
 enum class Modifier(val name: String) {
     PUBLIC: Modifier("public")
     PROTECTED: Modifier("protected")
     PRIVATE: Modifier("private")
-    INTERNAL: Modifier("internal")
     STATIC: Modifier("static")
     ABSTRACT: Modifier("abstract")
     FINAL: Modifier("final")
     OPEN: Modifier("open")
     OVERRIDE: Modifier("override")
 
-    public fun toKotlin(): String? {
+    public fun toKotlin(): String = name
-        return when(this) {
-            INTERNAL -> null
-            else -> name
-        }
-    }
 }
 
-val ACCESS_MODIFIERS = setOf(PUBLIC, PROTECTED, PRIVATE, INTERNAL)
+val ACCESS_MODIFIERS = setOf(Modifier.PUBLIC, Modifier.PROTECTED, Modifier.PRIVATE)
 
 fun Collection<Modifier>.accessModifier(): Modifier? {
     return firstOrNull { ACCESS_MODIFIERS.contains(it) }
 }
 
 fun Collection<Modifier>.toKotlin(): String
-        = if (isNotEmpty()) map { it.toKotlin() }.filterNotNull().makeString(" ") + " " else ""
+        = if (isNotEmpty()) map { it.toKotlin() }.makeString(" ") + " " else ""
 
 

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

@@ -17,6 +17,7 @@
 package org.jetbrains.jet.j2k.ast
 
 import org.jetbrains.jet.j2k.Converter
+import java.util.ArrayList
 
 class Trait(
         converter: Converter,
@@ -35,5 +36,11 @@ class Trait(
         get() = "trait"
 
     override fun primaryConstructorSignatureToKotlin() = ""
-    override fun needsOpenModifier() = false
+
+    override fun modifiersToKotlin(): String {
+        val modifierList = ArrayList<Modifier>()
+        modifiers.accessModifier()?.let { modifierList.add(it) }
+        return modifierList.toKotlin()
+    }
+
 }

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

@@ -1982,6 +1982,11 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("j2k/tests/testData/ast/settings"), Pattern.compile("^(.+)\\.java$"), true);
         }
         
+        @TestMetadata("openByDefault.java")
+        public void testOpenByDefault() throws Exception {
+            doTest("j2k/tests/testData/ast/settings/openByDefault.java");
+        }
+        
         @TestMetadata("specifyLocalVariableTypeByDefault.java")
         public void testSpecifyLocalVariableTypeByDefault() throws Exception {
             doTest("j2k/tests/testData/ast/settings/specifyLocalVariableTypeByDefault.java");

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

@@ -0,0 +1,34 @@
+// !openByDefault: true
+
+open class A() {
+    open fun foo1() {
+    }
+    private fun foo2() {
+    }
+    fun foo3() {
+    }
+}
+
+class B() {
+    fun foo() {
+    }
+}
+
+abstract class C() {
+    abstract fun foo()
+}
+
+trait I {
+    public fun foo()
+}
+
+open class D() : I {
+    override fun foo() {
+    }
+}
+
+enum class E {
+    fun foo(): Int {
+        return 0
+    }
+}

j2k/tests/testData/ast/trait/abstactInterface.kt

@@ -1 +1 @@
-abstract trait I
+trait I

j2k/tests/testData/ast/trait/interfaceWithFields.kt

@@ -1,7 +1,7 @@
 trait INode {
     class object {
-        val IN: String = "in"
+        public val IN: String = "in"
-        val AT: String = "@"
+        public val AT: String = "@"
-        val COMMA_WITH_SPACE: String = COMMA + SPACE
+        public val COMMA_WITH_SPACE: String = COMMA + SPACE
     }
 }

j2k/tests/testData/ast/trait/interfaceWithMethodDeclaration.kt

@@ -1,4 +1,4 @@
 trait INode {
-    fun getTag(): Tag
+    public fun getTag(): Tag
-    fun toKotlin(): String
+    public fun toKotlin(): String
 }