New J2K: handle correctly short names which are imported by default in kotlin (like List or Result)

Kotlin default import inserter was unable to correctly insert such imports

#KT-34165 fixed

nj2k/src/org/jetbrains/kotlin/nj2k/JKImportStorage.kt

@@ -6,18 +6,28 @@
 package org.jetbrains.kotlin.nj2k
 
 import com.intellij.psi.CommonClassNames
+import org.jetbrains.kotlin.config.LanguageVersionSettings
 import org.jetbrains.kotlin.idea.refactoring.fqName.getKotlinFqName
 import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.load.java.NULLABILITY_ANNOTATIONS
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.KtQualifiedExpression
 import org.jetbrains.kotlin.resolve.calls.callUtil.getCalleeExpressionIfAny
+import org.jetbrains.kotlin.resolve.jvm.platform.JvmPlatformAnalyzerServices
 
-class JKImportStorage {
+class JKImportStorage(languageSettings: LanguageVersionSettings) {
     private val imports = mutableSetOf<FqName>()
 
+    private val defaultImports: Set<FqName> =
+        JvmPlatformAnalyzerServices.getDefaultImports(
+            languageSettings,
+            includeLowPriorityImports = true
+        ).mapNotNull { import ->
+            if (import.isAllUnder) null else import.fqName
+        }.toSet()
+
     fun addImport(import: FqName) {
-        if (isImportNeeded(import)) {
+        if (isImportNeeded(import, allowSingleIdentifierImport = false)) {
             imports += import
         }
     }
@@ -28,23 +38,17 @@ class JKImportStorage {
 
     fun getImports(): Set<FqName> = imports
 
+    fun isImportNeeded(fqName: FqName, allowSingleIdentifierImport: Boolean = false): Boolean {
+        if (!allowSingleIdentifierImport && fqName.asString().count { it == '.' } < 1) return false
+        if (fqName in NULLABILITY_ANNOTATIONS) return false
+        if (fqName in defaultImports) return false
+        return true
+    }
+
+    fun isImportNeeded(fqName: String, allowSingleIdentifierImport: Boolean = false): Boolean =
+        isImportNeeded(FqName(fqName), allowSingleIdentifierImport)
+
     companion object {
-        fun isImportNeeded(fqName: FqName): Boolean {
-            if (fqName in NULLABILITY_ANNOTATIONS) return false
-            if (fqName in JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES) return false
-            return true
-        }
-
-        fun isImportNeededForCall(qualifiedExpression: KtQualifiedExpression): Boolean {
-            val shortName = qualifiedExpression.getCalleeExpressionIfAny()?.text ?: return true
-            if (shortName !in SHORT_NAMES) return true
-            val fqName = qualifiedExpression.selectorExpression?.mainReference?.resolve()?.getKotlinFqName() ?: return true
-            return isImportNeeded(fqName)
-        }
-
-        fun isImportNeeded(fqName: String): Boolean = isImportNeeded(FqName(fqName))
-
-
         private val JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES = setOf(
             FqName(CommonClassNames.JAVA_LANG_BYTE),
             FqName(CommonClassNames.JAVA_LANG_SHORT),
@@ -53,9 +57,15 @@ class JKImportStorage {
             FqName(CommonClassNames.JAVA_LANG_DOUBLE)
         )
 
-        private val SHORT_NAMES =
+        private val SHORT_NAMES = JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES
-            (NULLABILITY_ANNOTATIONS + JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES)
+            .map { it.shortName().identifier }
-                .map { it.shortName().asString() }
+            .toSet()
-                .toSet()
+
+        fun isImportNeededForCall(qualifiedExpression: KtQualifiedExpression): Boolean {
+            val shortName = qualifiedExpression.getCalleeExpressionIfAny()?.text ?: return true
+            if (shortName !in SHORT_NAMES) return true
+            val fqName = qualifiedExpression.selectorExpression?.mainReference?.resolve()?.getKotlinFqName() ?: return true
+            return fqName !in JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES
+        }
     }
 }

nj2k/src/org/jetbrains/kotlin/nj2k/NewJavaToKotlinConverter.kt

@@ -17,7 +17,6 @@
 package org.jetbrains.kotlin.nj2k
 
 import com.intellij.openapi.application.ApplicationManager
-import com.intellij.openapi.application.ModalityState
 import com.intellij.openapi.command.CommandProcessor
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.module.Module
@@ -27,13 +26,18 @@ import com.intellij.openapi.progress.ProgressManager
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.Computable
 import com.intellij.psi.*
-import kotlinx.coroutines.runBlocking
+import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl
-import kotlinx.coroutines.withContext
+import org.jetbrains.kotlin.idea.project.languageVersionSettings
-import org.jetbrains.kotlin.idea.core.util.EDT
+import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.j2k.*
+import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.nj2k.printing.JKCodeBuilder
 import org.jetbrains.kotlin.nj2k.types.JKTypeFactory
+import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.psi.KtFile
+import org.jetbrains.kotlin.psi.KtImportList
 import org.jetbrains.kotlin.psi.KtPsiFactory
+import org.jetbrains.kotlin.resolve.ImportPath
 
 class NewJavaToKotlinConverter(
     val project: Project,
@@ -66,12 +70,11 @@ class NewJavaToKotlinConverter(
                     val kotlinFile = ApplicationManager.getApplication().runReadAction(Computable {
                         KtPsiFactory(project).createFileWithLightClassSupport("dummy.kt", result!!.text, files[i])
                     })
-                    runBlocking(EDT.ModalityStateElement(ModalityState.defaultModalityState())) {
+
-                        withContext(EDT) {
+                    ApplicationManager.getApplication().invokeAndWait {
-                            CommandProcessor.getInstance().runUndoTransparentAction {
+                        CommandProcessor.getInstance().runUndoTransparentAction {
-                                result!!.importsToAdd.forEach {
+                            runWriteAction {
-                                    postProcessor.insertImport(kotlinFile, it)
+                                kotlinFile.addImports(result!!.importsToAdd)
-                                }
                             }
                         }
                     }
@@ -95,6 +98,33 @@ class NewJavaToKotlinConverter(
         }
     }
 
+    private fun KtFile.addImports(imports: Collection<FqName>) {
+        val factory = KtPsiFactory(this)
+        var importList = importList
+        for (import in imports) {
+            val importDirective = factory.createImportDirective(ImportPath(import, isAllUnder = false))
+            if (importList == null) {
+                importList = addImportList(importDirective.parent as KtImportList)
+            } else {
+                importList.add(importDirective)
+            }
+        }
+    }
+
+    private fun KtFile.addImportList(importList: KtImportList): KtImportList {
+        if (packageDirective != null) {
+            return addAfter(importList, packageDirective) as KtImportList
+        }
+
+        val firstDeclaration = findChildByClass(KtDeclaration::class.java)
+        if (firstDeclaration != null) {
+            return addBefore(importList, firstDeclaration) as KtImportList
+        }
+
+        return add(importList) as KtImportList
+    }
+
+
     override fun elementsToKotlin(inputElements: List<PsiElement>, processor: WithProgressProcessor): Result {
         val phaseDescription = "Converting Java code to Kotlin code"
         val contextElement = inputElements.firstOrNull() ?: return Result(emptyList(), null, null)
@@ -102,9 +132,14 @@ class NewJavaToKotlinConverter(
         val typeFactory = JKTypeFactory(symbolProvider)
         symbolProvider.typeFactory = typeFactory
         symbolProvider.preBuildTree(inputElements)
-        val importStorage = JKImportStorage()
-        val treeBuilder = JavaToJKTreeBuilder(symbolProvider, typeFactory, converterServices, importStorage)
 
+        val languageVersion = when {
+            contextElement.isPhysical -> contextElement.languageVersionSettings
+            else -> LanguageVersionSettingsImpl.DEFAULT
+        }
+
+        val importStorage = JKImportStorage(languageVersion)
+        val treeBuilder = JavaToJKTreeBuilder(symbolProvider, typeFactory, converterServices, importStorage)
 
         // we want to leave all imports as is in the case when user is converting only imports
         val saveImports = inputElements.all { element ->

nj2k/src/org/jetbrains/kotlin/nj2k/conversions/FilterImportsConversion.kt

@@ -5,7 +5,6 @@
 
 package org.jetbrains.kotlin.nj2k.conversions
 
-import org.jetbrains.kotlin.nj2k.JKImportStorage
 import org.jetbrains.kotlin.nj2k.NewJ2kConverterContext
 import org.jetbrains.kotlin.nj2k.tree.JKImportList
 import org.jetbrains.kotlin.nj2k.tree.JKTreeElement
@@ -14,7 +13,7 @@ class FilterImportsConversion(context: NewJ2kConverterContext) : RecursiveApplic
     override fun applyToElement(element: JKTreeElement): JKTreeElement {
         if (element !is JKImportList) return recurse(element)
         element.imports = element.imports.filter { import ->
-            JKImportStorage.isImportNeeded(import.name.value)
+            context.importStorage.isImportNeeded(import.name.value, allowSingleIdentifierImport = true)
         }
         return recurse(element)
     }

nj2k/src/org/jetbrains/kotlin/nj2k/printing/JKSymbolRenderer.kt

@@ -6,9 +6,9 @@
 package org.jetbrains.kotlin.nj2k.printing
 
 import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiClass
 import com.intellij.psi.search.GlobalSearchScope
 import com.intellij.psi.search.PsiShortNamesCache
-import com.intellij.util.Processor
 import org.jetbrains.kotlin.name.SpecialNames
 import org.jetbrains.kotlin.nj2k.JKImportStorage
 import org.jetbrains.kotlin.nj2k.escaped
@@ -77,7 +77,13 @@ private class CanBeShortenedCache(project: Project) {
     }
 
     fun canBeShortened(symbol: JKClassSymbol): Boolean = canBeShortenedCache.getOrPut(symbol.name) {
-        !shortNameCache.processClassesWithName(symbol.name, { false }, searchScope, null)
+        var symbolsWithSuchNameCount = 0
+        val processSymbol = { _: PsiClass ->
+            symbolsWithSuchNameCount++
+            symbolsWithSuchNameCount <= 1 //stop if met more than one symbol with such name
+        }
+        shortNameCache.processClassesWithName(symbol.name, processSymbol, searchScope, null)
+        symbolsWithSuchNameCount == 1
     }
 
     companion object {

nj2k/testData/copyPastePlainText/PostProcessing.expected.kt

@@ -1,6 +1,7 @@
 package to
 
 import java.io.File
+import java.util.ArrayList
 
 
 class JavaClass {

nj2k/testData/copyPastePlainText/WholeFile.expected.kt

@@ -1,5 +1,7 @@
 package to
 
+import java.util.ArrayList
+
 
 class JavaClass {
     internal fun foo() {

nj2k/testData/newJ2k/assignmentExpression/nullability-settings.kt

@@ -1,3 +1,5 @@
+import java.util.HashSet
+
 internal class Foo {
     fun foo(o: HashSet<*>) {
         val o2: HashSet<*> = o

nj2k/testData/newJ2k/assignmentExpression/nullability.kt

@@ -1,3 +1,5 @@
+import java.util.HashSet
+
 internal class Foo {
     fun foo(o: HashSet<*>) {
         var foo = 0

nj2k/testData/newJ2k/class/kt-639.kt

@@ -1,5 +1,7 @@
 package demo
 
+import java.util.HashMap
+
 internal class Test {
     constructor() {}
     constructor(s: String?) {}

nj2k/testData/newJ2k/detectProperties/PropertyType.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class A {
     private val list1 = ArrayList<String>()
     private val list2: MutableList<String> = ArrayList()

nj2k/testData/newJ2k/field/specifyType.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class A {
     private val field1: List<String> = ArrayList()
     val field2: List<String> = ArrayList()

nj2k/testData/newJ2k/for/forThroughListIndices.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class C {
     fun foo1(list: MutableList<String?>) {
         for (i in list.indices) {

nj2k/testData/newJ2k/for/overMapEntries.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 internal object Test {
     @JvmStatic
     fun main(args: Array<String>) {

nj2k/testData/newJ2k/issues/kt-1048.kt

@@ -1,6 +1,8 @@
 // ERROR: The integer literal does not conform to the expected type CapturedType(*)
 // ERROR: The integer literal does not conform to the expected type CapturedType(*)
 // ERROR: Type argument is not within its bounds: should be subtype of 'String?'
+import java.util.HashMap
+
 internal class G<T : String?>(t: T)
 class Java {
     internal fun test() {

nj2k/testData/newJ2k/issues/kt-15791.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal object A {
     @JvmStatic
     fun main(args: Array<String>) {

nj2k/testData/newJ2k/issues/kt-19382.kt

@@ -1,5 +1,7 @@
 package test
 
+import java.util.HashMap
+
 class TestPrimitiveFromMap {
     fun foo(map: HashMap<String?, Int?>): Int {
         return map["zzz"]!!

nj2k/testData/newJ2k/issues/kt-19604.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 class TestMethodReference {
     private val hashMap = HashMap<String, String>()
     fun update(key: String, msg: String) {

nj2k/testData/newJ2k/issues/kt-19606.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 class TestMethodReference {
     private val hashMap = HashMap<String, String>()
     fun update(key: String, msg: String) {

nj2k/testData/newJ2k/issues/kt-24677.kt

@@ -1,4 +1,5 @@
 import Foo.SomeClass
+import java.util.ArrayList
 
 internal interface FooInterface {
     fun foo(): ArrayList<out SomeClass?>?

nj2k/testData/newJ2k/issues/kt-32572.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 class TestClass {
     private val hashMap: Map<String, List<Int>> = HashMap()
 }

nj2k/testData/newJ2k/issues/kt-32702.kt

@@ -1,3 +1,4 @@
+import java.util.ArrayList
 import java.util.function.Consumer
 
 internal class Test {

nj2k/testData/newJ2k/issues/kt-809-string.kt

@@ -1,5 +1,7 @@
 package demo
 
+import demo.One
+
 internal class Container {
     var myString = "1"
 }

nj2k/testData/newJ2k/issues/kt-809.kt

@@ -1,5 +1,7 @@
 package demo
 
+import demo.One
+
 internal class Container {
     var myInt = 1
 }

nj2k/testData/newJ2k/issues/kt-820-field.kt

@@ -1,5 +1,7 @@
 package demo
 
+import demo.One
+
 internal class Container {
     var myInt = 1
 }

nj2k/testData/newJ2k/issues/kt-820.kt

@@ -1,5 +1,7 @@
 package demo
 
+import demo.One
+
 internal class Container {
     var myInt = 1
 }

nj2k/testData/newJ2k/issues/kt-824.kt

@@ -1,5 +1,7 @@
 package demo
 
+import demo.One
+
 internal class Container {
     var myBoolean = true
 }

nj2k/testData/newJ2k/issues/spaceBeforeAssignment.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class A {
     var list: List<String> = ArrayList()
 }

nj2k/testData/newJ2k/javaStreamsApi/formatting.kt

@@ -1,3 +1,4 @@
+import java.util.Comparator
 import java.util.stream.Collectors
 
 internal class Test {

nj2k/testData/newJ2k/javaStreamsApi/streamOperations.kt

@@ -1,3 +1,4 @@
+import java.util.Comparator
 import java.util.stream.Collectors
 
 internal class Test {

nj2k/testData/newJ2k/list/ForEach.kt

@@ -1,4 +1,6 @@
 // ERROR: Unresolved reference: LinkedList
+import java.util.ArrayList
+
 class ForEach {
     fun test() {
         val xs = ArrayList<Any>()

nj2k/testData/newJ2k/list/Lists.kt

@@ -1,4 +1,6 @@
 // ERROR: Unresolved reference: LinkedList
+import java.util.ArrayList
+
 class Lists {
     fun test() {
         val xs: MutableList<Any?> = ArrayList()

nj2k/testData/newJ2k/methodCallExpression/specialBuiltinMembers.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 internal enum class E {
     A, B, C
 }

nj2k/testData/newJ2k/mutableCollections/FunctionReturnValue.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class A {
     fun createCollection(): MutableCollection<String> {
         return ArrayList()

nj2k/testData/newJ2k/mutableCollections/FunctionReturnValue2.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class A {
     private val collection: MutableCollection<String>
     fun createCollection(): MutableCollection<String> {

nj2k/testData/newJ2k/mutableCollections/Iterator.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 internal class IteratorTest {
     var mutableMap1: MutableMap<String, String> = HashMap()
     var mutableMap2: MutableMap<String, String> = HashMap()

nj2k/testData/newJ2k/mutableCollections/Iterator2.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 class TestMutableCollection {
     val list: MutableList<String> = ArrayList()
     fun test() {

nj2k/testData/newJ2k/mutableCollections/listOfMutableList.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 class Test {
     internal var list: List<MutableList<Int>> = ArrayList()
     fun test() {

nj2k/testData/newJ2k/mutableCollections/mutableListInOtherClass.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 class Owner {
     var list: MutableList<String> = ArrayList()
 }

nj2k/testData/newJ2k/newClassExpression/fullQualifiedName.kt

@@ -1,5 +1,7 @@
 package test
 
+import java.util.ArrayList
+
 internal class User {
     fun main() {
         val list: List<*> = ArrayList<Any?>()

nj2k/testData/newJ2k/newClassExpression/fullQualifiedName2.kt

@@ -1,7 +1,9 @@
 package test
 
+import java.util.ArrayList
+
 internal class User {
     fun main() {
         val list: List<*> = ArrayList<Any?>()
     }
-}
+}

nj2k/testData/newJ2k/newClassExpression/genericClassInvocation.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class User {
     fun main() {
         val list: List<String> = ArrayList()

nj2k/testData/newJ2k/newClassExpression/newListAndNewMember.kt

@@ -1,5 +1,7 @@
 package org.test
 
+import java.util.ArrayList
+
 internal class Member
 internal class User {
     fun main() {

nj2k/testData/newJ2k/nullability/nullableField.kt

@@ -1,5 +1,7 @@
 package test
 
+import java.util.ArrayList
+
 class Test {
     private var myProp: String? = null
     private var myIntProp: Int? = null

nj2k/testData/newJ2k/postProcessing/GetOperator.kt

@@ -1,5 +1,6 @@
 import javaApi.JavaClass
 import kotlinApi.KotlinClass
+import java.util.HashMap
 
 internal class X {
     operator fun get(index: Int): Int {

nj2k/testData/newJ2k/postProcessing/java8MapForEachWithFullJdk.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 internal class Test {
     fun test(map: HashMap<String?, String?>) {
         map.forEach { (key: String?, value: String?) -> foo(key, value) }

nj2k/testData/newJ2k/projections/projections.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class C<T> {
     fun foo1(src: Collection<T>) {
         val t = src.iterator().next()

nj2k/testData/newJ2k/rawGenerics/kt-540-map.kt

@@ -1,5 +1,7 @@
 package demo
 
+import java.util.HashMap
+
 internal class Test {
     fun main() {
         val commonMap = HashMap<String, Int>()

nj2k/testData/newJ2k/rawGenerics/kt-540.kt

@@ -1,5 +1,7 @@
 package demo
 
+import java.util.ArrayList
+
 internal class Test {
     fun main() {
         val common: List<String> = ArrayList()

nj2k/testData/newJ2k/settings/specifyFieldTypeByDefault.kt

@@ -1,3 +1,5 @@
+import java.util.ArrayList
+
 internal class A {
     private val field1: List<String> = ArrayList()
     val field2: List<String> = ArrayList()

nj2k/testData/newJ2k/toKotlinClasses/LibraryFunctions.kt

@@ -1,5 +1,7 @@
 package demo
 
+import java.util.ArrayList
+
 class TestJava {
     fun f(result: Function1<String?, Unit>) {
         result.invoke("a")

nj2k/testData/newJ2k/toKotlinClasses/TypeParameterBound.kt

@@ -1,2 +1,4 @@
+import java.util.ArrayList
+
 internal interface I<T : List<Iterator<String?>?>?>
 internal class C : I<ArrayList<Iterator<String?>?>?> 

nj2k/testData/newJ2k/typeParameters/needTypeArgs.kt

@@ -1,3 +1,5 @@
+import java.util.HashMap
+
 internal class A {
     fun foo() {
         val map1 = getMap1<String, Int>()

nj2k/testData/newJ2k/typeParameters/rawTypeCast.kt

@@ -2,7 +2,8 @@
 // ERROR: Type mismatch: inferred type is (CapturedType(*)!!..Any?) but String? was expected
 // ERROR: Type mismatch: inferred type is Any? but String? was expected
 // ERROR: Type mismatch: inferred type is (CapturedType(*)!!..Any?) but String? was expected
-// ERROR: Type mismatch: inferred type is kotlin.collections.HashMap<Any?, Any?> /* = java.util.HashMap<Any?, Any?> */ but Map<String?, String?> was expected
+// ERROR: Type mismatch: inferred type is HashMap<Any?, Any?> but Map<String?, String?> was expected
+import java.util.HashMap
 import java.util.Properties
 
 internal object A {