diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallReplacementEngine.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallReplacementEngine.kt index e2a311f7d41..767e054817c 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallReplacementEngine.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallReplacementEngine.kt @@ -56,7 +56,7 @@ class CallReplacementEngine( 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( 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( private fun postProcessInsertedCode(range: PsiChildRange): PsiChildRange { val elements = range.filterIsInstance().toList() + if (elements.isEmpty()) return PsiChildRange.EMPTY elements.forEach { introduceNamedArguments(it) diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallableUsageReplacementStrategy.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallableUsageReplacementStrategy.kt index d579a9f534a..2394d5631a6 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallableUsageReplacementStrategy.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/CallableUsageReplacementStrategy.kt @@ -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 diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ClassUsageReplacementStrategy.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ClassUsageReplacementStrategy.kt index 7a7586e19e3..345366db8f7 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ClassUsageReplacementStrategy.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ClassUsageReplacementStrategy.kt @@ -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 } diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt index 7bd8d7c2989..20c90887d84 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt @@ -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( @@ -32,7 +36,7 @@ internal abstract class ReplacementPerformer( ) { 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(replacement, expressionToBeReplaced) { - override fun doIt(postProcessing: (PsiChildRange) -> PsiChildRange): KtExpression { + override fun doIt(postProcessing: (PsiChildRange) -> PsiChildRange): KtExpression? { val insertedStatements = ArrayList() 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! } /** diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/UsageReplacementStrategy.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/UsageReplacementStrategy.kt index d429299f9f9..cadfa76f28f 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/UsageReplacementStrategy.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/UsageReplacementStrategy.kt @@ -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) diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt index 8764d9cdb52..ac8073fea27 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt @@ -125,7 +125,9 @@ internal fun MutableReplacementCode.introduceValue( appendFixedText("\n") } - appendExpression(mainExpression!!) //TODO: mainExpression == null + if (mainExpression != null) { + appendExpression(mainExpression) + } appendFixedText("}") } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt index eb15735be17..5d8bd746e52 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt @@ -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 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() - .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 diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFix.kt index bca46bf8f16..4ca83fe33fb 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFix.kt @@ -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() { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/inline/KotlinInlineFunctionHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/inline/KotlinInlineFunctionHandler.kt index 902317e238b..be83b65db9b 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/inline/KotlinInlineFunctionHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/inline/KotlinInlineFunctionHandler.kt @@ -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 { diff --git a/idea/testData/refactoring/inline/function/EmptyFunction.kt b/idea/testData/refactoring/inline/function/EmptyFunction.kt new file mode 100644 index 00000000000..a1156401d97 --- /dev/null +++ b/idea/testData/refactoring/inline/function/EmptyFunction.kt @@ -0,0 +1,14 @@ +fun f(p1: Int, p2: Int) { +} + +fun 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) } diff --git a/idea/testData/refactoring/inline/function/EmptyFunction.kt.after b/idea/testData/refactoring/inline/function/EmptyFunction.kt.after new file mode 100644 index 00000000000..f56a3ed7bb9 --- /dev/null +++ b/idea/testData/refactoring/inline/function/EmptyFunction.kt.after @@ -0,0 +1,10 @@ +fun doIt(p: () -> T): T = TODO() + +fun g(p: String?) { + + p?.let { } +} + +fun h() = Unit + +fun x() = doIt { } diff --git a/idea/testData/refactoring/inline/function/UnitReturnType.kt b/idea/testData/refactoring/inline/function/UnitReturnType.kt new file mode 100644 index 00000000000..d9fa5728871 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType.kt @@ -0,0 +1,20 @@ +class C { + fun f(p1: Int, p2: Int) { + println(p1) + println(p2) + } + + fun 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) } +} diff --git a/idea/testData/refactoring/inline/function/UnitReturnType.kt.after b/idea/testData/refactoring/inline/function/UnitReturnType.kt.after new file mode 100644 index 00000000000..2dfff0d52b7 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType.kt.after @@ -0,0 +1,29 @@ +class C { + + fun 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) + } +} diff --git a/idea/testData/refactoring/inline/function/UnitReturnType2.kt.after b/idea/testData/refactoring/inline/function/UnitReturnType2.kt.after new file mode 100644 index 00000000000..5fcded0ffa7 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType2.kt.after @@ -0,0 +1,25 @@ +fun nonUnit(p: Int): Int = p + +fun 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 +} diff --git a/idea/testData/refactoring/inline/function/UnitReturnType2.kt.todo b/idea/testData/refactoring/inline/function/UnitReturnType2.kt.todo new file mode 100644 index 00000000000..c2801d8b2f0 --- /dev/null +++ b/idea/testData/refactoring/inline/function/UnitReturnType2.kt.todo @@ -0,0 +1,18 @@ +fun f(p1: Int, p2: Int) { + println(p1) + nonUnit(p2) +} + +fun nonUnit(p: Int): Int = p + +fun 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) } diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java index a335446c4e7..ca24efc2efa 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java @@ -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)