From a662d777e8b371cf2e344f78c9938f84b91727cf Mon Sep 17 00:00:00 2001 From: shiraji Date: Fri, 28 Oct 2016 14:45:56 +0900 Subject: [PATCH] Implement "Remove redundant '.let' call" binary operator support #KT-14396 Fixed --- .../org/jetbrains/kotlin/psi/KtPsiFactory.kt | 3 + .../ReplaceSingleLineLetIntention.kt | 85 ++++++++++++++++--- .../lambdaWithBinaryExpression.kt | 6 ++ .../lambdaWithBinaryExpression2.kt | 6 ++ .../letUseItAsParamWithBinaryExpression.kt | 6 ++ .../letUseItWithBinaryExpression.kt | 6 ++ .../letUseItWithBinaryExpression2.kt | 6 ++ .../letWithBinaryExpression.kt | 6 ++ .../letWithBinaryExpression.kt.after | 6 ++ .../letWithSimpleBinaryExpression.kt | 6 ++ .../letWithSimpleBinaryExpression.kt.after | 6 ++ .../letWithThisBinaryExpression.kt | 6 ++ .../letWithThisBinaryExpression.kt.after | 6 ++ .../letWithThisShortBinaryExpression.kt | 6 ++ .../letWithThisShortBinaryExpression.kt.after | 6 ++ .../intentions/IntentionTestGenerated.java | 54 ++++++++++++ 16 files changed, 208 insertions(+), 12 deletions(-) create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression2.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letUseItAsParamWithBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression2.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt.after create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt.after create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt.after create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt create mode 100644 idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt.after diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt index f126a222be5..d580bf72710 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt @@ -78,6 +78,9 @@ class KtPsiFactory(private val project: Project) { return if (expression.text == text) expression else null } + fun createThisExpression() = + (createExpression("this.x") as KtQualifiedExpression).receiverExpression as KtThisExpression + fun createClassLiteral(className: String): KtClassLiteralExpression = createExpression("$className::class") as KtClassLiteralExpression diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceSingleLineLetIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceSingleLineLetIntention.kt index 8bba28a66f6..f214890da55 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceSingleLineLetIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceSingleLineLetIntention.kt @@ -31,18 +31,49 @@ class ReplaceSingleLineLetIntention : SelfTargetingOffsetIndependentIntention it.applyTo(element) + is KtBinaryExpression -> it.applyTo(element) + } + } + } + + private fun KtBinaryExpression.applyTo(element: KtCallExpression) { + val left = left ?: return + val factory = KtPsiFactory(element.project) + val parent = element.parent + when (parent) { + is KtQualifiedExpression -> { + val receiver = parent.receiverExpression + val newLeft = when (left) { + is KtDotQualifiedExpression -> left.replaceFirstReceiver(factory, receiver, parent is KtSafeQualifiedExpression) + else -> receiver + } + val newExpression = factory.createExpressionByPattern("$0 $1 $2", newLeft, operationReference, right!!) + parent.replace(newExpression) + } + else -> { + val newLeft = when (left) { + is KtDotQualifiedExpression -> left.deleteFirstReceiver() + else -> factory.createThisExpression() + } + val newExpression = factory.createExpressionByPattern("$0 $1 $2", newLeft, operationReference, right!!) + element.replace(newExpression) + } + } + } + + private fun KtDotQualifiedExpression.applyTo(element: KtCallExpression) { val parent = element.parent when (parent) { is KtQualifiedExpression -> { val factory = KtPsiFactory(element.project) val receiver = parent.receiverExpression - parent.replace(dotQualifiedExpression.replaceFirstReceiver(factory, receiver, parent.operationSign == KtTokens.SAFE_ACCESS)) + parent.replace(replaceFirstReceiver(factory, receiver, parent is KtSafeQualifiedExpression)) } else -> { - element.replace(dotQualifiedExpression.deleteFirstReceiver()) + element.replace(deleteFirstReceiver()) } } } @@ -59,17 +90,47 @@ class ReplaceSingleLineLetIntention : SelfTargetingOffsetIndependentIntention()) return false + val bodyExpression = lambdaExpression.bodyExpression?.children?.singleOrNull() ?: return false + + return when (bodyExpression) { + is KtBinaryExpression -> bodyExpression.isApplicable(parameterName) + is KtDotQualifiedExpression -> bodyExpression.isApplicable(parameterName) + else -> false } - return !dotQualifiedExpression.receiverUsedAsArgument(parameterName) } + private fun KtBinaryExpression.isApplicable(parameterName: String): Boolean { + val left = left ?: return false + when (left) { + is KtNameReferenceExpression -> if (left.text != parameterName) return false + is KtDotQualifiedExpression -> if (!left.isApplicable(parameterName)) return false + } + + val right = right ?: return false + when (right) { + is KtNameReferenceExpression -> return right.text != parameterName + is KtDotQualifiedExpression -> { + if (right.isParameterLeftMostReceiver(parameterName)) return false + if (right.hasLambdaExpression()) return false + return !right.receiverUsedAsArgument(parameterName) + } + else -> return true + } + } + + private fun KtDotQualifiedExpression.isApplicable(parameterName: String): Boolean { + if (!isParameterLeftMostReceiver(parameterName)) return false + if (hasLambdaExpression()) return false + return !receiverUsedAsArgument(parameterName) + } + + private fun KtDotQualifiedExpression.hasLambdaExpression() + = selectorExpression?.anyDescendantOfType() ?: false + + private fun KtDotQualifiedExpression.isParameterLeftMostReceiver(parameterName: String) + = getLeftMostReceiverExpression().text == parameterName + private fun isLetMethod(element: KtCallExpression) = element.calleeExpression?.text == "let" && element.isMethodCall("kotlin.let") diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression.kt new file mode 100644 index 00000000000..23db56267bc --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false + +fun baz(foo: String) { + foo.let { it.indexOfLast { c -> c == it[0] } + 1 } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression2.kt b/idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression2.kt new file mode 100644 index 00000000000..67e99fb1e30 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression2.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false + +fun baz(foo: String) { + foo.let { it.length + "".indexOfLast { c -> c == it[0] } } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letUseItAsParamWithBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letUseItAsParamWithBinaryExpression.kt new file mode 100644 index 00000000000..61609c11507 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letUseItAsParamWithBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false + +fun foo() { + "".let { it.length + "".indexOf(it) } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression.kt new file mode 100644 index 00000000000..5b207566566 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false + +fun foo() { + "".let { it.length + it.length } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression2.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression2.kt new file mode 100644 index 00000000000..25d349fdf99 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression2.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false + +fun foo() { + "".let { it.substring(0, 1) + it } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt new file mode 100644 index 00000000000..9a57302c24b --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun foo() { + "".let { it.length + 1 } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt.after b/idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt.after new file mode 100644 index 00000000000..8bcd30515f3 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun foo() { + "".length + 1 +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt new file mode 100644 index 00000000000..61e653185a0 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun foo() { + "".let { it + 1 } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt.after b/idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt.after new file mode 100644 index 00000000000..3ae32ba8c97 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun foo() { + "" + 1 +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt new file mode 100644 index 00000000000..2c3fc8bccb0 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun Int.foo() { + let { it.dec() + 1 } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt.after b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt.after new file mode 100644 index 00000000000..413570fd47d --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun Int.foo() { + dec() + 1 +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt new file mode 100644 index 00000000000..f05639c5168 --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun Int.foo() { + let { it + 1 } +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt.after b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt.after new file mode 100644 index 00000000000..21e066415dd --- /dev/null +++ b/idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// IS_APPLICABLE: true + +fun Int.foo() { + this + 1 +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 53bde42d3f9..f1af0a0dfbc 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -11799,6 +11799,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/replaceSingleLineLetIntention"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("lambdaWithBinaryExpression.kt") + public void testLambdaWithBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression.kt"); + doTest(fileName); + } + + @TestMetadata("lambdaWithBinaryExpression2.kt") + public void testLambdaWithBinaryExpression2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/lambdaWithBinaryExpression2.kt"); + doTest(fileName); + } + @TestMetadata("let.kt") public void testLet() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/let.kt"); @@ -11835,6 +11847,24 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("letUseItAsParamWithBinaryExpression.kt") + public void testLetUseItAsParamWithBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letUseItAsParamWithBinaryExpression.kt"); + doTest(fileName); + } + + @TestMetadata("letUseItWithBinaryExpression.kt") + public void testLetUseItWithBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression.kt"); + doTest(fileName); + } + + @TestMetadata("letUseItWithBinaryExpression2.kt") + public void testLetUseItWithBinaryExpression2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithBinaryExpression2.kt"); + doTest(fileName); + } + @TestMetadata("letUseItWithMultipleMethodCall1.kt") public void testLetUseItWithMultipleMethodCall1() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letUseItWithMultipleMethodCall1.kt"); @@ -11859,6 +11889,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("letWithBinaryExpression.kt") + public void testLetWithBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letWithBinaryExpression.kt"); + doTest(fileName); + } + @TestMetadata("letWithMethodCall.kt") public void testLetWithMethodCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letWithMethodCall.kt"); @@ -11877,6 +11913,24 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("letWithSimpleBinaryExpression.kt") + public void testLetWithSimpleBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letWithSimpleBinaryExpression.kt"); + doTest(fileName); + } + + @TestMetadata("letWithThisBinaryExpression.kt") + public void testLetWithThisBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letWithThisBinaryExpression.kt"); + doTest(fileName); + } + + @TestMetadata("letWithThisShortBinaryExpression.kt") + public void testLetWithThisShortBinaryExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/letWithThisShortBinaryExpression.kt"); + doTest(fileName); + } + @TestMetadata("multipleReceiver.kt") public void testMultipleReceiver() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSingleLineLetIntention/multipleReceiver.kt");