ConstantConditionIfInspection.replaceWithBranch shouldn't remove subjectVariable with side effects

Relates to #KT-30975
2019-05-29 15:38:28 +07:00
parent e2d5e9a184
commit 1c4fb9b00b
22 changed files with 89 additions and 57 deletions
@@ -17,9 +17,13 @@ import org.jetbrains.kotlin.idea.core.replaced
 import org.jetbrains.kotlin.idea.intentions.branchedTransformations.isElseIf
 import org.jetbrains.kotlin.idea.intentions.branchedTransformations.unwrapBlockOrParenthesis
 import org.jetbrains.kotlin.idea.references.mainReference
+import org.jetbrains.kotlin.idea.util.hasNoSideEffects
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.*
+import org.jetbrains.kotlin.psi.psiUtil.containingClass
+import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType
+import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
+import org.jetbrains.kotlin.psi.psiUtil.startOffset
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
 import org.jetbrains.kotlin.resolve.calls.callUtil.getType
@@ -158,43 +162,29 @@ private fun KtExpression.enumEntry(): KtEnumEntry? {
 fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boolean, keepBraces: Boolean = false) {
    val caretModel = findExistingEditor()?.caretModel

-    val subjectVariable = (this as? KtWhenExpression)?.subjectVariable?.let { it ->
-        if (it.annotationEntries.isNotEmpty()) return@let it
-        val initializer = it.initializer ?: return@let it
-        val references = ReferencesSearch.search(it, LocalSearchScope(this)).toList()
-        when (references.size) {
-            0 -> when (initializer) {
-                is KtSimpleNameExpression, is KtStringTemplateExpression, is KtConstantExpression -> null
-                else -> it
-            }
+    val subjectVariable = (this as? KtWhenExpression)?.subjectVariable?.let(fun(property: KtProperty): KtProperty? {
+        if (property.annotationEntries.isNotEmpty()) return property
+        val initializer = property.initializer ?: return property
+        val references = ReferencesSearch.search(property, LocalSearchScope(this)).toList()
+        return when (references.size) {
+            0 -> property.takeUnless { initializer.hasNoSideEffects() }
            1 -> {
-                references.firstOrNull()?.element?.replace(initializer)
-                null
+                if (initializer.hasNoSideEffects()) {
+                    references.first().element.replace(initializer)
+                    null
+                } else
+                    property
            }
-            else -> it
+            else -> property
        }
-    }
-    val wrapSubjectVariableByRun = if (subjectVariable != null) {
-        val subjectVariableName = subjectVariable.nameAsName
-        val parentBlock = getStrictParentOfType<KtBlockExpression>()
-        parentBlock?.anyDescendantOfType<KtProperty> { it != subjectVariable && it.nameAsName == subjectVariableName } == true
-    } else {
-        false
-    }
+    })

    val factory = KtPsiFactory(this)
    val parent = this.parent
    val replaced = when {
        branch !is KtBlockExpression -> {
            if (subjectVariable != null) {
-                if (isUsedAsExpression || wrapSubjectVariableByRun) {
-                    replaced(KtPsiFactory(this).createExpressionByPattern("run { $0\n$1 }", subjectVariable, branch))
-                } else {
-                    parent.addBefore(subjectVariable, this).also {
-                        parent.addAfter(factory.createNewLine(), it)
-                        replaced(branch)
-                    }
-                }
+                replaced(KtPsiFactory(this).createExpressionByPattern("run { $0\n$1 }", subjectVariable, branch))
            } else {
                replaced(branch)
            }
@@ -212,12 +202,11 @@ fun KtExpression.replaceWithBranch(branch: KtExpression, isUsedAsExpression: Boo
                if (keepBraces) {
                    parent.addAfter(branch, this)
                } else {
-                    if (subjectVariable != null && wrapSubjectVariableByRun) {
+                    if (subjectVariable != null) {
                        branch.addAfter(subjectVariable, branch.lBrace)
                        parent.addAfter(KtPsiFactory(this).createExpression("run ${branch.text}"), this)
                    } else {
                        parent.addRangeAfter(firstChildSibling, lastChild.prevSibling, this)
-                        subjectVariable?.let { parent.addBefore(subjectVariable, this) }
                    }
                }
            } else {
@@ -5,12 +5,12 @@

 package org.jetbrains.kotlin.idea.util

+import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.lexer.KtTokens
-import org.jetbrains.kotlin.psi.KtCallExpression
-import org.jetbrains.kotlin.psi.KtModifierListOwner
-import org.jetbrains.kotlin.psi.KtPrimaryConstructor
-import org.jetbrains.kotlin.psi.KtTypeArgumentList
+import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.containingClass
+import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator
+import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode


 fun KtCallExpression.replaceOrCreateTypeArgumentList(newTypeArgumentList: KtTypeArgumentList) {
@@ -25,4 +25,11 @@ fun KtModifierListOwner.hasInlineModifier() = hasModifier(KtTokens.INLINE_KEYWOR

 fun KtPrimaryConstructor.allowedValOrVar(): Boolean = containingClass()?.let {
    it.isAnnotation() || it.hasInlineModifier()
-} ?: false
+} ?: false
+
+// TODO: add cases
+fun KtExpression.hasNoSideEffects(): Boolean = when (this) {
+    is KtStringTemplateExpression -> !hasInterpolation()
+    is KtConstantExpression -> true
+    else -> ConstantExpressionEvaluator.getConstant(this, analyze(BodyResolveMode.PARTIAL)) != null
+}
@@ -1,6 +1,6 @@
 fun println(s: String) {}

 fun foo() {
-    <caret>println("")
+    println("")
    1
 }
@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = create()) {
        else -> {
@@ -1,7 +1,10 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>val a = create()
-    use(a, a)
-    foo()
+    run {
+        val a = create()
+        use(a, a)
+        foo()
+    }
 }

 fun create(): String = ""
@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = create()) {
        else -> {
@@ -1,7 +1,10 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>val a = create()
-    use("")
-    foo()
+    run {
+        val a = create()
+        use("")
+        foo()
+    }
 }

 fun create(): String = ""
@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = 42) {
        else -> {
@@ -1,5 +1,6 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>use("")
+    use("")
    foo()
 }

@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = create()) {
        else -> {
@@ -1,6 +1,10 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>use(create())
-    foo()
+    run {
+        val a = create()
+        use(a)
+        foo()
+    }
 }

 fun create(): String = ""
@@ -1,7 +1,8 @@
 // WITH_RUNTIME
 fun test() {
-    val x = <caret>run {
-        use(create())
+    val x = run {
+        val a = create()
+        use(a)
    }
 }

@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun hasAnnotation() {
    when<caret> (@Bar val a = create()) {
        else -> use(a)
@@ -1,6 +1,9 @@
+// WITH_RUNTIME
 fun hasAnnotation() {
-    <caret>@Bar val a = create()
-    use(a)
+    run {
+        @Bar val a = create()
+        use(a)
+    }
 }

 fun create(): String = ""
@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = create()) {
        else -> use(a, a)
@@ -1,6 +1,9 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>val a = create()
-    use(a, a)
+    run {
+        val a = create()
+        use(a, a)
+    }
 }

 fun create(): String = ""
@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = create()) {
        else -> use("")
@@ -1,6 +1,9 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>val a = create()
-    use("")
+    run {
+        val a = create()
+        use("")
+    }
 }

 fun create(): String = ""
@@ -1,5 +1,5 @@
 fun test() {
-    <caret>use("")
+    use("")
 }

 fun use(s: String) {}
@@ -1,3 +1,4 @@
+// WITH_RUNTIME
 fun test() {
    when<caret> (val a = create()) {
        else -> use(a)
@@ -1,5 +1,9 @@
+// WITH_RUNTIME
 fun test() {
-    <caret>use(create())
+    run {
+        val a = create()
+        use(a)
+    }
 }

 fun create(): String = ""
@@ -1,6 +1,9 @@
 // WITH_RUNTIME
 fun test() {
-    val x = <caret>use(create())
+    val x = run {
+        val a = create()
+        use(a)
+    }
 }

 fun create(): String = ""