From 74b41cff0e3359530d316d83361b7aa9437a8284 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Fri, 29 Apr 2016 18:03:46 +0300 Subject: [PATCH] Minor changes on code review --- j2k/src/org/jetbrains/kotlin/j2k/Converter.kt | 2 +- .../jetbrains/kotlin/j2k/importConversion.kt | 163 +++++++++--------- .../jetbrains/kotlin/j2k/propertyDetection.kt | 4 +- 3 files changed, 88 insertions(+), 81 deletions(-) diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt index 8658f9d667c..cf8eb23a761 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt +++ b/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 -> { diff --git a/j2k/src/org/jetbrains/kotlin/j2k/importConversion.kt b/j2k/src/org/jetbrains/kotlin/j2k/importConversion.kt index dadcd8f7b3f..fc7b8e015dc 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/importConversion.kt +++ b/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.PsiImportStatementBase -import com.intellij.psi.PsiImportStaticStatement -import com.intellij.psi.PsiJavaCodeReferenceElement +import com.intellij.psi.* 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 { - fun doConvert(): List { - val reference = anImport.importReference ?: return emptyList() - val fqName = FqName(reference.qualifiedName!!) - val onDemand = anImport.isOnDemand - return if (filter) { - filterImport(fqName, reference, onDemand, anImport is PsiImportStaticStatement) - .map { Import(it) } - } - else { - listOf(Import(renderImportName(fqName, onDemand))) - } +fun Converter.convertImport(anImport: PsiImportStatementBase, dumpConversion: Boolean = false): List { + val reference = anImport.importReference ?: return emptyList() + val fqName = FqName(reference.qualifiedName!!) + val onDemand = anImport.isOnDemand + val convertedImports = if (dumpConversion) { + listOf(Import(renderImportName(fqName, onDemand))) } - - return doConvert().map { it.assignPrototype(anImport) } + else { + 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 { +private fun Converter.convertImport(fqName: FqName, ref: PsiJavaCodeReferenceElement, isOnDemand: Boolean, isImportStatic: Boolean): List { 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) { - 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() } - } - } - } - } + return convertStaticImportOnDemand(fqName, target) } else { - 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 convertStaticExplicitImport(fqName, target) } } else { - when (target) { - is KtLightClassForFacade -> return listOf(target.getFqName().parent().render() + ".*") + return convertNonStaticImport(fqName, isOnDemand, target) + } +} - is KtLightClass -> { - if (!isOnDemand) { - if (isFacadeClassFromLibrary(target)) return emptyList() +private fun Converter.convertStaticImportOnDemand(fqName: FqName, target: PsiElement?): List { + when (target) { + 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 { + 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 { + 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)) } diff --git a/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt b/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt index 76c1ba9f0ca..30c226561dc 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/propertyDetection.kt @@ -329,7 +329,7 @@ private class PropertyDetector( private fun dropPropertiesConflictingWithFields(memberToPropertyInfo: MutableMap) { 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)