Replace with binary operator: don't suggest for non-operator function

#KT-12273 Fixed
2018-12-12 19:06:25 +09:00
parent ea0f9d124d
commit f2cf3a8e7b
14 changed files with 102 additions and 13 deletions
@@ -22,13 +22,17 @@ import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiElement
 import com.intellij.psi.tree.IElementType
 import org.jetbrains.kotlin.cfg.pseudocode.containingDeclarationForPseudocode
+import org.jetbrains.kotlin.descriptors.FunctionDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
 import org.jetbrains.kotlin.idea.inspections.AbstractApplicabilityBasedInspection
-import org.jetbrains.kotlin.idea.intentions.*
+import org.jetbrains.kotlin.idea.intentions.callExpression
 import org.jetbrains.kotlin.idea.intentions.conventionNameCalls.isAnyEquals
+import org.jetbrains.kotlin.idea.intentions.isOperatorOrCompatible
+import org.jetbrains.kotlin.idea.intentions.isReceiverExpressionWithValue
+import org.jetbrains.kotlin.idea.intentions.toResolvedCall
 import org.jetbrains.kotlin.idea.project.languageVersionSettings
 import org.jetbrains.kotlin.lexer.KtSingleValueToken
 import org.jetbrains.kotlin.lexer.KtTokens
@@ -63,7 +67,7 @@ class ReplaceCallWithBinaryOperatorInspection : AbstractApplicabilityBasedInspec

    override fun isApplicable(element: KtDotQualifiedExpression): Boolean {
        val calleeExpression = element.callExpression?.calleeExpression as? KtSimpleNameExpression ?: return false
-        val operation = operation(calleeExpression) ?: return false
+        if (operation(calleeExpression) == null) return false

        val resolvedCall = element.toResolvedCall(BodyResolveMode.PARTIAL) ?: return false
        if (!resolvedCall.isReallySuccess()) return false
@@ -137,6 +141,9 @@ class ReplaceCallWithBinaryOperatorInspection : AbstractApplicabilityBasedInspec
    private fun operation(calleeExpression: KtSimpleNameExpression): KtSingleValueToken? {
        val identifier = calleeExpression.getReferencedNameAsName()
        val dotQualified = calleeExpression.parent.parent as? KtDotQualifiedExpression ?: return null
+        val isOperatorOrCompatible by lazy {
+            (calleeExpression.resolveToCall()?.resultingDescriptor as? FunctionDescriptor)?.isOperatorOrCompatible == true
+        }
        return when (identifier) {
            OperatorNameConventions.EQUALS -> {
                if (!dotQualified.isAnyEquals()) return null
@@ -145,6 +152,7 @@ class ReplaceCallWithBinaryOperatorInspection : AbstractApplicabilityBasedInspec
                else KtTokens.EQEQ
            }
            OperatorNameConventions.COMPARE_TO -> {
+                if (!isOperatorOrCompatible) return null
                // callee -> call -> DotQualified -> Binary
                val binaryParent = dotQualified.parent as? KtBinaryExpression ?: return null
                val notZero = when {
@@ -160,7 +168,10 @@ class ReplaceCallWithBinaryOperatorInspection : AbstractApplicabilityBasedInspec
                    null
                }
            }
-            else -> OperatorConventions.BINARY_OPERATION_NAMES.inverse()[identifier]
+            else -> {
+                if (!isOperatorOrCompatible) return null
+                OperatorConventions.BINARY_OPERATION_NAMES.inverse()[identifier]
+            }
        }
    }

@@ -28,6 +28,7 @@ import org.jetbrains.kotlin.idea.core.setType
 import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.idea.search.usagesSearch.descriptor
 import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.containingClass
@@ -42,6 +43,7 @@ import org.jetbrains.kotlin.resolve.source.getPsi
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.kotlin.types.isFlexible
 import org.jetbrains.kotlin.types.typeUtil.isUnit
+import org.jetbrains.kotlin.util.OperatorChecks

 fun KtContainerNode.description(): String? {
    when (node.elementType) {
@@ -330,3 +332,11 @@ fun KtDotQualifiedExpression.isToString(): Boolean {
    val callableDescriptor = resolvedCall.resultingDescriptor as? CallableMemberDescriptor ?: return false
    return callableDescriptor.getDeepestSuperDeclarations().any { it.fqNameUnsafe.asString() == "kotlin.Any.toString" }
 }
+
+val FunctionDescriptor.isOperatorOrCompatible: Boolean
+    get() {
+        if (this is JavaMethodDescriptor) {
+            return OperatorChecks.check(this).isSuccess
+        }
+        return isOperator
+    }
@@ -23,13 +23,11 @@ import org.jetbrains.kotlin.descriptors.FunctionDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
 import org.jetbrains.kotlin.idea.intentions.*
 import org.jetbrains.kotlin.lexer.KtTokens
-import org.jetbrains.kotlin.load.java.descriptors.JavaMethodDescriptor
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.resolve.calls.model.ArgumentMatch
 import org.jetbrains.kotlin.resolve.calls.model.isReallySuccess
 import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
-import org.jetbrains.kotlin.util.OperatorChecks
 import org.jetbrains.kotlin.util.OperatorNameConventions

 class ReplaceContainsIntention : SelfTargetingRangeIntention<KtDotQualifiedExpression>(KtDotQualifiedExpression::class.java, "Replace 'contains' call with 'in' operator"), HighPriorityAction {
@@ -54,14 +52,6 @@ class ReplaceContainsIntention : SelfTargetingRangeIntention<KtDotQualifiedExpre
        return element.callExpression!!.calleeExpression!!.textRange
    }

-    private val FunctionDescriptor.isOperatorOrCompatible: Boolean
-        get() {
-            if (this is JavaMethodDescriptor) {
-                return OperatorChecks.check(this).isSuccess
-            }
-            return isOperator
-        }
-
    override fun applyTo(element: KtDotQualifiedExpression, editor: Editor?) {
        val argument = element.callExpression!!.valueArguments.single().getArgumentExpression()!!
        val receiver = element.receiverExpression
@@ -0,0 +1,5 @@
+public class Operation {
+    public int compareTo(Operation other) {
+        return 0;
+    }
+}
@@ -0,0 +1,5 @@
+public class Operation {
+    public int compareTo(Operation other) {
+        return 0;
+    }
+}
@@ -0,0 +1,5 @@
+// FIX: Replace with '<'
+
+fun test(p1: Operation, p2: Operation) {
+    p1.<caret>compareTo(p2) < 0
+}
@@ -0,0 +1,5 @@
+// FIX: Replace with '<'
+
+fun test(p1: Operation, p2: Operation) {
+    p1 < p2
+}
@@ -0,0 +1,9 @@
+// PROBLEM: none
+
+fun test(p1: Operation, p2: Operation) {
+    p1.<caret>compareTo(p2) < 0
+}
+
+class Operation {
+    fun compareTo(other: Operation) = 0
+}
@@ -0,0 +1,9 @@
+// PROBLEM: none
+
+fun test(p1: Operation, p2: Operation) {
+    p1.<caret>plus(p2)
+}
+
+class Operation {
+    fun plus(other: Operation) = 0
+}
@@ -0,0 +1,5 @@
+public class Operation {
+    public int plus(Operation other) {
+        return 0;
+    }
+}
@@ -0,0 +1,5 @@
+public class Operation {
+    public int plus(Operation other) {
+        return 0;
+    }
+}
@@ -0,0 +1,5 @@
+// FIX: Replace with '+'
+
+fun test(p1: Operation, p2: Operation) {
+    p1.<caret>plus(p2)
+}
@@ -0,0 +1,5 @@
+// FIX: Replace with '+'
+
+fun test(p1: Operation, p2: Operation) {
+    p1 + p2
+}
@@ -1594,6 +1594,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
                KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
            }

+            @TestMetadata("compareToFromJava.kt")
+            public void testCompareToFromJava() throws Exception {
+                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/compareToFromJava.kt");
+            }
+
            @TestMetadata("divSanityTest.kt")
            public void testDivSanityTest() throws Exception {
                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/divSanityTest.kt");
@@ -1699,6 +1704,21 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/notEqualsBracketsComplex.kt");
            }

+            @TestMetadata("notOperatorCompareTo.kt")
+            public void testNotOperatorCompareTo() throws Exception {
+                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/notOperatorCompareTo.kt");
+            }
+
+            @TestMetadata("notOperatorPlus.kt")
+            public void testNotOperatorPlus() throws Exception {
+                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/notOperatorPlus.kt");
+            }
+
+            @TestMetadata("plusFromJava.kt")
+            public void testPlusFromJava() throws Exception {
+                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/plusFromJava.kt");
+            }
+
            @TestMetadata("plusSanityTest.kt")
            public void testPlusSanityTest() throws Exception {
                runTest("idea/testData/inspectionsLocal/conventionNameCalls/replaceCallWithBinaryOperator/plusSanityTest.kt");