Minor changes on code review

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

@@ -114,7 +114,7 @@ class Converter private constructor(
         is PsiStatement -> createDefaultCodeConverter().convertStatement(element)
         is PsiExpression -> createDefaultCodeConverter().convertExpression(element)
         is PsiImportList -> convertImportList(element)
-        is PsiImportStatementBase -> convertImport(element, false).singleOrNull()
+        is PsiImportStatementBase -> convertImport(element, dumpConversion = true).singleOrNull()
         is PsiAnnotation -> annotationConverter.convertAnnotation(element, newLineAfter = false)
         is PsiPackageStatement -> PackageStatement(quoteKeywords(element.packageName ?: "")).assignPrototype(element)
         is PsiJavaCodeReferenceElement -> {

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

@@ -16,10 +16,7 @@
 
 package org.jetbrains.kotlin.j2k
 
-import com.intellij.psi.PsiImportList
+import com.intellij.psi.*
-import com.intellij.psi.PsiImportStatementBase
-import com.intellij.psi.PsiImportStaticStatement
-import com.intellij.psi.PsiJavaCodeReferenceElement
 import org.jetbrains.kotlin.asJava.KtLightClass
 import org.jetbrains.kotlin.asJava.KtLightClassForFacade
 import org.jetbrains.kotlin.asJava.KtLightDeclaration
@@ -39,29 +36,26 @@ import org.jetbrains.kotlin.utils.singletonOrEmptyList
 
 fun Converter.convertImportList(importList: PsiImportList): ImportList {
     val imports = importList.allImportStatements
-            .flatMap { convertImport(it, true) }
+            .flatMap { convertImport(it) }
             .distinctBy { it.name } // duplicated imports may appear
     return ImportList(imports).assignPrototype(importList)
 }
 
-fun Converter.convertImport(anImport: PsiImportStatementBase, filter: Boolean): List<Import> {
+fun Converter.convertImport(anImport: PsiImportStatementBase, dumpConversion: Boolean = false): List<Import> {
-    fun doConvert(): List<Import> {
+    val reference = anImport.importReference ?: return emptyList()
-        val reference = anImport.importReference ?: return emptyList()
+    val fqName = FqName(reference.qualifiedName!!)
-        val fqName = FqName(reference.qualifiedName!!)
+    val onDemand = anImport.isOnDemand
-        val onDemand = anImport.isOnDemand
+    val convertedImports = if (dumpConversion) {
-        return if (filter) {
+        listOf(Import(renderImportName(fqName, onDemand)))
-            filterImport(fqName, reference, onDemand, anImport is PsiImportStaticStatement)
-                    .map { Import(it) }
-        }
-        else {
-            listOf(Import(renderImportName(fqName, onDemand)))
-        }
     }
-
+    else {
-    return doConvert().map { it.assignPrototype(anImport) }
+        convertImport(fqName, reference, onDemand, anImport is PsiImportStaticStatement)
+                .map { Import(it) }
+    }
+    return convertedImports.map { it.assignPrototype(anImport) }
 }
 
-private fun Converter.filterImport(fqName: FqName, ref: PsiJavaCodeReferenceElement, isOnDemand: Boolean, isImportStatic: Boolean): List<String> {
+private fun Converter.convertImport(fqName: FqName, ref: PsiJavaCodeReferenceElement, isOnDemand: Boolean, isImportStatic: Boolean): List<String> {
     if (!isOnDemand) {
         if (annotationConverter.isImportNotRequired(fqName)) return emptyList()
 
@@ -73,76 +67,89 @@ private fun Converter.filterImport(fqName: FqName, ref: PsiJavaCodeReferenceElem
     val target = ref.resolve()
     if (isImportStatic) {
         if (isOnDemand) {
-            when (target) {
+            return convertStaticImportOnDemand(fqName, target)
-                is KtLightClassForFacade -> return listOf(target.getFqName().parent().render() + ".*")
-
-                is KtLightClass -> {
-                    val kotlinOrigin = target.kotlinOrigin
-                    val importFromObject = when (kotlinOrigin) {
-                        is KtObjectDeclaration -> kotlinOrigin
-                        is KtClass -> kotlinOrigin.getCompanionObjects().singleOrNull()
-                        else -> null
-                    }
-                    if (importFromObject != null) {
-                        val objectFqName = importFromObject.fqName
-                        if (objectFqName != null) {
-                            // cannot import on demand from object, generate imports for all members
-                            return importFromObject.declarations
-                                    .mapNotNull {
-                                        val descriptor = services.resolverForConverter.resolveToDescriptor(it) ?: return@mapNotNull null
-                                        if (descriptor.hasJvmStaticAnnotation() || descriptor is PropertyDescriptor && descriptor.isConst)
-                                            descriptor.name
-                                        else
-                                            null
-                                    }
-                                    .distinct()
-                                    .map { objectFqName.child(it).render() }
-                        }
-                    }
-                }
-            }
         }
         else {
-            if (target is KtLightDeclaration<*, *>) {
+            return convertStaticExplicitImport(fqName, target)
-                val kotlinOrigin = target.kotlinOrigin
-
-                val nameToImport = if (target is KtLightMethod && kotlinOrigin is KtProperty)
-                    kotlinOrigin.nameAsName
-                else
-                    fqName.shortName()
-
-                if (nameToImport != null) {
-                    val originParent = kotlinOrigin?.parent
-                    when (originParent) {
-                        is KtFile -> { // import of function or property accessor from file facade
-                            return listOf(originParent.packageFqName.child(nameToImport).render())
-                        }
-
-                        is KtClassBody -> {
-                            val parentClass = originParent.parent as KtClassOrObject
-                            if (parentClass is KtObjectDeclaration && parentClass.isCompanion()) {
-                                return parentClass.getFqName()?.child(nameToImport)?.render().singletonOrEmptyList()
-                            }
-                        }
-                    }
-                }
-            }
         }
     }
     else {
-        when (target) {
+        return convertNonStaticImport(fqName, isOnDemand, target)
-            is KtLightClassForFacade -> return listOf(target.getFqName().parent().render() + ".*")
+    }
+}
 
-            is KtLightClass -> {
+private fun Converter.convertStaticImportOnDemand(fqName: FqName, target: PsiElement?): List<String> {
-                if (!isOnDemand) {
+    when (target) {
-                    if (isFacadeClassFromLibrary(target)) return emptyList()
+        is KtLightClassForFacade -> return listOf(target.getFqName().parent().render() + ".*")
 
-                    if (isImportedByDefault(target)) return emptyList()
+        is KtLightClass -> {
+            val kotlinOrigin = target.kotlinOrigin
+            val importFromObject = when (kotlinOrigin) {
+                is KtObjectDeclaration -> kotlinOrigin
+                is KtClass -> kotlinOrigin.getCompanionObjects().singleOrNull()
+                else -> null
+            }
+            if (importFromObject != null) {
+                val objectFqName = importFromObject.fqName
+                if (objectFqName != null) {
+                    // cannot import on demand from object, generate imports for all members
+                    return importFromObject.declarations
+                            .mapNotNull {
+                                val descriptor = services.resolverForConverter.resolveToDescriptor(it) ?: return@mapNotNull null
+                                if (descriptor.hasJvmStaticAnnotation() || descriptor is PropertyDescriptor && descriptor.isConst)
+                                    descriptor.name
+                                else
+                                    null
+                            }
+                            .distinct()
+                            .map { objectFqName.child(it).render() }
                 }
             }
         }
     }
+    return listOf(renderImportName(fqName, isOnDemand = true))
+}
 
+private fun convertStaticExplicitImport(fqName: FqName, target: PsiElement?): List<String> {
+    if (target is KtLightDeclaration<*, *>) {
+        val kotlinOrigin = target.kotlinOrigin
+
+        val nameToImport = if (target is KtLightMethod && kotlinOrigin is KtProperty)
+            kotlinOrigin.nameAsName
+        else
+            fqName.shortName()
+
+        if (nameToImport != null) {
+            val originParent = kotlinOrigin?.parent
+            when (originParent) {
+                is KtFile -> { // import of function or property accessor from file facade
+                    return listOf(originParent.packageFqName.child(nameToImport).render())
+                }
+
+                is KtClassBody -> {
+                    val parentClass = originParent.parent as KtClassOrObject
+                    if (parentClass is KtObjectDeclaration && parentClass.isCompanion()) {
+                        return parentClass.getFqName()?.child(nameToImport)?.render().singletonOrEmptyList()
+                    }
+                }
+            }
+        }
+    }
+    return listOf(renderImportName(fqName, isOnDemand = false))
+}
+
+private fun convertNonStaticImport(fqName: FqName, isOnDemand: Boolean, target: PsiElement?): List<String> {
+    when (target) {
+        is KtLightClassForFacade -> return listOf(target.getFqName().parent().render() + ".*")
+
+        is KtLightClass -> {
+            if (!isOnDemand) {
+                if (isFacadeClassFromLibrary(target)) return emptyList()
+
+                if (isImportedByDefault(target)) return emptyList()
+            }
+        }
+    }
     return listOf(renderImportName(fqName, isOnDemand))
 }
 

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

@@ -329,7 +329,7 @@ private class PropertyDetector(
     private fun dropPropertiesConflictingWithFields(memberToPropertyInfo: MutableMap<PsiMember, PropertyInfo>) {
         val fieldsByName = psiClass.fields.associateBy { it.name } //TODO: fields from base
 
-        fun isPropertyNameProhibited(name: String): Boolean {
+        fun isPropertyNameInUse(name: String): Boolean {
             val field = fieldsByName[name] ?: return false
             return !memberToPropertyInfo.containsKey(field)
         }
@@ -338,7 +338,7 @@ private class PropertyDetector(
         do {
             repeatLoop = false
             for (propertyInfo in memberToPropertyInfo.values.distinct()) {
-                if (isPropertyNameProhibited(propertyInfo.name)) {
+                if (isPropertyNameInUse(propertyInfo.name)) {
                     //TODO: what about overrides in this case?
                     memberToPropertyInfo.dropProperty(propertyInfo)