Refactoring (+ slightly more correct logic)

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

@@ -201,12 +201,11 @@ class ConstructorConverter(
                     else {
                         val (field, type) = parameterToField[parameter]!!
                         val propertyInfo = fieldToPropertyInfo(field)!!
-                        val parameterModifiers = converter.convertModifiers(propertyInfo, classKind)
                         FunctionParameter(propertyInfo.identifier,
                                           type,
                                           if (propertyInfo.isVar) FunctionParameter.VarValModifier.Var else FunctionParameter.VarValModifier.Val,
                                           converter.convertAnnotations(parameter) + converter.convertAnnotations(field),
-                                          parameterModifiers,
+                                          propertyInfo.modifiers,
                                           default)
                                 .assignPrototypes(
                                         PrototypeInfo(parameter, CommentsAndSpacesInheritance.LINE_BREAKS),

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

@@ -30,7 +30,6 @@ import org.jetbrains.kotlin.j2k.usageProcessing.UsageProcessing
 import org.jetbrains.kotlin.j2k.usageProcessing.UsageProcessingExpressionConverter
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.types.expressions.OperatorConventions.*
-import org.jetbrains.kotlin.utils.addIfNotNull
 import org.jetbrains.kotlin.utils.addToStdlib.check
 import org.jetbrains.kotlin.utils.addToStdlib.singletonOrEmptyList
 import java.util.*
@@ -109,7 +108,7 @@ class Converter private constructor(
         is PsiJavaFile -> convertFile(element)
         is PsiClass -> convertClass(element)
         is PsiMethod -> convertMethod(element, null, null, null, ClassKind.FINAL_CLASS)
-        is PsiField -> convertProperty(PropertyInfo.fromFieldWithNoAccessors(element, element.isVar(referenceSearcher)), ClassKind.FINAL_CLASS)
+        is PsiField -> convertProperty(PropertyInfo.fromFieldWithNoAccessors(element, this), ClassKind.FINAL_CLASS)
         is PsiStatement -> createDefaultCodeConverter().convertStatement(element)
         is PsiExpression -> createDefaultCodeConverter().convertExpression(element)
         is PsiImportList -> convertImportList(element)
@@ -208,7 +207,7 @@ class Converter private constructor(
         }.assignPrototype(psiClass)
     }
 
-    private fun needOpenModifier(psiClass: PsiClass): Boolean {
+    public fun needOpenModifier(psiClass: PsiClass): Boolean {
         return if (psiClass.hasModifierProperty(PsiModifier.FINAL) || psiClass.hasModifierProperty(PsiModifier.ABSTRACT))
             false
         else
@@ -309,7 +308,7 @@ class Converter private constructor(
         //TODO: annotations from getter/setter?
         val annotations = field?.let { convertAnnotations(it) } ?: Annotations.Empty
 
-        val modifiers = convertModifiers(propertyInfo, classKind)
+        val modifiers = propertyInfo.modifiers
 
         val name = propertyInfo.identifier
         if (field is PsiEnumConstant) {
@@ -347,7 +346,7 @@ class Converter private constructor(
                     getter = PropertyAccessor(AccessorKind.GETTER, method.annotations, Modifiers.Empty, method.parameterList, method.body)
                     getter.assignPrototype(getMethod, CommentsAndSpacesInheritance.NO_SPACES)
                 }
-                else if (propertyInfo.isOverride) {
+                else if (propertyInfo.modifiers.contains(Modifier.OVERRIDE)) {
                     val superExpression = SuperExpression(Identifier.Empty).assignNoPrototype()
                     val superAccess = QualifiedExpression(superExpression, propertyInfo.identifier).assignNoPrototype()
                     val returnStatement = ReturnStatement(superAccess).assignNoPrototype()
@@ -372,7 +371,7 @@ class Converter private constructor(
                             method.body)
                     setter.assignPrototype(setMethod, CommentsAndSpacesInheritance.NO_SPACES)
                 }
-                else if (propertyInfo.isOverride) {
+                else if (propertyInfo.modifiers.contains(Modifier.OVERRIDE)) {
                     val superExpression = SuperExpression(Identifier.Empty).assignNoPrototype()
                     val superAccess = QualifiedExpression(superExpression, propertyInfo.identifier).assignNoPrototype()
                     val valueIdentifier = Identifier("value", false).assignNoPrototype()
@@ -669,41 +668,6 @@ class Converter private constructor(
         return without(Modifier.INTERNAL).with(Modifier.PUBLIC)
     }
 
-    public fun convertModifiers(propertyInfo: PropertyInfo, classKind: ClassKind): Modifiers {
-        val fieldModifiers = propertyInfo.field?.let { convertModifiers(it, false) } ?: Modifiers.Empty
-        val getterModifiers = propertyInfo.getMethod?.let { convertModifiers(it, classKind.isOpen()) } ?: Modifiers.Empty
-        val setterModifiers = propertyInfo.setMethod?.let { convertModifiers(it, classKind.isOpen()) } ?: Modifiers.Empty
-
-        val modifiers = ArrayList<Modifier>()
-
-        if (propertyInfo.isAbstract) {
-            modifiers.add(Modifier.ABSTRACT)
-        }
-
-        if (getterModifiers.contains(Modifier.OPEN) || setterModifiers.contains(Modifier.OPEN)) {
-            modifiers.add(Modifier.OPEN)
-        }
-
-        if (propertyInfo.isOverride) {
-            modifiers.add(Modifier.OVERRIDE)
-        }
-
-        if (propertyInfo.getMethod != null) {
-            modifiers.addIfNotNull(getterModifiers.accessModifier())
-        }
-        else if (propertyInfo.setMethod != null) {
-            modifiers.addIfNotNull(getterModifiers.accessModifier())
-        }
-        else {
-            modifiers.addIfNotNull(fieldModifiers.accessModifier())
-        }
-
-        val prototypes = listOf<PsiElement?>(propertyInfo.field, propertyInfo.getMethod, propertyInfo.setMethod)
-                .filterNotNull()
-                .map { PrototypeInfo(it, CommentsAndSpacesInheritance.NO_SPACES) }
-        return Modifiers(modifiers).assignPrototypes(*prototypes.toTypedArray())
-    }
-
     public fun convertAnonymousClassBody(anonymousClass: PsiAnonymousClass): AnonymousClassBody {
         return AnonymousClassBody(ClassBodyConverter(anonymousClass, ClassKind.ANONYMOUS_OBJECT, this).convertBody(),
                                   anonymousClass.getBaseClassType().resolve()?.isInterface() ?: false).assignPrototype(anonymousClass)

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

@@ -19,14 +19,13 @@ package org.jetbrains.kotlin.j2k
 import com.intellij.psi.*
 import com.intellij.psi.util.MethodSignatureUtil
 import org.jetbrains.kotlin.asJava.KotlinLightMethod
-import org.jetbrains.kotlin.j2k.ast.Identifier
+import org.jetbrains.kotlin.j2k.ast.*
-import org.jetbrains.kotlin.j2k.ast.Modifier
-import org.jetbrains.kotlin.j2k.ast.assignNoPrototype
-import org.jetbrains.kotlin.j2k.ast.declarationIdentifier
 import org.jetbrains.kotlin.load.java.JvmAbi
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.JetProperty
 import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor
+import org.jetbrains.kotlin.utils.addIfNotNull
+import org.jetbrains.kotlin.utils.addToStdlib.check
 import java.util.*
 
 class PropertyInfo(
@@ -38,9 +37,8 @@ class PropertyInfo(
         val setMethod: PsiMethod?,
         val isGetMethodBodyFieldAccess: Boolean,
         val isSetMethodBodyFieldAccess: Boolean,
-        val specialSetterAccess: Modifier?,
+        val modifiers: Modifiers,
-        val isOverride: Boolean,
+        val specialSetterAccess: Modifier?
-        val isAbstract: Boolean //TODO: modifiers here
 ) {
     init {
         assert(field != null || getMethod != null || setMethod != null)
@@ -59,7 +57,7 @@ class PropertyInfo(
     val needExplicitGetter: Boolean
         get() {
             if (getMethod != null && getMethod.body != null && !isGetMethodBodyFieldAccess) return true
-            return isOverride && this.field == null && !isAbstract
+            return modifiers.contains(Modifier.OVERRIDE) && this.field == null && !modifiers.contains(Modifier.ABSTRACT)
         }
 
     //TODO: what if annotations are not empty?
@@ -68,12 +66,15 @@ class PropertyInfo(
             if (!isVar) return false
             if (specialSetterAccess != null) return true
             if (setMethod != null && setMethod.body != null && !isSetMethodBodyFieldAccess) return true
-            return isOverride && this.field == null && !isAbstract
+            return modifiers.contains(Modifier.OVERRIDE) && this.field == null && !modifiers.contains(Modifier.ABSTRACT)
         }
 
     companion object {
-        fun fromFieldWithNoAccessors(field: PsiField, isVar: Boolean)
+        fun fromFieldWithNoAccessors(field: PsiField, converter: Converter): PropertyInfo {
-                = PropertyInfo(field.declarationIdentifier(), isVar, field.type, field, null, null, false, false, null, false, false)
+            val isVar = field.isVar(converter.referenceSearcher)
+            val modifiers = converter.convertModifiers(field, false)
+            return PropertyInfo(field.declarationIdentifier(), isVar, field.type, field, null, null, false, false, modifiers, null)
+        }
     }
 }
 
@@ -95,6 +96,8 @@ private class PropertyDetector(
         private val psiClass: PsiClass,
         private val converter: Converter
 ) {
+    private val isOpenClass = converter.needOpenModifier(psiClass)
+
     public fun detectProperties(): Map<PsiMember, PropertyInfo> {
         val propertyNameToGetterInfo = LinkedHashMap<String, AccessorInfo>()
         val propertyNameToSetterInfo = LinkedHashMap<String, AccessorInfo>()
@@ -139,18 +142,6 @@ private class PropertyDetector(
                 field = null
             }
 
-            var specialSetterAccess: Modifier? = null
-            val getterAccess = if (getterInfo != null) converter.convertModifiers(getterInfo.method, false).accessModifier() else Modifier.PUBLIC //TODO
-            val setterAccess = if (setterInfo != null)
-                converter.convertModifiers(setterInfo.method, false).accessModifier()
-            else if (field != null && field.isVar(converter.referenceSearcher))
-                converter.convertModifiers(field, false).accessModifier()
-            else
-                getterAccess
-            if (setterAccess != getterAccess) {
-                specialSetterAccess = setterAccess
-            }
-
             //TODO: no body for getter OR setter
 
             val isVar = if (setterInfo != null)
@@ -164,10 +155,16 @@ private class PropertyDetector(
 
             val isOverride = getterInfo?.superProperty != null || setterInfo?.superProperty != null
 
-            //TODO: what if one is abstract and another is not?
+            val modifiers = convertModifiers(field, getterInfo?.method, setterInfo?.method, isOverride)
-            val isGetterAbstract = getterInfo?.method?.hasModifierProperty(PsiModifier.ABSTRACT) ?: true
+
-            val isSetterAbstract = setterInfo?.method?.hasModifierProperty(PsiModifier.ABSTRACT) ?: true
+            val propertyAccess = modifiers.accessModifier()
-            val isAbstract = field == null && isGetterAbstract && isSetterAbstract
+            val setterAccess = if (setterInfo != null)
+                converter.convertModifiers(setterInfo.method, false).accessModifier()
+            else if (field != null && field.isVar(converter.referenceSearcher))
+                converter.convertModifiers(field, false).accessModifier()
+            else
+                propertyAccess
+            val specialSetterAccess = setterAccess?.check { it != propertyAccess }
 
             val propertyInfo = PropertyInfo(Identifier(propertyName).assignNoPrototype(),
                                             isVar,
@@ -177,9 +174,8 @@ private class PropertyDetector(
                                             setterInfo?.method,
                                             field != null && getterInfo?.field == field,
                                             field != null && setterInfo?.field == field,
-                                            specialSetterAccess,
+                                            modifiers,
-                                            isOverride,
+                                            specialSetterAccess)
-                                            isAbstract)
 
             if (field != null) {
                 memberToPropertyInfo[field] = propertyInfo
@@ -202,8 +198,7 @@ private class PropertyDetector(
         // map all other fields
         for (field in psiClass.fields) {
             if (field !in mappedFields) {
-                val propertyInfo = PropertyInfo.fromFieldWithNoAccessors(field, field.isVar(converter.referenceSearcher))
+                memberToPropertyInfo[field] = PropertyInfo.fromFieldWithNoAccessors(field, converter)
-                memberToPropertyInfo[field] = propertyInfo
             }
         }
 
@@ -228,7 +223,7 @@ private class PropertyDetector(
         }
 
         for (propertyInfo in propertyInfos) {
-            if (propertyInfo.isOverride) continue // cannot drop override
+            if (propertyInfo.modifiers.contains(Modifier.OVERRIDE)) continue // cannot drop override
 
             //TODO: test this case
             val getterName = JvmAbi.getterName(propertyInfo.name)
@@ -249,6 +244,44 @@ private class PropertyDetector(
         }
     }
 
+    private fun convertModifiers(field: PsiField?, getMethod: PsiMethod?, setMethod: PsiMethod?, isOverride: Boolean): Modifiers {
+        val fieldModifiers = field?.let { converter.convertModifiers(it, false) } ?: Modifiers.Empty
+        val getterModifiers = getMethod?.let { converter.convertModifiers(it, isOpenClass) } ?: Modifiers.Empty
+        val setterModifiers = setMethod?.let { converter.convertModifiers(it, isOpenClass) } ?: Modifiers.Empty
+
+        val modifiers = ArrayList<Modifier>()
+
+        //TODO: what if one is abstract and another is not?
+        val isGetterAbstract = getMethod?.hasModifierProperty(PsiModifier.ABSTRACT) ?: true
+        val isSetterAbstract = setMethod?.hasModifierProperty(PsiModifier.ABSTRACT) ?: true
+        if (field == null && isGetterAbstract && isSetterAbstract) {
+            modifiers.add(Modifier.ABSTRACT)
+        }
+
+        if (getterModifiers.contains(Modifier.OPEN) || setterModifiers.contains(Modifier.OPEN)) {
+            modifiers.add(Modifier.OPEN)
+        }
+
+        if (isOverride) {
+            modifiers.add(Modifier.OVERRIDE)
+        }
+
+        if (getMethod != null) {
+            modifiers.addIfNotNull(getterModifiers.accessModifier())
+        }
+        else if (setMethod != null) {
+            modifiers.addIfNotNull(getterModifiers.accessModifier())
+        }
+        else {
+            modifiers.addIfNotNull(fieldModifiers.accessModifier())
+        }
+
+        val prototypes = listOf<PsiElement?>(field, getMethod, setMethod)
+                .filterNotNull()
+                .map { PrototypeInfo(it, CommentsAndSpacesInheritance.NO_SPACES) }
+        return Modifiers(modifiers).assignPrototypes(*prototypes.toTypedArray())
+    }
+
     private class AccessorInfo(
             val method: PsiMethod,
             val field: PsiField?,
@@ -278,23 +311,22 @@ private class PropertyDetector(
 
     private fun createAccessorInfo(getOrSetMethod: PsiMethod, field: PsiField?, kind: AccessorKind, propertyName: String): AccessorInfo? {
         //TODO: multiple
-        for (superMethod in converter.services.superMethodsSearcher.findDeepestSuperMethods(getOrSetMethod)) {
+        val superMethod = converter.services.superMethodsSearcher.findDeepestSuperMethods(getOrSetMethod).firstOrNull()
-            val containingClass = superMethod.containingClass!!
+                          ?: return AccessorInfo(getOrSetMethod, field, kind, propertyName, null)
-            val superPropertyInfo: SuperPropertyInfo? = if (converter.inConversionScope(containingClass)) {
-                val propertyInfo = converter.propertyDetectionCache[containingClass][superMethod]
-                if (propertyInfo != null) SuperPropertyInfo(propertyInfo.isVar) else null
-            }
-            else if (superMethod is KotlinLightMethod) {
-                val origin = superMethod.getOrigin()
-                if (origin is JetProperty) SuperPropertyInfo(origin.isVar) else null
-            }
-            else {
-                null
-            }
-            return superPropertyInfo?.let { AccessorInfo(getOrSetMethod, field, kind, propertyName, it) }
-        }
 
-        return AccessorInfo(getOrSetMethod, field, kind, propertyName, null)
+        val containingClass = superMethod.containingClass!!
+        val superPropertyInfo: SuperPropertyInfo? = if (converter.inConversionScope(containingClass)) {
+            val propertyInfo = converter.propertyDetectionCache[containingClass][superMethod]
+            if (propertyInfo != null) SuperPropertyInfo(propertyInfo.isVar) else null
+        }
+        else if (superMethod is KotlinLightMethod) {
+            val origin = superMethod.getOrigin()
+            if (origin is JetProperty) SuperPropertyInfo(origin.isVar) else null
+        }
+        else {
+            null
+        }
+        return superPropertyInfo?.let { AccessorInfo(getOrSetMethod, field, kind, propertyName, it) }
     }
 
     private fun propertyNameByGetMethod(method: PsiMethod): String? {