From a0d2b002873cff97d97203e2adc03b1622ad6213 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Thu, 9 Apr 2015 18:56:06 +0300 Subject: [PATCH] Code corrections on code review --- .../kotlin/idea/actions/JavaToKotlinAction.kt | 4 +- .../KotlinCopyPasteReferenceProcessor.kt | 14 ++- .../copy/ConvertJavaCopyPastePostProcessor.kt | 3 +- .../idea/conversion/copy/DataForConversion.kt | 15 +-- .../kotlin/j2k/ClassBodyConverter.kt | 4 +- j2k/src/org/jetbrains/kotlin/j2k/Converter.kt | 2 +- .../kotlin/j2k/JavaToKotlinConverter.kt | 98 ++++++++++--------- .../jetbrains/kotlin/j2k/ReferenceSearcher.kt | 27 +++-- .../org/jetbrains/kotlin/j2k/ast/ClassBody.kt | 3 +- ...tractJavaToKotlinConverterMultiFileTest.kt | 2 +- 10 files changed, 91 insertions(+), 81 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/actions/JavaToKotlinAction.kt b/idea/src/org/jetbrains/kotlin/idea/actions/JavaToKotlinAction.kt index 1abe69f0462..3b8938ce1a7 100644 --- a/idea/src/org/jetbrains/kotlin/idea/actions/JavaToKotlinAction.kt +++ b/idea/src/org/jetbrains/kotlin/idea/actions/JavaToKotlinAction.kt @@ -75,7 +75,7 @@ public class JavaToKotlinAction : AnAction() { ProgressManager.getInstance().runProcessWithProgressSynchronously( { runReadAction { - externalCodeUpdate = converterResult!!.externalCodeProcessing(ProgressManager.getInstance().getProgressIndicator()) + externalCodeUpdate = converterResult!!.externalCodeProcessing.prepareWriteOperation(ProgressManager.getInstance().getProgressIndicator()) } }, title, @@ -89,7 +89,7 @@ public class JavaToKotlinAction : AnAction() { val newFiles = saveResults(javaFiles, converterResult!!.results.map { it!!.text /*conversion of a file always succeeds*/ }) - externalCodeUpdate?.invoke() + externalCodeUpdate!!.invoke() newFiles.singleOrNull()?.let { FileEditorManager.getInstance(project).openFile(it, true) diff --git a/idea/src/org/jetbrains/kotlin/idea/codeInsight/KotlinCopyPasteReferenceProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/codeInsight/KotlinCopyPasteReferenceProcessor.kt index 4bb0cb65ed8..92b45b24111 100644 --- a/idea/src/org/jetbrains/kotlin/idea/codeInsight/KotlinCopyPasteReferenceProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/codeInsight/KotlinCopyPasteReferenceProcessor.kt @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.codeInsight import com.intellij.codeInsight.CodeInsightSettings import com.intellij.codeInsight.editorActions.CopyPastePostProcessor +import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.RangeMarker @@ -28,7 +29,9 @@ import com.intellij.openapi.util.Ref import com.intellij.openapi.util.TextRange import com.intellij.psi.* import com.intellij.psi.util.PsiTreeUtil -import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.annotations.TestOnly +import org.jetbrains.kotlin.descriptors.CallableDescriptor +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.codeInsight.shorten.performDelayedShortening @@ -51,7 +54,6 @@ import org.jetbrains.kotlin.resolve.ImportPath import org.jetbrains.kotlin.resolve.JetModuleUtil import org.jetbrains.kotlin.resolve.QualifiedExpressionResolver import org.jetbrains.kotlin.resolve.QualifiedExpressionResolver.LookupMode -import org.jetbrains.kotlin.resolve.descriptorUtil.getImportableDescriptor import org.jetbrains.kotlin.resolve.descriptorUtil.isExtension import org.jetbrains.kotlin.resolve.scopes.JetScope import org.jetbrains.kotlin.types.ErrorUtils @@ -268,7 +270,7 @@ public class KotlinCopyPasteReferenceProcessor() : CopyPastePostProcessor): Collection { val fqNames = referencesToRestore.map { it.refData.fqName }.toSortedSet() - declarationsToImportSuggested = fqNames + if (ApplicationManager.getApplication().isUnitTestMode()) { + declarationsToImportSuggested = fqNames + } val shouldShowDialog = CodeInsightSettings.getInstance().ADD_IMPORTS_ON_PASTE == CodeInsightSettings.ASK if (!shouldShowDialog || referencesToRestore.isEmpty()) { @@ -352,7 +356,7 @@ public class KotlinCopyPasteReferenceProcessor() : CopyPastePostProcessor = emptyList() } } diff --git a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPastePostProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPastePostProcessor.kt index 1364e616483..324cd3012cf 100644 --- a/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPastePostProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPastePostProcessor.kt @@ -29,6 +29,7 @@ import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.psi.PsiJavaFile +import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.idea.codeInsight.KotlinCopyPasteReferenceProcessor import org.jetbrains.kotlin.idea.codeInsight.KotlinReferenceData import org.jetbrains.kotlin.idea.editor.JetEditorOptions @@ -223,7 +224,7 @@ public class ConvertJavaCopyPastePostProcessor : CopyPastePostProcessor() - for (i in startOffsets.indices) { - val start = startOffsets[i] - val end = endOffsets[i] - val range = TextRange(start, end) + for (range in ranges) { + val start = range.start + val end = range.end + if (start == end) continue val startToken = file.findElementAt(start)!! val elementToClipLeft = startToken.maximalParentToClip(range) @@ -197,7 +198,7 @@ data class DataForConversion private( if (!canDropElementFromText(child)) return null clipTo = childRange.end } - else { // rangeBound is inside element'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) } diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ClassBodyConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/ClassBodyConverter.kt index 47b3b136c9b..48ba50ae14f 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ClassBodyConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ClassBodyConverter.kt @@ -93,7 +93,7 @@ class ClassBodyConverter(private val psiClass: PsiClass, } } - return ClassBody(null, null, convertedMembers.values().toList(), emptyList(), lBrace, rBrace, overloadReducer) + return ClassBody(null, null, convertedMembers.values().toList(), emptyList(), lBrace, rBrace) } val useCompanionObject = shouldGenerateCompanionObject(convertedMembers) @@ -128,7 +128,7 @@ class ClassBodyConverter(private val psiClass: PsiClass, primaryConstructorSignature = null // no "()" after class name is needed in this case } - return ClassBody(primaryConstructorSignature, constructorConverter?.baseClassParams, members, companionObjectMembers, lBrace, rBrace, overloadReducer) + return ClassBody(primaryConstructorSignature, constructorConverter?.baseClassParams, members, companionObjectMembers, lBrace, rBrace) } private fun Converter.convertMember(member: PsiMember, diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt index 6f723c567d0..b59936413e4 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt @@ -277,7 +277,7 @@ class Converter private( // to convert fields and nested types - they are not allowed in Kotlin but we convert them and let user refactor code var classBody = ClassBodyConverter(psiClass, this, isOpenClass = false, isObject = false).convertBody() classBody = ClassBody(constructorSignature, classBody.baseClassParams, classBody.members, - classBody.companionObjectMembers, classBody.lBrace, classBody.rBrace, classBody.overloadReducer) + classBody.companionObjectMembers, classBody.lBrace, classBody.rBrace) val annotationAnnotation = Annotation(Identifier("annotation").assignNoPrototype(), listOf(), false, false).assignNoPrototype() return Class(psiClass.declarationIdentifier(), diff --git a/j2k/src/org/jetbrains/kotlin/j2k/JavaToKotlinConverter.kt b/j2k/src/org/jetbrains/kotlin/j2k/JavaToKotlinConverter.kt index ee655d440f2..c68655a7c61 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/JavaToKotlinConverter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/JavaToKotlinConverter.kt @@ -40,7 +40,10 @@ import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.isAncestor import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.resolve.BindingContext -import java.util.* +import java.util.ArrayList +import java.util.Comparator +import java.util.LinkedHashMap +import kotlin.properties.Delegates public trait PostProcessor { public fun analyzeFile(file: JetFile, range: TextRange?): BindingContext @@ -88,7 +91,11 @@ public class JavaToKotlinConverter(private val project: Project, public data class ElementResult(val text: String, val parseContext: ParseContext) - public data class Result(val results: List, val externalCodeProcessing: ((ProgressIndicator) -> (() -> Unit)?)?) //TODO: change interface to not perform write-actions under progress + public trait ExternalCodeProcessing { + public fun prepareWriteOperation(progress: ProgressIndicator): () -> Unit + } + + public data class Result(val results: List, val externalCodeProcessing: ExternalCodeProcessing?) public fun elementsToKotlin( inputElements: List, @@ -200,12 +207,14 @@ public class JavaToKotlinConverter(private val project: Project, val target: PsiElement, val file: PsiFile, val processings: Collection - ) + ) { + val depth: Int by Delegates.lazy { target.parents(withItself = true).takeWhile { it !is PsiFile }.count() } + } private fun buildExternalCodeProcessing( usageProcessings: Map>, inConversionScope: (PsiElement) -> Boolean - ): ((ProgressIndicator) -> (() -> Unit)?)? { + ): ExternalCodeProcessing? { if (usageProcessings.isEmpty()) return null val map: Map> = usageProcessings.values() @@ -214,51 +223,53 @@ public class JavaToKotlinConverter(private val project: Project, .groupBy { it.targetElement } if (map.isEmpty()) return null - return fun(progress: ProgressIndicator): (() -> Unit)? { - val refs = ArrayList() + return object: ExternalCodeProcessing { + override fun prepareWriteOperation(progress: ProgressIndicator): () -> Unit { + val refs = ArrayList() - progress.setText("Searching usages to update...") + progress.setText("Searching usages to update...") - for ((i, entry) in map.entrySet().withIndex()) { - val psiElement = entry.key - val processings = entry.value + for ((i, entry) in map.entrySet().withIndex()) { + val psiElement = entry.key + val processings = entry.value - progress.setText2((psiElement as? PsiNamedElement)?.getName() ?: "") - progress.checkCanceled() + progress.setText2((psiElement as? PsiNamedElement)?.getName() ?: "") + progress.checkCanceled() - ProgressManager.getInstance().runProcess( - { - val searchJava = processings.any { it.javaCodeProcessor != null } - val searchKotlin = processings.any { it.kotlinCodeProcessor != null } - referenceSearcher.findExternalCodeProcessingUsages(psiElement, searchJava, searchKotlin) - .filterNot { inConversionScope(it.getElement()) } - .mapTo(refs) { ReferenceInfo(it, psiElement, it.getElement().getContainingFile(), processings) } - }, - ProgressPortionReporter(progress, i / map.size().toDouble(), 1.0 / map.size())) + ProgressManager.getInstance().runProcess( + { + val searchJava = processings.any { it.javaCodeProcessor != null } + val searchKotlin = processings.any { it.kotlinCodeProcessor != null } + referenceSearcher.findUsagesForExternalCodeProcessing(psiElement, searchJava, searchKotlin) + .filterNot { inConversionScope(it.getElement()) } + .mapTo(refs) { ReferenceInfo(it, psiElement, it.getElement().getContainingFile(), processings) } + }, + ProgressPortionReporter(progress, i / map.size().toDouble(), 1.0 / map.size())) + } + + return { processUsages(refs) } } - - if (refs.isEmpty()) return null - - return { processUsages(refs) } } } private fun processUsages(refs: Collection) { - @ReferenceLoop - for ((reference, target, file, processings) in refs.sortBy(ReferenceComparator)) { - val processors = when (reference.getElement().getLanguage()) { - JavaLanguage.INSTANCE -> processings.map { it.javaCodeProcessor }.filterNotNull() - JetLanguage.INSTANCE -> processings.map { it.kotlinCodeProcessor }.filterNotNull() - else -> continue@ReferenceLoop - } + for (fileRefs in refs.groupBy { it.file }.values()) { // group by file for faster sorting + @ReferenceLoop + for ((reference, target, file, processings) in fileRefs.sortBy(ReferenceComparator)) { + val processors = when (reference.getElement().getLanguage()) { + JavaLanguage.INSTANCE -> processings.map { it.javaCodeProcessor }.filterNotNull() + JetLanguage.INSTANCE -> processings.map { it.kotlinCodeProcessor }.filterNotNull() + else -> continue@ReferenceLoop + } - checkReferenceValid(reference) + checkReferenceValid(reference) - var references = listOf(reference) - for (processor in processors) { - references = references.flatMap { processor.processUsage(it) ?: listOf(it) } - references.forEach { checkReferenceValid(it) } + var references = listOf(reference) + for (processor in processors) { + references = references.flatMap { processor.processUsage(it) ?: listOf(it) } + references.forEach { checkReferenceValid(it) } + } } } } @@ -273,18 +284,15 @@ public class JavaToKotlinConverter(private val project: Project, val element1 = info1.reference.getElement() val element2 = info2.reference.getElement() - val deepness1 = element1.deepnessInTree() - val deepness2 = element2.deepnessInTree() - if (deepness1 != deepness2) { // put deeper elements first to not invalidate them when processing ancestors - return -deepness1.compareTo(deepness2) + val depth1 = info1.depth + val depth2 = info2.depth + if (depth1 != depth2) { // put deeper elements first to not invalidate them when processing ancestors + return -depth1.compareTo(depth2) } - // process elements of the same deepness from right to left so that right-side of assignments is not invalidated by processing of the left one + // process elements with the same parent from right to left so that right-side of assignments is not invalidated by processing of the left one return -element1.getStartOffsetInParent().compareTo(element2.getStartOffsetInParent()) } - - private fun PsiElement.deepnessInTree() = parents(withItself = true).takeWhile { it !is PsiFile }.count() - } private class ProgressPortionReporter( diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt b/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt index 400d1d3179c..257f25026bd 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt @@ -17,23 +17,22 @@ package org.jetbrains.kotlin.j2k import com.intellij.lang.java.JavaLanguage -import com.intellij.openapi.fileTypes.FileTypeManager -import com.intellij.openapi.vfs.VirtualFile +import com.intellij.openapi.fileTypes.FileType import com.intellij.psi.* -import com.intellij.psi.search.DelegatingGlobalSearchScope import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.search.LocalSearchScope import com.intellij.psi.search.searches.ClassInheritorsSearch import com.intellij.psi.search.searches.OverridingMethodsSearch import com.intellij.psi.search.searches.ReferencesSearch import org.jetbrains.kotlin.idea.JetLanguage +import java.util.ArrayList public trait ReferenceSearcher { fun findLocalUsages(element: PsiElement, scope: PsiElement): Collection fun hasInheritors(`class`: PsiClass): Boolean fun hasOverrides(method: PsiMethod): Boolean - fun findExternalCodeProcessingUsages(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection + fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection } public fun ReferenceSearcher.findVariableUsages(variable: PsiVariable, scope: PsiElement): Collection @@ -55,7 +54,7 @@ public object EmptyReferenceSearcher: ReferenceSearcher { override fun findLocalUsages(element: PsiElement, scope: PsiElement): Collection = emptyList() override fun hasInheritors(`class`: PsiClass) = false override fun hasOverrides(method: PsiMethod) = false - override fun findExternalCodeProcessingUsages(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection + override fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection = throw UnsupportedOperationException() } @@ -66,17 +65,15 @@ public object IdeaReferenceSearcher: ReferenceSearcher { override fun hasOverrides(method: PsiMethod) = OverridingMethodsSearch.search(method, false).any() - override fun findExternalCodeProcessingUsages(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection { - val searchScope = object : DelegatingGlobalSearchScope(GlobalSearchScope.projectScope(element.getProject())) { - override fun contains(file: VirtualFile): Boolean { - if (!super.contains(file)) return false - return when (FileTypeManager.getInstance().getFileTypeByFile(file)) { - JavaLanguage.INSTANCE.getAssociatedFileType() -> searchJava - JetLanguage.INSTANCE.getAssociatedFileType() -> searchKotlin - else -> false - } - } + override fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection { + val fileTypes = ArrayList() + if (searchJava) { + fileTypes.add(JavaLanguage.INSTANCE.getAssociatedFileType()) } + if (searchKotlin) { + fileTypes.add(JetLanguage.INSTANCE.getAssociatedFileType()) + } + val searchScope = GlobalSearchScope.getScopeRestrictedByFileTypes(GlobalSearchScope.projectScope(element.getProject()), *fileTypes.copyToArray()) return ReferencesSearch.search(element, searchScope).findAll() } } diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ast/ClassBody.kt b/j2k/src/org/jetbrains/kotlin/j2k/ast/ClassBody.kt index 09349732fbe..11ad8f325a4 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ast/ClassBody.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ast/ClassBody.kt @@ -30,8 +30,7 @@ class ClassBody ( val members: List, val companionObjectMembers: List, val lBrace: LBrace, - val rBrace: RBrace, - val overloadReducer: OverloadReducer) { + val rBrace: RBrace) { fun append(builder: CodeBuilder) { val membersFiltered = members.filter { !it.isEmpty } diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/AbstractJavaToKotlinConverterMultiFileTest.kt b/j2k/tests/org/jetbrains/kotlin/j2k/AbstractJavaToKotlinConverterMultiFileTest.kt index 547eb3784b3..926577e004f 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/AbstractJavaToKotlinConverterMultiFileTest.kt +++ b/j2k/tests/org/jetbrains/kotlin/j2k/AbstractJavaToKotlinConverterMultiFileTest.kt @@ -59,7 +59,7 @@ public abstract class AbstractJavaToKotlinConverterMultiFileTest : AbstractJavaT val inputElements = psiFilesToConvert.map { JavaToKotlinConverter.InputElement(it, it) } val (results, externalCodeProcessor) = converter.elementsToKotlin(inputElements) - val process = externalCodeProcessor?.invoke(EmptyProgressIndicator()) + val process = externalCodeProcessor?.prepareWriteOperation(EmptyProgressIndicator()) project.executeWriteCommand("") { process?.invoke() } fun expectedResultFile(i: Int) = File(filesToConvert[i].getPath().replace(".java", ".kt"))