Code corrections on code review

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)

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<Kotlin
         if (referencedFqNames.singleOrNull() == originalFqName) return null
 
         // check that descriptor to import exists and is accessible from the current module
-        if (!findImportableDescriptors(originalFqName, file).any { KotlinReferenceData.Kind.fromDescriptor(it) == refData.kind }) {
+        if (findImportableDescriptors(originalFqName, file).none { KotlinReferenceData.Kind.fromDescriptor(it) == refData.kind }) {
             return null
         }
 
@@ -327,7 +329,9 @@ public class KotlinCopyPasteReferenceProcessor() : CopyPastePostProcessor<Kotlin
     private fun showRestoreReferencesDialog(project: Project, referencesToRestore: List<ReferenceToRestoreData>): Collection<ReferenceToRestoreData> {
         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<Kotlin
     }
 
     companion object {
-        // for tests
+        [TestOnly]
         public var declarationsToImportSuggested: Collection<String> = emptyList()
     }
 }

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<TextBloc
     }
 
     companion object {
-        // used for testing
+        [TestOnly]
         public var conversionPerformed: Boolean = false
     }
 }

idea/src/org/jetbrains/kotlin/idea/conversion/copy/DataForConversion.kt

@@ -57,14 +57,15 @@ data class DataForConversion private(
         }
 
         private fun clipTextIfNeeded(file: PsiJavaFile, fileText: String, startOffsets: IntArray, endOffsets: IntArray): String? {
-            fun canDropRange(range: TextRange)
+            val ranges = startOffsets.indices.map { TextRange(startOffsets[it], endOffsets[it]) }.sortBy { it.start }
-                    = startOffsets.indices.all { range !in TextRange(startOffsets[it], endOffsets[it]) }
+
+            fun canDropRange(range: TextRange) = ranges.all { range !in it }
 
             val rangesToDrop = ArrayList<TextRange>()
-            for (i in startOffsets.indices) {
+            for (range in ranges) {
-                val start = startOffsets[i]
+                val start = range.start
-                val end = endOffsets[i]
+                val end = range.end
-                val range = TextRange(start, 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)
                 }

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,

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(),

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<ElementResult?>, 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<ElementResult?>, val externalCodeProcessing: ExternalCodeProcessing?)
 
     public fun elementsToKotlin(
             inputElements: List<InputElement>,
@@ -200,12 +207,14 @@ public class JavaToKotlinConverter(private val project: Project,
             val target: PsiElement,
             val file: PsiFile,
             val processings: Collection<UsageProcessing>
-    )
+    ) {
+        val depth: Int by Delegates.lazy { target.parents(withItself = true).takeWhile { it !is PsiFile }.count() }
+    }
 
     private fun buildExternalCodeProcessing(
             usageProcessings: Map<PsiElement, Collection<UsageProcessing>>,
             inConversionScope: (PsiElement) -> Boolean
-    ): ((ProgressIndicator) -> (() -> Unit)?)? {
+    ): ExternalCodeProcessing? {
         if (usageProcessings.isEmpty()) return null
 
         val map: Map<PsiElement, Collection<UsageProcessing>> = 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)? {
+        return object: ExternalCodeProcessing {
-            val refs = ArrayList<ReferenceInfo>()
+            override fun prepareWriteOperation(progress: ProgressIndicator): () -> Unit {
+                val refs = ArrayList<ReferenceInfo>()
 
-            progress.setText("Searching usages to update...")
+                progress.setText("Searching usages to update...")
 
-            for ((i, entry) in map.entrySet().withIndex()) {
+                for ((i, entry) in map.entrySet().withIndex()) {
-                val psiElement = entry.key
+                    val psiElement = entry.key
-                val processings = entry.value
+                    val processings = entry.value
 
-                progress.setText2((psiElement as? PsiNamedElement)?.getName() ?: "")
+                    progress.setText2((psiElement as? PsiNamedElement)?.getName() ?: "")
-                progress.checkCanceled()
+                    progress.checkCanceled()
 
-                ProgressManager.getInstance().runProcess(
+                    ProgressManager.getInstance().runProcess(
-                        {
+                            {
-                            val searchJava = processings.any { it.javaCodeProcessor != null }
+                                val searchJava = processings.any { it.javaCodeProcessor != null }
-                            val searchKotlin = processings.any { it.kotlinCodeProcessor != null }
+                                val searchKotlin = processings.any { it.kotlinCodeProcessor != null }
-                            referenceSearcher.findExternalCodeProcessingUsages(psiElement, searchJava, searchKotlin)
+                                referenceSearcher.findUsagesForExternalCodeProcessing(psiElement, searchJava, searchKotlin)
-                                    .filterNot { inConversionScope(it.getElement()) }
+                                        .filterNot { inConversionScope(it.getElement()) }
-                                    .mapTo(refs) { ReferenceInfo(it, psiElement, it.getElement().getContainingFile(), processings) }
+                                        .mapTo(refs) { ReferenceInfo(it, psiElement, it.getElement().getContainingFile(), processings) }
-                        },
+                            },
-                        ProgressPortionReporter(progress, i / map.size().toDouble(), 1.0 / map.size()))
+                            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<ReferenceInfo>) {
-        @ReferenceLoop
+        for (fileRefs in refs.groupBy { it.file }.values()) { // group by file for faster sorting
-        for ((reference, target, file, processings) in refs.sortBy(ReferenceComparator)) {
+            @ReferenceLoop
-            val processors = when (reference.getElement().getLanguage()) {
+            for ((reference, target, file, processings) in fileRefs.sortBy(ReferenceComparator)) {
-                JavaLanguage.INSTANCE -> processings.map { it.javaCodeProcessor }.filterNotNull()
+                val processors = when (reference.getElement().getLanguage()) {
-                JetLanguage.INSTANCE -> processings.map { it.kotlinCodeProcessor }.filterNotNull()
+                    JavaLanguage.INSTANCE -> processings.map { it.javaCodeProcessor }.filterNotNull()
-                else -> continue@ReferenceLoop
+                    JetLanguage.INSTANCE -> processings.map { it.kotlinCodeProcessor }.filterNotNull()
-            }
+                    else -> continue@ReferenceLoop
+                }
 
-            checkReferenceValid(reference)
+                checkReferenceValid(reference)
 
-            var references = listOf(reference)
+                var references = listOf(reference)
-            for (processor in processors) {
+                for (processor in processors) {
-                references = references.flatMap { processor.processUsage(it) ?: listOf(it) }
+                    references = references.flatMap { processor.processUsage(it) ?: listOf(it) }
-                references.forEach { checkReferenceValid(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 depth1 = info1.depth
-            val deepness2 = element2.deepnessInTree()
+            val depth2 = info2.depth
-            if (deepness1 != deepness2) { // put deeper elements first to not invalidate them when processing ancestors
+            if (depth1 != depth2) { // put deeper elements first to not invalidate them when processing ancestors
-                return -deepness1.compareTo(deepness2)
+                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(

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.fileTypes.FileType
-import com.intellij.openapi.vfs.VirtualFile
 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<PsiReference>
     fun hasInheritors(`class`: PsiClass): Boolean
     fun hasOverrides(method: PsiMethod): Boolean
 
-    fun findExternalCodeProcessingUsages(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection<PsiReference>
+    fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection<PsiReference>
 }
 
 public fun ReferenceSearcher.findVariableUsages(variable: PsiVariable, scope: PsiElement): Collection<PsiReferenceExpression>
@@ -55,7 +54,7 @@ public object EmptyReferenceSearcher: ReferenceSearcher {
     override fun findLocalUsages(element: PsiElement, scope: PsiElement): Collection<PsiReference> = emptyList()
     override fun hasInheritors(`class`: PsiClass) = false
     override fun hasOverrides(method: PsiMethod) = false
-    override fun findExternalCodeProcessingUsages(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection<PsiReference>
+    override fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection<PsiReference>
             = 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<PsiReference> {
+    override fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection<PsiReference> {
-        val searchScope = object : DelegatingGlobalSearchScope(GlobalSearchScope.projectScope(element.getProject())) {
+        val fileTypes = ArrayList<FileType>()
-            override fun contains(file: VirtualFile): Boolean {
+        if (searchJava) {
-                if (!super.contains(file)) return false
+            fileTypes.add(JavaLanguage.INSTANCE.getAssociatedFileType())
-                return when (FileTypeManager.getInstance().getFileTypeByFile(file)) {
-                    JavaLanguage.INSTANCE.getAssociatedFileType() -> searchJava
-                    JetLanguage.INSTANCE.getAssociatedFileType() -> searchKotlin
-                    else -> false
-                }
-            }
         }
+        if (searchKotlin) {
+            fileTypes.add(JetLanguage.INSTANCE.getAssociatedFileType())
+        }
+        val searchScope = GlobalSearchScope.getScopeRestrictedByFileTypes(GlobalSearchScope.projectScope(element.getProject()), *fileTypes.copyToArray())
         return ReferencesSearch.search(element, searchScope).findAll()
     }
 }

j2k/src/org/jetbrains/kotlin/j2k/ast/ClassBody.kt

@@ -30,8 +30,7 @@ class ClassBody (
         val members: List<Member>,
         val companionObjectMembers: List<Member>,
         val lBrace: LBrace,
-        val rBrace: RBrace,
+        val rBrace: RBrace) {
-        val overloadReducer: OverloadReducer) {
 
     fun append(builder: CodeBuilder) {
         val membersFiltered = members.filter { !it.isEmpty }

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"))