Convert 'if' with 'is' check to 'as?' with safe call #KT-17054 Fixed

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<KtIfExpression>(IfThenToSafeAccessIntention::class)
 
@@ -32,50 +37,75 @@ class IfThenToSafeAccessIntention : SelfTargetingOffsetIndependentIntention<KtIf
 ) {
 
     override fun isApplicableTo(element: KtIfExpression): Boolean {
-        val condition = element.condition as? KtBinaryExpression ?: return false
+        val condition = element.condition
         val thenClause = element.then
         val elseClause = element.`else`
-
-        val receiverExpression = condition.expressionComparedToNull() ?: return false
-        if (!receiverExpression.isStableVariable()) return false
-
-        return when (condition.operationToken) {
-            KtTokens.EQEQ ->
-                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
 

idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt

@@ -1,3 +1,5 @@
+// IS_APPLICABLE: false
+
 fun maybeFoo(): String? {
     return "foo"
 }

idea/testData/intentions/branched/ifThenToSafeAccess/emptyElseBlock.kt.after

@@ -1,7 +0,0 @@
-fun maybeFoo(): String? {
-    return "foo"
-}
-
-fun main(args: Array<String>) {
-    maybeFoo()?.length
-}

idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt

@@ -1,3 +1,5 @@
+// IS_APPLICABLE: false
+
 fun maybeFoo(): String? {
     return "foo"
 }

idea/testData/intentions/branched/ifThenToSafeAccess/emptyThenBlock.kt.after

@@ -1,7 +0,0 @@
-fun maybeFoo(): String? {
-    return "foo"
-}
-
-fun main(args: Array<String>) {
-    maybeFoo()?.length
-}

idea/testData/intentions/branched/ifThenToSafeAccess/inspectionData/expected.xml

@@ -71,22 +71,6 @@
     <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
     <description>Replace 'if' expression with safe access expression</description>
   </problem>
-  <problem>
-    <file>emptyThenBlock.kt</file>
-    <line>7</line>
-    <module>light_idea_test_case</module>
-    <entry_point TYPE="file" FQNAME="temp:///src/src/emptyThenBlock.kt" />
-    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
-    <description>Replace 'if' expression with safe access expression</description>
-  </problem>
-  <problem>
-    <file>emptyElseBlock.kt</file>
-    <line>7</line>
-    <module>light_idea_test_case</module>
-    <entry_point TYPE="file" FQNAME="temp:///src/src/emptyElseBlock.kt" />
-    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
-    <description>Replace 'if' expression with safe access expression</description>
-  </problem>
   <problem>
     <file>doesNotinlineValueOutsideOfScope.kt</file>
     <line>8</line>
@@ -111,4 +95,20 @@
     <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
     <description>Replace 'if' expression with safe access expression</description>
   </problem>
+  <problem>
+    <file>isCondition.kt</file>
+    <line>1</line>
+    <module>light_idea_test_case</module>
+    <entry_point TYPE="file" FQNAME="temp:///src/src/isCondition.kt" />
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
+    <description>Replace 'if' expression with safe access expression</description>
+  </problem>
+  <problem>
+    <file>isNotCondition.kt</file>
+    <line>1</line>
+    <module>light_idea_test_case</module>
+    <entry_point TYPE="file" FQNAME="temp:///src/src/isNotCondition.kt" />
+    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
+    <description>Replace 'if' expression with safe access expression</description>
+  </problem>
 </problems>

idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt

@@ -0,0 +1 @@
+fun foo(arg: Any) = if (arg is String<caret>) arg.length else null

idea/testData/intentions/branched/ifThenToSafeAccess/isCondition.kt.after

@@ -0,0 +1 @@
+fun foo(arg: Any) = (arg as? String)?.length

idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt

@@ -0,0 +1 @@
+fun foo(arg: Any) = if (arg !is String<caret>) null else arg.length

idea/testData/intentions/branched/ifThenToSafeAccess/isNotCondition.kt.after

@@ -0,0 +1 @@
+fun foo(arg: Any) = (arg as? String)?.length

idea/testData/intentions/branched/ifThenToSafeAccess/isNotNullable.kt

@@ -0,0 +1,2 @@
+// IS_APPLICABLE: false
+fun foo(arg: Any) = if (arg !is String?<caret>) null else arg?.length

idea/testData/intentions/branched/ifThenToSafeAccess/isNullable.kt

@@ -0,0 +1,2 @@
+// IS_APPLICABLE: false
+fun foo(arg: Any) = if (arg is String?<caret>) arg?.length else null

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");