Supported unit return type functions

2016-10-24 23:21:17 +03:00
parent 152e77bad3
commit 4c4c85e37a
16 changed files with 191 additions and 34 deletions
@@ -56,7 +56,7 @@ class CallReplacementEngine<TCallElement : KtElement>(
    private val project = nameExpression.project
    private val psiFactory = KtPsiFactory(project)

-    fun performReplacement(): KtElement {
+    fun performReplacement(): KtElement? {
        val descriptor = resolvedCall.resultingDescriptor
        val file = nameExpression.getContainingKtFile()

@@ -128,7 +128,9 @@ class CallReplacementEngine<TCallElement : KtElement>(

        return replacementPerformer.doIt(postProcessing = { range ->
            val newRange = postProcessInsertedCode(range)
-            commentSaver.restore(newRange)
+            if (!newRange.isEmpty) {
+                commentSaver.restore(newRange)
+            }
            newRange
        })
    }
@@ -356,6 +358,7 @@ class CallReplacementEngine<TCallElement : KtElement>(

    private fun postProcessInsertedCode(range: PsiChildRange): PsiChildRange {
        val elements = range.filterIsInstance<KtElement>().toList()
+        if (elements.isEmpty()) return PsiChildRange.EMPTY

        elements.forEach {
            introduceNamedArguments(it)
@@ -28,7 +28,7 @@ class CallableUsageReplacementStrategy(
        private val replacement: ReplacementCode
 ) : UsageReplacementStrategy {

-    override fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement)? {
+    override fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)? {
        val bindingContext = usage.analyze(BodyResolveMode.PARTIAL)
        val resolvedCall = usage.getResolvedCall(bindingContext) ?: return null
        if (!resolvedCall.isReallySuccess()) return null
@@ -36,7 +36,7 @@ class ClassUsageReplacementStrategy(

    private val constructorReplacementStrategy = constructorReplacement?.let(::CallableUsageReplacementStrategy)

-    override fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement)? {
+    override fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)? {
        if (usage !is KtNameReferenceExpression) return null

        constructorReplacementStrategy?.createReplacer(usage)?.let { return it }
@@ -17,13 +17,17 @@
 package org.jetbrains.kotlin.idea.replacement

 import com.intellij.openapi.util.Key
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.core.replaced
 import org.jetbrains.kotlin.idea.intentions.ConvertToBlockBodyIntention
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.PsiChildRange
 import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType
 import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
+import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.utils.addToStdlib.singletonList
+import org.jetbrains.kotlin.utils.addToStdlib.singletonOrEmptyList
 import java.util.*

 internal abstract class ReplacementPerformer<TElement : KtElement>(
@@ -32,7 +36,7 @@ internal abstract class ReplacementPerformer<TElement : KtElement>(
 ) {
    protected val psiFactory = KtPsiFactory(elementToBeReplaced)

-    abstract fun doIt(postProcessing: (PsiChildRange) -> PsiChildRange): TElement
+    abstract fun doIt(postProcessing: (PsiChildRange) -> PsiChildRange): TElement?
 }

 internal class AnnotationEntryReplacementPerformer(
@@ -65,7 +69,7 @@ internal class ExpressionReplacementPerformer(
        expressionToBeReplaced: KtExpression
 ) : ReplacementPerformer<KtExpression>(replacement, expressionToBeReplaced) {

-    override fun doIt(postProcessing: (PsiChildRange) -> PsiChildRange): KtExpression {
+    override fun doIt(postProcessing: (PsiChildRange) -> PsiChildRange): KtExpression? {
        val insertedStatements = ArrayList<KtExpression>()
        for (statement in replacement.statementsBefore) {
            // copy the statement if it can get invalidated by findOrCreateBlockToInsertStatement()
@@ -79,21 +83,44 @@ internal class ExpressionReplacementPerformer(
            insertedStatements.add(inserted)
        }

-        val replaced = elementToBeReplaced.replace(replacement.mainExpression!!) //TODO: support null here
-
-        replacement.performPostInsertionActions(insertedStatements + replaced.singletonList())
-
-        var range = if (insertedStatements.isEmpty()) {
-            PsiChildRange.singleElement(replaced)
+        val replaced = if (replacement.mainExpression != null) {
+            elementToBeReplaced.replace(replacement.mainExpression!!)
        }
        else {
-            val statement = insertedStatements.first()
-            PsiChildRange(statement, replaced.parentsWithSelf.first { it.parent == statement.parent })
+            val bindingContext = elementToBeReplaced.analyze(BodyResolveMode.FULL)
+            val canDropElementToBeReplaced = !elementToBeReplaced.isUsedAsExpression(bindingContext)
+            if (canDropElementToBeReplaced) {
+                elementToBeReplaced.delete()
+                null
+            }
+            else {
+                elementToBeReplaced.replace(psiFactory.createExpression("Unit"))
+            }
+        }
+
+        replacement.performPostInsertionActions(insertedStatements + replaced.singletonOrEmptyList())
+
+        var range = if (replaced != null) {
+            if (insertedStatements.isEmpty()) {
+                PsiChildRange.singleElement(replaced)
+            }
+            else {
+                val statement = insertedStatements.first()
+                PsiChildRange(statement, replaced.parentsWithSelf.first { it.parent == statement.parent })
+            }
+        }
+        else {
+            if (insertedStatements.isEmpty()) {
+                PsiChildRange.EMPTY
+            }
+            else {
+                PsiChildRange(insertedStatements.first(), insertedStatements.last())
+            }
        }

        range = postProcessing(range)

-        return range.last as KtExpression //TODO: return value not correct!
+        return range.last as KtExpression? //TODO: return value not correct!
    }

    /**
@@ -36,7 +36,7 @@ import org.jetbrains.kotlin.psi.KtSimpleNameExpression
 import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType

 interface UsageReplacementStrategy {
-    fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement)?
+    fun createReplacer(usage: KtSimpleNameExpression): (() -> KtElement?)?
 }

 private val LOG = Logger.getInstance(UsageReplacementStrategy::class.java)
@@ -125,7 +125,9 @@ internal fun MutableReplacementCode.introduceValue(
                appendFixedText("\n")
            }

-            appendExpression(mainExpression!!) //TODO: mainExpression == null
+            if (mainExpression != null) {
+                appendExpression(mainExpression)
+            }

            appendFixedText("}")
        }
@@ -39,6 +39,7 @@ import org.jetbrains.kotlin.psi.psiUtil.*
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DelegatingBindingTrace
 import org.jetbrains.kotlin.resolve.bindingContextUtil.getDataFlowInfo
+import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
 import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
@@ -237,10 +238,10 @@ fun <TExpression : KtExpression> tryChangeAndCheckErrors(
 ): Boolean {
    val bindingContext = expressionToChange.analyze(BodyResolveMode.FULL)

-    // analyze the closest block which is not used as expression
+    // analyze the closest block whose value is not used
    val block = expressionToChange.parents
                        .filterIsInstance<KtBlockExpression>()
-                        .firstOrNull { bindingContext[BindingContext.USED_AS_EXPRESSION, it] != true }
+                        .firstOrNull { !it.isUsedAsExpression(bindingContext) }
                ?: return true

    // we declare these keys locally to avoid possible race-condition problems if this code is executed in 2 threads simultaneously
@@ -386,7 +387,7 @@ private fun isEmbeddedBreakOrContinue(expression: KtExpressionWithLabel): Boolea

        is KtContainerNode -> {
            val containerExpression = parent.parent as KtExpression
-            return containerExpression.analyze(BodyResolveMode.PARTIAL)[BindingContext.USED_AS_EXPRESSION, containerExpression] == true
+            return containerExpression.isUsedAsExpression(containerExpression.analyze(BodyResolveMode.PARTIAL))
        }

        else -> return true
@@ -21,10 +21,10 @@ import com.intellij.codeInsight.intention.IntentionAction
 import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.project.Project
 import org.jetbrains.kotlin.diagnostics.Diagnostic
+import org.jetbrains.kotlin.idea.core.moveCaret
 import org.jetbrains.kotlin.idea.core.targetDescriptors
 import org.jetbrains.kotlin.idea.quickfix.CleanupFix
 import org.jetbrains.kotlin.idea.quickfix.KotlinSingleIntentionActionFactory
-import org.jetbrains.kotlin.idea.core.moveCaret
 import org.jetbrains.kotlin.idea.replacement.UsageReplacementStrategy
 import org.jetbrains.kotlin.psi.KtImportDirective
 import org.jetbrains.kotlin.psi.KtSimpleNameExpression
@@ -42,8 +42,10 @@ class DeprecatedSymbolUsageFix(
    override fun invoke(replacementStrategy: UsageReplacementStrategy, project: Project, editor: Editor?) {
        val element = element ?: return
        val result = replacementStrategy.createReplacer(element)!!.invoke()
-        val offset = (result.getCalleeExpressionIfAny() ?: result).textOffset
-        editor?.moveCaret(offset)
+        if (result != null) {
+            val offset = (result.getCalleeExpressionIfAny() ?: result).textOffset
+            editor?.moveCaret(offset)
+        }
    }

    companion object : KotlinSingleIntentionActionFactory() {
@@ -71,19 +71,13 @@ class KotlinInlineFunctionHandler: InlineActionHandler() {
        val replacement = if (element.hasBlockBody()) {
            bodyCopy as KtBlockExpression
            val statements = bodyCopy.statements
-            if (statements.isEmpty()) {
-                //TODO
-                throw UnsupportedOperationException()
+            //TODO: check no other return's!
+            val lastReturn = statements.lastOrNull() as? KtReturnExpression
+            if (lastReturn != null) {
+                replacementBuilder.buildReplacementCode(lastReturn.returnedExpression, statements.dropLast(1), ::analyzeBodyCopy)
            }
            else {
-                //TODO: check no other return's
-                val lastReturn = statements.last() as? KtReturnExpression
-                if (lastReturn == null) {
-                    //TODO
-                    throw UnsupportedOperationException()
-                }
-
-                replacementBuilder.buildReplacementCode(lastReturn.returnedExpression, statements.dropLast(1), ::analyzeBodyCopy)
+                replacementBuilder.buildReplacementCode(null, statements, ::analyzeBodyCopy)
            }
        }
        else {
@@ -0,0 +1,14 @@
+fun <caret>f(p1: Int, p2: Int) {
+}
+
+fun <T> doIt(p: () -> T): T = TODO()
+
+fun g(p: String?) {
+    f(1, 2)
+
+    p?.let { f(3, 4) }
+}
+
+fun h() = f(5, 6)
+
+fun x() = doIt { f(7, 8) }
@@ -0,0 +1,10 @@
+fun <T> doIt(p: () -> T): T = TODO()
+
+fun g(p: String?) {
+
+    p?.let { }
+}
+
+fun h() = Unit
+
+fun x() = doIt { }
@@ -0,0 +1,20 @@
+class C {
+    fun <caret>f(p1: Int, p2: Int) {
+        println(p1)
+        println(p2)
+    }
+
+    fun <T> doIt(p: () -> T): T = TODO()
+
+    fun g(p: String?, other: C) {
+        f(1, 2)
+
+        p?.let { f(3, 4) }
+
+        other?.f(5, 6)
+    }
+
+    fun h() = f(7, 8)
+
+    fun x() = doIt { f(9, 10) }
+}
@@ -0,0 +1,29 @@
+class C {
+
+    fun <T> doIt(p: () -> T): T = TODO()
+
+    fun g(p: String?, other: C) {
+        println(1)
+        println(2)
+
+        p?.let {
+            println(3)
+            println(4)
+        }
+
+        if (other != null) {
+            println(5)
+            println(6)
+        }
+    }
+
+    fun h() {
+        println(7)
+        println(8)
+    }
+
+    fun x() = doIt {
+        println(9)
+        println(10)
+    }
+}
@@ -0,0 +1,25 @@
+fun nonUnit(p: Int): Int = p
+
+fun <T> doIt(p: () -> T): T = TODO()
+
+fun g(p: String?) {
+    println(1)
+    nonUnit(2)
+
+    p?.let {
+        println(3)
+        nonUnit(4)
+        Unit
+    }
+}
+
+fun h() {
+    println(5)
+    nonUnit(6)
+}
+
+fun x() = doIt {
+    println(7)
+    nonUnit(8)
+    Unit
+}
@@ -0,0 +1,18 @@
+fun <caret>f(p1: Int, p2: Int) {
+    println(p1)
+    nonUnit(p2)
+}
+
+fun nonUnit(p: Int): Int = p
+
+fun <T> doIt(p: () -> T): T = TODO()
+
+fun g(p: String?) {
+    f(1, 2)
+
+    p?.let { f(3, 4) }
+}
+
+fun h() = f(5, 6)
+
+fun x() = doIt { f(7, 8) }
@@ -43,6 +43,18 @@ public class InlineTestGenerated extends AbstractInlineTest {
            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/function"), Pattern.compile("^(\\w+)\\.kt$"), true);
        }

+        @TestMetadata("EmptyFunction.kt")
+        public void testEmptyFunction() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/EmptyFunction.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("UnitReturnType.kt")
+        public void testUnitReturnType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/UnitReturnType.kt");
+            doTest(fileName);
+        }
+
        @TestMetadata("idea/testData/refactoring/inline/function/expressionBody")
        @TestDataPath("$PROJECT_ROOT")
        @RunWith(JUnit3RunnerWithInners.class)