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 fb4a52ef27c..37a4580c40d 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 @@ -45,12 +45,11 @@ import org.jetbrains.kotlin.idea.core.replaced 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.refactoring.pullUp.checkVisibilityInAbstractedMembers import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.runSynchronouslyWithProgress import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers @@ -175,15 +174,7 @@ class ExtractSuperRefactoring( 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) - } - } + checkVisibilityInAbstractedMembers(memberInfos, originalClass.getResolutionFacade(), conflicts) } } 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 c1a66aed3c7..0bbea8bd657 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt @@ -31,12 +31,15 @@ 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.references.KtReference +import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.idea.resolve.ResolutionFacade import org.jetbrains.kotlin.idea.search.declarationsSearch.HierarchySearchRequest import org.jetbrains.kotlin.idea.search.declarationsSearch.searchInheritors 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.forEachDescendantOfType import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf import org.jetbrains.kotlin.renderer.DescriptorRenderer import org.jetbrains.kotlin.renderer.ParameterNameRenderingPolicy @@ -67,23 +70,53 @@ 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) - } } } + checkVisibilityInAbstractedMembers(memberInfos, pullUpData.resolutionFacade, conflicts) project.checkConflictsInteractively(conflicts, onShowConflicts, onAccept) } +internal fun checkVisibilityInAbstractedMembers( + memberInfos: List, + resolutionFacade: ResolutionFacade, + conflicts: MultiMap +) { + val membersToMove = ArrayList() + val membersToAbstract = ArrayList() + + for (memberInfo in memberInfos) { + val member = memberInfo.member ?: continue + (if (memberInfo.isToAbstract) membersToAbstract else membersToMove).add(member) + } + + for (member in membersToAbstract) { + val memberDescriptor = member.resolveToDescriptorWrapperAware(resolutionFacade) + member.forEachDescendantOfType { + val target = it.mainReference.resolve() as? KtNamedDeclaration ?: return@forEachDescendantOfType + if (!willBeMoved(target, membersToMove)) return@forEachDescendantOfType + if (target.hasModifier(KtTokens.PRIVATE_KEYWORD)) { + val targetDescriptor = target.resolveToDescriptorWrapperAware(resolutionFacade) + val memberText = memberDescriptor.renderForConflicts() + val targetText = targetDescriptor.renderForConflicts() + val message = "$memberText uses $targetText which won't be accessible from the subclass." + conflicts.putValue(target, message.capitalize()) + } + } + } +} + +internal fun willBeMoved(element: PsiElement, membersToMove: Collection) = + element.parentsWithSelf.any { it in membersToMove } + internal fun willBeUsedInSourceClass( - member: PsiElement, - sourceClass: KtClassOrObject, - membersToMove: Collection + member: PsiElement, + sourceClass: KtClassOrObject, + membersToMove: Collection ): Boolean { return !ReferencesSearch - .search(member, LocalSearchScope(sourceClass), false) - .all { it.element.parentsWithSelf.any { it in membersToMove } } + .search(member, LocalSearchScope(sourceClass), false) + .all { willBeMoved(it.element, membersToMove) } } private val CALLABLE_RENDERER = IdeDescriptorRenderers.SOURCE_CODE_SHORT_NAMES_NO_ANNOTATIONS.withOptions { @@ -224,18 +257,4 @@ 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/abstractedMemberUsingPrivate.kt b/idea/testData/refactoring/extractInterface/abstractedMemberUsingPrivate.kt new file mode 100644 index 00000000000..51d758f1d3f --- /dev/null +++ b/idea/testData/refactoring/extractInterface/abstractedMemberUsingPrivate.kt @@ -0,0 +1,9 @@ +// NAME: I + +// SIBLING: +class PrivateRef { + // INFO: {checked: "true", toAbstract: "false"} + private fun privateFun() = 0 + // INFO: {checked: "true", toAbstract: "true"} + fun refer() = privateFun() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractInterface/abstractedMemberUsingPrivate.kt.conflicts b/idea/testData/refactoring/extractInterface/abstractedMemberUsingPrivate.kt.conflicts new file mode 100644 index 00000000000..eff95491034 --- /dev/null +++ b/idea/testData/refactoring/extractInterface/abstractedMemberUsingPrivate.kt.conflicts @@ -0,0 +1 @@ +Function 'fun refer(): Int' uses function 'fun privateFun(): Int' which won't be accessible from the subclass. diff --git a/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.after b/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.after new file mode 100644 index 00000000000..30b0a8017c5 --- /dev/null +++ b/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.after @@ -0,0 +1,11 @@ +interface I { + // INFO: {checked: "true"} + fun privateFun() = 0 +} + +// NAME: I +// SIBLING: +class Foo : I { + + 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 deleted file mode 100644 index b6cee1e6d13..00000000000 --- a/idea/testData/refactoring/extractInterface/privateMemberWithUsages.kt.conflicts +++ /dev/null @@ -1 +0,0 @@ -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/abstractedMemberUsingPrivate.kt b/idea/testData/refactoring/extractSuperclass/abstractedMemberUsingPrivate.kt new file mode 100644 index 00000000000..64c6d9c9f81 --- /dev/null +++ b/idea/testData/refactoring/extractSuperclass/abstractedMemberUsingPrivate.kt @@ -0,0 +1,9 @@ +// NAME: PrivateRefBase + +// SIBLING: +class PrivateRef { + // INFO: {checked: "true", toAbstract: "false"} + private fun privateFun() = 0 + // INFO: {checked: "true", toAbstract: "true"} + fun refer() = privateFun() +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractSuperclass/abstractedMemberUsingPrivate.kt.conflicts b/idea/testData/refactoring/extractSuperclass/abstractedMemberUsingPrivate.kt.conflicts new file mode 100644 index 00000000000..eff95491034 --- /dev/null +++ b/idea/testData/refactoring/extractSuperclass/abstractedMemberUsingPrivate.kt.conflicts @@ -0,0 +1 @@ +Function 'fun refer(): Int' uses function 'fun privateFun(): Int' which won't be accessible from the subclass. diff --git a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.after b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.after new file mode 100644 index 00000000000..945eadc8801 --- /dev/null +++ b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.after @@ -0,0 +1,9 @@ +interface I { + // INFO: {checked: "true"} + fun privateFun() = 0 +} + +class Foo : I { + + fun refer() = privateFun() +} diff --git a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.messages b/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.messages deleted file mode 100644 index 5f476a7a244..00000000000 --- a/idea/testData/refactoring/pullUp/k2k/privateMemberWithUsagesToInterface.kt.messages +++ /dev/null @@ -1 +0,0 @@ -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 715824d3099..d67a1c44177 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java @@ -3931,6 +3931,11 @@ public class ExtractionTestGenerated extends AbstractExtractionTest { KotlinTestUtils.runTest(this::doExtractSuperclassTest, TargetBackend.ANY, testDataFilePath); } + @TestMetadata("abstractedMemberUsingPrivate.kt") + public void testAbstractedMemberUsingPrivate() throws Exception { + runTest("idea/testData/refactoring/extractSuperclass/abstractedMemberUsingPrivate.kt"); + } + @TestMetadata("addSuperclassNoSecondaryConstructors.kt") public void testAddSuperclassNoSecondaryConstructors() throws Exception { runTest("idea/testData/refactoring/extractSuperclass/addSuperclassNoSecondaryConstructors.kt"); @@ -4029,6 +4034,11 @@ public class ExtractionTestGenerated extends AbstractExtractionTest { KotlinTestUtils.runTest(this::doExtractInterfaceTest, TargetBackend.ANY, testDataFilePath); } + @TestMetadata("abstractedMemberUsingPrivate.kt") + public void testAbstractedMemberUsingPrivate() throws Exception { + runTest("idea/testData/refactoring/extractInterface/abstractedMemberUsingPrivate.kt"); + } + @TestMetadata("addInterface.kt") public void testAddInterface() throws Exception { runTest("idea/testData/refactoring/extractInterface/addInterface.kt");