Extract Interface/Pull Up: Warn about private members with usages in the original class

#KT-15606 Fixed

ChangeLog.md

@@ -481,6 +481,7 @@ These artifacts include extensions for the types available in the latter JDKs, s
 - [`KT-15607`](https://youtrack.jetbrains.com/issue/KT-15607) Extract Interface/Pull Up: Disable internal/protected members when moving to an interface
 - [`KT-15640`](https://youtrack.jetbrains.com/issue/KT-15640) Extract Interface/Pull Up: Drop 'final' modifier when moving to an interface
 - [`KT-15639`](https://youtrack.jetbrains.com/issue/KT-15639) Extract Superclass/Interface/Pull Up: Add spaces between 'abstract' modifier and annotations
+- [`KT-15606`](https://youtrack.jetbrains.com/issue/KT-15606) Extract Interface/Pull Up: Warn about private members with usages in the original class
 
 #### Intention actions, inspections and quickfixes
 

idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractClass/ExtractSuperRefactoring.kt

@@ -34,6 +34,7 @@ import org.jetbrains.kotlin.asJava.toLightMethods
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.idea.actions.NewKotlinFileAction
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
 import org.jetbrains.kotlin.idea.codeInsight.shorten.performDelayedShortening
@@ -41,10 +42,12 @@ import org.jetbrains.kotlin.idea.core.*
 import org.jetbrains.kotlin.idea.refactoring.introduce.insertDeclaration
 import org.jetbrains.kotlin.idea.refactoring.memberInfo.KotlinMemberInfo
 import org.jetbrains.kotlin.idea.refactoring.memberInfo.getChildrenToAnalyze
+import org.jetbrains.kotlin.idea.refactoring.memberInfo.resolveToDescriptorWrapperAware
 import org.jetbrains.kotlin.idea.refactoring.memberInfo.toJavaMemberInfo
 import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.KotlinMoveTargetForDeferredFile
 import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.KotlinMoveTargetForExistingElement
 import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.MoveConflictChecker
+import org.jetbrains.kotlin.idea.refactoring.pullUp.checkPrivateMembersWithUsages
 import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.idea.runSynchronouslyWithProgress
 import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
@@ -159,6 +162,16 @@ class ExtractSuperRefactoring(
                     if (targetParent is PsiDirectory) {
                         ExtractSuperClassUtil.checkSuperAccessible(targetParent, conflicts, originalClass.toLightClass())
                     }
+
+                    if (isExtractInterface) {
+                        val resolutionFacade = originalClass.getResolutionFacade()
+
+                        val membersToMove = elementsToMove.filterIsInstance<KtNamedDeclaration>()
+                        for (member in membersToMove) {
+                            val memberDescriptor = member.resolveToDescriptorWrapperAware(resolutionFacade)
+                            checkPrivateMembersWithUsages(member, memberDescriptor, originalClass, membersToMove, conflicts)
+                        }
+                    }
                 }
             }
 

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

@@ -21,8 +21,6 @@ import com.intellij.psi.codeStyle.CodeStyleManager
 import com.intellij.psi.codeStyle.JavaCodeStyleManager
 import com.intellij.psi.impl.light.LightField
 import com.intellij.psi.search.GlobalSearchScope
-import com.intellij.psi.search.LocalSearchScope
-import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.refactoring.classMembers.MemberInfoBase
 import com.intellij.refactoring.memberPullUp.PullUpData
 import com.intellij.refactoring.memberPullUp.PullUpHelper
@@ -51,7 +49,10 @@ import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.*
+import org.jetbrains.kotlin.psi.psiUtil.allChildren
+import org.jetbrains.kotlin.psi.psiUtil.asAssignment
+import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
+import org.jetbrains.kotlin.psi.psiUtil.parameterIndex
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
@@ -271,15 +272,9 @@ class KotlinPullUpHelper(
         }
     }
 
-    private fun willBeUsedInSourceClass(member: PsiElement): Boolean {
-        return !ReferencesSearch
-                .search(member, LocalSearchScope(data.sourceClass), false)
-                .all { it.element.parentsWithSelf.any { it in data.membersToMove } }
-    }
-
     private fun liftToProtected(declaration: KtNamedDeclaration, ignoreUsages: Boolean = false) {
         if (!declaration.hasModifier(KtTokens.PRIVATE_KEYWORD)) return
-        if (ignoreUsages || willBeUsedInSourceClass(declaration)) declaration.addModifier(KtTokens.PROTECTED_KEYWORD)
+        if (ignoreUsages || willBeUsedInSourceClass(declaration, data.sourceClass, data.membersToMove)) declaration.addModifier(KtTokens.PROTECTED_KEYWORD)
     }
 
     override fun setCorrectVisibility(info: MemberInfoBase<PsiMember>) {

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

@@ -20,6 +20,8 @@ import com.intellij.openapi.project.Project
 import com.intellij.psi.PsiClass
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiNamedElement
+import com.intellij.psi.search.LocalSearchScope
+import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.refactoring.RefactoringBundle
 import com.intellij.util.containers.MultiMap
 import org.jetbrains.kotlin.asJava.unwrapped
@@ -35,6 +37,7 @@ import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
+import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
 import org.jetbrains.kotlin.renderer.DescriptorRenderer
 import org.jetbrains.kotlin.renderer.ParameterNameRenderingPolicy
 import org.jetbrains.kotlin.resolve.source.getPsi
@@ -64,12 +67,25 @@ fun checkConflicts(project: Project,
             checkAccidentalOverrides(member, memberDescriptor, conflicts)
             checkInnerClassToInterface(member, memberDescriptor, conflicts)
             checkVisibility(memberInfo, memberDescriptor, conflicts)
+            if (isInterfaceTarget) {
+                checkPrivateMembersWithUsages(member, memberDescriptor, sourceClass, pullUpData.membersToMove, conflicts)
+            }
         }
     }
 
     project.checkConflictsInteractively(conflicts, onShowConflicts, onAccept)
 }
 
+internal fun willBeUsedInSourceClass(
+        member: PsiElement,
+        sourceClass: KtClassOrObject,
+        membersToMove: Collection<KtNamedDeclaration>
+): Boolean {
+    return !ReferencesSearch
+            .search(member, LocalSearchScope(sourceClass), false)
+            .all { it.element.parentsWithSelf.any { it in membersToMove } }
+}
+
 private val CALLABLE_RENDERER = IdeDescriptorRenderers.SOURCE_CODE_SHORT_NAMES_IN_TYPES.withOptions {
     parameterNameRenderingPolicy = ParameterNameRenderingPolicy.NONE
     modifiers = emptySet()
@@ -209,3 +225,17 @@ private fun KotlinPullUpData.checkVisibility(
         )
     }
 }
+
+internal fun checkPrivateMembersWithUsages(
+        member: KtNamedDeclaration,
+        memberDescriptor: DeclarationDescriptor,
+        sourceClass: KtClassOrObject,
+        membersToMove: Collection<KtNamedDeclaration>,
+        conflicts: MultiMap<PsiElement, String>
+) {
+    if (member.hasModifier(KtTokens.PRIVATE_KEYWORD) &&
+        willBeUsedInSourceClass(member, sourceClass, membersToMove)) {
+        val message = "${memberDescriptor.renderForConflicts()} can't be moved to the interface because it's private and has usages in the original class"
+        conflicts.putValue(member, message.capitalize())
+    }
+}

idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt

@@ -0,0 +1,8 @@
+// NAME: I
+// SIBLING:
+class <caret>Foo {
+    // INFO: {checked: "true"}
+    private fun privateFun() = 0
+
+    fun refer() = privateFun()
+}

idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.conflicts

@@ -0,0 +1 @@
+Function 'fun privateFun(): Int' can't be moved to the interface because it's private and has usages in the original class

idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt

@@ -0,0 +1,8 @@
+// NAME: A
+// SIBLING:
+class <caret>Foo {
+    // INFO: {checked: "true"}
+    private fun privateFun() = 0
+
+    fun refer() = privateFun()
+}

idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt.after

@@ -0,0 +1,11 @@
+open class A {
+    // INFO: {checked: "true"}
+    protected fun privateFun() = 0
+}
+
+// NAME: A
+// SIBLING:
+class Foo : A() {
+
+    fun refer() = privateFun()
+}

idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt

@@ -0,0 +1,8 @@
+open class A
+
+class <caret>Foo : A() {
+    // INFO: {checked: "true"}
+    private fun privateFun() = 0
+
+    fun refer() = privateFun()
+}

idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt.after

@@ -0,0 +1,9 @@
+open class A {
+    // INFO: {checked: "true"}
+    protected fun privateFun() = 0
+}
+
+class Foo : A() {
+
+    fun refer() = privateFun()
+}

idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt

@@ -0,0 +1,8 @@
+interface I
+
+class <caret>Foo : I {
+    // INFO: {checked: "true"}
+    private fun privateFun() = 0
+
+    fun refer() = privateFun()
+}

idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.messages

@@ -0,0 +1 @@
+Function 'fun privateFun(): Int' can't be moved to the interface because it's private and has usages in the original class

idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java

@@ -4307,6 +4307,12 @@ public class ExtractionTestGenerated extends AbstractExtractionTest {
             doExtractSuperclassTest(fileName);
         }
 
+        @TestMetadata("privateMemberWithUsages.kt")
+        public void testPrivateMemberWithUsages() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt");
+            doExtractSuperclassTest(fileName);
+        }
+
         @TestMetadata("reformatModifierList.kt")
         public void testReformatModifierList() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/extractSuperclass/reformatModifierList.kt");
@@ -4370,6 +4376,12 @@ public class ExtractionTestGenerated extends AbstractExtractionTest {
             doExtractInterfaceTest(fileName);
         }
 
+        @TestMetadata("privateMemberWithUsages.kt")
+        public void testPrivateMemberWithUsages() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt");
+            doExtractInterfaceTest(fileName);
+        }
+
         @TestMetadata("specialName.kt")
         public void testSpecialName() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/extractInterface/specialName.kt");

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

@@ -290,6 +290,18 @@ public class PullUpTestGenerated extends AbstractPullUpTest {
             doKotlinTest(fileName);
         }
 
+        @TestMetadata("privateMemberWithUsagesToClass.kt")
+        public void testPrivateMemberWithUsagesToClass() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt");
+            doKotlinTest(fileName);
+        }
+
+        @TestMetadata("privateMemberWithUsagesToInterface.kt")
+        public void testPrivateMemberWithUsagesToInterface() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt");
+            doKotlinTest(fileName);
+        }
+
         @TestMetadata("propertyDependenceSatisfied.kt")
         public void testPropertyDependenceSatisfied() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/propertyDependenceSatisfied.kt");