From 2dabb91a1fe443b367cb20e3e53208c343805771 Mon Sep 17 00:00:00 2001 From: Ilya Kirillov Date: Fri, 2 Aug 2019 20:30:34 +0300 Subject: [PATCH] J2K, minor: clean copy-paste conversion code --- .../copy/ConvertJavaCopyPasteProcessor.kt | 107 +++++++++++------- .../copy/ConvertTextJavaCopyPasteProcessor.kt | 73 +++++++----- .../idea/conversion/copy/DataForConversion.kt | 63 +++++------ .../conversion/copy/ElementAndTextList.kt | 8 +- .../copy/JavaContextDeclarationRenderer.kt | 1 - .../copy/KotlinFilePasteProvider.kt | 10 +- .../copy/PlainTextPasteImportResolver.kt | 104 ++++++++--------- 7 files changed, 202 insertions(+), 164 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPasteProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPasteProcessor.kt index d2a658874a5..c07774eddfd 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPasteProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPasteProcessor.kt @@ -54,6 +54,7 @@ import java.util.* import kotlin.system.measureTimeMillis class ConvertJavaCopyPasteProcessor : CopyPastePostProcessor() { + @Suppress("PrivatePropertyName") private val LOG = Logger.getInstance(ConvertJavaCopyPasteProcessor::class.java) override fun extractTransferableData(content: Transferable): List { @@ -61,20 +62,30 @@ class ConvertJavaCopyPasteProcessor : CopyPastePostProcessor { + override fun collectTransferableData( + file: PsiFile, + editor: Editor, + startOffsets: IntArray, + endOffsets: IntArray + ): List { if (file !is PsiJavaFile) return listOf() - return listOf(CopiedJavaCode(file.getText()!!, startOffsets, endOffsets)) } - override fun processTransferableData(project: Project, editor: Editor, bounds: RangeMarker, caretOffset: Int, indented: Ref, values: List) { + override fun processTransferableData( + project: Project, + editor: Editor, + bounds: RangeMarker, + caretOffset: Int, + indented: Ref, + values: List + ) { if (DumbService.getInstance(project).isDumb) return if (!KotlinEditorOptions.getInstance().isEnableJavaToKotlinConversion) return @@ -104,7 +115,11 @@ class ConvertJavaCopyPasteProcessor : CopyPastePostProcessor, explicitImports: Collection): TextRange? { + fun insertImports( + bounds: TextRange, + referenceData: Collection, + explicitImports: Collection + ): TextRange? { if (referenceData.isEmpty() && explicitImports.isEmpty()) return bounds PsiDocumentManager.getInstance(project).commitAllDocuments() @@ -133,31 +148,31 @@ class ConvertJavaCopyPasteProcessor : CopyPastePostProcessor { + private fun buildReferenceData( + text: String, + parseContext: ParseContext, + importsAndPackage: String, + targetFile: KtFile + ): Collection { var blockStart: Int? = null var blockEnd: Int? = null val fileText = buildString { @@ -202,7 +222,6 @@ class ConvertJavaCopyPasteProcessor : CopyPastePostProcessor, - val textChanged: Boolean, - val converterContext: ConverterContext? + val text: String, + val parseContext: ParseContext, + val importsToAdd: Set, + val textChanged: Boolean, + val converterContext: ConverterContext? ) internal fun ElementAndTextList.convertCodeToKotlin(project: Project, targetModule: Module?, useNewJ2k: Boolean): ConversionResult { @@ -237,16 +257,16 @@ internal fun ElementAndTextList.convertCodeToKotlin(project: Project, targetModu IdeaJavaToKotlinServices ) - val inputElements = this.toList().filterIsInstance() + val inputElements = toList().filterIsInstance() val (results, _, converterContext) = - ProgressManager.getInstance().runProcessWithProgressSynchronously( - ThrowableComputable { - runReadAction { converter.elementsToKotlin(inputElements) } - }, - JavaToKotlinAction.title, - false, - project - ) + ProgressManager.getInstance().runProcessWithProgressSynchronously( + ThrowableComputable { + runReadAction { converter.elementsToKotlin(inputElements) } + }, + JavaToKotlinAction.title, + false, + project + ) val importsToAdd = LinkedHashSet() @@ -267,8 +287,7 @@ internal fun ElementAndTextList.convertCodeToKotlin(project: Project, targetModu parseContext = result.parseContext } importsToAdd.addAll(result.importsToAdd) - } - else { // failed to convert element to Kotlin, insert "as is" + } else { // failed to convert element to Kotlin, insert "as is" convertedCodeBuilder.append(originalText) } } @@ -297,11 +316,11 @@ internal fun isNoConversionPosition(file: KtFile, offset: Int): Boolean { if (token !is PsiWhiteSpace && token.endOffset != offset) return true // pasting into the middle of token for (element in token.parentsWithSelf) { - if (element is PsiComment) { - return element.node.elementType == KtTokens.EOL_COMMENT || offset != element.endOffset + when (element) { + is PsiComment -> return element.node.elementType == KtTokens.EOL_COMMENT || offset != element.endOffset + is KtStringTemplateEntryWithExpression -> return false + is KtStringTemplateExpression -> return true } - if (element is KtStringTemplateEntryWithExpression) return false - if (element is KtStringTemplateExpression) return true } return false } diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertTextJavaCopyPasteProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertTextJavaCopyPasteProcessor.kt index f3040d9b5ef..3d32a407c85 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertTextJavaCopyPasteProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertTextJavaCopyPasteProcessor.kt @@ -61,11 +61,17 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor { + override fun collectTransferableData( + file: PsiFile, + editor: Editor, + startOffsets: IntArray, + endOffsets: IntArray + ): List { if (file is KtFile) return listOf(CopiedKotlinCode(file.text, startOffsets, endOffsets)) return emptyList() } @@ -73,22 +79,28 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor { try { if (content.isDataFlavorSupported(DataFlavor.stringFlavor)) { - if (content.isDataFlavorSupported(CopiedKotlinCode.DATA_FLAVOR) || /* Handled by ConvertJavaCopyPasteProcessor */ - content.isDataFlavorSupported(CopiedJavaCode.DATA_FLAVOR)) return emptyList() + content.isDataFlavorSupported(CopiedJavaCode.DATA_FLAVOR) + ) return emptyList() val text = content.getTransferData(DataFlavor.stringFlavor) as String return listOf(MyTransferableData(text)) } - } - catch (e: Throwable) { + } catch (e: Throwable) { LOG.error(e) } return emptyList() } - override fun processTransferableData(project: Project, editor: Editor, bounds: RangeMarker, caretOffset: Int, indented: Ref, values: List) { + override fun processTransferableData( + project: Project, + editor: Editor, + bounds: RangeMarker, + caretOffset: Int, + indented: Ref, + values: List + ) { if (DumbService.getInstance(project).isDumb) return if (!KotlinEditorOptions.getInstance().isEnableJavaToKotlinConversion) return //TODO: use another option? @@ -100,11 +112,8 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor 0) convertedImportsText = "\n" + convertedImportsText + if (convertedImportsText.isNotBlank()) editor.document.insertString(importsInsertOffset, convertedImportsText) @@ -139,9 +149,7 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor { @@ -238,19 +245,21 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor + listOf("\n", importStatement) + } + "\n\n") //TODO Non-manual formatting for import list } private fun prepareCopiedJavaCodeByContext(text: String, context: JavaContext, target: KtElement): CopiedJavaCode { val targetFile = target.containingFile as KtFile - val (localDeclarations, memberDeclarations) = javaContextDeclarationRenderer.render(target) val prefix = buildString { @@ -265,16 +274,25 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor "" - JavaContext.CLASS_BODY, - JavaContext.IN_BLOCK, - JavaContext.EXPRESSION -> { + JavaContext.CLASS_BODY, JavaContext.IN_BLOCK, JavaContext.EXPRESSION -> { val lightClass = target.getParentOfType(false)?.toLightClass() buildString { append("class ") append(lightClass?.name ?: "Dummy") - lightClass?.extendsListTypes?.ifNotEmpty { joinTo(this@buildString, prefix = " extends ") { it.getCanonicalText(true) } } - lightClass?.implementsListTypes?.ifNotEmpty { joinTo(this@buildString, prefix = " implements ") { it.getCanonicalText(true) } } + lightClass?.extendsListTypes?.ifNotEmpty { + joinTo( + this@buildString, + prefix = " extends " + ) { it.getCanonicalText(true) } + } + lightClass?.implementsListTypes?.ifNotEmpty { + joinTo(this@buildString, prefix = " implements ") { + it.getCanonicalText( + true + ) + } + } } } } @@ -300,6 +318,7 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor= range.end) { this[i] = offset - range.length - } - else { + } else { assert(offset <= range.start) } } @@ -141,7 +138,10 @@ data class DataForConversion private constructor( } private fun PsiElement.maximalParentToClip(range: TextRange): PsiElement? { - val firstNotInRange = parentsWithSelf.takeWhile { it !is PsiDirectory }.firstOrNull { it.range !in range } ?: return null + val firstNotInRange = parentsWithSelf + .takeWhile { it !is PsiDirectory } + .firstOrNull { it.range !in range } + ?: return null return firstNotInRange.parentsWithSelf.lastOrNull { it.minimizedTextRange() in range } } @@ -155,41 +155,42 @@ data class DataForConversion private constructor( private fun canDropElementFromText(element: PsiElement): Boolean { return when (element) { is PsiWhiteSpace, is PsiComment, is PsiModifierList, is PsiAnnotation -> true - is PsiJavaToken -> { when (element.tokenType) { // modifiers JavaTokenType.PUBLIC_KEYWORD, JavaTokenType.PROTECTED_KEYWORD, JavaTokenType.PRIVATE_KEYWORD, JavaTokenType.STATIC_KEYWORD, JavaTokenType.ABSTRACT_KEYWORD, JavaTokenType.FINAL_KEYWORD, JavaTokenType.NATIVE_KEYWORD, JavaTokenType.SYNCHRONIZED_KEYWORD, JavaTokenType.STRICTFP_KEYWORD, - JavaTokenType.TRANSIENT_KEYWORD, JavaTokenType.VOLATILE_KEYWORD, JavaTokenType.DEFAULT_KEYWORD -> element.getParent() is PsiModifierList - + JavaTokenType.TRANSIENT_KEYWORD, JavaTokenType.VOLATILE_KEYWORD, JavaTokenType.DEFAULT_KEYWORD -> + element.getParent() is PsiModifierList JavaTokenType.SEMICOLON -> true - else -> false } } - is PsiCodeBlock -> element.getParent() is PsiMethod - else -> element.firstChild == null } } - private fun tryClipLeftSide(element: PsiElement, leftBound: Int) - = tryClipSide(element, leftBound, { range }, { allChildren }) + private fun tryClipLeftSide(element: PsiElement, leftBound: Int) = + tryClipSide(element, leftBound, { range }, { allChildren }) private fun tryClipRightSide(element: PsiElement, rightBound: Int): Int? { fun Int.transform() = Int.MAX_VALUE - this fun TextRange.transform() = TextRange(end.transform(), start.transform()) - return tryClipSide(element, rightBound.transform(), { range.transform() }, { lastChild.siblings(forward = false) })?.transform() + return tryClipSide( + element, + rightBound.transform(), + { range.transform() }, + { lastChild.siblings(forward = false) } + )?.transform() } private fun tryClipSide( - element: PsiElement, - rangeBound: Int, - rangeFunction: PsiElement.() -> TextRange, - childrenFunction: PsiElement.() -> Sequence + element: PsiElement, + rangeBound: Int, + rangeFunction: PsiElement.() -> TextRange, + childrenFunction: PsiElement.() -> Sequence ): Int? { if (element.firstChild == null) return null @@ -202,12 +203,10 @@ data class DataForConversion private constructor( if (childRange.start >= rangeBound) { // we have cut enough already break - } - else if (childRange.end <= rangeBound) { // need to drop the whole element + } else if (childRange.end <= rangeBound) { // need to drop the whole element if (!canDropElementFromText(child)) return null clipTo = childRange.end - } - else { // rangeBound is inside child's range + } else { // rangeBound is inside child's range if (child is PsiWhiteSpace) break // no need to cut whitespace - we can leave it as is return tryClipSide(child, rangeBound, rangeFunction, childrenFunction) } @@ -217,15 +216,14 @@ data class DataForConversion private constructor( } private fun ElementAndTextList.collectElementsToConvert( - file: PsiJavaFile, - fileText: String, - range: TextRange + file: PsiJavaFile, + fileText: String, + range: TextRange ) { val elements = file.elementsInRange(range) if (elements.isEmpty()) { add(fileText.substring(range.start, range.end)) - } - else { + } else { add(fileText.substring(range.start, elements.first().range.start)) elements.forEach { if (shouldExpandToChildren(it)) @@ -243,7 +241,7 @@ data class DataForConversion private constructor( private fun buildImportsAndPackage(sourceFile: PsiJavaFile): String { return buildString { val packageName = sourceFile.packageName - if (!packageName.isEmpty()) { + if (packageName.isNotEmpty()) { append("package $packageName\n") } @@ -253,8 +251,7 @@ data class DataForConversion private constructor( val qualifiedName = import.qualifiedName ?: continue if (import.isOnDemand) { append("import $qualifiedName.*\n") - } - else { + } else { val fqName = FqNameUnsafe(qualifiedName) // skip explicit imports of platform classes mapped into Kotlin classes if (fqName.isSafe && JavaToKotlinClassMap.isJavaPlatformClass(fqName.toSafe())) continue diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ElementAndTextList.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ElementAndTextList.kt index 3c02003fbf2..c74b7f1a83f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ElementAndTextList.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ElementAndTextList.kt @@ -42,10 +42,10 @@ class ElementAndTextList() { fun toList(): List = elementsAndTexts.toList() fun process(processor: ElementsAndTextsProcessor) { - elementsAndTexts.forEach { - when (it) { - is PsiElement -> processor.processElement(it) - is String -> processor.processText(it) + elementsAndTexts.forEach { element -> + when (element) { + is PsiElement -> processor.processElement(element) + is String -> processor.processText(element) } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/JavaContextDeclarationRenderer.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/JavaContextDeclarationRenderer.kt index 84b1346f417..3962a953c4f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/JavaContextDeclarationRenderer.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/JavaContextDeclarationRenderer.kt @@ -26,7 +26,6 @@ data class ContextDeclarations( val memberDeclarationsJavaStubs: String ) - class JavaContextDeclarationRenderer { private val KtElement.memberDeclarations get() = parents() diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/KotlinFilePasteProvider.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/KotlinFilePasteProvider.kt index 4199537a504..4a7075cd45c 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/KotlinFilePasteProvider.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/KotlinFilePasteProvider.kt @@ -34,6 +34,7 @@ class KotlinFilePasteProvider : PasteProvider { val ktFile = KtPsiFactory(project).createFile(text) val fileName = (ktFile.declarations.firstOrNull()?.name ?: return) + ".kt" + @Suppress("UsePropertyAccessSyntax") val directory = ideView.getOrChooseDirectory() ?: return project.executeWriteCommand("Create Kotlin file") { val file = try { @@ -47,9 +48,9 @@ class KotlinFilePasteProvider : PasteProvider { if (document != null) { document.setText(text) documentManager.commitDocument(document) - val qualName = JavaDirectoryService.getInstance()?.getPackage(directory)?.qualifiedName - if (qualName != null && file is KtFile) { - file.packageFqName = FqName(qualName) + val qualifiedName = JavaDirectoryService.getInstance()?.getPackage(directory)?.qualifiedName + if (qualifiedName != null && file is KtFile) { + file.packageFqName = FqName(qualifiedName) } OpenFileDescriptor(project, file.virtualFile).navigate(true) } @@ -64,7 +65,8 @@ class KotlinFilePasteProvider : PasteProvider { //todo: KT-25329, to remove these heuristics if (text.contains(";\n") || ((text.contains("public interface") || text.contains("public class")) && - !text.contains("fun "))) return false //Optimisation for Java. Kotlin doesn't need that... + !text.contains("fun ")) + ) return false //Optimisation for Java. Kotlin doesn't need that... val file = KtPsiFactory(project).createFile(text) return !PsiTreeUtil.hasErrorElements(file) } diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/PlainTextPasteImportResolver.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/PlainTextPasteImportResolver.kt index 3036517c6c0..6f10e6a8139 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/PlainTextPasteImportResolver.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/PlainTextPasteImportResolver.kt @@ -37,8 +37,7 @@ import org.jetbrains.kotlin.psi.KtImportDirective import org.jetbrains.kotlin.psi.psiUtil.referenceExpression import java.util.* -class PlainTextPasteImportResolver(val dataForConversion: DataForConversion, val targetFile: KtFile) { - +class PlainTextPasteImportResolver(private val dataForConversion: DataForConversion, val targetFile: KtFile) { private val file = dataForConversion.file private val project = targetFile.project @@ -59,8 +58,8 @@ class PlainTextPasteImportResolver(val dataForConversion: DataForConversion, val private fun canBeImported(descriptor: DeclarationDescriptorWithVisibility?): Boolean { return descriptor != null - && descriptor.canBeReferencedViaImport() - && descriptor.isVisible(targetFile, null, bindingContext, resolutionFacade) + && descriptor.canBeReferencedViaImport() + && descriptor.isVisible(targetFile, null, bindingContext, resolutionFacade) } private fun addImport(importStatement: PsiImportStatementBase, shouldAddToTarget: Boolean = false) { @@ -76,46 +75,54 @@ class PlainTextPasteImportResolver(val dataForConversion: DataForConversion, val val importedReference = importDirective.importedReference if (importPath != null && !importPath.hasAlias() && importedReference is KtDotQualifiedExpression) { val receiver = importedReference - .receiverExpression - .referenceExpression() - ?.mainReference - ?.resolve() + .receiverExpression + .referenceExpression() + ?.mainReference + ?.resolve() val selector = importedReference - .selectorExpression - ?.referenceExpression() - ?.mainReference - ?.resolve() + .selectorExpression + ?.referenceExpression() + ?.mainReference + ?.resolve() val isPackageReceiver = receiver is PsiPackage val isClassReceiver = receiver is PsiClass val isClassSelector = selector is PsiClass if (importPath.isAllUnder) { - if (isClassReceiver) - addImport(psiElementFactory.createImportStaticStatement(receiver as PsiClass, "*")) - else if (isPackageReceiver) - addImport(psiElementFactory.createImportStatementOnDemand((receiver as PsiPackage).qualifiedName)) - } - else { - if (isClassSelector) - addImport(psiElementFactory.createImportStatement(selector as PsiClass)) - else if (isClassReceiver) - addImport(psiElementFactory.createImportStaticStatement(receiver as PsiClass, importPath.importedName!!.asString())) + when { + isClassReceiver -> + addImport(psiElementFactory.createImportStaticStatement(receiver as PsiClass, "*")) + isPackageReceiver -> + addImport(psiElementFactory.createImportStatementOnDemand((receiver as PsiPackage).qualifiedName)) + } + } else { + when { + isClassSelector -> + addImport(psiElementFactory.createImportStatement(selector as PsiClass)) + isClassReceiver -> + addImport( + psiElementFactory.createImportStaticStatement( + receiver as PsiClass, + importPath.importedName!!.asString() + ) + ) + } } } } - if (importList !in dataForConversion.elementsAndTexts.toList()) + if (importList !in dataForConversion.elementsAndTexts.toList()) { runWriteAction { targetFile.importDirectives.forEach(::tryConvertKotlinImport) } + } } fun tryResolveReferences() { - - val elementsWithUnresolvedRef = PsiTreeUtil.collectElements(file) { - it.reference != null - && it.reference is PsiQualifiedReference - && it.reference?.resolve() == null + val elementsWithUnresolvedRef = PsiTreeUtil.collectElements(file) { element -> + element.reference != null + && element.reference is PsiQualifiedReference + && element.reference?.resolve() == null } fun tryResolveReference(reference: PsiQualifiedReference): Boolean { @@ -123,28 +130,27 @@ class PlainTextPasteImportResolver(val dataForConversion: DataForConversion, val val referenceName = reference.referenceName ?: return false if (referenceName in failedToResolveReferenceNames) return false val classes = shortNameCache.getClassesByName(referenceName, scope) - .mapNotNull { psiClass -> - val containingFile = psiClass.containingFile - if (ProjectRootsUtil.isInProjectOrLibraryContent(containingFile)) { - psiClass to psiClass.getJavaMemberDescriptor() as? ClassDescriptor - } - else { - null - } - } - .filter { canBeImported(it.second) } + .mapNotNull { psiClass -> + val containingFile = psiClass.containingFile + if (ProjectRootsUtil.isInProjectOrLibraryContent(containingFile)) { + psiClass to psiClass.getJavaMemberDescriptor() as? ClassDescriptor + } else null + }.filter { canBeImported(it.second) } - classes.find { (_, descriptor) -> JavaToKotlinClassMap.mapPlatformClass(descriptor!!).isNotEmpty() } - ?.let { (psiClass, _) -> addImport(psiElementFactory.createImportStatement(psiClass)) } + classes.find { (_, descriptor) -> + JavaToKotlinClassMap.mapPlatformClass(descriptor!!).isNotEmpty() + }?.let { (psiClass, _) -> + addImport(psiElementFactory.createImportStatement(psiClass)) + } if (reference.resolve() != null) return true classes.singleOrNull()?.let { (psiClass, _) -> addImport(psiElementFactory.createImportStatement(psiClass), true) } - if (reference.resolve() != null) return true - else { - if (classes.isNotEmpty()) { + when { + reference.resolve() != null -> return true + classes.isNotEmpty() -> { ambiguityInResolution = true return false } @@ -163,14 +169,10 @@ class PlainTextPasteImportResolver(val dataForConversion: DataForConversion, val addImport(psiElementFactory.createImportStaticStatement(psiMember.containingClass!!, psiMember.name!!), true) } - if (reference.resolve() != null) return false - else { - if (members.isNotEmpty()) { - ambiguityInResolution = true - } - else { - couldNotResolve = true - } + when { + reference.resolve() != null -> return false + members.isNotEmpty() -> ambiguityInResolution = true + else -> couldNotResolve = true } return false }