Handle implicit receivers more accurately in if-then to safe access

#KT-26599 Fixed
This commit is contained in:
Mikhail Glukhikh
2018-09-07 14:52:46 +03:00
parent ccb17caf7b
commit 485e098ced
10 changed files with 114 additions and 7 deletions
@@ -215,14 +215,14 @@ data class IfThenToSelectData(
baseClause is KtDotQualifiedExpression -> baseClause.replaceFirstReceiver(
factory, newReceiver!!, safeAccess = true
)
hasImplicitReceiver() -> factory.createExpressionByPattern("$0?.$1", newReceiver!!, baseClause).insertSafeCalls(
hasImplicitReceiverReplaceableBySafeCall() -> factory.createExpressionByPattern("$0?.$1", newReceiver!!, baseClause).insertSafeCalls(
factory
)
baseClause is KtCallExpression -> replacedBaseClauseWithLet(baseClause, newReceiver!!, factory)
else -> error("Illegal state")
}
}
hasImplicitReceiver() -> factory.createExpressionByPattern("this?.$0", baseClause).insertSafeCalls(factory)
hasImplicitReceiverReplaceableBySafeCall() -> factory.createExpressionByPattern("this?.$0", baseClause).insertSafeCalls(factory)
baseClause is KtCallExpression -> replacedBaseClauseWithLet(baseClause, receiverExpression, factory)
else -> baseClause.insertSafeCalls(factory)
}
@@ -235,7 +235,8 @@ data class IfThenToSelectData(
return resolvedCall.getImplicitReceiverValue()
}
internal fun hasImplicitReceiver(): Boolean = getImplicitReceiver() != null
internal fun hasImplicitReceiverReplaceableBySafeCall(): Boolean =
receiverExpression is KtThisExpression && getImplicitReceiver() != null
private fun replacedBaseClauseWithLet(baseClause: KtCallExpression, receiver: KtExpression, factory: KtPsiFactory): KtExpression {
val needExplicitParameter = baseClause.valueArguments.any { it.getArgumentExpression()?.text == "it" }
@@ -295,15 +296,15 @@ internal fun KtIfExpression.buildSelectTransformationData(): IfThenToSelectData?
return IfThenToSelectData(context, condition, receiverExpression, baseClause, negatedClause)
}
internal fun KtExpression?.isClauseTransformableToLetOnly() =
this is KtCallExpression && resolveToCall()?.getImplicitReceiverValue() == null
internal fun KtExpression?.isClauseTransformableToLetOnly(receiver: KtExpression?) =
this is KtCallExpression && (resolveToCall()?.getImplicitReceiverValue() == null || receiver !is KtThisExpression)
internal fun KtIfExpression.shouldBeTransformed(): Boolean {
val condition = condition
return when (condition) {
is KtBinaryExpression -> {
val baseClause = (if (condition.operationToken == KtTokens.EQEQ) `else` else then)?.unwrapBlockOrParenthesis()
!baseClause.isClauseTransformableToLetOnly()
!baseClause.isClauseTransformableToLetOnly(checkedExpression())
}
else -> false
}
@@ -54,7 +54,7 @@ class IfThenToElvisIntention : SelfTargetingOffsetIndependentIntention<KtIfExpre
false
baseClause.evaluatesTo(receiverExpression) ->
true
receiverExpression is KtThisExpression && hasImplicitReceiver() || baseClause.hasFirstReceiverOf(receiverExpression) ->
hasImplicitReceiverReplaceableBySafeCall() || baseClause.hasFirstReceiverOf(receiverExpression) ->
!baseClause.hasNullableType(context)
else ->
false
@@ -0,0 +1,14 @@
// HIGHLIGHT: INFORMATION
// WITH_RUNTIME
class Test {
fun doAThing(param1: String) {
}
fun doAThingIfPresent(param1: String?) {
<caret>if (param1 != null) {
doAThing(param1)
}
}
}
@@ -0,0 +1,12 @@
// HIGHLIGHT: INFORMATION
// WITH_RUNTIME
class Test {
fun doAThing(param1: String) {
}
fun doAThingIfPresent(param1: String?) {
param1?.let { doAThing(it) }
}
}
@@ -0,0 +1,14 @@
// HIGHLIGHT: INFORMATION
// WITH_RUNTIME
class Test {
fun doAThing(param1: String) {
}
fun doAThingIfPresent(param1: String?) {
<caret>if (param1 is String) {
doAThing(param1)
}
}
}
@@ -0,0 +1,12 @@
// HIGHLIGHT: INFORMATION
// WITH_RUNTIME
class Test {
fun doAThing(param1: String) {
}
fun doAThingIfPresent(param1: String?) {
(param1 as? String)?.let { doAThing(it) }
}
}
@@ -0,0 +1,17 @@
// IS_APPLICABLE: false
// WITH_RUNTIME
class Test {
fun doAThing(param1: String): String {
return param1
}
fun doAThingIfPresent(param1: String?) {
// In theory could propose transformation to 'let'
<caret>if (param1 != null) {
doAThing(param1)
} else {
""
}
}
}
@@ -0,0 +1,17 @@
// IS_APPLICABLE: false
// WITH_RUNTIME
class Test {
fun doAThing(param1: String): String {
return param1
}
fun doAThingIfPresent(param1: String?) {
// In theory could propose transformation to 'let'
<caret>if (param1 is String) {
doAThing(param1)
} else {
""
}
}
}
@@ -286,6 +286,16 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/propertyWithProperty.kt");
}
@TestMetadata("replaceWithLetInMember.kt")
public void testReplaceWithLetInMember() throws Exception {
runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt");
}
@TestMetadata("replaceWithLetInMemberWithIs.kt")
public void testReplaceWithLetInMemberWithIs() throws Exception {
runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt");
}
@TestMetadata("rhsEqualsNull.kt")
public void testRhsEqualsNull() throws Exception {
runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/rhsEqualsNull.kt");
@@ -2426,6 +2426,16 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
runTest("idea/testData/intentions/branched/ifThenToElvis/otherBlockHasMoreThanOneStatement.kt");
}
@TestMetadata("replaceWithLetInMember.kt")
public void testReplaceWithLetInMember() throws Exception {
runTest("idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMember.kt");
}
@TestMetadata("replaceWithLetInMemberWithIs.kt")
public void testReplaceWithLetInMemberWithIs() throws Exception {
runTest("idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMemberWithIs.kt");
}
@TestMetadata("rhsEqualsNull.kt")
public void testRhsEqualsNull() throws Exception {
runTest("idea/testData/intentions/branched/ifThenToElvis/rhsEqualsNull.kt");