diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt index e2a044fba2d..ed933c4ab47 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt @@ -19,8 +19,10 @@ package org.jetbrains.kotlin.j2k import com.intellij.openapi.project.Project import com.intellij.psi.* import com.intellij.psi.CommonClassNames.* +import com.intellij.psi.util.InheritanceUtil import com.intellij.psi.util.PsiMethodUtil import com.intellij.psi.util.PsiTreeUtil +import com.intellij.psi.util.PsiUtil import org.jetbrains.kotlin.j2k.ast.* import org.jetbrains.kotlin.j2k.ast.Annotation import org.jetbrains.kotlin.j2k.ast.Enum @@ -29,6 +31,7 @@ import org.jetbrains.kotlin.j2k.usageProcessing.FieldToPropertyProcessing import org.jetbrains.kotlin.j2k.usageProcessing.UsageProcessing import org.jetbrains.kotlin.j2k.usageProcessing.UsageProcessingExpressionConverter import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf import org.jetbrains.kotlin.types.expressions.OperatorConventions.* import org.jetbrains.kotlin.utils.addToStdlib.singletonOrEmptyList import java.util.* @@ -714,28 +717,34 @@ class Converter private constructor( val originalClass = member.containingClass ?: return this // Search for usages only in Java because java-protected member cannot be used in Kotlin from same package val usages = referenceSearcher.findUsagesForExternalCodeProcessing(member, true, false) - for (usage in usages) { - val element = usage.element - var allowProtected = false - var parent: PsiElement? = element - while (parent != null) { - if (parent is PsiClass && allowProtected(parent, originalClass)) { - allowProtected = true - break - } - parent = parent.parent - } - - if (!allowProtected) { - return without(Modifier.PROTECTED).with(Modifier.PUBLIC) - } - } - return this + return if (usages.any { !allowProtected(it.element, member, originalClass) }) + without(Modifier.PROTECTED).with(Modifier.PUBLIC) + else this } - private fun allowProtected(containingClass: PsiClass, originalClass: PsiClass): Boolean { - return originalClass == containingClass || containingClass.isInheritor(originalClass, true) + private fun allowProtected(element: PsiElement, member: PsiMember, originalClass: PsiClass): Boolean { + if (element.parent is PsiNewExpression && member is PsiMethod && member.isConstructor) { + // calls to for protected constructors are allowed only within same class or as super calls + return element.parentsWithSelf.contains(originalClass) + } + + return element.parentsWithSelf.filterIsInstance().any { accessContainingClass -> + if (!InheritanceUtil.isInheritorOrSelf(accessContainingClass, originalClass, true)) return@any false + + if (element !is PsiReferenceExpression) return@any true + + val qualifierExpression = element.qualifierExpression ?: return@any true + + // super.foo is allowed if 'foo' is protected + if (qualifierExpression is PsiSuperExpression) return@any true + + val receiverType = qualifierExpression.type ?: return@any true + val resolvedClass = PsiUtil.resolveGenericsClassInType(receiverType).element ?: return@any true + + // receiver type should be subtype of containing class + InheritanceUtil.isInheritorOrSelf(resolvedClass, accessContainingClass, true) + } } fun convertAnonymousClassBody(anonymousClass: PsiAnonymousClass): AnonymousClassBody { diff --git a/j2k/testData/fileOrElement/detectProperties/DifferentFieldNameAndSuperClass.kt b/j2k/testData/fileOrElement/detectProperties/DifferentFieldNameAndSuperClass.kt index ff0baf94ce3..4f4367c2530 100644 --- a/j2k/testData/fileOrElement/detectProperties/DifferentFieldNameAndSuperClass.kt +++ b/j2k/testData/fileOrElement/detectProperties/DifferentFieldNameAndSuperClass.kt @@ -1,6 +1,5 @@ open class Base internal constructor(x: Int) { var x = 42 - protected set init { this.x = x diff --git a/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java index 4fb861b1a23..36d5562d96e 100644 --- a/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java +++ b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java @@ -5,10 +5,14 @@ public class BaseInheritorSamePackage { } - protected void foo() { + protected BaseInheritorSamePackage(int x) { } + protected void foo() { + new BaseInheritorSamePackage(1); + } + protected int i = 1; } diff --git a/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt index 8ab6d0bca1d..a7109b907ab 100644 --- a/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt +++ b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt @@ -1,12 +1,19 @@ package test -open class BaseInheritorSamePackage protected constructor() { - - protected fun foo() { +open class BaseInheritorSamePackage { + constructor() { } - protected var i = 1 + protected constructor(x: Int) { + + } + + fun foo() { + BaseInheritorSamePackage(1) + } + + var i = 1 } internal class DerivedInheritorSamePackage : BaseInheritorSamePackage() { diff --git a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java index 13e6d3b16f0..374d48fe83b 100644 --- a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java +++ b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java @@ -3,9 +3,9 @@ package test2; import test.*; public class DerivedOtherPackage extends BaseOtherPackage { - public void usage1() { - BaseOtherPackage base = new BaseOtherPackage(); - base.foo(); - int i = base.i; + protected DerivedOtherPackage() { + super(); + foo(); + int i = this.i; } } \ No newline at end of file diff --git a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt index 1c6304e4ab7..80e22e9ff08 100644 --- a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt +++ b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt @@ -2,10 +2,9 @@ package test2 import test.* -class DerivedOtherPackage : BaseOtherPackage() { - fun usage1() { - val base = BaseOtherPackage() - base.foo() - val i = base.i +class DerivedOtherPackage protected constructor() : BaseOtherPackage() { + init { + foo() + val i = this.i } } \ No newline at end of file diff --git a/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt b/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt index 2945ee7c6a8..66860d86c0e 100644 --- a/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt +++ b/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt @@ -3,9 +3,8 @@ package test3 import test.* class DerivedOtherPackageKotlin : BaseOtherPackage() { - fun usage1() { - val base = BaseOtherPackage() - base.foo() - val i = base.i + init { + foo() + val i = this.i } } \ No newline at end of file diff --git a/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt.expected b/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt.expected index 72cf39e94a8..d5f1d723fea 100644 --- a/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt.expected +++ b/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt.expected @@ -3,9 +3,8 @@ package test3 import test.* class DerivedOtherPackageKotlin : BaseOtherPackage() { - fun usage1() { - val base = BaseOtherPackage() - base.foo() - val i = base.i + init { + foo() + val i = this.i } }