From fd6d1520c759e0f19d437ae134d7a3fd3b01f1a7 Mon Sep 17 00:00:00 2001 From: Dmitry Neverov Date: Sat, 29 Apr 2017 06:27:20 +0200 Subject: [PATCH] Convert 'if' with 'is' check to 'as?' with safe call #KT-17054 Fixed --- .../intentions/IfThenToSafeAccessIntention.kt | 90 ++++++++++++------- .../ifThenToSafeAccess/emptyElseBlock.kt | 2 + .../emptyElseBlock.kt.after | 7 -- .../ifThenToSafeAccess/emptyThenBlock.kt | 2 + .../emptyThenBlock.kt.after | 7 -- .../inspectionData/expected.xml | 32 +++---- .../ifThenToSafeAccess/isCondition.kt | 1 + .../ifThenToSafeAccess/isCondition.kt.after | 1 + .../ifThenToSafeAccess/isNotCondition.kt | 1 + .../isNotCondition.kt.after | 1 + .../ifThenToSafeAccess/isNotNullable.kt | 2 + .../branched/ifThenToSafeAccess/isNullable.kt | 2 + .../intentions/IntentionTestGenerated.java | 24 +++++ 13 files changed, 112 insertions(+), 60 deletions(-) delete mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt.after delete mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt.after create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt.after create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt.after create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isNotNullable.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isNullable.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToSafeAccessIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToSafeAccessIntention.kt index fe360dd4da0..db6ae174304 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToSafeAccessIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToSafeAccessIntention.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions import com.intellij.openapi.editor.Editor +import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.idea.intentions.SelfTargetingOffsetIndependentIntention @@ -24,6 +25,10 @@ import org.jetbrains.kotlin.idea.intentions.branchedTransformations.* import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.callUtil.getType +import org.jetbrains.kotlin.types.TypeUtils +import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf class IfThenToSafeAccessInspection : IntentionBasedInspection(IfThenToSafeAccessIntention::class) @@ -32,50 +37,75 @@ class IfThenToSafeAccessIntention : SelfTargetingOffsetIndependentIntention - thenClause?.isNullExpressionOrEmptyBlock() ?: true && - elseClause != null && clauseContainsAppropriateDotQualifiedExpression(elseClause, receiverExpression) - - KtTokens.EXCLEQ -> - elseClause?.isNullExpressionOrEmptyBlock() ?: true && - thenClause != null && clauseContainsAppropriateDotQualifiedExpression(thenClause, receiverExpression) - - else -> - false + return when (condition) { + is KtBinaryExpression -> { + val receiverExpression = condition.expressionComparedToNull() ?: return false + if (!receiverExpression.isStableVariable()) return false + when (condition.operationToken) { + KtTokens.EQEQ -> canBeReplacedWithSafeCall(receiverExpression, thenClause, elseClause) + KtTokens.EXCLEQ -> canBeReplacedWithSafeCall(receiverExpression, elseClause, thenClause) + else -> false + } + } + is KtIsExpression -> { + val context = element.analyze() + val receiverExpression = condition.leftHandSide + if (!receiverExpression.isStableVariable(context)) return false + val targetType = context[BindingContext.TYPE, condition.typeReference] ?: return false + if (TypeUtils.isNullableType(targetType)) return false + val originalType = receiverExpression.getType(context) ?: return false + if (!targetType.isSubtypeOf(originalType)) return false + when(condition.isNegated) { + true -> canBeReplacedWithSafeCall(receiverExpression, thenClause, elseClause) + false -> canBeReplacedWithSafeCall(receiverExpression, elseClause, thenClause) + } + } + else -> false } } override fun startInWriteAction() = false override fun applyTo(element: KtIfExpression, editor: Editor?) { - val condition = element.condition as KtBinaryExpression - val receiverExpression = condition.expressionComparedToNull()!! + val condition = element.condition + val newExpr = when (condition) { + is KtBinaryExpression -> { + val receiverExpression = condition.expressionComparedToNull()!! - val selectorExpression = - when(condition.operationToken) { - KtTokens.EQEQ -> findSelectorExpressionInClause(element.`else`!!, receiverExpression)!! + val selectorExpression = + when(condition.operationToken) { + KtTokens.EQEQ -> findSelectorExpressionInClause(element.`else`!!, receiverExpression)!! + KtTokens.EXCLEQ -> findSelectorExpressionInClause(element.then!!, receiverExpression)!! + else -> throw IllegalArgumentException() + } - KtTokens.EXCLEQ -> findSelectorExpressionInClause(element.then!!, receiverExpression)!! + KtPsiFactory(element).createExpressionByPattern("$0?.$1", receiverExpression, selectorExpression) as KtSafeQualifiedExpression + } + is KtIsExpression -> { + val typeRef = condition.typeReference!! + val lhs = condition.leftHandSide + val selector = if (condition.isNegated) element.`else`!! else element.then!! + val selectorExpression = findSelectorExpressionInClause(selector, lhs)!! + KtPsiFactory(element).createExpressionByPattern("($0 as? $1)?.$2", lhs, typeRef, selectorExpression) as KtSafeQualifiedExpression + } + else -> null + } - else -> throw IllegalArgumentException() - } - - val newExpr = KtPsiFactory(element).createExpressionByPattern("$0?.$1", receiverExpression, selectorExpression) as KtSafeQualifiedExpression - val safeAccessExpr = runWriteAction { element.replaced(newExpr) } - - if (editor != null) { - safeAccessExpr.inlineReceiverIfApplicableWithPrompt(editor) + if (newExpr != null) { + val safeAccessExpr = runWriteAction { element.replaced(newExpr) } + if (editor != null) { + safeAccessExpr.inlineReceiverIfApplicableWithPrompt(editor) + } } } + private fun canBeReplacedWithSafeCall(receiver: KtExpression, nullClause: KtExpression?, notNullClause: KtExpression?) = + nullClause?.isNullExpression() ?: true && + notNullClause != null && clauseContainsAppropriateDotQualifiedExpression(notNullClause, receiver) + private fun clauseContainsAppropriateDotQualifiedExpression(clause: KtExpression, receiverExpression: KtExpression) = findSelectorExpressionInClause(clause, receiverExpression) != null diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt index 1175234b604..141c2ba4744 100644 --- a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt @@ -1,3 +1,5 @@ +// IS_APPLICABLE: false + fun maybeFoo(): String? { return "foo" } diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt.after deleted file mode 100644 index cb42a95887b..00000000000 --- a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt.after +++ /dev/null @@ -1,7 +0,0 @@ -fun maybeFoo(): String? { - return "foo" -} - -fun main(args: Array) { - maybeFoo()?.length -} diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt index 6898c3e73b6..4c59d0563b2 100644 --- a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt @@ -1,3 +1,5 @@ +// IS_APPLICABLE: false + fun maybeFoo(): String? { return "foo" } diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt.after deleted file mode 100644 index cb42a95887b..00000000000 --- a/idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt.after +++ /dev/null @@ -1,7 +0,0 @@ -fun maybeFoo(): String? { - return "foo" -} - -fun main(args: Array) { - maybeFoo()?.length -} diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml b/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml index 24ef90af04b..56807d80d23 100644 --- a/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml @@ -71,22 +71,6 @@ If-Then foldable to '?.' Replace 'if' expression with safe access expression - - emptyThenBlock.kt - 7 - light_idea_test_case - - If-Then foldable to '?.' - Replace 'if' expression with safe access expression - - - emptyElseBlock.kt - 7 - light_idea_test_case - - If-Then foldable to '?.' - Replace 'if' expression with safe access expression - doesNotinlineValueOutsideOfScope.kt 8 @@ -111,4 +95,20 @@ If-Then foldable to '?.' Replace 'if' expression with safe access expression + + isCondition.kt + 1 + light_idea_test_case + + If-Then foldable to '?.' + Replace 'if' expression with safe access expression + + + isNotCondition.kt + 1 + light_idea_test_case + + If-Then foldable to '?.' + Replace 'if' expression with safe access expression + diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt new file mode 100644 index 00000000000..be1017880e1 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt @@ -0,0 +1 @@ +fun foo(arg: Any) = if (arg is String) arg.length else null \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt.after new file mode 100644 index 00000000000..7f51059f5a3 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt.after @@ -0,0 +1 @@ +fun foo(arg: Any) = (arg as? String)?.length \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt new file mode 100644 index 00000000000..5157a0199c9 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt @@ -0,0 +1 @@ +fun foo(arg: Any) = if (arg !is String) null else arg.length \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt.after new file mode 100644 index 00000000000..7f51059f5a3 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt.after @@ -0,0 +1 @@ +fun foo(arg: Any) = (arg as? String)?.length \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isNotNullable.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/isNotNullable.kt new file mode 100644 index 00000000000..bc66928af0c --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isNotNullable.kt @@ -0,0 +1,2 @@ +// IS_APPLICABLE: false +fun foo(arg: Any) = if (arg !is String?) null else arg?.length \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isNullable.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/isNullable.kt new file mode 100644 index 00000000000..627a1637059 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isNullable.kt @@ -0,0 +1,2 @@ +// IS_APPLICABLE: false +fun foo(arg: Any) = if (arg is String?) arg?.length else null \ 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 74e2234b355..83a40866632 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -1766,6 +1766,30 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("isCondition.kt") + public void testIsCondition() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt"); + doTest(fileName); + } + + @TestMetadata("isNotCondition.kt") + public void testIsNotCondition() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt"); + doTest(fileName); + } + + @TestMetadata("isNotNullable.kt") + public void testIsNotNullable() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isNotNullable.kt"); + doTest(fileName); + } + + @TestMetadata("isNullable.kt") + public void testIsNullable() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isNullable.kt"); + doTest(fileName); + } + @TestMetadata("lhsEqualsNull.kt") public void testLhsEqualsNull() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/lhsEqualsNull.kt");