Enhancement for "join declaration and assignment": now can handle also local variables, relevant inspection added #KT-12095 Fixed

2016-11-17 16:02:19 +03:00
parent c44ecded42
commit 45e28bef1c
27 changed files with 244 additions and 61 deletions
@@ -0,0 +1,5 @@
 <html>
 <body>
 This inspection reports any property declaration that can be joined with the following assignment
 </body>
 </html>
@@ -1928,6 +1928,14 @@
                     language="kotlin"
    />
    <localInspection implementationClass="org.jetbrains.kotlin.idea.intentions.JoinDeclarationAndAssignmentInspection"
                     displayName="Join declaration and assignment"
                     groupName="Kotlin"
                     enabledByDefault="true"
                     level="WEAK WARNING"
                     language="kotlin"
    />
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -24,38 +24,44 @@ import com.intellij.psi.util.PsiTreeUtil
 import org.jetbrains.kotlin.idea.core.canOmitDeclaredType
 import org.jetbrains.kotlin.idea.core.moveCaret
 import org.jetbrains.kotlin.idea.core.unblockDocument
 import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.*
 import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
-class JoinDeclarationAndAssignmentIntention :
+class JoinDeclarationAndAssignmentInspection : IntentionBasedInspection<KtProperty>(
-        SelfTargetingIntention<KtBinaryExpression>(KtBinaryExpression::class.java, "Join declaration and assignment") {
+        JoinDeclarationAndAssignmentIntention::class,
        "Can be joined with assignment"
 )
-    override fun isApplicableTo(element: KtBinaryExpression, caretOffset: Int): Boolean {
+class JoinDeclarationAndAssignmentIntention : SelfTargetingOffsetIndependentIntention<KtProperty>(
-        if (element.operationToken != KtTokens.EQ) {
+        KtProperty::class.java,
        "Join declaration and assignment"
 ) {
    override fun isApplicableTo(element: KtProperty): Boolean {
        if (element.hasDelegate()
            || element.hasInitializer()
            || element.setter != null
            || element.getter != null
            || element.receiverTypeReference != null
            || element.name == null) {
            return false
        }
        val rightExpression = element.right ?: return false
        val initializer = PsiTreeUtil.getParentOfType(element,
                                                      KtAnonymousInitializer::class.java,
                                                      KtSecondaryConstructor::class.java) ?: return false
        val target = findTargetProperty(element)
        return target != null && target.initializer == null && target.receiverTypeReference == null &&
               target.getNonStrictParentOfType<KtClassOrObject>() == element.getNonStrictParentOfType<KtClassOrObject>() &&
               hasNoLocalDependencies(rightExpression, initializer)
        val assignment = findAssignment(element) ?: return false
        return assignment.right?.let { hasNoLocalDependencies(it, element.parent) } ?: false
    }
-    override fun applyTo(element: KtBinaryExpression, editor: Editor?) {
+    override fun applyTo(element: KtProperty, editor: Editor?) {
-        val property = findTargetProperty(element) ?: return
+        val typeReference = element.typeReference ?: return
        val initializer = element.right ?: return
        val newInitializer = property.setInitializer(initializer)!!
-        val initializerBlock = element.getStrictParentOfType<KtAnonymousInitializer>()
+        val assignment = findAssignment(element) ?: return
-        element.delete()
+        val initializer = assignment.right ?: return
        val newInitializer = element.setInitializer(initializer)!!
        val initializerBlock = assignment.parent?.parent as? KtAnonymousInitializer
        assignment.delete()
        if (initializerBlock != null && (initializerBlock.body as? KtBlockExpression)?.isEmpty() == true) {
            initializerBlock.delete()
        }
@@ -63,11 +69,10 @@ class JoinDeclarationAndAssignmentIntention :
        editor?.apply {
            unblockDocument()
-            val typeRef = property.typeReference
+            if (element.canOmitDeclaredType(newInitializer, canChangeTypeToSubtype = !element.isVar)) {
-            if (typeRef != null && property.canOmitDeclaredType(newInitializer, canChangeTypeToSubtype = !property.isVar)) {
+                val colon = element.colon!!
-                val colon = property.colon!!
+                selectionModel.setSelection(colon.startOffset, typeReference.endOffset)
-                selectionModel.setSelection(colon.startOffset, typeRef.endOffset)
+                moveCaret(typeReference.endOffset, ScrollType.CENTER)
                moveCaret(typeRef.endOffset, ScrollType.CENTER)
            }
            else {
                moveCaret(newInitializer.startOffset, ScrollType.CENTER)
@@ -75,15 +80,40 @@ class JoinDeclarationAndAssignmentIntention :
        }
    }
-    private fun findTargetProperty(expr: KtBinaryExpression): KtProperty? {
+    private fun findAssignment(property: KtProperty): KtBinaryExpression? {
-        val leftExpression = expr.left as? KtNameReferenceExpression ?: return null
+        val propertyContainer = property.parent as? KtElement ?: return null
-        return leftExpression.resolveAllReferences().firstIsInstanceOrNull<KtProperty>()
+        property.typeReference ?: return null
        val assignments = mutableListOf<KtBinaryExpression>()
        fun process(binaryExpr: KtBinaryExpression) {
            if (binaryExpr.operationToken != KtTokens.EQ) return
            val leftReference = binaryExpr.left as? KtNameReferenceExpression ?: return
            if (leftReference.getReferencedName() != property.name) return
            assignments += binaryExpr
        }
        propertyContainer.forEachDescendantOfType(::process)
        fun PsiElement?.invalidParent(): Boolean {
            when {
                this == null -> return true
                this === propertyContainer -> return false
                else -> {
                    val grandParent = parent
                    if (grandParent.parent !== propertyContainer) return true
                    return grandParent !is KtAnonymousInitializer && grandParent !is KtSecondaryConstructor
                }
            }
        }
        if (assignments.any { it.parent.invalidParent() }) return null
        val first = assignments.firstOrNull() ?: return null
        if (assignments.any { it !== first && it.parent?.parent is KtSecondaryConstructor}) return null
        return first
    }
-    fun KtBlockExpression.isEmpty(): Boolean {
+    // a block that only contains comments is not empty
-        // a block that only contains comments is not empty
+    private fun KtBlockExpression.isEmpty() = contentRange().isEmpty
        return contentRange().isEmpty
    }
    private fun hasNoLocalDependencies(element: KtElement, localContext: PsiElement): Boolean {
        return !element.anyDescendantOfType<PsiElement> { child ->
@@ -97,5 +127,3 @@ private fun PsiElement.resolveAllReferences(): Sequence<PsiElement?> {
            .asSequence()
            .map { it.resolve() }
 }
@@ -0,0 +1,8 @@
 // IS_APPLICABLE: false
 fun foo(flag: Boolean) {
    var x: Double<caret>
    if (flag) {
        x = 3.14
    }
    x = 2.71
 }
@@ -1,7 +1,7 @@
 class A {
-    var a: String?
+    var a<caret>: String?
    init {
-        <caret>a = null
+        a = null
    }
 }
@@ -1,9 +1,9 @@
 // WITH_RUNTIME
 class A {
-    var a: List<String>
+    var a<caret>: List<String>
    init {
-        <caret>a = emptyList()
+        a = emptyList()
    }
 }
@@ -1,7 +0,0 @@
 class A {
    val a: String?
    init {
        <caret>a = null
    }
 }
@@ -1,4 +0,0 @@
 class A {
    val a<selection>: String?</selection><caret> = null
 }
@@ -0,0 +1,11 @@
 // IS_APPLICABLE: false
 fun bar() {
    var x: <caret>String
    fun foo() {
        x = "456"
        x.hashCode()
    }
    foo()
    x = "123"
    x.hashCode()
 }
@@ -1,8 +1,8 @@
 class A {
-    var a: Int
+    var a<caret>: Int
    init {
        // Initialize a
-        <caret>a = 1
+        a = 1
    }
 }
@@ -0,0 +1,4 @@
 fun foo(flag: Boolean) {
    val x: Double<caret>
    x = if (flag) 3.14 else 2.71
 }
@@ -0,0 +1,3 @@
 fun foo(flag: Boolean) {
    val x<selection>: Double</selection><caret> = if (flag) 3.14 else 2.71
 }
@@ -1,7 +1,7 @@
 class A {
-    var a: Int
+    var a<caret>: Int
    init {
-        <caret>a = 1
+        a = 1
    }
 }
@@ -0,0 +1,6 @@
 // IS_APPLICABLE: false
 fun foo() {
    val x: Double<caret>
    val flag = false
    x = if (flag) 3.14 else 2.71
 }
@@ -0,0 +1,12 @@
 // IS_APPLICABLE: false
 class A {
    constructor() {
        a = 1
    }
    constructor(aa: Int) {
        a = aa
    }
    val a<caret>: Int
 }
@@ -0,0 +1,9 @@
 class C {
    var s<caret>: Int
    init {
        s = 1
        s.hashCode()
        s = 2
    }
 }
@@ -0,0 +1,8 @@
 class C {
    var s<selection>: Int</selection><caret> = 1
    init {
        s.hashCode()
        s = 2
    }
 }
@@ -1,9 +1,9 @@
 class A {
-    var a: Int
+    var a<caret>: Int
    var b: Int
    init {
-        <caret>a = 1
+        a = 1
        b = 2
    }
 }
@@ -0,0 +1,4 @@
 fun foo() {
    val s: String<caret>
    s = "Hello"
 }
@@ -0,0 +1,3 @@
 fun foo() {
    val s<selection>: String</selection><caret> = "Hello"
 }
@@ -0,0 +1,7 @@
 class A {
    constructor() {
        a = 1
    }
    val a<caret>: Int
 }
@@ -0,0 +1,6 @@
 class A {
    constructor() {
    }
    val a<selection>: Int</selection><caret> = 1
 }
@@ -1,9 +1,9 @@
 // IS_APPLICABLE: false
 class A {
-    var b: Int
+    var b<caret>: Int
    init {
        val i = 0
-        b = <caret>i
+        b = i
    }
 }
@@ -0,0 +1,7 @@
 // IS_APPLICABLE: false
 fun foo() {
    // It's possible here to join with assignment but move the result after declaration of i
    var b<caret>: Int
    val i = 0
    b = i
 }
@@ -0,0 +1,6 @@
 fun foo() {
    var s<caret>: Int
    s = 1
    s.hashCode()
    s = 2
 }
@@ -0,0 +1,5 @@
 fun foo() {
    var s<selection>: Int</selection><caret> = 1
    s.hashCode()
    s = 2
 }
@@ -8046,6 +8046,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/joinDeclarationAndAssignment"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
        }
        @TestMetadata("assignmentInIf.kt")
        public void testAssignmentInIf() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/assignmentInIf.kt");
            doTest(fileName);
        }
        @TestMetadata("cannotRemoveType.kt")
        public void testCannotRemoveType() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType.kt");
@@ -8058,9 +8064,9 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
            doTest(fileName);
        }
-        @TestMetadata("cannotRemoveType3.kt")
+        @TestMetadata("capturedInitialization.kt")
-        public void testCannotRemoveType3() throws Exception {
+        public void testCapturedInitialization() throws Exception {
-            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt");
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/capturedInitialization.kt");
            doTest(fileName);
        }
@@ -8070,23 +8076,71 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
            doTest(fileName);
        }
        @TestMetadata("correctConditionalAssignment.kt")
        public void testCorrectConditionalAssignment() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt");
            doTest(fileName);
        }
        @TestMetadata("deleteInitBlock.kt")
        public void testDeleteInitBlock() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/deleteInitBlock.kt");
            doTest(fileName);
        }
        @TestMetadata("incorrectConditionalAssignment.kt")
        public void testIncorrectConditionalAssignment() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/incorrectConditionalAssignment.kt");
            doTest(fileName);
        }
        @TestMetadata("multipleConstructors.kt")
        public void testMultipleConstructors() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/multipleConstructors.kt");
            doTest(fileName);
        }
        @TestMetadata("propertyReassignment.kt")
        public void testPropertyReassignment() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt");
            doTest(fileName);
        }
        @TestMetadata("simple.kt")
        public void testSimple() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/simple.kt");
            doTest(fileName);
        }
        @TestMetadata("simpleLocal.kt")
        public void testSimpleLocal() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt");
            doTest(fileName);
        }
        @TestMetadata("singleConstructor.kt")
        public void testSingleConstructor() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt");
            doTest(fileName);
        }
        @TestMetadata("usedLocal.kt")
        public void testUsedLocal() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/usedLocal.kt");
            doTest(fileName);
        }
        @TestMetadata("usedLocal2.kt")
        public void testUsedLocal2() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/usedLocal2.kt");
            doTest(fileName);
        }
        @TestMetadata("varReassignment.kt")
        public void testVarReassignment() throws Exception {
            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt");
            doTest(fileName);
        }
    }
    @TestMetadata("idea/testData/intentions/loopToCallChain")