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 38176a85abc..5c9aa19a2e3 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt @@ -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 } 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 32f96ced99a..91b404ed7d5 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 @@ -54,7 +54,7 @@ class IfThenToElvisIntention : SelfTargetingOffsetIndependentIntention true - receiverExpression is KtThisExpression && hasImplicitReceiver() || baseClause.hasFirstReceiverOf(receiverExpression) -> + hasImplicitReceiverReplaceableBySafeCall() || baseClause.hasFirstReceiverOf(receiverExpression) -> !baseClause.hasNullableType(context) else -> false diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt new file mode 100644 index 00000000000..1f4b3e6c60e --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt @@ -0,0 +1,14 @@ +// HIGHLIGHT: INFORMATION +// WITH_RUNTIME + +class Test { + fun doAThing(param1: String) { + + } + + fun doAThingIfPresent(param1: String?) { + if (param1 != null) { + doAThing(param1) + } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt.after b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt.after new file mode 100644 index 00000000000..7bd44514091 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMember.kt.after @@ -0,0 +1,12 @@ +// HIGHLIGHT: INFORMATION +// WITH_RUNTIME + +class Test { + fun doAThing(param1: String) { + + } + + fun doAThingIfPresent(param1: String?) { + param1?.let { doAThing(it) } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt new file mode 100644 index 00000000000..a710782de47 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt @@ -0,0 +1,14 @@ +// HIGHLIGHT: INFORMATION +// WITH_RUNTIME + +class Test { + fun doAThing(param1: String) { + + } + + fun doAThingIfPresent(param1: String?) { + if (param1 is String) { + doAThing(param1) + } + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt.after b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt.after new file mode 100644 index 00000000000..daba9239d2b --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/replaceWithLetInMemberWithIs.kt.after @@ -0,0 +1,12 @@ +// HIGHLIGHT: INFORMATION +// WITH_RUNTIME + +class Test { + fun doAThing(param1: String) { + + } + + fun doAThingIfPresent(param1: String?) { + (param1 as? String)?.let { doAThing(it) } + } +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMember.kt b/idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMember.kt new file mode 100644 index 00000000000..17a7a7644d2 --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMember.kt @@ -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' + if (param1 != null) { + doAThing(param1) + } else { + "" + } + } +} \ No newline at end of file diff --git a/idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMemberWithIs.kt b/idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMemberWithIs.kt new file mode 100644 index 00000000000..ec95d2b02cc --- /dev/null +++ b/idea/testData/intentions/branched/ifThenToElvis/replaceWithLetInMemberWithIs.kt @@ -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' + if (param1 is String) { + doAThing(param1) + } else { + "" + } + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index bf30541c0a0..13b9bf56e04 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 1547a6d3641..b6e3abee929 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -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");