From f1934fa49d148ca5d9ef5b17dd3ef7953fa92bec Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Sat, 9 Nov 2019 18:06:58 +0300 Subject: [PATCH] Amend "Terminate preceding call with semicolon" quickfix Quickfix wasn't aware of cases, when expression to be fixed is a part of parent call expression or dot-qualified expression. Incorrect position for extracting trailing lambdas led to broken formatting. #KT-34694 Fixed --- .../AddSemicolonBeforeLambdaExpressionFix.kt | 139 ++++++++++++++---- .../dotExpressionAndCallExpression.kt | 10 ++ .../dotExpressionAndCallExpression.kt.after | 10 ++ .../multipleCalls.kt | 9 ++ .../multipleCalls.kt.after | 9 ++ .../onlyDotBefore.kt | 7 + .../onlyDotBefore.kt.after | 7 + .../twoLambdasTwoInvokes.kt | 7 + .../twoLambdasTwoInvokes.kt.after | 7 + .../idea/quickfix/QuickFixTestGenerated.java | 20 +++ 10 files changed, 196 insertions(+), 29 deletions(-) create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt.after create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt.after create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt.after create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt create mode 100644 idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/AddSemicolonBeforeLambdaExpressionFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddSemicolonBeforeLambdaExpressionFix.kt index 08c3d64b91b..b43675cdfc0 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/AddSemicolonBeforeLambdaExpressionFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/AddSemicolonBeforeLambdaExpressionFix.kt @@ -21,46 +21,127 @@ class AddSemicolonBeforeLambdaExpressionFix(element: KtLambdaExpression) : Kotli override fun getFamilyName(): String = text override fun invoke(project: Project, editor: Editor?, file: KtFile) { - val lambdaExpressionArgument = element?.parent?.safeAs() - ?: return - val callExpression = lambdaExpressionArgument.parent.safeAs() - ?: return - val desiredEndOfCallExpression = - PsiTreeUtil.findSiblingBackward( - lambdaExpressionArgument, - KtNodeTypes.LAMBDA_ARGUMENT, - null - ) ?: PsiTreeUtil.findSiblingBackward( - lambdaExpressionArgument, - KtNodeTypes.VALUE_ARGUMENT_LIST, - null - ) + val lambdaExpressionArgument = element?.parent?.safeAs() ?: return + val callExpression = lambdaExpressionArgument.parent.safeAs() ?: return + + val desiredEndOfCallExpression = lambdaExpressionArgument.findCorrectEndOfCall() + desiredEndOfCallExpression?.let { endOfCall -> - makeNewExpressionsFromFollowingLambdas(callExpression, endOfCall) - val semicolon = callExpression.parent.addAfter( - KtPsiFactory(project).createSemicolon(), - callExpression - ) - editor?.caretModel?.moveToOffset(semicolon.startOffset) + val psiFactory = KtPsiFactory(project) + + val addedSemicolon = when (val parent = callExpression.parent) { + // Parent call is the call to the right, we want to give it this call's last lambda argument as a new receiver + is KtCallExpression -> liftTrailingNodesAndRelocateLastLambda( + psiFactory, callExpression, endOfCall, + lastLambdaAcceptor = parent, + nodeBeforeSemicolon = callExpression + ) + // Incorrect call is a part of dot-qualified expression before it + is KtDotQualifiedExpression -> { + val grandparent = parent.parent + if (grandparent.isCallOrDotExpression) { + // Similar to call expression parent, but here correct lambda receiver is one level higher, + // since our parent is the dot expression to the left + liftTrailingNodesAndRelocateLastLambda( + psiFactory, callExpression, endOfCall, + lastLambdaAcceptor = grandparent, + nodeBeforeSemicolon = parent + ) + } else { + // Extract trailing lambdas (and possible formatting) two levels higher, + // before dot-qualified expression to the left of this call expression + liftTrailingNodes( + psiFactory, callExpression, endOfCall, + addNodesAfter = parent + ) + } + } + // Simple case: extract all trailing nodes right after call - it is a standalone call expression + else -> liftTrailingNodes( + psiFactory, callExpression, endOfCall, + addNodesAfter = callExpression + ) + } + editor?.caretModel?.moveToOffset(addedSemicolon.startOffset) } } - private fun makeNewExpressionsFromFollowingLambdas( + private fun KtLambdaArgument.findCorrectEndOfCall() = + PsiTreeUtil.findSiblingBackward(this, KtNodeTypes.LAMBDA_ARGUMENT, null) + ?: PsiTreeUtil.findSiblingBackward(this, KtNodeTypes.VALUE_ARGUMENT_LIST, null) + + private fun liftTrailingNodesAndRelocateLastLambda( + psiFactory: KtPsiFactory, + callExpression: KtCallExpression, + endOfCall: PsiElement, + lastLambdaAcceptor: PsiElement, + nodeBeforeSemicolon: PsiElement + ): PsiElement { + val (topCall, callHolder) = topLevelHolder(callExpression) + val semicolon = callHolder.addBefore(psiFactory.createSemicolon(), topCall) + + makeNewExpressionsFromTrailingLambdas(callExpression, endOfCall, addNodesAfter = semicolon) { lastLambdaExpression -> + lastLambdaAcceptor.addAfter(lastLambdaExpression, nodeBeforeSemicolon) + } + + callHolder.addBefore(nodeBeforeSemicolon, semicolon) + nodeBeforeSemicolon.delete() + + return semicolon + } + + private fun liftTrailingNodes( + psiFactory: KtPsiFactory, + callExpression: KtCallExpression, + endOfCall: PsiElement, + addNodesAfter: PsiElement + ): PsiElement { + makeNewExpressionsFromTrailingLambdas(callExpression, endOfCall, addNodesAfter) + return addNodesAfter.parent.addAfter( + psiFactory.createSemicolon(), + addNodesAfter + ) + } + + private val PsiElement.isCallOrDotExpression + get() = this is KtCallExpression || this is KtDotQualifiedExpression + + data class TopExpressionAndHolder(val top: PsiElement, val holder: PsiElement) + + private fun topLevelHolder(callExpression: KtCallExpression): TopExpressionAndHolder { + var me: PsiElement = callExpression + var parent: PsiElement = callExpression.parent + while (parent.isCallOrDotExpression) { + me = parent + parent = parent.parent + } + return TopExpressionAndHolder(me, parent) + } + + private fun makeNewExpressionsFromTrailingLambdas( oldCallExpression: KtCallExpression, - endOfArguments: PsiElement + endOfArguments: PsiElement, + addNodesAfter: PsiElement, + lastLambdaHandler: ((PsiElement) -> Unit)? = null ) { var lastSibling = oldCallExpression.lastChild - val parentForCallExpression = oldCallExpression.parent + var lastLambdaWasProcessed = false while (lastSibling != endOfArguments) { when (lastSibling) { - is KtLambdaArgument -> parentForCallExpression.addAfter( - lastSibling.getLambdaExpression() ?: lastSibling, - oldCallExpression - ) - else -> parentForCallExpression.addAfter( + is KtLambdaArgument -> { + val lambdaExpression: PsiElement = lastSibling.getLambdaExpression() ?: lastSibling + + if (lastLambdaHandler != null && !lastLambdaWasProcessed) { + lastLambdaWasProcessed = true + lastLambdaHandler(lambdaExpression) + } else { + addNodesAfter.parent.addAfter(lambdaExpression, addNodesAfter) + } + } + else -> addNodesAfter.parent.addAfter( lastSibling, - oldCallExpression + addNodesAfter ) } lastSibling = lastSibling.prevSibling diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt new file mode 100644 index 00000000000..4f7b93408f4 --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt @@ -0,0 +1,10 @@ +// "Terminate preceding call with semicolon" "true" + +fun doSomething() {} +fun Any.foo() {} + +fun test() { + doSomething().foo() + // comment and formatting + { doSomething() }() +} \ No newline at end of file diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt.after b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt.after new file mode 100644 index 00000000000..1836a34e6b9 --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt.after @@ -0,0 +1,10 @@ +// "Terminate preceding call with semicolon" "true" + +fun doSomething() {} +fun Any.foo() {} + +fun test() { + doSomething().foo(); + // comment and formatting + { doSomething() }() +} \ No newline at end of file diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt new file mode 100644 index 00000000000..faa993ab598 --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt @@ -0,0 +1,9 @@ +// "Terminate preceding call with semicolon" "true" + +fun foo() {} + +fun test { + foo() + // comment and formatting + { { { foo() } } }()()() +} \ No newline at end of file diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt.after b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt.after new file mode 100644 index 00000000000..875c2383113 --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt.after @@ -0,0 +1,9 @@ +// "Terminate preceding call with semicolon" "true" + +fun foo() {} + +fun test { + foo(); + // comment and formatting + { { { foo() } } }()()() +} \ No newline at end of file diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt new file mode 100644 index 00000000000..394e597a772 --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt @@ -0,0 +1,7 @@ +// "Terminate preceding call with semicolon" "true" + +fun foo() { + 15.toString() + // comment and formatting + {} +} diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt.after b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt.after new file mode 100644 index 00000000000..da8ad413adb --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt.after @@ -0,0 +1,7 @@ +// "Terminate preceding call with semicolon" "true" + +fun foo() { + 15.toString(); + // comment and formatting + {} +} diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt new file mode 100644 index 00000000000..63bfa757484 --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt @@ -0,0 +1,7 @@ +// "Terminate preceding call with semicolon" "true" + +fun foo() { + { "first" }.invoke() + // comment and formatting + { "second" }.invoke() +} diff --git a/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt.after b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt.after new file mode 100644 index 00000000000..339ff73642f --- /dev/null +++ b/idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt.after @@ -0,0 +1,7 @@ +// "Terminate preceding call with semicolon" "true" + +fun foo() { + { "first" }.invoke(); + // comment and formatting + { "second" }.invoke() +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index c053bee9521..eb0e0143fb8 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -1113,11 +1113,31 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/basic.kt"); } + @TestMetadata("dotExpressionAndCallExpression.kt") + public void testDotExpressionAndCallExpression() throws Exception { + runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/dotExpressionAndCallExpression.kt"); + } + + @TestMetadata("multipleCalls.kt") + public void testMultipleCalls() throws Exception { + runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleCalls.kt"); + } + @TestMetadata("multipleLambdas.kt") public void testMultipleLambdas() throws Exception { runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/multipleLambdas.kt"); } + @TestMetadata("onlyDotBefore.kt") + public void testOnlyDotBefore() throws Exception { + runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/onlyDotBefore.kt"); + } + + @TestMetadata("twoLambdasTwoInvokes.kt") + public void testTwoLambdasTwoInvokes() throws Exception { + runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/twoLambdasTwoInvokes.kt"); + } + @TestMetadata("withComments.kt") public void testWithComments() throws Exception { runTest("idea/testData/quickfix/addSemicolonBeforeLambdaExpression/withComments.kt");