Inline Property also supported for properties with getter

2017-05-23 10:42:57 +03:00
parent 20686297e5
commit e6bfa55534
15 changed files with 233 additions and 142 deletions
@@ -29,24 +29,15 @@ import com.intellij.refactoring.util.CommonRefactoringUtil
 import org.jetbrains.kotlin.descriptors.FunctionDescriptor
 import org.jetbrains.kotlin.descriptors.SimpleFunctionDescriptor
 import org.jetbrains.kotlin.idea.KotlinLanguage
-import org.jetbrains.kotlin.idea.analysis.analyzeInContext
 import org.jetbrains.kotlin.idea.caches.resolve.analyzeFully
-import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
-import org.jetbrains.kotlin.idea.codeInliner.CodeToInlineBuilder
-import org.jetbrains.kotlin.idea.core.copied
 import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
-import org.jetbrains.kotlin.idea.util.getResolutionScope
-import org.jetbrains.kotlin.psi.KtBlockExpression
 import org.jetbrains.kotlin.psi.KtExpression
 import org.jetbrains.kotlin.psi.KtNamedFunction
-import org.jetbrains.kotlin.psi.KtReturnExpression
 import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
-import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
-import org.jetbrains.kotlin.types.TypeUtils
 import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull

 class KotlinInlineFunctionHandler: InlineActionHandler() {
@@ -58,49 +49,6 @@ class KotlinInlineFunctionHandler: InlineActionHandler() {
    override fun inlineElement(project: Project, editor: Editor?, element: PsiElement) {
        element as KtNamedFunction

-        val descriptor = element.resolveToDescriptor() as SimpleFunctionDescriptor
-        val bodyExpression = element.bodyExpression!!
-        val bodyCopy = bodyExpression.copied()
-
-        val expectedType = if (!element.hasBlockBody() && element.hasDeclaredReturnType())
-            descriptor.returnType ?: TypeUtils.NO_EXPECTED_TYPE
-        else
-            TypeUtils.NO_EXPECTED_TYPE
-
-        fun analyzeBodyCopy(): BindingContext {
-            return bodyCopy.analyzeInContext(bodyExpression.getResolutionScope(),
-                                             contextExpression = bodyExpression,
-                                             expectedType = expectedType)
-        }
-
-        val replacementBuilder = CodeToInlineBuilder(descriptor, element.getResolutionFacade())
-        val replacement = if (element.hasBlockBody()) {
-            bodyCopy as KtBlockExpression
-            val statements = bodyCopy.statements
-
-            val returnStatements = bodyCopy.collectDescendantsOfType<KtReturnExpression> {
-                it.getLabelName().let { it == null || it == element.name }
-            }
-
-            val lastReturn = statements.lastOrNull() as? KtReturnExpression
-            if (returnStatements.any { it != lastReturn }) {
-                val message = RefactoringBundle.getCannotRefactorMessage(
-                        if (returnStatements.size > 1)
-                            "Inline Function is not supported for functions with multiple return statements."
-                        else
-                            "Inline Function is not supported for functions with return statements not at the end of the body."
-                )
-                CommonRefactoringUtil.showErrorHint(project, editor, message, "Inline Function", null)
-                return
-            }
-
-            replacementBuilder.prepareCodeToInline(lastReturn?.returnedExpression,
-                                                   statements.dropLast(returnStatements.size), ::analyzeBodyCopy)
-        }
-        else {
-            replacementBuilder.prepareCodeToInline(bodyCopy, emptyList(), ::analyzeBodyCopy)
-        }
-
        val reference = editor?.let { TargetElementUtil.findReference(it, it.caretModel.offset) }
        val nameReference = when (reference) {
            is KtSimpleNameReference -> reference
@@ -114,7 +62,10 @@ class KotlinInlineFunctionHandler: InlineActionHandler() {
            return
        }

-        val replacementStrategy = CallableUsageReplacementStrategy(replacement)
+        val descriptor = element.resolveToDescriptor() as SimpleFunctionDescriptor
+        val codeToInline = buildCodeToInline(element, descriptor.returnType, editor) ?: return
+
+        val replacementStrategy = CallableUsageReplacementStrategy(codeToInline)

        val dialog = KotlinInlineFunctionDialog(project, element, nameReference, replacementStrategy,
                                                allowInlineThisOnly = recursive)
@@ -29,12 +29,14 @@ import com.intellij.refactoring.HelpID
 import com.intellij.refactoring.RefactoringBundle
 import com.intellij.refactoring.util.CommonRefactoringUtil
 import com.intellij.util.containers.MultiMap
+import org.jetbrains.kotlin.descriptors.PropertyDescriptor
 import org.jetbrains.kotlin.descriptors.VariableDescriptor
 import org.jetbrains.kotlin.idea.KotlinLanguage
 import org.jetbrains.kotlin.idea.analysis.analyzeInContext
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
+import org.jetbrains.kotlin.idea.codeInliner.CodeToInline
 import org.jetbrains.kotlin.idea.codeInliner.CodeToInlineBuilder
 import org.jetbrains.kotlin.idea.core.copied
 import org.jetbrains.kotlin.idea.refactoring.checkConflictsInteractively
@@ -58,18 +60,70 @@ class KotlinInlineValHandler : InlineActionHandler() {
    override fun isEnabledForLanguage(l: Language) = l == KotlinLanguage.INSTANCE

    override fun canInlineElement(element: PsiElement): Boolean {
-        if (element !is KtProperty) return false
-        return element.getter == null && element.receiverTypeReference == null
+        return element is KtProperty && element.name != null
    }

    override fun inlineElement(project: Project, editor: Editor?, element: PsiElement) {
        val declaration = element as KtProperty
+        val name = declaration.name!!
+
        val file = declaration.containingKtFile
-        val name = declaration.name ?: return
        if (file.isCompiled) {
            return showErrorHint(project, editor, "Cannot inline '$name' from a decompiled file")
        }

+        val getter = declaration.getter?.takeIf { it.hasBody() }
+        val setter = declaration.setter?.takeIf { it.hasBody() }
+
+        if (getter != null || setter != null) {
+            if (declaration.initializer != null) {
+                return showErrorHint(project, editor, "Cannot inline property with accessor(s) and backing field")
+            }
+
+            if (setter != null) {
+                return showErrorHint(project, editor, "Inline property not supported for properties with setter")
+            }
+        }
+
+        val (referenceExpressions, foreignUsages) = findUsages(declaration)
+
+        if (referenceExpressions.isEmpty()) {
+            val kind = if (declaration.isLocal) "Variable" else "Property"
+            return showErrorHint(project, editor, "$kind '$name' is never used") //TODO: foreign usages!
+        }
+
+        val referencesInOriginalFile = referenceExpressions.filter { it.containingFile == file }
+        val isHighlighting = referencesInOriginalFile.isNotEmpty()
+        highlightElements(project, editor, referencesInOriginalFile)
+
+        val codeToInline: CodeToInline
+        val assignment: KtBinaryExpression?
+        if (getter == null) {
+            val initialization = extractInitialization(declaration, referenceExpressions, project, editor) ?: return
+            codeToInline = buildCodeToInline(declaration, initialization.value)
+            assignment = initialization.assignment
+        }
+        else {
+            val descriptor = declaration.resolveToDescriptor() as PropertyDescriptor
+            codeToInline = buildCodeToInline(getter, descriptor.type, editor) ?: return
+            assignment = null
+        }
+
+        if (foreignUsages.isNotEmpty()) {
+            val conflicts = MultiMap<PsiElement, String>().apply {
+                putValue(null, "Property '$name' has non-Kotlin usages. They won't be processed by the Inline refactoring.")
+                foreignUsages.forEach { putValue(it, it.text) }
+            }
+            project.checkConflictsInteractively(conflicts) { performRefactoring(declaration, codeToInline, editor, assignment, isHighlighting) }
+        }
+        else {
+            performRefactoring(declaration, codeToInline, editor, assignment, isHighlighting)
+        }
+    }
+
+    private data class Usages(val referenceExpressions: Collection<KtExpression>, val foreignUsages: Collection<PsiElement>)
+
+    private fun findUsages(declaration: KtProperty): Usages {
        val references = ReferencesSearch.search(declaration)
        val referenceExpressions = mutableListOf<KtExpression>()
        val foreignUsages = mutableListOf<PsiElement>()
@@ -81,59 +135,41 @@ class KotlinInlineValHandler : InlineActionHandler() {
            }
            referenceExpressions.addIfNotNull((refElement as? KtExpression)?.getQualifiedExpressionForSelectorOrThis())
        }
+        return Usages(referenceExpressions, foreignUsages)
+    }

-        if (referenceExpressions.isEmpty()) {
-            val kind = if (declaration.isLocal) "Variable" else "Property"
-            return showErrorHint(project, editor, "$kind '$name' is never used")
-        }
+    private data class Initialization(val value: KtExpression, val assignment: KtBinaryExpression?)

+    private fun extractInitialization(
+            declaration: KtProperty,
+            referenceExpressions: Collection<KtExpression>,
+            project: Project,
+            editor: Editor?
+    ): Initialization? {
        val writeUsages = referenceExpressions.filter { it.readWriteAccess(useResolveForReadWrite = true) != ReferenceAccess.READ }
-        
+
        val initializerInDeclaration = declaration.initializer
-        val initializer: KtExpression
-        val assignment: KtBinaryExpression?
        if (initializerInDeclaration != null) {
            if (!writeUsages.isEmpty()) {
-                return reportAmbiguousAssignment(project, editor, name, writeUsages)
+                reportAmbiguousAssignment(project, editor, declaration.name!!, writeUsages)
+                return null
            }
-            initializer = initializerInDeclaration
-            assignment = null
+            return Initialization(initializerInDeclaration, assignment = null)
        }
        else {
-            assignment = writeUsages.singleOrNull()
+            val assignment = writeUsages.singleOrNull()
                    ?.getAssignmentByLHS()
                    ?.takeIf { it.operationToken == KtTokens.EQ }
-            initializer = assignment?.right
-                          ?: return reportAmbiguousAssignment(project, editor, name, writeUsages)
-        }
-
-        val referencesInOriginalFile = referenceExpressions.filter { it.containingFile == file }
-        val isHighlighting = referencesInOriginalFile.isNotEmpty()
-        highlightElements(project, editor, referencesInOriginalFile)
-
-        if (referencesInOriginalFile.size != referenceExpressions.size) {
-            preProcessInternalUsages(initializer, referenceExpressions)
-        }
-
-        if (foreignUsages.isNotEmpty()) {
-            val conflicts = MultiMap<PsiElement, String>().apply {
-                putValue(null, "Property '$name' has non-Kotlin usages. They won't be processed by the Inline refactoring.")
-                foreignUsages.forEach { putValue(it, it.text) }
+            val initializer = assignment?.right
+            if (initializer == null) {
+                reportAmbiguousAssignment(project, editor, declaration.name!!, writeUsages)
+                return null
            }
-            project.checkConflictsInteractively(conflicts) { performRefactoring(declaration, initializer, editor, assignment, isHighlighting) }
-        }
-        else {
-            performRefactoring(declaration, initializer, editor, assignment, isHighlighting)
+            return Initialization(initializer, assignment)
        }
    }

-    fun performRefactoring(
-            declaration: KtProperty,
-            initializer: KtExpression,
-            editor: Editor?,
-            assignment: KtBinaryExpression?,
-            isHighlighting: Boolean
-    ) {
+    private fun buildCodeToInline(declaration: KtProperty, initializer: KtExpression): CodeToInline {
        val descriptor = declaration.resolveToDescriptor() as VariableDescriptor
        val expectedType = if (declaration.typeReference != null)
            descriptor.returnType ?: TypeUtils.NO_EXPECTED_TYPE
@@ -147,11 +183,21 @@ class KotlinInlineValHandler : InlineActionHandler() {
                                                    expectedType = expectedType)
        }

-        val reference = editor?.let { TargetElementUtil.findReference(it, it.caretModel.offset) } as? KtSimpleNameReference
-        val replacementBuilder = CodeToInlineBuilder(descriptor, declaration.getResolutionFacade())
-        val replacement = replacementBuilder.prepareCodeToInline(initializerCopy, emptyList(), ::analyzeInitializerCopy)
+        val codeToInlineBuilder = CodeToInlineBuilder(descriptor, declaration.getResolutionFacade())
+        return codeToInlineBuilder.prepareCodeToInline(initializerCopy, emptyList(), ::analyzeInitializerCopy)
+    }
+
+    private fun performRefactoring(
+            declaration: KtProperty,
+            replacement: CodeToInline,
+            editor: Editor?,
+            assignment: KtBinaryExpression?,
+            isHighlighting: Boolean
+    ) {
        val replacementStrategy = CallableUsageReplacementStrategy(replacement)

+        val reference = editor?.let { TargetElementUtil.findReference(it, it.caretModel.offset) } as? KtSimpleNameReference
+
        val dialog = KotlinInlineValDialog(declaration, reference, replacementStrategy, assignment)

        if (!ApplicationManager.getApplication().isUnitTestMode) {
@@ -25,16 +25,29 @@ import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.Key
 import com.intellij.psi.PsiElement
 import com.intellij.refactoring.RefactoringBundle
+import com.intellij.refactoring.util.CommonRefactoringUtil
 import com.intellij.refactoring.util.RefactoringMessageDialog
 import com.intellij.usageView.UsageInfo
+import org.jetbrains.kotlin.descriptors.CallableDescriptor
+import org.jetbrains.kotlin.descriptors.SimpleFunctionDescriptor
+import org.jetbrains.kotlin.idea.analysis.analyzeInContext
+import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
+import org.jetbrains.kotlin.idea.codeInliner.CodeToInline
+import org.jetbrains.kotlin.idea.codeInliner.CodeToInlineBuilder
+import org.jetbrains.kotlin.idea.core.copied
 import org.jetbrains.kotlin.idea.refactoring.move.ContainerChangeInfo
 import org.jetbrains.kotlin.idea.refactoring.move.ContainerInfo
-import org.jetbrains.kotlin.idea.refactoring.move.processInternalReferencesToUpdateOnPackageNameChange
 import org.jetbrains.kotlin.idea.refactoring.move.postProcessMoveUsages
+import org.jetbrains.kotlin.idea.refactoring.move.processInternalReferencesToUpdateOnPackageNameChange
+import org.jetbrains.kotlin.idea.util.getResolutionScope
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
 import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.types.KotlinType
+import org.jetbrains.kotlin.types.TypeUtils
 import java.util.*

 fun highlightElements(project: Project, editor: Editor?, elements: List<PsiElement>) {
@@ -76,7 +89,7 @@ fun showDialog(
 internal var KtSimpleNameExpression.internalUsageInfos: MutableMap<FqName, (KtSimpleNameExpression) -> UsageInfo?>?
        by CopyableUserDataProperty(Key.create("INTERNAL_USAGE_INFOS"))

-internal fun preProcessInternalUsages(element: KtElement, usages: List<KtElement>) {
+internal fun preProcessInternalUsages(element: KtElement, usages: Collection<KtElement>) {
    val mainFile = element.containingKtFile
    val targetPackages = usages.mapNotNullTo(LinkedHashSet()) { it.containingKtFile.packageFqName }
    for (targetPackage in targetPackages) {
@@ -99,4 +112,49 @@ internal fun <E : KtElement> postProcessInternalReferences(inlinedElement: E): E
    expressionsToProcess.forEach { it.internalUsageInfos = null }
    postProcessMoveUsages(internalUsages)
    return pointer.element
-}
+}
+
+internal fun buildCodeToInline(declaration: KtDeclarationWithBody, returnType: KotlinType?, editor: Editor?): CodeToInline? {
+    val bodyExpression = declaration.bodyExpression!!
+    val bodyCopy = bodyExpression.copied()
+
+    val expectedType = if (!declaration.hasBlockBody() && declaration.hasDeclaredReturnType())
+        returnType ?: TypeUtils.NO_EXPECTED_TYPE
+    else
+        TypeUtils.NO_EXPECTED_TYPE
+
+    fun analyzeBodyCopy(): BindingContext {
+        return bodyCopy.analyzeInContext(bodyExpression.getResolutionScope(),
+                                         contextExpression = bodyExpression,
+                                         expectedType = expectedType)
+    }
+
+    val descriptor = declaration.resolveToDescriptor()
+    val builder = CodeToInlineBuilder(descriptor as CallableDescriptor, declaration.getResolutionFacade())
+    if (declaration.hasBlockBody()) {
+        bodyCopy as KtBlockExpression
+        val statements = bodyCopy.statements
+
+        val returnStatements = bodyCopy.collectDescendantsOfType<KtReturnExpression> {
+            it.getLabelName().let { it == null || it == declaration.name }
+        }
+
+        val lastReturn = statements.lastOrNull() as? KtReturnExpression
+        if (returnStatements.any { it != lastReturn }) {
+            val message = RefactoringBundle.getCannotRefactorMessage(
+                    if (returnStatements.size > 1)
+                        "Inline Function is not supported for functions with multiple return statements."
+                    else
+                        "Inline Function is not supported for functions with return statements not at the end of the body."
+            )
+            CommonRefactoringUtil.showErrorHint(declaration.project, editor, message, "Inline Function", null)
+            return null
+        }
+
+        return builder.prepareCodeToInline(lastReturn?.returnedExpression,
+                                           statements.dropLast(returnStatements.size), ::analyzeBodyCopy)
+    }
+    else {
+        return builder.prepareCodeToInline(bodyCopy, emptyList(), ::analyzeBodyCopy)
+    }
+}
@@ -1,7 +0,0 @@
-val Int.C = 239
-
-// not implemented yet
-
-fun f() {
-    println(5.<caret>C)
-}
@@ -1,8 +0,0 @@
-val C: Int
-    get() = 239
-
-// not implemented yet
-
-fun f() {
-    println(<caret>C)
-}
@@ -1,9 +0,0 @@
-val C = 239
-    get() = $C + 1
-
-
-// not implemented yet
-
-fun f() {
-    println(<caret>C)
-}
@@ -0,0 +1,9 @@
+val <caret>property: Int
+    get {
+        println("access!")
+        return 1
+    }
+
+fun foo() {
+    println(property)
+}
@@ -0,0 +1,4 @@
+fun foo() {
+    println("access!")
+    println(1)
+}
@@ -0,0 +1,8 @@
+import java.util.*
+
+val <caret>property: Int
+    get() = Random().nextInt()
+
+fun foo() {
+    println(property)
+}
@@ -0,0 +1,5 @@
+import java.util.*
+
+fun foo() {
+    println(Random().nextInt())
+}
@@ -0,0 +1,7 @@
+val String.<caret>property: Int
+    get() = length * 2
+
+fun String.foo() {
+    println("a".property)
+    println(property)
+}
@@ -0,0 +1,4 @@
+fun String.foo() {
+    println("a".length * 2)
+    println(length * 2)
+}
@@ -0,0 +1,8 @@
+// ERROR: Cannot inline property with accessor(s) and backing field
+
+val C = 239
+    get() = field + 1
+
+fun f() {
+    println(<caret>C)
+}
@@ -65,9 +65,9 @@ abstract class AbstractInlineTest : KotlinLightCodeInsightFixtureTestCase() {
                }
            }
            catch (e: CommonRefactoringUtil.RefactoringErrorHintException) {
-                TestCase.assertFalse(afterFileExists)
-                TestCase.assertEquals(1, expectedErrors.size)
-                TestCase.assertEquals(expectedErrors[0].replace("\\n", "\n"), e.message)
+                TestCase.assertFalse("Refactoring not available: ${e.message}", afterFileExists)
+                TestCase.assertEquals("Expected errors", 1, expectedErrors.size)
+                TestCase.assertEquals("Error message", expectedErrors[0].replace("\\n", "\n"), e.message)
            }
        }
        else {
@@ -994,12 +994,6 @@ public class InlineTestGenerated extends AbstractInlineTest {
                doTest(fileName);
            }

-            @TestMetadata("ExtensionProperty.kt")
-            public void testExtensionProperty() throws Exception {
-                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/ExtensionProperty.kt");
-                doTest(fileName);
-            }
-
            @TestMetadata("InstanceProperty.kt")
            public void testInstanceProperty() throws Exception {
                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/InstanceProperty.kt");
@@ -1048,16 +1042,37 @@ public class InlineTestGenerated extends AbstractInlineTest {
                doTest(fileName);
            }

-            @TestMetadata("WithGetter.kt")
-            public void testWithGetter() throws Exception {
-                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/WithGetter.kt");
-                doTest(fileName);
-            }
+            @TestMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors")
+            @TestDataPath("$PROJECT_ROOT")
+            @RunWith(JUnit3RunnerWithInners.class)
+            public static class Accessors extends AbstractInlineTest {
+                public void testAllFilesPresentInAccessors() throws Exception {
+                    KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors"), Pattern.compile("^(\\w+)\\.kt$"), TargetBackend.ANY, true);
+                }

-            @TestMetadata("WithInitializerAndGetter.kt")
-            public void testWithInitializerAndGetter() throws Exception {
-                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/WithInitializerAndGetter.kt");
-                doTest(fileName);
+                @TestMetadata("BlockBody.kt")
+                public void testBlockBody() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/BlockBody.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("ExpressionBody.kt")
+                public void testExpressionBody() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/ExpressionBody.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("ExtensionProperty.kt")
+                public void testExtensionProperty() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/ExtensionProperty.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("WithInitializer.kt")
+                public void testWithInitializer() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/accessors/WithInitializer.kt");
+                    doTest(fileName);
+                }
            }
        }