Extraction Engine: Do not extract type parameter if it's resolved in the target scope

#KT-7246 Fixed

idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractionEngine/extractableAnalysisUtil.kt

@@ -460,14 +460,14 @@ private fun JetType.processTypeIfExtractable(
         } as? JetTypeParameter
 
         when {
+            typeToCheck.isResolvableInScope(targetScope, true) ->
+                extractable
+
             typeParameter != null -> {
                 typeParameters.add(TypeParameter(typeParameter, typeParameter.collectRelevantConstraints()))
                 extractable
             }
 
-            typeToCheck.isResolvableInScope(targetScope, false) ->
-                extractable
-
             options.allowSpecialClassNames && typeToCheck.isSpecial() ->
                 extractable
 

idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceProperty/KotlinIntroducePropertyHandler.kt

@@ -28,6 +28,7 @@ import org.jetbrains.kotlin.idea.util.psi.patternMatching.*
 import kotlin.test.*
 import com.intellij.openapi.application.*
 import org.jetbrains.kotlin.idea.core.refactoring.getExtractionContainers
+import org.jetbrains.kotlin.idea.refactoring.introduce.extractFunction.EXTRACT_FUNCTION
 import org.jetbrains.kotlin.idea.refactoring.introduce.introduceProperty.*
 import java.util.*
 
@@ -52,53 +53,60 @@ public class KotlinIntroducePropertyHandler(
         }
     }
 
-    override fun invoke(project: Project, editor: Editor, file: PsiFile, dataContext: DataContext?) {
+    public fun selectElements(editor: Editor, file: PsiFile, continuation: (elements: List<PsiElement>, targetSibling: PsiElement) -> Unit) {
-        if (file !is JetFile) return
         selectElementsWithTargetSibling(
-                operationName = INTRODUCE_PROPERTY,
+                INTRODUCE_PROPERTY,
-                editor = editor,
+                editor,
-                file = file,
+                file,
-                getContainers = { elements, parent ->
+                { elements, parent ->
                     parent.getExtractionContainers(strict = true, includeAll = true).filter { it is JetClassBody || it is JetFile }
-                }
+                },
-        ) { elements, targetSibling ->
+                continuation
-            val adjustedElements = (elements.singleOrNull() as? JetBlockExpression)?.getStatements() ?: elements
+        )
-            if (adjustedElements.isNotEmpty()) {
+    }
-                val options = ExtractionOptions(extractAsProperty = true)
-                val extractionData = ExtractionData(file, adjustedElements.toRange(), targetSibling, null, options)
-                ExtractionEngine(helper).run(editor, extractionData) {
-                    val property = it.declaration as JetProperty
-                    val descriptor = it.config.descriptor
 
-                    editor.getCaretModel().moveToOffset(property.getTextOffset())
+    public fun doInvoke(project: Project, editor: Editor, file: JetFile, elements: List<PsiElement>, targetSibling: PsiElement) {
-                    editor.getSelectionModel().removeSelection()
+        val adjustedElements = (elements.singleOrNull() as? JetBlockExpression)?.getStatements() ?: elements
-                    if (editor.getSettings().isVariableInplaceRenameEnabled() && !ApplicationManager.getApplication().isUnitTestMode()) {
+        if (adjustedElements.isNotEmpty()) {
-                        with(PsiDocumentManager.getInstance(project)) {
+            val options = ExtractionOptions(extractAsProperty = true)
-                            commitDocument(editor.getDocument())
+            val extractionData = ExtractionData(file, adjustedElements.toRange(), targetSibling, null, options)
-                            doPostponedOperationsAndUnblockDocument(editor.getDocument())
+            ExtractionEngine(helper).run(editor, extractionData) {
-                        }
+                val property = it.declaration as JetProperty
+                val descriptor = it.config.descriptor
 
-                        val introducer = KotlinInplacePropertyIntroducer(
+                editor.getCaretModel().moveToOffset(property.getTextOffset())
-                                property = property,
+                editor.getSelectionModel().removeSelection()
-                                editor = editor,
+                if (editor.getSettings().isVariableInplaceRenameEnabled() && !ApplicationManager.getApplication().isUnitTestMode()) {
-                                project = project,
+                    with(PsiDocumentManager.getInstance(project)) {
-                                title = INTRODUCE_PROPERTY,
+                        commitDocument(editor.getDocument())
-                                doNotChangeVar = false,
+                        doPostponedOperationsAndUnblockDocument(editor.getDocument())
-                                exprType = descriptor.controlFlow.outputValueBoxer.returnType,
-                                extractionResult = it,
-                                availableTargets = propertyTargets.filter { it.isAvailable(descriptor) }
-                        )
-                        introducer.performInplaceRefactoring(LinkedHashSet(descriptor.suggestedNames))
-                    }
-                    else {
-                        processDuplicatesSilently(it.duplicateReplacers, project)
                     }
+
+                    val introducer = KotlinInplacePropertyIntroducer(
+                            property = property,
+                            editor = editor,
+                            project = project,
+                            title = INTRODUCE_PROPERTY,
+                            doNotChangeVar = false,
+                            exprType = descriptor.controlFlow.outputValueBoxer.returnType,
+                            extractionResult = it,
+                            availableTargets = propertyTargets.filter { it.isAvailable(descriptor) }
+                    )
+                    introducer.performInplaceRefactoring(LinkedHashSet(descriptor.suggestedNames))
+                }
+                else {
+                    processDuplicatesSilently(it.duplicateReplacers, project)
                 }
-            }
-            else {
-                showErrorHintByKey(project, editor, "cannot.refactor.no.expression", INTRODUCE_PROPERTY)
             }
         }
+        else {
+            showErrorHintByKey(project, editor, "cannot.refactor.no.expression", INTRODUCE_PROPERTY)
+        }
+    }
+
+    override fun invoke(project: Project, editor: Editor, file: PsiFile, dataContext: DataContext?) {
+        if (file !is JetFile) return
+        selectElements(editor, file) { elements, targetSibling -> doInvoke(project, editor, file, elements, targetSibling) }
     }
 
     override fun invoke(project: Project, elements: Array<out PsiElement>, dataContext: DataContext?) {

idea/testData/refactoring/extractFunction/typeParameters/typeParameterNotResolvableInTargetScope.kt

@@ -0,0 +1,16 @@
+// WITH_RUNTIME
+// PARAM_TYPES: Foo<T>
+// PARAM_TYPES: kotlin.String, Comparable<String>, CharSequence, kotlin.Any
+// PARAM_DESCRIPTOR: internal final class Foo<T> defined in root package
+// PARAM_DESCRIPTOR: value-parameter val l: kotlin.String defined in Foo.test
+
+import java.util.*
+
+// SIBLING:
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(l: String): T {
+        return <selection>map[l]</selection>
+    }
+}

idea/testData/refactoring/extractFunction/typeParameters/typeParameterNotResolvableInTargetScope.kt.after

@@ -0,0 +1,18 @@
+// WITH_RUNTIME
+// PARAM_TYPES: Foo<T>
+// PARAM_TYPES: kotlin.String, Comparable<String>, CharSequence, kotlin.Any
+// PARAM_DESCRIPTOR: internal final class Foo<T> defined in root package
+// PARAM_DESCRIPTOR: value-parameter val l: kotlin.String defined in Foo.test
+
+import java.util.*
+
+// SIBLING:
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(l: String): T {
+        return t(l)
+    }
+}
+
+private fun <T> Foo<T>.t(l: String) = map[l]

idea/testData/refactoring/extractFunction/typeParameters/typeParameterResolvableInTargetScope.kt

@@ -0,0 +1,13 @@
+// WITH_RUNTIME
+// PARAM_TYPES: kotlin.String, Comparable<String>, CharSequence, kotlin.Any
+// PARAM_DESCRIPTOR: value-parameter val l: kotlin.String defined in Foo.test
+
+import java.util.*
+
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(l: String): T {
+        return <selection>map[l]</selection>
+    }
+}

idea/testData/refactoring/extractFunction/typeParameters/typeParameterResolvableInTargetScope.kt.after

@@ -0,0 +1,15 @@
+// WITH_RUNTIME
+// PARAM_TYPES: kotlin.String, Comparable<String>, CharSequence, kotlin.Any
+// PARAM_DESCRIPTOR: value-parameter val l: kotlin.String defined in Foo.test
+
+import java.util.*
+
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(l: String): T {
+        return t(l)
+    }
+
+    private fun t(l: String) = map[l]
+}

idea/testData/refactoring/introduceProperty/typeParameterNotResolvableInTargetScope.kt

@@ -0,0 +1,13 @@
+// EXTRACTION_TARGET: property with getter
+// WITH_RUNTIME
+
+import java.util.*
+
+// SIBLING:
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(): T {
+        return <selection>map[""]</selection>
+    }
+}

idea/testData/refactoring/introduceProperty/typeParameterNotResolvableInTargetScope.kt.after

@@ -0,0 +1,16 @@
+// EXTRACTION_TARGET: property with getter
+// WITH_RUNTIME
+
+import java.util.*
+
+private val <T> Foo<T>.t: T?
+    get() = map[""]
+
+// SIBLING:
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(): T {
+        return t
+    }
+}

idea/testData/refactoring/introduceProperty/typeParameterResolvableInTargetScope.kt

@@ -0,0 +1,12 @@
+// EXTRACTION_TARGET: property with initializer
+// WITH_RUNTIME
+
+import java.util.*
+
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    fun test(): T {
+        return <selection>map[""]</selection>
+    }
+}

idea/testData/refactoring/introduceProperty/typeParameterResolvableInTargetScope.kt.after

@@ -0,0 +1,14 @@
+// EXTRACTION_TARGET: property with initializer
+// WITH_RUNTIME
+
+import java.util.*
+
+class Foo<T> {
+    val map = HashMap<String, T>()
+
+    private val t = map[""]
+
+    fun test(): T {
+        return t
+    }
+}

idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/AbstractJetExtractionTest.kt

@@ -207,6 +207,7 @@ public abstract class AbstractJetExtractionTest() : JetLightCodeInsightFixtureTe
             val extractionTarget = propertyTargets.single {
                 it.name == InTextDirectivesUtils.findStringWithPrefixes(file.getText(), "// EXTRACTION_TARGET: ")
             }
+            val explicitPreviousSibling = file.findElementByComment("// SIBLING:")
             val helper = object : ExtractionEngineHelper(INTRODUCE_PROPERTY) {
                 override fun configureAndRun(
                         project: Project,
@@ -223,12 +224,11 @@ public abstract class AbstractJetExtractionTest() : JetLightCodeInsightFixtureTe
                     )
                 }
             }
-            KotlinIntroducePropertyHandler(helper).invoke(
+            val handler = KotlinIntroducePropertyHandler(helper)
-                    fixture.getProject(),
+            val editor = fixture.getEditor()
-                    fixture.getEditor(),
+            handler.selectElements(editor, file) { elements, previousSibling ->
-                    file,
+                handler.doInvoke(getProject(), editor, file as JetFile, elements, explicitPreviousSibling ?: previousSibling)
-                    DataManager.getInstance().getDataContext(fixture.getEditor().getComponent())
+            }
-            )
         }
     }
 

idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/JetExtractionTestGenerated.java

@@ -1989,6 +1989,18 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
                 doExtractFunctionTest(fileName);
             }
 
+            @TestMetadata("typeParameterNotResolvableInTargetScope.kt")
+            public void testTypeParameterNotResolvableInTargetScope() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/typeParameters/typeParameterNotResolvableInTargetScope.kt");
+                doExtractFunctionTest(fileName);
+            }
+
+            @TestMetadata("typeParameterResolvableInTargetScope.kt")
+            public void testTypeParameterResolvableInTargetScope() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/typeParameters/typeParameterResolvableInTargetScope.kt");
+                doExtractFunctionTest(fileName);
+            }
+
             @TestMetadata("typeParametersAndConstraintsCombined1.kt")
             public void testTypeParametersAndConstraintsCombined1() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/typeParameters/typeParametersAndConstraintsCombined1.kt");
@@ -2196,6 +2208,18 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
             String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceProperty/replaceDuplicates.kt");
             doIntroducePropertyTest(fileName);
         }
+
+        @TestMetadata("typeParameterNotResolvableInTargetScope.kt")
+        public void testTypeParameterNotResolvableInTargetScope() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceProperty/typeParameterNotResolvableInTargetScope.kt");
+            doIntroducePropertyTest(fileName);
+        }
+
+        @TestMetadata("typeParameterResolvableInTargetScope.kt")
+        public void testTypeParameterResolvableInTargetScope() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/introduceProperty/typeParameterResolvableInTargetScope.kt");
+            doIntroducePropertyTest(fileName);
+        }
     }
 
     @TestMetadata("idea/testData/refactoring/introduceParameter")