diff --git a/idea/resources/inspectionDescriptions/JoinDeclarationAndAssignment.html b/idea/resources/inspectionDescriptions/JoinDeclarationAndAssignment.html new file mode 100644 index 00000000000..250fd965be9 --- /dev/null +++ b/idea/resources/inspectionDescriptions/JoinDeclarationAndAssignment.html @@ -0,0 +1,5 @@ + + +This inspection reports any property declaration that can be joined with the following assignment + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 4dcd6dd4e83..1d8cbaa39a9 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1928,6 +1928,14 @@ language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/JoinDeclarationAndAssignmentIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/JoinDeclarationAndAssignmentIntention.kt index da83e388ee3..702b98c3f6b 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/JoinDeclarationAndAssignmentIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/JoinDeclarationAndAssignmentIntention.kt @@ -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 : - SelfTargetingIntention(KtBinaryExpression::class.java, "Join declaration and assignment") { +class JoinDeclarationAndAssignmentInspection : IntentionBasedInspection( + JoinDeclarationAndAssignmentIntention::class, + "Can be joined with assignment" +) - override fun isApplicableTo(element: KtBinaryExpression, caretOffset: Int): Boolean { - if (element.operationToken != KtTokens.EQ) { +class JoinDeclarationAndAssignmentIntention : SelfTargetingOffsetIndependentIntention( + 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() == element.getNonStrictParentOfType() && - 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?) { - val property = findTargetProperty(element) ?: return - val initializer = element.right ?: return - val newInitializer = property.setInitializer(initializer)!! + override fun applyTo(element: KtProperty, editor: Editor?) { + val typeReference = element.typeReference ?: return - val initializerBlock = element.getStrictParentOfType() - element.delete() + val assignment = findAssignment(element) ?: return + 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 (typeRef != null && property.canOmitDeclaredType(newInitializer, canChangeTypeToSubtype = !property.isVar)) { - val colon = property.colon!! - selectionModel.setSelection(colon.startOffset, typeRef.endOffset) - moveCaret(typeRef.endOffset, ScrollType.CENTER) + if (element.canOmitDeclaredType(newInitializer, canChangeTypeToSubtype = !element.isVar)) { + val colon = element.colon!! + selectionModel.setSelection(colon.startOffset, typeReference.endOffset) + moveCaret(typeReference.endOffset, ScrollType.CENTER) } else { moveCaret(newInitializer.startOffset, ScrollType.CENTER) @@ -75,15 +80,40 @@ class JoinDeclarationAndAssignmentIntention : } } - private fun findTargetProperty(expr: KtBinaryExpression): KtProperty? { - val leftExpression = expr.left as? KtNameReferenceExpression ?: return null - return leftExpression.resolveAllReferences().firstIsInstanceOrNull() + private fun findAssignment(property: KtProperty): KtBinaryExpression? { + val propertyContainer = property.parent as? KtElement ?: return null + property.typeReference ?: return null + + val assignments = mutableListOf() + 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 - return contentRange().isEmpty - } + // a block that only contains comments is not empty + private fun KtBlockExpression.isEmpty() = contentRange().isEmpty private fun hasNoLocalDependencies(element: KtElement, localContext: PsiElement): Boolean { return !element.anyDescendantOfType { child -> @@ -97,5 +127,3 @@ private fun PsiElement.resolveAllReferences(): Sequence { .asSequence() .map { it.resolve() } } - - diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/assignmentInIf.kt b/idea/testData/intentions/joinDeclarationAndAssignment/assignmentInIf.kt new file mode 100644 index 00000000000..99e3ed1ef61 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/assignmentInIf.kt @@ -0,0 +1,8 @@ +// IS_APPLICABLE: false +fun foo(flag: Boolean) { + var x: Double + if (flag) { + x = 3.14 + } + x = 2.71 +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType.kt b/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType.kt index 370f64d0290..9db80b33256 100644 --- a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType.kt +++ b/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType.kt @@ -1,7 +1,7 @@ class A { - var a: String? + var a: String? init { - a = null + a = null } } diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType2.kt b/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType2.kt index e965b72a83d..dee8c203de9 100644 --- a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType2.kt +++ b/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType2.kt @@ -1,9 +1,9 @@ // WITH_RUNTIME class A { - var a: List + var a: List init { - a = emptyList() + a = emptyList() } } diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt b/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt deleted file mode 100644 index 1106c9191fd..00000000000 --- a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt +++ /dev/null @@ -1,7 +0,0 @@ -class A { - val a: String? - - init { - a = null - } -} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt.after b/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt.after deleted file mode 100644 index cd8f33753c1..00000000000 --- a/idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt.after +++ /dev/null @@ -1,4 +0,0 @@ -class A { - val a: String? = null - -} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/capturedInitialization.kt b/idea/testData/intentions/joinDeclarationAndAssignment/capturedInitialization.kt new file mode 100644 index 00000000000..51cd9c5f5f1 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/capturedInitialization.kt @@ -0,0 +1,11 @@ +// IS_APPLICABLE: false +fun bar() { + var x: String + fun foo() { + x = "456" + x.hashCode() + } + foo() + x = "123" + x.hashCode() +} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/comment.kt b/idea/testData/intentions/joinDeclarationAndAssignment/comment.kt index ffccaacf78e..8126eaa36ea 100644 --- a/idea/testData/intentions/joinDeclarationAndAssignment/comment.kt +++ b/idea/testData/intentions/joinDeclarationAndAssignment/comment.kt @@ -1,8 +1,8 @@ class A { - var a: Int + var a: Int init { // Initialize a - a = 1 + a = 1 } } diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt b/idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt new file mode 100644 index 00000000000..e4a1e42a2e1 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt @@ -0,0 +1,4 @@ +fun foo(flag: Boolean) { + val x: Double + x = if (flag) 3.14 else 2.71 +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt.after b/idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt.after new file mode 100644 index 00000000000..0969d2c86f9 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/correctConditionalAssignment.kt.after @@ -0,0 +1,3 @@ +fun foo(flag: Boolean) { + val x: Double = if (flag) 3.14 else 2.71 +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/deleteInitBlock.kt b/idea/testData/intentions/joinDeclarationAndAssignment/deleteInitBlock.kt index bf15b61352d..8328ba6857f 100644 --- a/idea/testData/intentions/joinDeclarationAndAssignment/deleteInitBlock.kt +++ b/idea/testData/intentions/joinDeclarationAndAssignment/deleteInitBlock.kt @@ -1,7 +1,7 @@ class A { - var a: Int + var a: Int init { - a = 1 + a = 1 } } diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/incorrectConditionalAssignment.kt b/idea/testData/intentions/joinDeclarationAndAssignment/incorrectConditionalAssignment.kt new file mode 100644 index 00000000000..71030faa4f2 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/incorrectConditionalAssignment.kt @@ -0,0 +1,6 @@ +// IS_APPLICABLE: false +fun foo() { + val x: Double + val flag = false + x = if (flag) 3.14 else 2.71 +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/multipleConstructors.kt b/idea/testData/intentions/joinDeclarationAndAssignment/multipleConstructors.kt new file mode 100644 index 00000000000..d0191793a9a --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/multipleConstructors.kt @@ -0,0 +1,12 @@ +// IS_APPLICABLE: false +class A { + constructor() { + a = 1 + } + + constructor(aa: Int) { + a = aa + } + + val a: Int +} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt b/idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt new file mode 100644 index 00000000000..ca4ff7161a6 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt @@ -0,0 +1,9 @@ +class C { + var s: Int + + init { + s = 1 + s.hashCode() + s = 2 + } +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt.after b/idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt.after new file mode 100644 index 00000000000..f04c1043748 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/propertyReassignment.kt.after @@ -0,0 +1,8 @@ +class C { + var s: Int = 1 + + init { + s.hashCode() + s = 2 + } +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/simple.kt b/idea/testData/intentions/joinDeclarationAndAssignment/simple.kt index 3d8bf8249b7..93410cefb94 100644 --- a/idea/testData/intentions/joinDeclarationAndAssignment/simple.kt +++ b/idea/testData/intentions/joinDeclarationAndAssignment/simple.kt @@ -1,9 +1,9 @@ class A { - var a: Int + var a: Int var b: Int init { - a = 1 + a = 1 b = 2 } } diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt b/idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt new file mode 100644 index 00000000000..6d35c2bff66 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt @@ -0,0 +1,4 @@ +fun foo() { + val s: String + s = "Hello" +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt.after b/idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt.after new file mode 100644 index 00000000000..47de0c68c3f --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/simpleLocal.kt.after @@ -0,0 +1,3 @@ +fun foo() { + val s: String = "Hello" +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt b/idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt new file mode 100644 index 00000000000..eb0686a3ae3 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt @@ -0,0 +1,7 @@ +class A { + constructor() { + a = 1 + } + + val a: Int +} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt.after b/idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt.after new file mode 100644 index 00000000000..b7e322e5d8e --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/singleConstructor.kt.after @@ -0,0 +1,6 @@ +class A { + constructor() { + } + + val a: Int = 1 +} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal.kt b/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal.kt index 04a5419b546..ddc1601be15 100644 --- a/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal.kt +++ b/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal.kt @@ -1,9 +1,9 @@ // IS_APPLICABLE: false class A { - var b: Int + var b: Int init { val i = 0 - b = i + b = i } } diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal2.kt b/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal2.kt new file mode 100644 index 00000000000..60ebac176f2 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/usedLocal2.kt @@ -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: Int + val i = 0 + b = i +} diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt b/idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt new file mode 100644 index 00000000000..4b988bc36e5 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt @@ -0,0 +1,6 @@ +fun foo() { + var s: Int + s = 1 + s.hashCode() + s = 2 +} \ No newline at end of file diff --git a/idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt.after b/idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt.after new file mode 100644 index 00000000000..1ef940d5cf0 --- /dev/null +++ b/idea/testData/intentions/joinDeclarationAndAssignment/varReassignment.kt.after @@ -0,0 +1,5 @@ +fun foo() { + var s: Int = 1 + s.hashCode() + s = 2 +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 190d073ce89..f6c064e8cb6 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -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") - public void testCannotRemoveType3() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/joinDeclarationAndAssignment/cannotRemoveType3.kt"); + @TestMetadata("capturedInitialization.kt") + public void testCapturedInitialization() throws Exception { + 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")