From 8c498af3d81b9d7a3ca32461cc96b1faf442ad30 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Thu, 24 Mar 2016 12:37:35 +0300 Subject: [PATCH] Checking that smart casts will not be broken by conversion --- .idea/dictionaries/valentin.xml | 1 + .../inspectionProfiles/Loop_to_call_chain.xml | 2133 ----------------- .../loopToCallChain/matchAndConvert.kt | 66 +- .../idea/intentions/loopToCallChain/utils.kt | 7 +- .../loopToCallChain/smartCastNotBroken.kt | 14 + .../smartCastNotBroken.kt.after | 11 + .../smartCastNotNullRequired.kt | 12 + .../loopToCallChain/smartCastNotRequired.kt | 11 + .../smartCastNotRequired.kt.after | 8 + .../loopToCallChain/smartCastRequired.kt | 12 + .../loopToCallChain/smartCastRequired2.kt | 11 + .../loopToCallChain/smartCastThisRequired.kt | 11 + 12 files changed, 162 insertions(+), 2135 deletions(-) delete mode 100644 .idea/inspectionProfiles/Loop_to_call_chain.xml create mode 100644 idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt create mode 100644 idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt.after create mode 100644 idea/testData/intentions/loopToCallChain/smartCastNotNullRequired.kt create mode 100644 idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt create mode 100644 idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt.after create mode 100644 idea/testData/intentions/loopToCallChain/smartCastRequired.kt create mode 100644 idea/testData/intentions/loopToCallChain/smartCastRequired2.kt create mode 100644 idea/testData/intentions/loopToCallChain/smartCastThisRequired.kt diff --git a/.idea/dictionaries/valentin.xml b/.idea/dictionaries/valentin.xml index b033708428b..1fa06691aa3 100644 --- a/.idea/dictionaries/valentin.xml +++ b/.idea/dictionaries/valentin.xml @@ -17,6 +17,7 @@ renderers rparenth selectioner + smartcast summand unpluralize weighers diff --git a/.idea/inspectionProfiles/Loop_to_call_chain.xml b/.idea/inspectionProfiles/Loop_to_call_chain.xml deleted file mode 100644 index 3d5a104dcb6..00000000000 --- a/.idea/inspectionProfiles/Loop_to_call_chain.xml +++ /dev/null @@ -1,2133 +0,0 @@ - - - - \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt index 3639f41a027..7d92ea5ece8 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt @@ -16,6 +16,10 @@ package org.jetbrains.kotlin.idea.intentions.loopToCallChain +import com.intellij.openapi.util.Key +import org.jetbrains.kotlin.idea.analysis.analyzeInContext +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.intentions.loopToCallChain.result.FindAndAssignTransformation import org.jetbrains.kotlin.idea.intentions.loopToCallChain.result.FindAndReturnTransformation import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FilterTransformation @@ -23,7 +27,15 @@ import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FlatMapTran import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.MapTransformation import org.jetbrains.kotlin.idea.intentions.negate import org.jetbrains.kotlin.idea.util.CommentSaver +import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType +import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.bindingContextUtil.getDataFlowInfo +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.utils.addToStdlib.check import java.util.* object MatcherRegistrar { @@ -59,7 +71,8 @@ fun match(loop: KtForExpression): ResultTransformationMatch? { val match = matcher.match(state) if (match != null) { sequenceTransformations.addAll(match.sequenceTransformations) - return ResultTransformationMatch(match.resultTransformation, sequenceTransformations) + val resultMatch = ResultTransformationMatch(match.resultTransformation, sequenceTransformations) + return resultMatch.check { checkSmartCastsPreserved(loop, it) } } } @@ -89,6 +102,57 @@ fun convertLoop(loop: KtForExpression, matchResult: ResultTransformationMatch): return result } +private fun checkSmartCastsPreserved(loop: KtForExpression, matchResult: ResultTransformationMatch): Boolean { + val bindingContext = loop.analyze(BodyResolveMode.FULL) + + val SMARTCAST_KEY = Key("SMARTCAST_KEY") + val IMPLICIT_RECEIVER_SMARTCAST_KEY = Key("IMPLICIT_RECEIVER_SMARTCAST") + + var smartCastsFound = false + try { + loop.forEachDescendantOfType { expression -> + bindingContext[BindingContext.SMARTCAST, expression]?.let { + expression.putCopyableUserData(SMARTCAST_KEY, it) + smartCastsFound = true + } + + bindingContext[BindingContext.IMPLICIT_RECEIVER_SMARTCAST, expression]?.let { + expression.putCopyableUserData(IMPLICIT_RECEIVER_SMARTCAST_KEY, it) + smartCastsFound = true + } + } + + if (!smartCastsFound) return true // optimization + + val callChain = matchResult.generateCallChain(loop) + + val resolutionScope = loop.getResolutionScope(bindingContext, loop.getResolutionFacade()) + val dataFlowInfo = bindingContext.getDataFlowInfo(loop) + val newBindingContext = callChain.analyzeInContext(resolutionScope, loop, dataFlowInfo = dataFlowInfo) + + val smartCastBroken = callChain.anyDescendantOfType { expression -> + val smartCastType = expression.getCopyableUserData(SMARTCAST_KEY) + if (smartCastType != null && newBindingContext[BindingContext.SMARTCAST, expression] != smartCastType && newBindingContext.getType(expression) != smartCastType) { + return@anyDescendantOfType true + } + + val implicitReceiverSmartCastType = expression.getCopyableUserData(IMPLICIT_RECEIVER_SMARTCAST_KEY) + if (implicitReceiverSmartCastType != null && newBindingContext[BindingContext.IMPLICIT_RECEIVER_SMARTCAST, expression] != implicitReceiverSmartCastType) { + return@anyDescendantOfType true + } + + false + } + + return !smartCastBroken + } + finally { + if (smartCastsFound) { + loop.forEachDescendantOfType { it.putCopyableUserData(SMARTCAST_KEY, null) } + } + } +} + private fun ResultTransformationMatch.generateCallChain(loop: KtForExpression): KtExpression { var sequenceTransformations = sequenceTransformations val last = sequenceTransformations.lastOrNull() diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt index 1851e50f680..13d96da85f4 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.idea.intentions.loopToCallChain +import com.intellij.openapi.util.UserDataHolderBase import com.intellij.psi.PsiWhiteSpace import com.intellij.psi.search.LocalSearchScope import com.intellij.psi.search.searches.ReferencesSearch @@ -23,6 +24,7 @@ import org.jetbrains.kotlin.KtNodeTypes import org.jetbrains.kotlin.idea.analysis.analyzeInContext import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade +import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.intentions.branchedTransformations.isNullExpression import org.jetbrains.kotlin.idea.references.KtSimpleNameReference import org.jetbrains.kotlin.idea.references.mainReference @@ -57,7 +59,10 @@ fun generateLambda(workingVariable: KtCallableDeclaration, expression: KtExpress val itExpr = psiFactory.createSimpleName("it") for (usage in usages) { - usage.replace(itExpr) + val replaced = usage.replaced(itExpr) + + // we need to copy user data for checkSmartCastsPreserved() to work + (usage.node as UserDataHolderBase).copyCopyableDataTo(replaced.node as UserDataHolderBase) } return psiFactory.createExpressionByPattern("{ $0 }", lambdaExpression.bodyExpression!!) as KtLambdaExpression diff --git a/idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt b/idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt new file mode 100644 index 00000000000..e1362dddb9d --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt @@ -0,0 +1,14 @@ +// WITH_RUNTIME +fun foo(list: List, o: Any): Int? { + if (o is Int) { + for (s in list) { + val length = s.length + o + if (length > 0) { + val x = length * o.hashCode() + return x + } + } + return null + } + return 0 +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt.after b/idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt.after new file mode 100644 index 00000000000..dfa0d74e515 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastNotBroken.kt.after @@ -0,0 +1,11 @@ +// WITH_RUNTIME +fun foo(list: List, o: Any): Int? { + if (o is Int) { + return list + .map { it.length + o } + .filter { it > 0 } + .map { it * o.hashCode() } + .firstOrNull() + } + return 0 +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastNotNullRequired.kt b/idea/testData/intentions/loopToCallChain/smartCastNotNullRequired.kt new file mode 100644 index 00000000000..9de5f591d3d --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastNotNullRequired.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false +fun foo(list: List, o: Int?): Int? { + for (s in list) { + val length = s.length + o!! + if (length > 0) { + val x = length * o + return x + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt b/idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt new file mode 100644 index 00000000000..48f6e21a119 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +fun foo(list: List, o: Any): Int? { + for (s in list) { + val length = s.length + (o as Int) + if (length > 0) { + val x = length * o.hashCode() + return x + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt.after b/idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt.after new file mode 100644 index 00000000000..06f65d63017 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastNotRequired.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME +fun foo(list: List, o: Any): Int? { + return list + .map { it.length + (o as Int) } + .filter { it > 0 } + .map { it * o.hashCode() } + .firstOrNull() +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastRequired.kt b/idea/testData/intentions/loopToCallChain/smartCastRequired.kt new file mode 100644 index 00000000000..9e62ef09670 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastRequired.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false +fun foo(list: List, o: Any): Int? { + for (s in list) { + val length = s.length + (o as Int) + if (length > 0) { + val x = length * o + return x + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastRequired2.kt b/idea/testData/intentions/loopToCallChain/smartCastRequired2.kt new file mode 100644 index 00000000000..7879fe2c9f1 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastRequired2.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false +fun foo(list: List, o: Any): Int? { + for (s in list) { + if (s is String && s.length > 0) { + val x = s.length * 2 + return x + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/smartCastThisRequired.kt b/idea/testData/intentions/loopToCallChain/smartCastThisRequired.kt new file mode 100644 index 00000000000..03755d6169a --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/smartCastThisRequired.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false +fun Any.foo(list: List): Int? { + for (s in list) { + if (s.length > 0 && this is String) { + val result = s.length + length + return result + } + } + return null +} \ No newline at end of file