From 66c5717adc121cb155d59f46ba03f5954950bc83 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Thu, 4 May 2017 15:51:59 +0300 Subject: [PATCH] Refactoring of IfThen: ToDoubleBang / ToElvis / ToSafeAccess Some common actions were extracted to IfThenUtils.kt Use consistent logic in all three intentions Also fixes potential PSI consistency problems in conversions dot->safe calls --- .../jetbrains/kotlin/idea/intentions/Utils.kt | 13 +- .../branchedTransformations/IfThenUtils.kt | 94 +++++++++++++- .../intentions/IfThenToDoubleBangIntention.kt | 42 ++----- .../intentions/IfThenToElvisIntention.kt | 118 +++--------------- .../intentions/IfThenToSafeAccessIntention.kt | 89 +++---------- .../inspectionData/expected.xml | 24 ++++ .../ifThenToSafeAccess/isCheckSimple.kt | 5 + .../ifThenToSafeAccess/isCheckSimple.kt.after | 5 + .../isCheckWithSelectorChain.kt | 5 + .../isCheckWithSelectorChain.kt.after | 5 + .../ifThenToSafeAccess/nullCheckSimple.kt | 5 + .../nullCheckWithSelectorCallChain.kt | 9 ++ .../nullCheckWithSelectorCallChain.kt.after | 5 + .../intentions/IntentionTestGenerated.java | 24 ++++ 14 files changed, 230 insertions(+), 213 deletions(-) create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt.after create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt.after create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckSimple.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt create mode 100644 idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt index 5074c3e148a..68774851235 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt @@ -22,6 +22,7 @@ import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.core.setType import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.lexer.KtTokens @@ -248,19 +249,19 @@ fun KtDotQualifiedExpression.replaceFirstReceiver( newReceiver: KtExpression, safeAccess: Boolean = false ): KtExpression { - val receiver = receiverExpression - if (safeAccess) { - operationTokenNode.psi.replace(factory.createSafeCallNode().psi) - } + val replaced = (if (safeAccess) { + this.replaced(factory.createExpressionByPattern("$0?.$1", receiverExpression, selectorExpression!!)) + } else this) as KtQualifiedExpression + val receiver = replaced.receiverExpression when (receiver) { is KtDotQualifiedExpression -> { - receiver.replaceFirstReceiver(factory, newReceiver, safeAccess) + receiver.replace(receiver.replaceFirstReceiver(factory, newReceiver, safeAccess)) } else -> { receiver.replace(newReceiver) } } - return this + return replaced } fun KtDotQualifiedExpression.deleteFirstReceiver(): KtExpression { diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt index 36e1604b5fc..be4742df7f2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt @@ -23,6 +23,8 @@ import org.jetbrains.kotlin.KtNodeTypes import org.jetbrains.kotlin.descriptors.VariableDescriptor import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.replaced +import org.jetbrains.kotlin.idea.intentions.getLeftMostReceiverExpression +import org.jetbrains.kotlin.idea.intentions.replaceFirstReceiver import org.jetbrains.kotlin.idea.refactoring.inline.KotlinInlineValHandler import org.jetbrains.kotlin.idea.refactoring.introduce.introduceVariable.KotlinIntroduceVariableHandler import org.jetbrains.kotlin.idea.references.mainReference @@ -31,8 +33,11 @@ import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingContextUtils import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.calls.callUtil.getType import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.types.TypeUtils +import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf import org.jetbrains.kotlin.utils.addToStdlib.constant fun KtBinaryExpression.expressionComparedToNull(): KtExpression? { @@ -62,8 +67,7 @@ fun KtExpression?.isNullExpression(): Boolean = this?.unwrapBlockOrParenthesis() fun KtExpression?.isNullExpressionOrEmptyBlock(): Boolean = this.isNullExpression() || this is KtBlockExpression && this.statements.isEmpty() fun KtThrowExpression.throwsNullPointerExceptionWithNoArguments(): Boolean { - val thrownExpression = this.thrownExpression - if (thrownExpression !is KtCallExpression) return false + val thrownExpression = this.thrownExpression as? KtCallExpression ?: return false val context = this.analyze(BodyResolveMode.PARTIAL) val nameExpression = thrownExpression.calleeExpression as? KtNameReferenceExpression ?: return false @@ -135,3 +139,89 @@ fun KtExpression.isStableVariable(context: BindingContext = this.analyze()): Boo return descriptor is VariableDescriptor && DataFlowValueFactory.isStableValue(descriptor, DescriptorUtils.getContainingModule(descriptor)) } + +data class IfThenToSelectData( + val context: BindingContext, + val condition: KtOperationExpression, + val receiverExpression: KtExpression, + val baseClause: KtExpression?, + val negatedClause: KtExpression? +) { + internal fun replacedBaseClause(factory: KtPsiFactory): KtExpression { + baseClause ?: error("Base clause must be not-null here") + val newReceiver = (condition as? KtIsExpression)?.let { + factory.createExpressionByPattern("$0 as? $1", + (baseClause as? KtDotQualifiedExpression)?.getLeftMostReceiverExpression() ?: baseClause, + it.typeReference!!) + } + return if (baseClause.evaluatesTo(receiverExpression)) { + if (condition is KtIsExpression) newReceiver!! else baseClause + } + else { + if (condition is KtIsExpression) { + (baseClause as KtDotQualifiedExpression).replaceFirstReceiver( + factory, newReceiver!!, safeAccess = true) + } + else { + baseClause.insertSafeCalls(factory) + } + } + } +} + +internal fun KtIfExpression.buildSelectTransformationData(): IfThenToSelectData? { + val context = analyze() + + val condition = condition as? KtOperationExpression ?: return null + val thenClause = then?.unwrapBlockOrParenthesis() + val elseClause = `else`?.unwrapBlockOrParenthesis() + val receiverExpression = condition.checkedExpression() ?: return null + + val (baseClause, negatedClause) = when (condition) { + is KtBinaryExpression -> when (condition.operationToken) { + KtTokens.EQEQ -> elseClause to thenClause + KtTokens.EXCLEQ -> thenClause to elseClause + else -> return null + } + is KtIsExpression -> { + val targetType = context[BindingContext.TYPE, condition.typeReference] ?: return null + if (TypeUtils.isNullableType(targetType)) return null + // TODO: the following check can be removed after fix of KT-14576 + val originalType = receiverExpression.getType(context) ?: return null + if (!targetType.isSubtypeOf(originalType)) return null + + when (condition.isNegated) { + true -> elseClause to thenClause + false -> thenClause to elseClause + else -> return null + } + } + else -> return null + } + return IfThenToSelectData(context, condition, receiverExpression, baseClause, negatedClause) +} + +private fun KtExpression.checkedExpression() = when (this) { + is KtBinaryExpression -> expressionComparedToNull() + is KtIsExpression -> leftHandSide + else -> null +} + +internal fun KtExpression.hasNullableType(context: BindingContext): Boolean { + val type = getType(context) ?: return true + return TypeUtils.isNullableType(type) +} + +internal fun KtExpression.hasFirstReceiverOf(receiver: KtExpression): Boolean { + val actualReceiver = (this as? KtDotQualifiedExpression)?.getLeftMostReceiverExpression() ?: return false + return actualReceiver.evaluatesTo(receiver) +} + +private fun KtExpression.insertSafeCalls(factory: KtPsiFactory): KtExpression { + if (this !is KtQualifiedExpression) return this + val replaced = (if (this is KtDotQualifiedExpression) { + this.replaced(factory.createExpressionByPattern("$0?.$1", receiverExpression, selectorExpression!!)) + } else this) as KtQualifiedExpression + replaced.receiverExpression.let { it.replace(it.insertSafeCalls(factory)) } + return replaced +} diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToDoubleBangIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToDoubleBangIntention.kt index b23122ab20d..bb135fed706 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToDoubleBangIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToDoubleBangIntention.kt @@ -18,43 +18,26 @@ package org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions import com.intellij.openapi.editor.Editor import com.intellij.openapi.util.TextRange -import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.intentions.SelfTargetingRangeIntention import org.jetbrains.kotlin.idea.intentions.branchedTransformations.* -import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsStatement -class IfThenToDoubleBangIntention : SelfTargetingRangeIntention(KtIfExpression::class.java, "Replace 'if' expression with '!!' expression") { +class IfThenToDoubleBangIntention : SelfTargetingRangeIntention( + KtIfExpression::class.java, "Replace 'if' expression with '!!' expression" +) { override fun applicabilityRange(element: KtIfExpression): TextRange? { - val condition = element.condition as? KtBinaryExpression ?: return null - val thenClause = element.then ?: return null - val elseClause = element.`else` + val (context, condition, receiverExpression, baseClause, negatedClause) = element.buildSelectTransformationData() ?: return null + // TODO: here "Replace with as" can be supported + if (condition is KtIsExpression) return null - val expression = condition.expressionComparedToNull() ?: return null + val throwExpression = negatedClause as? KtThrowExpression ?: return null - val token = condition.operationToken - - val throwExpression: KtThrowExpression - val matchingClause: KtExpression? - when (token) { - KtTokens.EQEQ -> { - throwExpression = thenClause.unwrapBlockOrParenthesis() as? KtThrowExpression ?: return null - matchingClause = elseClause - } - - KtTokens.EXCLEQ -> { - matchingClause = thenClause - throwExpression = elseClause?.unwrapBlockOrParenthesis() as? KtThrowExpression ?: return null - } - - else -> throw IllegalStateException() - } - - val matchesAsStatement = element.isUsedAsStatement(element.analyze()) && (matchingClause?.isNullExpressionOrEmptyBlock() ?: true) - if (!matchesAsStatement && !(matchingClause?.evaluatesTo(expression) ?: false && expression.isStableVariable())) return null + val matchesAsStatement = element.isUsedAsStatement(context) && (baseClause?.isNullExpressionOrEmptyBlock() ?: true) + if (!matchesAsStatement && + !(baseClause?.evaluatesTo(receiverExpression) ?: false && receiverExpression.isStableVariable())) return null var text = "Replace 'if' expression with '!!' expression" if (!throwExpression.throwsNullPointerExceptionWithNoArguments()) { @@ -67,9 +50,8 @@ class IfThenToDoubleBangIntention : SelfTargetingRangeIntention( } override fun applyTo(element: KtIfExpression, editor: Editor?) { - val condition = element.condition as KtBinaryExpression - val expression = condition.expressionComparedToNull()!! - val result = element.replace(KtPsiFactory(element).createExpressionByPattern("$0!!", expression)) as KtPostfixExpression + val (_, _, receiverExpression) = element.buildSelectTransformationData() ?: return + val result = element.replace(KtPsiFactory(element).createExpressionByPattern("$0!!", receiverExpression)) as KtPostfixExpression result.inlineBaseExpressionIfApplicableWithPrompt(editor) } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToElvisIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToElvisIntention.kt index ac43b480249..dc973638162 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToElvisIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/intentions/IfThenToElvisIntention.kt @@ -24,17 +24,11 @@ import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.idea.intentions.SelfTargetingOffsetIndependentIntention import org.jetbrains.kotlin.idea.intentions.branchedTransformations.* -import org.jetbrains.kotlin.idea.intentions.getLeftMostReceiverExpression -import org.jetbrains.kotlin.idea.intentions.replaceFirstReceiver 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.bindingContextUtil.isUsedAsExpression import org.jetbrains.kotlin.resolve.calls.callUtil.getType import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode -import org.jetbrains.kotlin.types.TypeUtils -import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf class IfThenToElvisInspection : IntentionBasedInspection( IfThenToElvisIntention::class, @@ -46,49 +40,21 @@ class IfThenToElvisIntention : SelfTargetingOffsetIndependentIntention expressionComparedToNull() - is KtIsExpression -> leftHandSide - else -> null - } + private fun IfThenToSelectData.clausesReplaceableByElvis() = + baseClause != null && negatedClause != null && + !negatedClause.isNullOrBlockExpression() && + (baseClause.evaluatesTo(receiverExpression) || + baseClause.hasFirstReceiverOf(receiverExpression) && !baseClause.hasNullableType(context)) && + !(negatedClause is KtThrowExpression && negatedClause.throwsNullPointerExceptionWithNoArguments()) override fun isApplicableTo(element: KtIfExpression): Boolean { - val context = element.analyze() - val type = element.getType(context) ?: return false + val ifThenToSelectData = element.buildSelectTransformationData() ?: return false + if (!ifThenToSelectData.receiverExpression.isStableVariable(ifThenToSelectData.context)) return false + + val type = element.getType(ifThenToSelectData.context) ?: return false if (KotlinBuiltIns.isUnit(type)) return false - val condition = element.condition as? KtOperationExpression ?: return false - val thenClause = element.then ?: return false - val elseClause = element.`else` ?: return false - - val checkedExpression = condition.checkedExpression() ?: return false - if (!checkedExpression.isStableVariable(context)) return false - - return when (condition) { - is KtBinaryExpression -> when (condition.operationToken) { - KtTokens.EQEQ -> checkedExpression.clausesReplaceableByElvis(thenClause, elseClause, context) - KtTokens.EXCLEQ -> checkedExpression.clausesReplaceableByElvis(elseClause, thenClause, context) - else -> false - } - is KtIsExpression -> { - val targetType = context[BindingContext.TYPE, condition.typeReference] ?: return false - if (TypeUtils.isNullableType(targetType)) return false - // The following check can be removed after fix of KT-14576 - val originalType = condition.leftHandSide.getType(context) ?: return false - if (!targetType.isSubtypeOf(originalType)) return false - - when (condition.isNegated) { - true -> checkedExpression.clausesReplaceableByElvis(thenClause, elseClause, context) - false -> checkedExpression.clausesReplaceableByElvis(elseClause, thenClause, context) - } - } - else -> false - } + return ifThenToSelectData.clausesReplaceableByElvis() } private fun KtExpression.isNullOrBlockExpression(): Boolean { @@ -96,69 +62,17 @@ class IfThenToElvisIntention : SelfTargetingOffsetIndependentIntention when (condition.operationToken) { - KtTokens.EQEQ -> Pair(elseExpression, thenExpression) - KtTokens.EXCLEQ -> Pair(thenExpression, elseExpression) - else -> throw IllegalArgumentException() - } - is KtIsExpression -> when (condition.isNegated) { - true -> Pair(elseExpression, thenExpression) - false -> Pair(thenExpression, elseExpression) - } - else -> throw IllegalArgumentException() - } + val ifThenToSelectData = element.buildSelectTransformationData() ?: return val factory = KtPsiFactory(element) - val newReceiver = (condition as? KtIsExpression)?.let { - factory.createExpressionByPattern("$0 as? $1", - (left as? KtDotQualifiedExpression)?.getLeftMostReceiverExpression() ?: left, - it.typeReference!!) - } - val checkedExpression = condition.checkedExpression()!! val elvis = runWriteAction { - val replacedLeft = if (left.evaluatesTo(checkedExpression)) { - if (condition is KtIsExpression) newReceiver!! else left - } - else { - if (condition is KtIsExpression) { - (left as KtDotQualifiedExpression).replaceFirstReceiver( - factory, newReceiver!!, safeAccess = true) - } - else { - left.insertSafeCalls(factory) - } - } - val newExpr = element.replaced(factory.createExpressionByPattern("$0 ?: $1", replacedLeft, right)) + val replacedBaseClause = ifThenToSelectData.replacedBaseClause(factory) + val newExpr = element.replaced(factory.createExpressionByPattern("$0 ?: $1", + replacedBaseClause, + ifThenToSelectData.negatedClause!!)) KtPsiUtil.deparenthesize(newExpr) as KtBinaryExpression } 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 db6ae174304..eab3fcaa77a 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,18 +17,12 @@ 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 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) @@ -37,83 +31,32 @@ class IfThenToSafeAccessIntention : SelfTargetingOffsetIndependentIntention { - 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 - } + val ifThenToSelectData = element.buildSelectTransformationData() ?: return false + if (!ifThenToSelectData.receiverExpression.isStableVariable(ifThenToSelectData.context)) return false + + return ifThenToSelectData.clausesReplaceableBySafeCall() } override fun startInWriteAction() = false override fun applyTo(element: KtIfExpression, editor: Editor?) { - val condition = element.condition - val newExpr = when (condition) { - is KtBinaryExpression -> { - val receiverExpression = condition.expressionComparedToNull()!! + val ifThenToSelectData = element.buildSelectTransformationData() ?: return - val selectorExpression = - when(condition.operationToken) { - KtTokens.EQEQ -> findSelectorExpressionInClause(element.`else`!!, receiverExpression)!! - KtTokens.EXCLEQ -> findSelectorExpressionInClause(element.then!!, receiverExpression)!! - else -> throw IllegalArgumentException() - } - - 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 + val factory = KtPsiFactory(element) + val resultExpr = runWriteAction { + val replacedBaseClause = ifThenToSelectData.replacedBaseClause(factory) + val newExpr = element.replaced(replacedBaseClause) + KtPsiUtil.deparenthesize(newExpr) } - if (newExpr != null) { - val safeAccessExpr = runWriteAction { element.replaced(newExpr) } - if (editor != null) { - safeAccessExpr.inlineReceiverIfApplicableWithPrompt(editor) - } + if (editor != null) { + (resultExpr as? KtSafeQualifiedExpression)?.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 - - private fun findSelectorExpressionInClause(clause: KtExpression, receiverExpression: KtExpression): KtExpression? { - val expression = clause.unwrapBlockOrParenthesis() as? KtDotQualifiedExpression ?: return null - - if (expression.receiverExpression.text != receiverExpression.text) return null - - return expression.selectorExpression + private fun IfThenToSelectData.clausesReplaceableBySafeCall(): Boolean { + if (baseClause == null || negatedClause != null && !negatedClause.isNullExpression()) return false + return baseClause.evaluatesTo(receiverExpression) && condition !is KtBinaryExpression || + baseClause.hasFirstReceiverOf(receiverExpression) && !baseClause.hasNullableType(context) } } diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml b/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml index 56807d80d23..b17cb39c71b 100644 --- a/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml @@ -111,4 +111,28 @@ If-Then foldable to '?.' Replace 'if' expression with safe access expression + + nullCheckWithSelectorCallChain.kt + 5 + light_idea_test_case + + If-Then foldable to '?.' + Replace 'if' expression with safe access expression + + + isCheckWithSelectorChain.kt + 4 + light_idea_test_case + + If-Then foldable to '?.' + Replace 'if' expression with safe access expression + + + isCheckSimple.kt + 4 + light_idea_test_case + + If-Then foldable to '?.' + Replace 'if' expression with safe access expression + diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt new file mode 100644 index 00000000000..f55661fb37c --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt @@ -0,0 +1,5 @@ +class My(val x: Int) + +fun foo(arg: Any?): My? { + return if (arg is My) arg else null +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt.after new file mode 100644 index 00000000000..4d902be1512 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt.after @@ -0,0 +1,5 @@ +class My(val x: Int) + +fun foo(arg: Any?): My? { + return arg as? My +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt new file mode 100644 index 00000000000..5b50836d765 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt @@ -0,0 +1,5 @@ +class My(val x: Int) + +fun foo(arg: Any?): Int? { + return if (arg is My) arg.x.hashCode() else null +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt.after new file mode 100644 index 00000000000..b8dc4aaf272 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt.after @@ -0,0 +1,5 @@ +class My(val x: Int) + +fun foo(arg: Any?): Int? { + return (arg as? My)?.x?.hashCode() +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckSimple.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckSimple.kt new file mode 100644 index 00000000000..3072b7bb38d --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckSimple.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false + +fun foo(arg: Any?): Any? { + return if (arg != null) arg else null +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt b/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt new file mode 100644 index 00000000000..865af0e4326 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME + +val nullableString: String? = "abc" + +val foo = if (nullableString != null) { + nullableString.toUpperCase().toLowerCase() +} else { + null +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt.after b/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt.after new file mode 100644 index 00000000000..d5117fa93cf --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt.after @@ -0,0 +1,5 @@ +// WITH_RUNTIME + +val nullableString: String? = "abc" + +val foo = nullableString?.toUpperCase()?.toLowerCase() diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 83a40866632..836d6500d2b 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -1766,6 +1766,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("isCheckSimple.kt") + public void testIsCheckSimple() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isCheckSimple.kt"); + doTest(fileName); + } + + @TestMetadata("isCheckWithSelectorChain.kt") + public void testIsCheckWithSelectorChain() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isCheckWithSelectorChain.kt"); + doTest(fileName); + } + @TestMetadata("isCondition.kt") public void testIsCondition() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt"); @@ -1856,6 +1868,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("nullCheckSimple.kt") + public void testNullCheckSimple() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckSimple.kt"); + doTest(fileName); + } + + @TestMetadata("nullCheckWithSelectorCallChain.kt") + public void testNullCheckWithSelectorCallChain() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/nullCheckWithSelectorCallChain.kt"); + doTest(fileName); + } + @TestMetadata("otherBlockHasMoreThanOneStatement.kt") public void testOtherBlockHasMoreThanOneStatement() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/branched/ifThenToSafeAccess/otherBlockHasMoreThanOneStatement.kt");