From b258af5288c61d76f6e6bb09d8f4d0ce42b9361f Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Thu, 7 Apr 2016 10:43:36 +0300 Subject: [PATCH] KT-11724 False suggestion to replace with compound assignment #KT-11724 Fixed --- .../ReplaceWithOperatorAssignmentIntention.kt | 32 +++++++++++++++---- .../flexibleTypeBug.kt | 7 ++++ .../plusAssignConflict.kt | 11 +++++++ .../intentions/IntentionTestGenerated.java | 12 +++++++ 4 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 idea/testData/intentions/replaceWithOperatorAssignment/flexibleTypeBug.kt create mode 100644 idea/testData/intentions/replaceWithOperatorAssignment/plusAssignConflict.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceWithOperatorAssignmentIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceWithOperatorAssignmentIntention.kt index e96ec1e292d..5dd667680b6 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceWithOperatorAssignmentIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ReplaceWithOperatorAssignmentIntention.kt @@ -20,14 +20,20 @@ import com.intellij.openapi.editor.Editor import com.intellij.psi.tree.IElementType import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.ClassDescriptor +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.inspections.IntentionBasedInspection +import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.idea.util.psi.patternMatching.matches import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.psi.KtNameReferenceExpression import org.jetbrains.kotlin.psi.KtPsiFactory +import org.jetbrains.kotlin.psi.doNotAnalyze import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.bindingContextUtil.getDataFlowInfo +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode class ReplaceWithOperatorAssignmentInspection : IntentionBasedInspection(ReplaceWithOperatorAssignmentIntention()) @@ -38,12 +44,22 @@ class ReplaceWithOperatorAssignmentIntention : SelfTargetingOffsetIndependentInt val right = element.right as? KtBinaryExpression ?: return false if (right.left == null || right.right == null) return false - return checkExpressionRepeat(left, right) + val bindingContext = right.analyze(BodyResolveMode.PARTIAL) + if (!checkExpressionRepeat(left, right, bindingContext)) return false + + // now check that the resulting operator assignment will be resolved + val opAssign = buildOperatorAssignment(element) + opAssign.getContainingKtFile().doNotAnalyze = null //TODO: strange hack + val resolutionScope = element.getResolutionScope(bindingContext, element.getResolutionFacade()) + val newBindingContext = opAssign.analyzeInContext(resolutionScope, + contextExpression = element, + dataFlowInfo = bindingContext.getDataFlowInfo(element), + isStatement = true) + return newBindingContext.diagnostics.forElement(opAssign.operationReference).isEmpty() } - private fun checkExpressionRepeat(variableExpression: KtNameReferenceExpression, expression: KtBinaryExpression): Boolean { - val context = expression.analyze() - val descriptor = context[BindingContext.REFERENCE_TARGET, expression.operationReference]?.containingDeclaration + private fun checkExpressionRepeat(variableExpression: KtNameReferenceExpression, expression: KtBinaryExpression, bindingContext: BindingContext): Boolean { + val descriptor = bindingContext[BindingContext.REFERENCE_TARGET, expression.operationReference]?.containingDeclaration val isPrimitiveOperation = descriptor is ClassDescriptor && KotlinBuiltIns.isPrimitiveType(descriptor.defaultType) val operationToken = expression.operationToken @@ -62,7 +78,7 @@ class ReplaceWithOperatorAssignmentIntention : SelfTargetingOffsetIndependentInt expressionLeft is KtBinaryExpression -> { val sameCommutativeOperation = expressionLeft.operationToken == operationToken && isCommutative(operationToken) - isPrimitiveOperation && sameCommutativeOperation && checkExpressionRepeat(variableExpression, expressionLeft) + isPrimitiveOperation && sameCommutativeOperation && checkExpressionRepeat(variableExpression, expressionLeft, bindingContext) } else -> { @@ -79,12 +95,16 @@ class ReplaceWithOperatorAssignmentIntention : SelfTargetingOffsetIndependentInt operationToken == KtTokens.PERC override fun applyTo(element: KtBinaryExpression, editor: Editor?) { + element.replace(buildOperatorAssignment(element)) + } + + private fun buildOperatorAssignment(element: KtBinaryExpression): KtBinaryExpression { val replacement = buildOperatorAssignmentText( element.left as KtNameReferenceExpression, element.right as KtBinaryExpression, "" ) - element.replace(KtPsiFactory(element).createExpression(replacement)) + return KtPsiFactory(element).createExpression(replacement) as KtBinaryExpression } tailrec private fun buildOperatorAssignmentText(variableExpression: KtNameReferenceExpression, expression: KtBinaryExpression, tail: String): String { diff --git a/idea/testData/intentions/replaceWithOperatorAssignment/flexibleTypeBug.kt b/idea/testData/intentions/replaceWithOperatorAssignment/flexibleTypeBug.kt new file mode 100644 index 00000000000..d83ae3480bc --- /dev/null +++ b/idea/testData/intentions/replaceWithOperatorAssignment/flexibleTypeBug.kt @@ -0,0 +1,7 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME +fun foo() { + var list1 = java.util.Collections.emptyList() + val list2 = listOf("b") + list1 = list1 + list2 +} \ No newline at end of file diff --git a/idea/testData/intentions/replaceWithOperatorAssignment/plusAssignConflict.kt b/idea/testData/intentions/replaceWithOperatorAssignment/plusAssignConflict.kt new file mode 100644 index 00000000000..cc8a92200c3 --- /dev/null +++ b/idea/testData/intentions/replaceWithOperatorAssignment/plusAssignConflict.kt @@ -0,0 +1,11 @@ +// IS_APPLICABLE: false +class A + +operator fun A.plus(a: A): A = A() +operator fun A.plusAssign(a: A){} + +fun foo() { + var a1 = A() + val a2 = A() + a1 = a1 + a2 +} \ 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 b48a907ea43..2d520b62243 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -8137,6 +8137,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/replaceWithOperatorAssignment"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); } + @TestMetadata("flexibleTypeBug.kt") + public void testFlexibleTypeBug() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceWithOperatorAssignment/flexibleTypeBug.kt"); + doTest(fileName); + } + @TestMetadata("illegalMultipleOperators.kt") public void testIllegalMultipleOperators() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceWithOperatorAssignment/illegalMultipleOperators.kt"); @@ -8179,6 +8185,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("plusAssignConflict.kt") + public void testPlusAssignConflict() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceWithOperatorAssignment/plusAssignConflict.kt"); + doTest(fileName); + } + @TestMetadata("rightSideRepeat.kt") public void testRightSideRepeat() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceWithOperatorAssignment/rightSideRepeat.kt");