diff --git a/ChangeLog.md b/ChangeLog.md index 01cb355befc..69f2fcc1aa7 100644 --- a/ChangeLog.md +++ b/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 diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractClass/ExtractSuperRefactoring.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractClass/ExtractSuperRefactoring.kt index 132ed7db032..b454421e4a2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractClass/ExtractSuperRefactoring.kt +++ b/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() + for (member in membersToMove) { + val memberDescriptor = member.resolveToDescriptorWrapperAware(resolutionFacade) + checkPrivateMembersWithUsages(member, memberDescriptor, originalClass, membersToMove, conflicts) + } + } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpHelper.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpHelper.kt index e87fe3fa3f7..a392aa86fa8 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpHelper.kt +++ b/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) { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt index 4d074fc5850..e96bfcefee7 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt +++ b/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 +): 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, + conflicts: MultiMap +) { + 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()) + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt b/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt new file mode 100644 index 00000000000..7b59472f4c5 --- /dev/null +++ b/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt @@ -0,0 +1,8 @@ +// NAME: I +// SIBLING: +class Foo { + // INFO: {checked: "true"} + private fun privateFun() = 0 + + fun refer() = privateFun() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.conflicts b/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.conflicts new file mode 100644 index 00000000000..b6cee1e6d13 --- /dev/null +++ b/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 diff --git a/idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt b/idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt new file mode 100644 index 00000000000..10a93c2802d --- /dev/null +++ b/idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt @@ -0,0 +1,8 @@ +// NAME: A +// SIBLING: +class Foo { + // INFO: {checked: "true"} + private fun privateFun() = 0 + + fun refer() = privateFun() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt.after b/idea/testData/refactoring/extractSuperclass/privateMemberWithUsages.kt.after new file mode 100644 index 00000000000..59ab383f623 --- /dev/null +++ b/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() +} \ No newline at end of file diff --git a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt new file mode 100644 index 00000000000..c57f2287d16 --- /dev/null +++ b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt @@ -0,0 +1,8 @@ +open class A + +class Foo : A() { + // INFO: {checked: "true"} + private fun privateFun() = 0 + + fun refer() = privateFun() +} \ No newline at end of file diff --git a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt.after b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToClass.kt.after new file mode 100644 index 00000000000..c73482f9af0 --- /dev/null +++ b/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() +} \ No newline at end of file diff --git a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt new file mode 100644 index 00000000000..54283e9e75d --- /dev/null +++ b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt @@ -0,0 +1,8 @@ +interface I + +class Foo : I { + // INFO: {checked: "true"} + private fun privateFun() = 0 + + fun refer() = privateFun() +} \ No newline at end of file diff --git a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.messages b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.messages new file mode 100644 index 00000000000..5f476a7a244 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java index 692f78d5f99..802a0b32382 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java +++ b/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"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java index e180ff09f74..204f124fc7f 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java +++ b/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");