Java to Kotlin converter: access modifiers of factory functions + factory functions for nested classes

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

@@ -120,7 +120,8 @@ class ConstructorConverter(private val psiClass: PsiClass, private val converter
                                                 typeParameterList.parameters,
                                                 Nullability.NotNull,
                                                 converter.settings).assignNoPrototype()
-            return FactoryFunction(constructor.declarationIdentifier(), annotations, modifiers, factoryFunctionType, params, typeParameterList, body)
+            return FactoryFunction(constructor.declarationIdentifier(), annotations, correctFactoryFunctionAccess(modifiers),
+                                   factoryFunctionType, params, typeParameterList, body)
         }
     }
 
@@ -344,6 +345,16 @@ class ConstructorConverter(private val psiClass: PsiClass, private val converter
         return statements
     }
 
+    private fun correctFactoryFunctionAccess(modifiers: Modifiers): Modifiers {
+        val classAccess = converter.convertModifiers(psiClass).accessModifier()
+        return when(modifiers.accessModifier()) {
+            Modifier.PUBLIC -> modifiers.without(Modifier.PUBLIC).with(classAccess)
+            Modifier.PROTECTED -> modifiers.without(Modifier.PROTECTED).with(classAccess)
+            Modifier.PRIVATE -> modifiers
+            else/*internal*/ -> if (classAccess != Modifier.PUBLIC) modifiers.with(classAccess) else modifiers
+        }
+    }
+
     private fun PsiMethodCallExpression.isSuperConstructorCall(): Boolean {
         val ref = getMethodExpression()
         return ref.getCanonicalText() == "super" && ref.resolve()?.isConstructor() ?: false

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

@@ -138,7 +138,10 @@ public class Converter private(val project: Project, val settings: ConverterSett
             else if (member is FactoryFunction) {
                 factoryFunctions.add(member)
             }
-            else if (useClassObject && psiMember !is PsiClass && psiMember.hasModifierProperty(PsiModifier.STATIC)) {
+            else if (useClassObject &&
+                    psiMember.hasModifierProperty(PsiModifier.STATIC) &&
+                    // we generate nested classes with factory functions into class object as a workaround until secondary constructors supported by Kotlin
+                    (psiMember !is PsiClass || (member as Class).body.factoryFunctions.isNotEmpty())) {
                 classObjectMembers.add(member)
             }
             else {
@@ -154,6 +157,10 @@ public class Converter private(val project: Project, val settings: ConverterSett
     // do not convert private static methods into class object if possible
     private fun shouldGenerateClassObject(psiClass: PsiClass, convertedMembers: Map<PsiMember, Member>): Boolean {
         if (psiClass.isEnum()) return false
+
+        // we generate nested classes with factory functions into class object as a workaround until secondary constructors supported by Kotlin
+        if (convertedMembers.values().any { it is Class && !it.modifiers.contains(Modifier.INNER) && it.body.factoryFunctions.isNotEmpty() }) return true
+
         val members = convertedMembers.keySet().filter { !it.isConstructor() }
         val classObjectMembers = members.filter { it !is PsiClass && it.hasModifierProperty(PsiModifier.STATIC) }
         val nestedClasses = members.filterIsInstance(javaClass<PsiClass>()).filter { it.hasModifierProperty(PsiModifier.STATIC) }

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

@@ -33,10 +33,6 @@ enum class Modifier(val name: String) {
 
 val ACCESS_MODIFIERS = setOf(Modifier.PUBLIC, Modifier.PROTECTED, Modifier.PRIVATE)
 
-fun Collection<Modifier>.accessModifier(): Modifier? {
-    return firstOrNull { it in ACCESS_MODIFIERS }
-}
-
 class Modifiers(val modifiers: Collection<Modifier>) : Element() {
     override fun generateCode(builder: CodeBuilder) {
         builder.append(modifiers.sortBy { it.ordinal() }.map { it.toKotlin() }.joinToString(" "))
@@ -45,7 +41,7 @@ class Modifiers(val modifiers: Collection<Modifier>) : Element() {
     override val isEmpty: Boolean
         get() = modifiers.isEmpty()
 
-    fun with(modifier: Modifier): Modifiers = Modifiers(modifiers + listOf(modifier)).assignPrototypesFrom(this)
+    fun with(modifier: Modifier?): Modifiers = if (modifier != null) Modifiers(modifiers + listOf(modifier)).assignPrototypesFrom(this) else this
 
     fun without(modifier: Modifier): Modifiers {
         val set = HashSet(modifiers)
@@ -58,7 +54,9 @@ class Modifiers(val modifiers: Collection<Modifier>) : Element() {
     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
+    val isInternal: Boolean get() = accessModifier() == null
+
+    fun accessModifier(): Modifier? = modifiers.firstOrNull { it in ACCESS_MODIFIERS }
 
     class object {
         val Empty = Modifiers(listOf())

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

@@ -16,17 +16,14 @@
 
 package org.jetbrains.jet.j2k.test;
 
-import junit.framework.Assert;
 import junit.framework.Test;
 import junit.framework.TestSuite;
-
-import java.io.File;
-import java.util.regex.Pattern;
 import org.jetbrains.jet.JetTestUtils;
 import org.jetbrains.jet.test.InnerTestClasses;
 import org.jetbrains.jet.test.TestMetadata;
 
-import org.jetbrains.jet.j2k.test.AbstractJavaToKotlinConverterTest;
+import java.io.File;
+import java.util.regex.Pattern;
 
 /** This class is generated by {@link org.jetbrains.jet.generators.tests.TestsPackage}. DO NOT MODIFY MANUALLY */
 @SuppressWarnings("all")
@@ -891,6 +888,21 @@ public class JavaToKotlinConverterTestGenerated extends AbstractJavaToKotlinConv
             doTest("j2k/tests/testData/ast/constructors/qualifiedRefInFactoryFun.java");
         }
         
+        @TestMetadata("secondaryConstructorsInInnerClass.java")
+        public void testSecondaryConstructorsInInnerClass() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/secondaryConstructorsInInnerClass.java");
+        }
+        
+        @TestMetadata("secondaryConstructorsInNestedClass.java")
+        public void testSecondaryConstructorsInNestedClass() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/secondaryConstructorsInNestedClass.java");
+        }
+        
+        @TestMetadata("secondaryConstructorsVisibility.java")
+        public void testSecondaryConstructorsVisibility() throws Exception {
+            doTest("j2k/tests/testData/ast/constructors/secondaryConstructorsVisibility.java");
+        }
+        
         @TestMetadata("staticFieldRefInFactoryFun.java")
         public void testStaticFieldRefInFactoryFun() throws Exception {
             doTest("j2k/tests/testData/ast/constructors/staticFieldRefInFactoryFun.java");

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

@@ -2,7 +2,7 @@ private fun C(arg1: Int, arg2: Int): C {
     return C(arg1, arg2, 0)
 }
 
-public fun C(arg1: Int): C {
+fun C(arg1: Int): C {
     return C(arg1, 0, 0)
 }
 

j2k/tests/testData/ast/constructors/secondaryConstructorsInInnerClass.java

@@ -0,0 +1,74 @@
+//file
+class Outer {
+    private class Inner1 {
+        public Inner1(){}
+
+        public Inner1(int a) {
+            this();
+        }
+
+        protected Inner1(char c) {
+            this();
+        }
+
+        private Inner1(boolean b) {
+            this();
+        }
+    }
+
+    protected class Inner2 {
+        public Inner2(){}
+
+        public Inner2(int a) {
+            this();
+        }
+
+        protected Inner2(char c) {
+            this();
+        }
+
+        private Inner2(boolean b) {
+            this();
+        }
+
+    }
+
+    class Inner3 {
+        public Inner3(){}
+
+        public Inner3(int a) {
+            this();
+        }
+
+        protected Inner3(char c) {
+            this();
+        }
+
+        private Inner3(boolean b) {
+            this();
+        }
+    }
+
+    public class Inner4 {
+        public Inner4(){}
+
+        public Inner4(int a) {
+            this();
+        }
+
+        protected Inner4(char c) {
+            this();
+        }
+
+        private Inner4(boolean b) {
+            this();
+        }
+    }
+
+    void foo() {
+        Inner1 inner1 = new Inner1(1);
+        Inner2 inner2 = new Inner2(2);
+        Inner3 inner3 = new Inner3(3);
+        Inner4 inner4 = new Inner4(4);
+    }
+}

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

@@ -0,0 +1,68 @@
+class Outer {
+
+    private fun Inner1(a: Int): Inner1 {
+        return Inner1()
+    }
+
+    private fun Inner1(c: Char): Inner1 {
+        return Inner1()
+    }
+
+    private fun Inner1(b: Boolean): Inner1 {
+        return Inner1()
+    }
+
+    private inner class Inner1
+
+
+    protected fun Inner2(a: Int): Inner2 {
+        return Inner2()
+    }
+
+    protected fun Inner2(c: Char): Inner2 {
+        return Inner2()
+    }
+
+    private fun Inner2(b: Boolean): Inner2 {
+        return Inner2()
+    }
+
+    protected inner class Inner2
+
+
+    fun Inner3(a: Int): Inner3 {
+        return Inner3()
+    }
+
+    fun Inner3(c: Char): Inner3 {
+        return Inner3()
+    }
+
+    private fun Inner3(b: Boolean): Inner3 {
+        return Inner3()
+    }
+
+    inner class Inner3
+
+
+    public fun Inner4(a: Int): Inner4 {
+        return Inner4()
+    }
+
+    public fun Inner4(c: Char): Inner4 {
+        return Inner4()
+    }
+
+    private fun Inner4(b: Boolean): Inner4 {
+        return Inner4()
+    }
+
+    public inner class Inner4
+
+    fun foo() {
+        val inner1 = Inner1(1)
+        val inner2 = Inner2(2)
+        val inner3 = Inner3(3)
+        val inner4 = Inner4(4)
+    }
+}

j2k/tests/testData/ast/constructors/secondaryConstructorsInNestedClass.java

@@ -0,0 +1,74 @@
+//file
+class Outer {
+    private static class Nested1 {
+        public Nested1(){}
+
+        public Nested1(int a) {
+            this();
+        }
+
+        protected Nested1(char c) {
+            this();
+        }
+
+        private Nested1(boolean b) {
+            this();
+        }
+    }
+
+    protected static class Nested2 {
+        public Nested2(){}
+
+        public Nested2(int a) {
+            this();
+        }
+
+        protected Nested2(char c) {
+            this();
+        }
+
+        private Nested2(boolean b) {
+            this();
+        }
+
+    }
+
+    static class Nested3 {
+        public Nested3(){}
+
+        public Nested3(int a) {
+            this();
+        }
+
+        protected Nested3(char c) {
+            this();
+        }
+
+        private Nested3(boolean b) {
+            this();
+        }
+    }
+
+    public static class Nested4 {
+        public Nested4(){}
+
+        public Nested4(int a) {
+            this();
+        }
+
+        protected Nested4(char c) {
+            this();
+        }
+
+        private Nested4(boolean b) {
+            this();
+        }
+    }
+
+    static void foo() {
+        Nested1 nested1 = new Nested1(1);
+        Nested2 nested2 = new Nested2(2);
+        Nested3 nested3 = new Nested3(3);
+        Nested4 nested4 = new Nested4(4);
+    }
+}

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

@@ -0,0 +1,70 @@
+class Outer {
+    class object {
+
+        private fun Nested1(a: Int): Nested1 {
+            return Nested1()
+        }
+
+        private fun Nested1(c: Char): Nested1 {
+            return Nested1()
+        }
+
+        private fun Nested1(b: Boolean): Nested1 {
+            return Nested1()
+        }
+
+        private class Nested1
+
+
+        protected fun Nested2(a: Int): Nested2 {
+            return Nested2()
+        }
+
+        protected fun Nested2(c: Char): Nested2 {
+            return Nested2()
+        }
+
+        private fun Nested2(b: Boolean): Nested2 {
+            return Nested2()
+        }
+
+        protected class Nested2
+
+
+        fun Nested3(a: Int): Nested3 {
+            return Nested3()
+        }
+
+        fun Nested3(c: Char): Nested3 {
+            return Nested3()
+        }
+
+        private fun Nested3(b: Boolean): Nested3 {
+            return Nested3()
+        }
+
+        class Nested3
+
+
+        public fun Nested4(a: Int): Nested4 {
+            return Nested4()
+        }
+
+        public fun Nested4(c: Char): Nested4 {
+            return Nested4()
+        }
+
+        private fun Nested4(b: Boolean): Nested4 {
+            return Nested4()
+        }
+
+        public class Nested4
+
+        fun foo() {
+            val nested1 = Nested1(1)
+            val nested2 = Nested2(2)
+            val nested3 = Nested3(3)
+            val nested4 = Nested4(4)
+        }
+    }
+}

j2k/tests/testData/ast/constructors/secondaryConstructorsVisibility.java

@@ -0,0 +1,24 @@
+//file
+class A {
+    public A() {}
+
+    public A(int a) { this(); }
+
+    protected A(char c) { this(); }
+
+    A(float f) { this(); }
+
+    private A(double d) { this(); }
+}
+
+public class B {
+    public B() {}
+
+    public B(int a) { this(); }
+
+    protected B(char c) { this(); }
+
+    B(float f) { this(); }
+
+    private B(double d) { this(); }
+}

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

@@ -0,0 +1,36 @@
+fun A(a: Int): A {
+    return A()
+}
+
+fun A(c: Char): A {
+    return A()
+}
+
+fun A(f: Float): A {
+    return A()
+}
+
+private fun A(d: Double): A {
+    return A()
+}
+
+class A
+
+
+public fun B(a: Int): B {
+    return B()
+}
+
+public fun B(c: Char): B {
+    return B()
+}
+
+fun B(f: Float): B {
+    return B()
+}
+
+private fun B(d: Double): B {
+    return B()
+}
+
+public class B