Pull Up: Implement conflict analysis

idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpDialog.kt

@@ -17,6 +17,7 @@
 package org.jetbrains.kotlin.idea.refactoring.pullUp
 
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.ui.DialogWrapper
 import com.intellij.psi.PsiComment
 import com.intellij.refactoring.JavaRefactoringSettings
 import com.intellij.refactoring.classMembers.AbstractMemberInfoModel
@@ -74,6 +75,8 @@ public class KotlinPullUpDialog(
 
     protected val memberInfoStorage: KotlinMemberInfoStorage get() = myMemberInfoStorage
 
+    protected val sourceClass: JetClassOrObject get() = myClass
+
     override fun getDimensionServiceKey() = "#" + javaClass.getName()
 
     override fun getSuperClass() = super.getSuperClass() as? JetClass
@@ -98,7 +101,11 @@ public class KotlinPullUpDialog(
             KotlinMemberSelectionTable(infos, null, "Make abstract")
 
     override fun doAction() {
-        invokeRefactoring(createProcessor(myClass, getSuperClass()!!, getSelectedMemberInfos()))
+        val selectedMembers = getSelectedMemberInfos()
+        val targetClass = getSuperClass()!!
+        checkConflicts(getProject(), sourceClass, targetClass, selectedMembers, { close(DialogWrapper.OK_EXIT_CODE) }) {
+            invokeRefactoring(createProcessor(sourceClass, targetClass, selectedMembers))
+        }
     }
 
     companion object {

idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpHandler.kt

@@ -148,9 +148,11 @@ public class KotlinPullUpHandler : RefactoringActionHandler, ElementsHandler {
 
         if (ApplicationManager.getApplication().isUnitTestMode()) {
             val helper = dataContext?.getData(PULLUP_TEST_HELPER_KEY) as TestHelper
-            KotlinPullUpDialog.createProcessor(classOrObject,
-                                               helper.chooseSuperClass(superClasses),
-                                               helper.adjustMembers(members)).run()
+            val selectedMembers = helper.adjustMembers(members)
+            val targetClass = helper.chooseSuperClass(superClasses)
+            checkConflicts(project, classOrObject, targetClass, selectedMembers) {
+                KotlinPullUpDialog.createProcessor(classOrObject, targetClass, selectedMembers).run()
+            }
         }
         else {
             val manager = classOrObject.getManager()

idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpHelper.kt

@@ -109,11 +109,6 @@ class KotlinPullUpHelper(
 
     }
 
-    private fun getClashingMemberInTargetClass(memberDescriptor: CallableMemberDescriptor): CallableMemberDescriptor? {
-        val memberInSuper = memberDescriptor.substitute(data.sourceToTargetClassSubstitutor) ?: return null
-        return data.targetClassDescriptor.findCallableMemberBySignature(memberInSuper as CallableMemberDescriptor)
-    }
-
     private fun fixOverrideAndGetClashingSuper(sourceMember: JetCallableDeclaration,
                                                targetMember: JetCallableDeclaration): JetCallableDeclaration? {
         val memberDescriptor = data.memberDescriptors[sourceMember] as CallableMemberDescriptor
@@ -123,7 +118,7 @@ class KotlinPullUpHelper(
             return null
         }
 
-        val clashingSuperDescriptor = getClashingMemberInTargetClass(memberDescriptor) ?: return null
+        val clashingSuperDescriptor = data.getClashingMemberInTargetClass(memberDescriptor) ?: return null
         if (clashingSuperDescriptor.getOverriddenDescriptors().isEmpty()) {
             targetMember.removeOverrideModifier()
         }

idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt

@@ -0,0 +1,209 @@
+/*
+ * Copyright 2010-2015 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.idea.refactoring.pullUp
+
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElement
+import com.intellij.refactoring.RefactoringBundle
+import com.intellij.refactoring.util.CommonRefactoringUtil
+import com.intellij.refactoring.util.RefactoringUIUtil
+import com.intellij.util.containers.MultiMap
+import org.jetbrains.kotlin.asJava.toLightClass
+import org.jetbrains.kotlin.asJava.unwrapped
+import org.jetbrains.kotlin.descriptors.*
+import org.jetbrains.kotlin.idea.core.refactoring.checkConflictsInteractively
+import org.jetbrains.kotlin.idea.refactoring.memberInfo.KotlinMemberInfo
+import org.jetbrains.kotlin.idea.references.JetReference
+import org.jetbrains.kotlin.idea.search.declarationsSearch.HierarchySearchRequest
+import org.jetbrains.kotlin.idea.search.declarationsSearch.searchInheritors
+import org.jetbrains.kotlin.idea.search.usagesSearch.DefaultSearchHelper
+import org.jetbrains.kotlin.idea.search.usagesSearch.UsagesSearchTarget
+import org.jetbrains.kotlin.idea.search.usagesSearch.search
+import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
+import org.jetbrains.kotlin.lexer.JetTokens
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.allChildren
+import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
+import org.jetbrains.kotlin.renderer.DescriptorRenderer
+import org.jetbrains.kotlin.renderer.ParameterNameRenderingPolicy
+import org.jetbrains.kotlin.resolve.DescriptorUtils
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue
+import org.jetbrains.kotlin.resolve.source.getPsi
+import org.jetbrains.kotlin.types.TypeSubstitutor
+import org.jetbrains.kotlin.types.Variance
+import org.jetbrains.kotlin.types.substitutions.getTypeSubstitutor
+import org.jetbrains.kotlin.util.findCallableMemberBySignature
+
+fun checkConflicts(project: Project,
+                   sourceClass: JetClassOrObject,
+                   targetClass: JetClass,
+                   memberInfos: List<KotlinMemberInfo>,
+                   onShowConflicts: () -> Unit = {},
+                   onAccept: () -> Unit) {
+    val conflicts = MultiMap<PsiElement, String>()
+
+    val pullUpData = KotlinPullUpData(sourceClass, targetClass, memberInfos.map { it.getMember() })
+
+    with(pullUpData) {
+        for (memberInfo in memberInfos) {
+            val member = memberInfo.getMember()
+            val memberDescriptor = resolutionFacade.resolveToDescriptor(member)
+
+            checkClashWithSuperDeclaration(member, memberDescriptor, conflicts)
+            checkAccidentalOverrides(member, memberDescriptor, conflicts)
+            checkInnerClassToInterface(member, memberDescriptor, conflicts)
+            checkVisibility(memberInfo, memberDescriptor, conflicts)
+        }
+    }
+
+    project.checkConflictsInteractively(conflicts, onShowConflicts, onAccept)
+}
+
+private val CALLABLE_RENDERER = IdeDescriptorRenderers.SOURCE_CODE_SHORT_NAMES_IN_TYPES.withOptions {
+    parameterNameRenderingPolicy = ParameterNameRenderingPolicy.NONE
+    modifiers = emptySet()
+    startFromName = false
+}
+
+private fun DeclarationDescriptor.renderForConflicts(): String {
+    return when (this) {
+        // todo: objects
+        is ClassDescriptor -> "${DescriptorRenderer.getClassKindPrefix(this)} ${IdeDescriptorRenderers.SOURCE_CODE.renderClassifierName(this)}"
+        is FunctionDescriptor -> "function '${CALLABLE_RENDERER.render(this)}'"
+        is PropertyDescriptor -> "property '${CALLABLE_RENDERER.render(this)}'"
+        else -> ""
+    }
+}
+
+private fun KotlinPullUpData.getClashingMemberInTargetClass(memberDescriptor: CallableMemberDescriptor): CallableMemberDescriptor? {
+    val memberInSuper = memberDescriptor.substitute(sourceToTargetClassSubstitutor) ?: return null
+    return targetClassDescriptor.findCallableMemberBySignature(memberInSuper as CallableMemberDescriptor)
+}
+
+private fun KotlinPullUpData.checkClashWithSuperDeclaration(
+        member: JetNamedDeclaration,
+        memberDescriptor: DeclarationDescriptor,
+        conflicts: MultiMap<PsiElement, String>) {
+    if (memberDescriptor is CallableMemberDescriptor) {
+        val clashingSuper = getClashingMemberInTargetClass(memberDescriptor)
+        if (clashingSuper != null && clashingSuper.getModality() != Modality.ABSTRACT) {
+            val message = "${targetClassDescriptor.renderForConflicts()} already contains ${memberDescriptor.renderForConflicts()}"
+            conflicts.putValue(member, message.capitalize())
+        }
+    }
+}
+
+private fun KotlinPullUpData.checkAccidentalOverrides(
+        member: JetNamedDeclaration,
+        memberDescriptor: DeclarationDescriptor,
+        conflicts: MultiMap<PsiElement, String>) {
+    if (memberDescriptor is CallableDescriptor && !member.hasModifier(JetTokens.PRIVATE_KEYWORD)) {
+        val memberDescriptorInTargetClass = memberDescriptor.substitute(sourceToTargetClassSubstitutor)
+        if (memberDescriptorInTargetClass != null) {
+            HierarchySearchRequest<PsiElement>(targetClass, targetClass.getUseScope())
+                    .searchInheritors()
+                    .asSequence()
+                    .filterNot { it.unwrapped == sourceClass || it.unwrapped == targetClass }
+                    .map { it.unwrapped as? JetClassOrObject }
+                    .filterNotNull()
+                    .forEach {
+                        val subClassDescriptor = resolutionFacade.resolveToDescriptor(it) as ClassDescriptor
+                        val substitutor = getTypeSubstitutor(targetClassDescriptor.getDefaultType(),
+                                                             subClassDescriptor.getDefaultType()) ?: TypeSubstitutor.EMPTY
+                        val memberDescriptorInSubClass =
+                                memberDescriptorInTargetClass.substitute(substitutor) as? CallableMemberDescriptor
+                        val clashingMemberDescriptor =
+                                memberDescriptorInSubClass?.let { subClassDescriptor.findCallableMemberBySignature(it) } ?: return
+                        val clashingMember = clashingMemberDescriptor.getSource().getPsi() ?: return
+
+                        val message = memberDescriptor.renderForConflicts() +
+                                      " in super class would clash with existing member of " +
+                                      resolutionFacade.resolveToDescriptor(it).renderForConflicts()
+                        conflicts.putValue(clashingMember, message.capitalize())
+                    }
+        }
+    }
+}
+
+private fun KotlinPullUpData.checkInnerClassToInterface(
+        member: JetNamedDeclaration,
+        memberDescriptor: DeclarationDescriptor,
+        conflicts: MultiMap<PsiElement, String>) {
+    if (targetClass.isInterface() && memberDescriptor is ClassDescriptor && memberDescriptor.isInner()) {
+        val message = "${memberDescriptor.renderForConflicts()} is an inner class. It can not be moved to the interface"
+        conflicts.putValue(member, message.capitalize())
+    }
+}
+
+private fun KotlinPullUpData.checkVisibility(
+        memberInfo: KotlinMemberInfo,
+        memberDescriptor: DeclarationDescriptor,
+        conflicts: MultiMap<PsiElement, String>
+) {
+    fun reportConflictIfAny(targetDescriptor: DeclarationDescriptor) {
+        val target = (targetDescriptor as? DeclarationDescriptorWithSource)?.getSource()?.getPsi() ?: return
+        if (targetDescriptor is DeclarationDescriptorWithVisibility
+            && !Visibilities.isVisible(ReceiverValue.IRRELEVANT_RECEIVER, targetDescriptor, targetClassDescriptor)) {
+            val message = RefactoringBundle.message(
+                    "0.uses.1.which.is.not.accessible.from.the.superclass",
+                    memberDescriptor.renderForConflicts(),
+                    targetDescriptor.renderForConflicts()
+            )
+            conflicts.putValue(target, message.capitalize())
+        }
+    }
+
+    val member = memberInfo.getMember()
+    val childrenToCheck = member.allChildren.toArrayList()
+    if (memberInfo.isToAbstract() && member is JetCallableDeclaration) {
+        when (member) {
+            is JetNamedFunction -> childrenToCheck.remove(member.getBodyExpression())
+            is JetProperty -> {
+                childrenToCheck.remove(member.getInitializer())
+                childrenToCheck.remove(member.getDelegateExpression())
+                childrenToCheck.removeAll(member.getAccessors())
+            }
+        }
+
+        if (member.getTypeReference() == null) {
+            (memberDescriptor as CallableDescriptor).getReturnType()?.let { returnType ->
+                val typeInTargetClass = sourceToTargetClassSubstitutor.substitute(returnType, Variance.INVARIANT)
+                val descriptorToCheck = typeInTargetClass?.getConstructor()?.getDeclarationDescriptor() as? ClassDescriptor
+                if (descriptorToCheck != null) {
+                    reportConflictIfAny(descriptorToCheck)
+                }
+            }
+        }
+    }
+
+    childrenToCheck.forEach {
+        it.accept(
+                object : JetTreeVisitorVoid() {
+                    override fun visitReferenceExpression(expression: JetReferenceExpression) {
+                        super.visitReferenceExpression(expression)
+
+                        val context = resolutionFacade.analyze(expression)
+                        expression.getReferences()
+                                .flatMap { (it as? JetReference)?.resolveToDescriptors(context) ?: emptyList() }
+                                .forEach(::reportConflictIfAny)
+
+                    }
+                }
+        )
+    }
+}

idea/testData/refactoring/pullUp/accidentalOverrides.kt

@@ -0,0 +1,21 @@
+open class A<T, U> {
+}
+
+class <caret>B<X>: A<String, X>() {
+    // INFO: {"checked": "true"}
+    fun foo(s: String, x: X) {
+
+    }
+}
+
+class C<X>: A<Int, B<X>>() {
+    fun foo(s: String, x: B<X>) {
+
+    }
+}
+
+class D<X>: A<Int, X>() {
+    fun foo(s: String, x: B<X>) {
+
+    }
+}

idea/testData/refactoring/pullUp/accidentalOverrides.kt.messages

@@ -0,0 +1 @@
+Function 'fun foo(String, X)' in super class would clash with existing member of class C

idea/testData/refactoring/pullUp/clashWithSuper.kt

@@ -0,0 +1,12 @@
+open class A<T, U> {
+    private fun foo(t: String, u: U) {
+
+    }
+}
+
+class <caret>B<X>: A<String, X>() {
+    // INFO: {"checked": "true"}
+    fun foo(s: String, x: X) {
+
+    }
+}

idea/testData/refactoring/pullUp/clashWithSuper.kt.messages

@@ -0,0 +1 @@
+Class A already contains function 'fun foo(String, X)'

idea/testData/refactoring/pullUp/fromClassToInterface.kt

@@ -15,11 +15,6 @@ abstract class <caret>B: T {
     // INFO: {"checked": "true"}
     abstract fun bar(s: String)
 
-    // INFO: {"checked": "true"}
-    inner class X {
-
-    }
-
     // INFO: {"checked": "true"}
     class Y {
 

idea/testData/refactoring/pullUp/fromClassToInterface.kt.after

@@ -15,11 +15,6 @@ interface T {
     // INFO: {"checked": "true"}
     fun bar(s: String)
 
-    // INFO: {"checked": "true"}
-    class X {
-
-    }
-
     // INFO: {"checked": "true"}
     class Y {
 

idea/testData/refactoring/pullUp/fromClassToInterfaceMakeAbstract.kt

@@ -15,11 +15,6 @@ abstract class <caret>B: T {
     // INFO: {"checked": "true", "toAbstract": "true"}
     abstract fun bar(s: String)
 
-    // INFO: {"checked": "true", "toAbstract": "true"}
-    inner class X {
-
-    }
-
     // INFO: {"checked": "true", "toAbstract": "true"}
     class Y {
 

idea/testData/refactoring/pullUp/fromClassToInterfaceMakeAbstract.kt.after

@@ -15,11 +15,6 @@ interface T {
     // INFO: {"checked": "true", "toAbstract": "true"}
     fun bar(s: String)
 
-    // INFO: {"checked": "true", "toAbstract": "true"}
-    class X {
-
-    }
-
     // INFO: {"checked": "true", "toAbstract": "true"}
     class Y {
 

idea/testData/refactoring/pullUp/inaccessibleMemberUsed.kt

@@ -0,0 +1,21 @@
+open class A {
+
+}
+
+class <caret>B: A() {
+    // INFO: {"checked": "false"}
+    private fun foo() = 1
+
+    // INFO: {"checked": "true"}
+    private class Z(n: Int)
+
+    // INFO: {"checked": "true"}
+    fun bar1() = foo() + 1
+    // INFO: {"checked": "true", "toAbstract": "true"}
+    fun bar2() = Z(foo() + 1)
+
+    // INFO: {"checked": "true"}
+    val x1 = foo() + 1
+    // INFO: {"checked": "true", "toAbstract": "true"}
+    val x2 = Z(foo() + 1)
+}

idea/testData/refactoring/pullUp/inaccessibleMemberUsed.kt.messages

@@ -0,0 +1,4 @@
+Function 'fun bar1(): Int' uses function 'fun foo(): Int', which is not accessible from the superclass
+Function 'fun bar2(): B.Z' uses class B.Z, which is not accessible from the superclass
+Property 'val x1: Int' uses function 'fun foo(): Int', which is not accessible from the superclass
+Property 'val x2: B.Z' uses class B.Z, which is not accessible from the superclass

idea/testData/refactoring/pullUp/innerClassToInterface.kt

@@ -0,0 +1,9 @@
+// WITH_RUNTIME
+interface T
+
+abstract class <caret>B: T {
+    // INFO: {"checked": "true"}
+    inner class X {
+
+    }
+}

idea/testData/refactoring/pullUp/innerClassToInterface.kt.messages

@@ -0,0 +1 @@
+Class B.X is an inner class. It can not be moved to the interface

idea/testData/refactoring/pullUp/noClashWithAbstractSuper.kt

@@ -0,0 +1,12 @@
+abstract class A<T, U> {
+    abstract fun foo(t: String, u: U) {
+
+    }
+}
+
+class <caret>B<X>: A<String, X>() {
+    // INFO: {"checked": "true"}
+    fun foo(s: String, x: X) {
+
+    }
+}

idea/testData/refactoring/pullUp/noClashWithAbstractSuper.kt.after

@@ -0,0 +1,9 @@
+abstract class A<T, U> {
+    // INFO: {"checked": "true"}
+    fun foo(s: String, x: U) {
+
+    }
+}
+
+class B<X>: A<String, X>() {
+}

idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java

@@ -31,10 +31,22 @@ import java.util.regex.Pattern;
 @TestDataPath("$PROJECT_ROOT")
 @RunWith(JUnit3RunnerWithInners.class)
 public class PullUpTestGenerated extends AbstractPullUpTest {
+    @TestMetadata("accidentalOverrides.kt")
+    public void testAccidentalOverrides() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/accidentalOverrides.kt");
+        doTest(fileName);
+    }
+
     public void testAllFilesPresentInPullUp() throws Exception {
         JetTestUtils.assertAllTestsPresentInSingleGeneratedClass(this.getClass(), new File("idea/testData/refactoring/pullUp"), Pattern.compile("^(.+)\\.kt$"));
     }
 
+    @TestMetadata("clashWithSuper.kt")
+    public void testClashWithSuper() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/clashWithSuper.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("fromClassToClass.kt")
     public void testFromClassToClass() throws Exception {
         String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/fromClassToClass.kt");
@@ -71,12 +83,30 @@ public class PullUpTestGenerated extends AbstractPullUpTest {
         doTest(fileName);
     }
 
+    @TestMetadata("inaccessibleMemberUsed.kt")
+    public void testInaccessibleMemberUsed() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/inaccessibleMemberUsed.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("innerClassToInterface.kt")
+    public void testInnerClassToInterface() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/innerClassToInterface.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("noCaret.kt")
     public void testNoCaret() throws Exception {
         String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/noCaret.kt");
         doTest(fileName);
     }
 
+    @TestMetadata("noClashWithAbstractSuper.kt")
+    public void testNoClashWithAbstractSuper() throws Exception {
+        String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/noClashWithAbstractSuper.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("noSuperClass.kt")
     public void testNoSuperClass() throws Exception {
         String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/noSuperClass.kt");