refactor ConvertFunctionToPropertyIntention and fix 'Convert function to property shouldn't insert explicit type if it was inferred previously' (#1011)

Fixes #KT-14820

compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt

@@ -612,7 +612,7 @@ class KtPsiFactory @JvmOverloads constructor(private val project: Project, val m
         private fun placeKeyword() {
             assert(state == State.MODIFIERS)
 
-            if (sb.isNotEmpty()) {
+            if (!sb.endsWith(" ")) {
                 sb.append(" ")
             }
             val keyword = when (target) {
@@ -625,9 +625,9 @@ class KtPsiFactory @JvmOverloads constructor(private val project: Project, val m
             state = State.RECEIVER
         }
 
-        private fun bodyPrefix() = when (target) {
+        private fun bodyPrefix(breakLine: Boolean = true) = when (target) {
             Target.FUNCTION, Target.CONSTRUCTOR -> ""
-            Target.READ_ONLY_PROPERTY -> "\nget()"
+            Target.READ_ONLY_PROPERTY -> (if (breakLine) "\n" else " ") + "get()"
         }
 
         fun modifier(modifier: String): CallableBuilder {
@@ -733,6 +733,16 @@ class KtPsiFactory @JvmOverloads constructor(private val project: Project, val m
             return this
         }
 
+        fun getterExpression(expression: String, breakLine: Boolean = true): CallableBuilder {
+            assert(target == Target.READ_ONLY_PROPERTY)
+            assert(state == State.BODY || state == State.TYPE_CONSTRAINTS)
+
+            sb.append(bodyPrefix(breakLine)).append(" = ").append(expression)
+            state = State.DONE
+
+            return this
+        }
+
         fun initializer(body: String): CallableBuilder {
             assert(target == Target.READ_ONLY_PROPERTY && (state == State.BODY || state == State.TYPE_CONSTRAINTS))
 

idea/src/org/jetbrains/kotlin/idea/intentions/ConvertFunctionToPropertyIntention.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2015 JetBrains s.r.o.
+ * Copyright 2010-2017 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,7 +21,10 @@ import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.Key
 import com.intellij.openapi.util.text.StringUtil
-import com.intellij.psi.*
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiMethod
+import com.intellij.psi.PsiNamedElement
+import com.intellij.psi.PsiReference
 import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.util.containers.MultiMap
 import org.jetbrains.kotlin.asJava.namedUnwrappedElement
@@ -32,6 +35,8 @@ import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
 import org.jetbrains.kotlin.idea.core.ShortenReferences
+import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.quickfix.createFromUsage.callableBuilder.getReturnTypeReference
 import org.jetbrains.kotlin.idea.refactoring.*
 import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
 import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
@@ -40,9 +45,11 @@ import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.load.java.JvmAbi
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.KtPsiFactory.CallableBuilder.Target.READ_ONLY_PROPERTY
+import org.jetbrains.kotlin.psi.psiUtil.endOffset
 import org.jetbrains.kotlin.psi.psiUtil.getParentOfTypeAndBranch
+import org.jetbrains.kotlin.psi.psiUtil.getStartOffsetIn
 import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
-import org.jetbrains.kotlin.psi.psiUtil.siblings
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
@@ -58,8 +65,9 @@ class ConvertFunctionToPropertyIntention : SelfTargetingIntention<KtNamedFunctio
 
     private inner class Converter(
             project: Project,
+            private val editor: Editor?,
             descriptor: FunctionDescriptor
-    ): CallableRefactoring<FunctionDescriptor>(project, descriptor, text) {
+    ) : CallableRefactoring<FunctionDescriptor>(project, descriptor, text) {
         private val elementsToShorten = ArrayList<KtElement>()
 
         private val newName: String by lazy {
@@ -68,30 +76,31 @@ class ConvertFunctionToPropertyIntention : SelfTargetingIntention<KtNamedFunctio
         }
 
         private fun convertFunction(originalFunction: KtNamedFunction, psiFactory: KtPsiFactory) {
-            val function = originalFunction.copy() as KtNamedFunction
+            val propertyString = KtPsiFactory.CallableBuilder(READ_ONLY_PROPERTY).apply {
+                // make sure to capture all comments and line breaks
+                modifier(originalFunction.text.substring(0, originalFunction.funKeyword!!.getStartOffsetIn(originalFunction)))
+                typeParams(originalFunction.typeParameters.map { it.text })
+                originalFunction.receiverTypeReference?.let { receiver(it.text) }
+                name(newName)
+                originalFunction.getReturnTypeReference()?.let { returnType(it.text) }
+                typeConstraints(originalFunction.typeConstraints.map { it.text })
 
-            val propertySample = psiFactory.createProperty("val foo: Int get() = 1")
+                if (originalFunction.equalsToken != null) {
+                    getterExpression(originalFunction.bodyExpression!!.text, breakLine = originalFunction.typeReference != null)
+                }
+                else {
+                    (originalFunction.bodyExpression as? KtBlockExpression)?.let { body ->
+                        transform {
+                            append("\nget() ")
+                            append(body.text)
+                        }
+                    }
+                }
+            }.asString()
 
-            val needsExplicitType = function.typeReference == null
+            val replaced = originalFunction.replaced(psiFactory.createDeclaration<KtProperty>(propertyString))
-            if (needsExplicitType) {
-                originalFunction.typeFqNameToAdd?.let { function.setTypeReference(psiFactory.createType(it)) }
-            }
 
-            function.funKeyword!!.replace(propertySample.valOrVarKeyword)
+            editor?.caretModel?.moveToOffset(replaced.nameIdentifier!!.endOffset)
-            function.valueParameterList?.delete()
-            val insertAfter = (function.equalsToken ?: function.bodyExpression)
-                    ?.siblings(forward = false, withItself = false)
-                    ?.firstOrNull { it !is PsiWhiteSpace }
-            if (insertAfter != null) {
-                function.addAfter(psiFactory.createParameterList("()"), insertAfter)
-                function.addAfter(propertySample.getter!!.namePlaceholder, insertAfter)
-            }
-            function.setName(newName)
-
-            val property = originalFunction.replace(psiFactory.createProperty(function.text)) as KtProperty
-            if (needsExplicitType) {
-                elementsToShorten.add(property.typeReference!!)
-            }
         }
 
         override fun performRefactoring(descriptorsForChange: Collection<CallableDescriptor>) {
@@ -211,6 +220,6 @@ class ConvertFunctionToPropertyIntention : SelfTargetingIntention<KtNamedFunctio
     override fun applyTo(element: KtNamedFunction, editor: Editor?) {
         val context = element.analyze(BodyResolveMode.PARTIAL)
         val descriptor = context[BindingContext.DECLARATION_TO_DESCRIPTOR, element] as FunctionDescriptor
-        Converter(element.project, descriptor).run()
+        Converter(element.project, editor, descriptor).run()
     }
 }

idea/testData/intentions/convertFunctionToProperty/addExplicitAnonymousType.kt.after

@@ -3,12 +3,11 @@ interface T {
 }
 
 class A(val n: Int) {
-    val foo: T
+    val foo get() = object : T {
-        get() = object : T {
+        override fun bar() {
-            override fun bar() {
 
-            }
         }
+    }
 }
 
 fun test() {

idea/testData/intentions/convertFunctionToProperty/addExplicitLocalType.kt.after

@@ -2,8 +2,7 @@ fun test() {
     class X
 
     class A(val n: Int) {
-        val foo: X
+        val foo get() = X()
-            get() = X()
     }
 
     val t = A(1).foo

idea/testData/intentions/convertFunctionToProperty/addExplicitType.kt.after

@@ -1,6 +1,5 @@
 class A(val n: Int) {
-    val foo: Boolean
+    val foo get() = n > 1
-        get() = n > 1
 }
 
 fun test() {

idea/testData/intentions/convertFunctionToProperty/annotationLineBreak.kt

@@ -0,0 +1,4 @@
+annotation class X(val s: String)
+
+@X("")
+fun foo<caret>(): String = ""

idea/testData/intentions/convertFunctionToProperty/annotationLineBreak.kt.after

@@ -0,0 +1,5 @@
+annotation class X(val s: String)
+
+@X("")
+val foo<caret>: String
+    get() = ""

idea/testData/intentions/convertFunctionToProperty/comments.kt

@@ -0,0 +1,8 @@
+annotation class X(val s: String)
+
+// 1
+@X("") // 2
+/* 3 */ fun foo<caret>(): String {
+    // 4
+    return ""
+}

idea/testData/intentions/convertFunctionToProperty/comments.kt.after

@@ -0,0 +1,9 @@
+annotation class X(val s: String)
+
+// 1
+@X("") // 2
+/* 3 */ val foo<caret>: String
+    get() {
+        // 4
+        return ""
+    }

idea/testData/intentions/convertFunctionToProperty/noExplicitType.kt

@@ -0,0 +1 @@
+fun foo<caret>() = ""

idea/testData/intentions/convertFunctionToProperty/noExplicitType.kt.after

@@ -0,0 +1 @@
+val foo<caret> get() = ""

idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java

@@ -4151,12 +4151,24 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/convertFunctionToProperty"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
         }
 
+        @TestMetadata("annotationLineBreak.kt")
+        public void testAnnotationLineBreak() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertFunctionToProperty/annotationLineBreak.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("blockBody.kt")
         public void testBlockBody() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertFunctionToProperty/blockBody.kt");
             doTest(fileName);
         }
 
+        @TestMetadata("comments.kt")
+        public void testComments() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertFunctionToProperty/comments.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("existingPropertyConflict.kt")
         public void testExistingPropertyConflict() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertFunctionToProperty/existingPropertyConflict.kt");
@@ -4223,6 +4235,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("noExplicitType.kt")
+        public void testNoExplicitType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertFunctionToProperty/noExplicitType.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("nothingFun.kt")
         public void testNothingFun() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertFunctionToProperty/nothingFun.kt");