From f5cfec4a916498f95f01c564f9b3ba5961a7bd91 Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Wed, 25 Jul 2018 08:21:26 +0300 Subject: [PATCH] "if-then to safe access" inspection: support call expression -> let #KT-7675 Fixed --- .../IfThenToSafeAccessInspection.kt | 26 ++++++++++++-- .../branchedTransformations/IfThenUtils.kt | 33 +++++++++++++++++ .../branched/ifThenToSafeAccess/call.kt | 6 ++++ .../branched/ifThenToSafeAccess/call.kt.after | 6 ++++ .../branched/ifThenToSafeAccess/call2.kt | 6 ++++ .../ifThenToSafeAccess/call2.kt.after | 6 ++++ .../branched/ifThenToSafeAccess/call3.kt | 6 ++++ .../ifThenToSafeAccess/call3.kt.after | 6 ++++ .../branched/ifThenToSafeAccess/call4.kt | 12 +++++++ .../ifThenToSafeAccess/call4.kt.after | 12 +++++++ .../inspectionData/expected.xml | 36 +++++++++++++++++++ .../LocalInspectionTestGenerated.java | 20 +++++++++++ 12 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt.after create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt.after create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt.after create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt create mode 100644 idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/branchedTransformations/IfThenToSafeAccessInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/branchedTransformations/IfThenToSafeAccessInspection.kt index 7158fe1496e..bbd0bcd172a 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/branchedTransformations/IfThenToSafeAccessInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/branchedTransformations/IfThenToSafeAccessInspection.kt @@ -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 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) } } 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 4d07b592de9..e8f83fda9a3 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/branchedTransformations/IfThenUtils.kt @@ -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? { diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt new file mode 100644 index 00000000000..ebceb7a3c04 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun convert(x: String, y: String) = "" + +fun foo(a: String?, b: String) { + if (a != null) convert(a, b) else null +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt.after b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt.after new file mode 100644 index 00000000000..c1eaace88c2 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun convert(x: String, y: String) = "" + +fun foo(a: String?, b: String) { + a?.let { convert(it, b) } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt new file mode 100644 index 00000000000..47693ec26c3 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun convert(x: String, y: String) = "" + +fun foo(a: Any?, b: String) { + if (a is String) convert(a, b) else null +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt.after b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt.after new file mode 100644 index 00000000000..294bea03985 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call2.kt.after @@ -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) } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt new file mode 100644 index 00000000000..a33521d90a8 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun convert(x: String, y: Int) = "" + +fun foo(a: String?, it: Int) { + if (a != null) convert(a, it) else null +} diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt.after b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt.after new file mode 100644 index 00000000000..0819e5302a9 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call3.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun convert(x: String, y: Int) = "" + +fun foo(a: String?, it: Int) { + a?.let { it1 -> convert(it1, it) } +} diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt new file mode 100644 index 00000000000..56779927ef3 --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME +fun maybeFoo(): String? { + return "foo" +} + +fun convert(x: String, y: Int) = "" + +fun foo(it: Int) { + val foo = maybeFoo() + if (foo == null) else convert(foo, it) +} + diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt.after b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt.after new file mode 100644 index 00000000000..cde5f47fc7f --- /dev/null +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/call4.kt.after @@ -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) } +} + + diff --git a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/inspectionData/expected.xml b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/inspectionData/expected.xml index bec92be1bf0..c612d3e163b 100644 --- a/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/inspectionData/expected.xml +++ b/idea/testData/inspectionsLocal/branched/ifThenToSafeAccess/inspectionData/expected.xml @@ -167,4 +167,40 @@ If-Then foldable to '?.' Foldable if-then + + call4.kt + 10 + light_idea_test_case + <default> + + If-Then foldable to '?.' + Foldable if-then + + + call3.kt + 5 + light_idea_test_case + <default> + + If-Then foldable to '?.' + Foldable if-then + + + call2.kt + 5 + light_idea_test_case + <default> + + If-Then foldable to '?.' + Foldable if-then + + + call.kt + 5 + light_idea_test_case + <default> + + If-Then foldable to '?.' + Foldable if-then + diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 60a7cb954b8..4303fe66398 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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");