From df4cd141616a95874836de90947ed36937bce219 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Mon, 26 Feb 2018 17:41:17 +0300 Subject: [PATCH] Move: Relax private/protected visibility checks for usages in Java code --- .../jetbrains/kotlin/util/descriptorUtils.kt | 5 ++ .../moveDeclarations/moveConflictUtils.kt | 65 +++++++++++++------ .../classAlreadyInaccessible/conflicts.txt | 1 + .../objectAlreadyInaccessible/conflicts.txt | 1 + 4 files changed, 53 insertions(+), 19 deletions(-) create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/classAlreadyInaccessible/conflicts.txt create mode 100644 idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/objectAlreadyInaccessible/conflicts.txt diff --git a/idea/ide-common/src/org/jetbrains/kotlin/util/descriptorUtils.kt b/idea/ide-common/src/org/jetbrains/kotlin/util/descriptorUtils.kt index cc88f6266ab..68534752387 100644 --- a/idea/ide-common/src/org/jetbrains/kotlin/util/descriptorUtils.kt +++ b/idea/ide-common/src/org/jetbrains/kotlin/util/descriptorUtils.kt @@ -17,6 +17,8 @@ package org.jetbrains.kotlin.util import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.load.java.descriptors.JavaCallableMemberDescriptor +import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor import org.jetbrains.kotlin.resolve.OverridingUtil import org.jetbrains.kotlin.resolve.OverridingUtil.OverrideCompatibilityInfo.Result.CONFLICT import org.jetbrains.kotlin.resolve.OverridingUtil.OverrideCompatibilityInfo.Result.OVERRIDABLE @@ -101,3 +103,6 @@ val ClassifierDescriptorWithTypeParameters.kind: ClassKind? is ClassDescriptor -> kind else -> null } + +val DeclarationDescriptor.isJavaDescriptor + get() = this is JavaClassDescriptor || this is JavaCallableMemberDescriptor \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt index ade7a7d30c8..f602e7ae8de 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/move/moveDeclarations/moveConflictUtils.kt @@ -55,6 +55,7 @@ import org.jetbrains.kotlin.idea.search.not import org.jetbrains.kotlin.idea.util.projectStructure.getModule import org.jetbrains.kotlin.idea.util.projectStructure.module import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.load.java.JavaVisibilities import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.* import org.jetbrains.kotlin.renderer.ClassifierNamePolicy @@ -70,6 +71,7 @@ import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.resolve.lazy.descriptors.findPackageFragmentForFile import org.jetbrains.kotlin.resolve.source.KotlinSourceElement import org.jetbrains.kotlin.resolve.source.getPsi +import org.jetbrains.kotlin.util.isJavaDescriptor import org.jetbrains.kotlin.utils.SmartSet import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull import java.util.* @@ -141,27 +143,50 @@ class MoveConflictChecker( } } - private fun DeclarationDescriptor.asPredicted(newContainer: DeclarationDescriptor): DeclarationDescriptor? { - val originalVisibility = (this as? DeclarationDescriptorWithVisibility)?.visibility ?: return null - val visibility = if (originalVisibility == Visibilities.PROTECTED && newContainer is PackageFragmentDescriptor) { + private fun DeclarationDescriptor.wrap( + newContainer: DeclarationDescriptor? = null, + newVisibility: Visibility? = null + ): DeclarationDescriptor? { + if (newContainer == null && newVisibility == null) return this + + val wrappedDescriptor = this + return when (wrappedDescriptor) { + // We rely on visibility not depending on more specific type of CallableMemberDescriptor + is CallableMemberDescriptor -> object : CallableMemberDescriptor by wrappedDescriptor { + override fun getOriginal() = this + override fun getContainingDeclaration() = newContainer ?: wrappedDescriptor.containingDeclaration + override fun getVisibility(): Visibility = newVisibility ?: wrappedDescriptor.visibility + override fun getSource() = + newContainer?.let { SourceElement { DescriptorUtils.getContainingSourceFile(it) } } ?: wrappedDescriptor.source + } + is ClassDescriptor -> object: ClassDescriptor by wrappedDescriptor { + override fun getOriginal() = this + override fun getContainingDeclaration() = newContainer ?: wrappedDescriptor.containingDeclaration + override fun getVisibility(): Visibility = newVisibility ?: wrappedDescriptor.visibility + override fun getSource() = + newContainer?.let { SourceElement { DescriptorUtils.getContainingSourceFile(it) } } ?: wrappedDescriptor.source + } + else -> null + } + } + + private fun DeclarationDescriptor.asPredicted(newContainer: DeclarationDescriptor, actualVisibility: Visibility?): DeclarationDescriptor? { + val visibility = actualVisibility ?: (this as? DeclarationDescriptorWithVisibility)?.visibility ?: return null + val adjustedVisibility = if (visibility == Visibilities.PROTECTED && newContainer is PackageFragmentDescriptor) { Visibilities.PUBLIC } else { - originalVisibility + visibility } - return when (this) { - // We rely on visibility not depending on more specific type of CallableMemberDescriptor - is CallableMemberDescriptor -> object : CallableMemberDescriptor by this { - override fun getOriginal() = this - override fun getContainingDeclaration() = newContainer - override fun getVisibility(): Visibility = visibility - override fun getSource() = SourceElement { DescriptorUtils.getContainingSourceFile(newContainer) } - } - is ClassDescriptor -> object: ClassDescriptor by this { - override fun getOriginal() = this - override fun getContainingDeclaration() = newContainer - override fun getVisibility(): Visibility = visibility - override fun getSource() = SourceElement { DescriptorUtils.getContainingSourceFile(newContainer) } + return wrap(newContainer, adjustedVisibility) + } + + private fun DeclarationDescriptor.visibilityAsViewedFromJava(): Visibility? { + if (this !is DeclarationDescriptorWithVisibility) return null + return when (visibility) { + Visibilities.PRIVATE -> { + if (this is ClassDescriptor && DescriptorUtils.isTopLevelDeclaration(this)) JavaVisibilities.PACKAGE_VISIBILITY else null } + Visibilities.PROTECTED -> JavaVisibilities.PROTECTED_AND_PACKAGE else -> null } } @@ -351,9 +376,11 @@ class MoveConflictChecker( is PsiMember -> container.getJavaMemberDescriptor() else -> null } ?: continue - val descriptorToCheck = referencedDescriptor.asPredicted(targetContainer) ?: continue + val actualVisibility = if (referencingDescriptor.isJavaDescriptor) referencedDescriptor.visibilityAsViewedFromJava() else null + val originalDescriptorToCheck = referencedDescriptor.wrap(newVisibility = actualVisibility) ?: referencedDescriptor + val newDescriptorToCheck = referencedDescriptor.asPredicted(targetContainer, actualVisibility) ?: continue - if (referencedDescriptor.isVisibleIn(referencingDescriptor) && !descriptorToCheck.isVisibleIn(referencingDescriptor)) { + if (originalDescriptorToCheck.isVisibleIn(referencingDescriptor) && !newDescriptorToCheck.isVisibleIn(referencingDescriptor)) { val message = "${render(container)} uses ${render(referencedElement)} which will be inaccessible after move" conflicts.putValue(element, message.capitalize()) } diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/classAlreadyInaccessible/conflicts.txt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/classAlreadyInaccessible/conflicts.txt new file mode 100644 index 00000000000..d6fe0a31673 --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/classAlreadyInaccessible/conflicts.txt @@ -0,0 +1 @@ +Method bar() uses class Test which will be inaccessible after move diff --git a/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/objectAlreadyInaccessible/conflicts.txt b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/objectAlreadyInaccessible/conflicts.txt new file mode 100644 index 00000000000..e0abc0552af --- /dev/null +++ b/idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/objectAlreadyInaccessible/conflicts.txt @@ -0,0 +1 @@ +Method bar() uses object Test which will be inaccessible after move