"if-then to safe access" inspection: support call expression -> let

#KT-7675 Fixed
2018-07-25 08:21:26 +03:00
parent 2dcbf6aa34
commit f5cfec4a91
12 changed files with 172 additions and 3 deletions
@@ -19,13 +19,16 @@ package org.jetbrains.kotlin.idea.inspections.branchedTransformations
 import com.intellij.codeInspection.ProblemHighlightType
 import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiDocumentManager
 import com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.idea.core.replaced
 import org.jetbrains.kotlin.idea.inspections.AbstractApplicabilityBasedInspection
 import org.jetbrains.kotlin.idea.intentions.branchedTransformations.*
+import org.jetbrains.kotlin.idea.refactoring.rename.KotlinVariableInplaceRenameHandler
 import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
+import org.jetbrains.kotlin.psi.psiUtil.startOffset
 import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
 import org.jetbrains.kotlin.resolve.calls.callUtil.getType

@@ -73,8 +76,9 @@ class IfThenToSafeAccessInspection : AbstractApplicabilityBasedInspection<KtIfEx
            KtPsiUtil.deparenthesize(newExpr)
        }

-        if (editor != null) {
-            (resultExpr as? KtSafeQualifiedExpression)?.inlineReceiverIfApplicableWithPrompt(editor)
+        if (editor != null && resultExpr is KtSafeQualifiedExpression) {
+            resultExpr.inlineReceiverIfApplicableWithPrompt(editor)
+            resultExpr.renameLetParameter(editor)
        }
    }

@@ -83,6 +87,22 @@ class IfThenToSafeAccessInspection : AbstractApplicabilityBasedInspection<KtIfEx
        negatedClause == null && baseClause.isUsedAsExpression(context) -> false
        negatedClause != null && !negatedClause.isNullExpression() -> false
        else -> baseClause.evaluatesTo(receiverExpression) || baseClause.hasFirstReceiverOf(receiverExpression) ||
-                receiverExpression is KtThisExpression && getImplicitReceiver()?.let { it.type == receiverExpression.getType(context) } == true
+                receiverExpression is KtThisExpression && getImplicitReceiver()?.let { it.type == receiverExpression.getType(context) } == true ||
+                replaceableCallExpression()
+    }
+
+    private fun IfThenToSelectData.replaceableCallExpression(): Boolean {
+        val callExpression = baseClause as? KtCallExpression ?: return false
+        val arguments = callExpression.valueArguments.map { it.getArgumentExpression() }
+        return arguments.any { it?.evaluatesTo(receiverExpression) == true } && arguments.all { it is KtNameReferenceExpression }
+    }
+
+    private fun KtSafeQualifiedExpression.renameLetParameter(editor: Editor) {
+        val callExpression = selectorExpression as? KtCallExpression ?: return
+        if (callExpression.calleeExpression?.text != "let") return
+        val parameter = callExpression.lambdaArguments.singleOrNull()?.getLambdaExpression()?.valueParameters?.singleOrNull() ?: return
+        PsiDocumentManager.getInstance(project).doPostponedOperationsAndUnblockDocument(editor.document)
+        editor.caretModel.moveToOffset(parameter.startOffset)
+        KotlinVariableInplaceRenameHandler().doRename(parameter, editor, null)
    }
 }
@@ -25,6 +25,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.caches.resolve.findModuleDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
+import org.jetbrains.kotlin.idea.core.KotlinNameSuggester
 import org.jetbrains.kotlin.idea.core.replaced
 import org.jetbrains.kotlin.idea.intentions.getLeftMostReceiverExpression
 import org.jetbrains.kotlin.idea.intentions.replaceFirstReceiver
@@ -33,7 +34,10 @@ import org.jetbrains.kotlin.idea.refactoring.introduce.introduceVariable.KotlinI
 import org.jetbrains.kotlin.idea.refactoring.isMultiLine
 import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.idea.resolve.frontendService
+import org.jetbrains.kotlin.idea.util.getResolutionScope
+import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
@@ -43,6 +47,7 @@ import org.jetbrains.kotlin.resolve.calls.resolvedCallUtil.getImplicitReceiverVa
 import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue
 import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory
 import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitReceiver
+import org.jetbrains.kotlin.resolve.scopes.utils.findVariable
 import org.jetbrains.kotlin.types.TypeUtils
 import org.jetbrains.kotlin.types.typeUtil.isSubtypeOf
 import org.jetbrains.kotlin.utils.addToStdlib.constant
@@ -211,10 +216,12 @@ data class IfThenToSelectData(
                        hasImplicitReceiver() -> factory.createExpressionByPattern("$0?.$1", newReceiver!!, baseClause).insertSafeCalls(
                            factory
                        )
+                        baseClause is KtCallExpression -> replacedBaseClause(baseClause, newReceiver!!, factory)
                        else -> error("Illegal state")
                    }
                }
                hasImplicitReceiver() -> factory.createExpressionByPattern("this?.$0", baseClause).insertSafeCalls(factory)
+                baseClause is KtCallExpression -> replacedBaseClause(baseClause, receiverExpression, factory)
                else -> baseClause.insertSafeCalls(factory)
            }
        }
@@ -223,6 +230,32 @@ data class IfThenToSelectData(
    internal fun getImplicitReceiver(): ImplicitReceiver? = baseClause.getResolvedCall(context)?.getImplicitReceiverValue()

    internal fun hasImplicitReceiver(): Boolean = getImplicitReceiver() != null
+
+    private fun replacedBaseClause(baseClause: KtCallExpression, receiver: KtExpression, factory: KtPsiFactory): KtExpression {
+        val needExplicitParameter = baseClause.valueArguments.any { it.getArgumentExpression()?.text == "it" }
+        val parameterName = if (needExplicitParameter) {
+            val scope = baseClause.getResolutionScope()
+            KotlinNameSuggester.suggestNameByName("it") { scope.findVariable(Name.identifier(it), NoLookupLocation.FROM_IDE) == null }
+        } else {
+            "it"
+        }
+        return factory.buildExpression {
+            appendExpression(receiver)
+            appendFixedText("?.let {")
+            if (needExplicitParameter) appendFixedText(" $parameterName ->")
+            appendExpression(baseClause.calleeExpression)
+            appendFixedText("(")
+            baseClause.valueArguments.forEachIndexed { index, arg ->
+                if (index != 0) appendFixedText(", ")
+                val argExpression = arg.getArgumentExpression()
+                if (argExpression?.evaluatesTo(receiverExpression) == true)
+                    appendFixedText(parameterName)
+                else
+                    appendExpression(argExpression)
+            }
+            appendFixedText(") }")
+        }
+    }
 }

 internal fun KtIfExpression.buildSelectTransformationData(): IfThenToSelectData? {
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun convert(x: String, y: String) = ""
+
+fun foo(a: String?, b: String) {
+    <caret>if (a != null) convert(a, b) else null
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun convert(x: String, y: String) = ""
+
+fun foo(a: String?, b: String) {
+    a?.let { convert(it, b) }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun convert(x: String, y: String) = ""
+
+fun foo(a: Any?, b: String) {
+    <caret>if (a is String) convert(a, b) else null
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun convert(x: String, y: String) = ""
+
+fun foo(a: Any?, b: String) {
+    (a as? String)?.let { convert(it, b) }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun convert(x: String, y: Int) = ""
+
+fun foo(a: String?, it: Int) {
+    <caret>if (a != null) convert(a, it) else null
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun convert(x: String, y: Int) = ""
+
+fun foo(a: String?, it: Int) {
+    a?.let { it1 -> convert(it1, it) }
+}
@@ -0,0 +1,12 @@
+// WITH_RUNTIME
+fun maybeFoo(): String? {
+    return "foo"
+}
+
+fun convert(x: String, y: Int) = ""
+
+fun foo(it: Int) {
+    val foo = maybeFoo()
+    <caret>if (foo == null) else convert(foo, it)
+}
+
@@ -0,0 +1,12 @@
+// WITH_RUNTIME
+fun maybeFoo(): String? {
+    return "foo"
+}
+
+fun convert(x: String, y: Int) = ""
+
+fun foo(it: Int) {
+    maybeFoo()?.let { it1 -> convert(it1, it) }
+}
+
+
@@ -167,4 +167,40 @@
    <problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
    <description>Foldable if-then</description>
  </problem>
+  <problem>
+    <file>call4.kt</file>
+    <line>10</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/call4.kt" />
+    <problem_class severity="INFO" attribute_key="INFO_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
+    <description>Foldable if-then</description>
+  </problem>
+  <problem>
+    <file>call3.kt</file>
+    <line>5</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/call3.kt" />
+    <problem_class severity="INFO" attribute_key="INFO_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
+    <description>Foldable if-then</description>
+  </problem>
+  <problem>
+    <file>call2.kt</file>
+    <line>5</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/call2.kt" />
+    <problem_class severity="INFO" attribute_key="INFO_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
+    <description>Foldable if-then</description>
+  </problem>
+  <problem>
+    <file>call.kt</file>
+    <line>5</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/call.kt" />
+    <problem_class severity="INFO" attribute_key="INFO_ATTRIBUTES">If-Then foldable to '?.'</problem_class>
+    <description>Foldable if-then</description>
+  </problem>
 </problems>
@@ -81,6 +81,26 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
                runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/blockUsesDifferentVar.kt");
            }

+            @TestMetadata("call.kt")
+            public void testCall() throws Exception {
+                runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt");
+            }
+
+            @TestMetadata("call2.kt")
+            public void testCall2() throws Exception {
+                runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt");
+            }
+
+            @TestMetadata("call3.kt")
+            public void testCall3() throws Exception {
+                runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt");
+            }
+
+            @TestMetadata("call4.kt")
+            public void testCall4() throws Exception {
+                runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt");
+            }
+
            @TestMetadata("conditionComparesNullWithNull.kt")
            public void testConditionComparesNullWithNull() throws Exception {
                runTest("idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/conditionComparesNullWithNull.kt");