diff --git a/Changelog.md b/Changelog.md index c318edf1a49..7e418a36093 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,3 +6,6 @@ ### JS - Safe calls (`x?.let { it }`) are now inlined + +### Tools. J2K +- Protected members used outside of inheritors are converted as public diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt index db2dbdce26d..59e1e8efe2b 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/Converter.kt @@ -665,10 +665,10 @@ class Converter private constructor( modifiers = modifiers.with(Modifier.OPEN) } - modifiers = modifiers.adaptForContainingClassVisibility(owner.containingClass) + modifiers = modifiers.adaptForContainingClassVisibility(owner.containingClass).adaptProtectedVisibility(owner) } else if (owner is PsiField) { - modifiers = modifiers.adaptForContainingClassVisibility(owner.containingClass) + modifiers = modifiers.adaptForContainingClassVisibility(owner.containingClass).adaptProtectedVisibility(owner) } else if (owner is PsiClass && owner.scope is PsiMethod) { // Local class should not have visibility modifiers @@ -685,6 +685,36 @@ class Converter private constructor( return without(Modifier.INTERNAL).with(Modifier.PUBLIC) } + private fun Modifiers.adaptProtectedVisibility(member: PsiMember): Modifiers { + if (!member.hasModifierProperty(PsiModifier.PROTECTED)) return this + + 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 + } + + private fun allowProtected(containingClass: PsiClass, originalClass: PsiClass): Boolean { + return originalClass == containingClass || containingClass.isInheritor(originalClass, true) + } + fun convertAnonymousClassBody(anonymousClass: PsiAnonymousClass): AnonymousClassBody { return AnonymousClassBody(ClassBodyConverter(anonymousClass, ClassKind.ANONYMOUS_OBJECT, this).convertBody(), anonymousClass.baseClassType.resolve()?.isInterface ?: false).assignPrototype(anonymousClass) diff --git a/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt b/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt index 9ff54dddc2e..c9b6c0ac2a5 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/ReferenceSearcher.kt @@ -72,5 +72,5 @@ object EmptyReferenceSearcher: ReferenceSearcher { override fun hasInheritors(`class`: PsiClass) = false override fun hasOverrides(method: PsiMethod) = false override fun findUsagesForExternalCodeProcessing(element: PsiElement, searchJava: Boolean, searchKotlin: Boolean): Collection - = throw UnsupportedOperationException() + = emptyList() } \ No newline at end of file diff --git a/j2k/src/org/jetbrains/kotlin/j2k/Utils.kt b/j2k/src/org/jetbrains/kotlin/j2k/Utils.kt index 31c858351a5..27661d5b716 100644 --- a/j2k/src/org/jetbrains/kotlin/j2k/Utils.kt +++ b/j2k/src/org/jetbrains/kotlin/j2k/Utils.kt @@ -82,6 +82,17 @@ fun PsiElement.getContainingMethod(): PsiMethod? { return null } +fun PsiElement.getContainingClass(): PsiClass? { + var context = context + while (context != null) { + val _context = context + if (_context is PsiClass) return _context + if (_context is PsiMember) return _context.containingClass + context = _context.context + } + return null +} + fun PsiElement.getContainingConstructor(): PsiMethod? { val method = getContainingMethod() return if (method?.isConstructor == true) method else null diff --git a/j2k/testData/fileOrElement/protected/constructorProperty.java b/j2k/testData/fileOrElement/protected/constructorProperty.java new file mode 100644 index 00000000000..cefca5d9f41 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/constructorProperty.java @@ -0,0 +1,26 @@ +package test; + +public abstract class Base { + protected int field; + + public Base(int value) { + field = value; + } +} + +public class Derived extends Base { + public Derived(int value) { + super(value); + } + + private View usage = new View() { + @Override + void click() { + int activity = field; + } + } +} + +abstract class View { + abstract void click(); +} diff --git a/j2k/testData/fileOrElement/protected/constructorProperty.kt b/j2k/testData/fileOrElement/protected/constructorProperty.kt new file mode 100644 index 00000000000..6b87028dd5a --- /dev/null +++ b/j2k/testData/fileOrElement/protected/constructorProperty.kt @@ -0,0 +1,16 @@ +package test + +abstract class Base(protected var field: Int) + +class Derived(value: Int) : Base(value) { + + private val usage = object : View() { + override fun click() { + val activity = field + } + } +} + +internal abstract class View { + internal abstract fun click() +} diff --git a/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java new file mode 100644 index 00000000000..4fb861b1a23 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.java @@ -0,0 +1,21 @@ +package test; + +public class BaseInheritorSamePackage { + protected BaseInheritorSamePackage() { + + } + + protected void foo() { + + } + + protected int i = 1; +} + +class DerivedInheritorSamePackage extends BaseInheritorSamePackage { + public void usage1() { + BaseInheritorSamePackage base = new BaseInheritorSamePackage(); + base.foo(); + int i = base.i; + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt new file mode 100644 index 00000000000..8ab6d0bca1d --- /dev/null +++ b/j2k/testData/fileOrElement/protected/inheritorsSamePackage.kt @@ -0,0 +1,18 @@ +package test + +open class BaseInheritorSamePackage protected constructor() { + + protected fun foo() { + + } + + protected var i = 1 +} + +internal class DerivedInheritorSamePackage : BaseInheritorSamePackage() { + fun usage1() { + val base = BaseInheritorSamePackage() + base.foo() + val i = base.i + } +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/protected/onlyProtectedConstructor.java b/j2k/testData/fileOrElement/protected/onlyProtectedConstructor.java new file mode 100644 index 00000000000..6455c409951 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/onlyProtectedConstructor.java @@ -0,0 +1,13 @@ +package test; + +public class BaseProtectedConstructor { + protected BaseSamePackage() { + + } +} + +class DerivedSamePackage extends BaseProtectedConstructor { + DerivedSamePackage() { + super(); + } +} diff --git a/j2k/testData/fileOrElement/protected/onlyProtectedConstructor.kt b/j2k/testData/fileOrElement/protected/onlyProtectedConstructor.kt new file mode 100644 index 00000000000..14d6ed954df --- /dev/null +++ b/j2k/testData/fileOrElement/protected/onlyProtectedConstructor.kt @@ -0,0 +1,5 @@ +package test + +open class BaseProtectedConstructor protected constructor() + +internal class DerivedSamePackage : BaseProtectedConstructor() diff --git a/j2k/testData/fileOrElement/protected/protectedInSamePackage.java b/j2k/testData/fileOrElement/protected/protectedInSamePackage.java new file mode 100644 index 00000000000..41a36ade5c2 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/protectedInSamePackage.java @@ -0,0 +1,21 @@ +package test; + +public class BaseSamePackage { + protected BaseSamePackage() { + + } + + protected void foo() { + + } + + protected int i = 1; +} + +class DerivedSamePackage { + public void usage1() { + BaseSamePackage base = new BaseSamePackage(); + base.foo(); + int i = base.i; + } +} diff --git a/j2k/testData/fileOrElement/protected/protectedInSamePackage.kt b/j2k/testData/fileOrElement/protected/protectedInSamePackage.kt new file mode 100644 index 00000000000..3d3cfe0b922 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/protectedInSamePackage.kt @@ -0,0 +1,18 @@ +package test + +class BaseSamePackage { + + fun foo() { + + } + + var i = 1 +} + +internal class DerivedSamePackage { + fun usage1() { + val base = BaseSamePackage() + base.foo() + val i = base.i + } +} diff --git a/j2k/testData/fileOrElement/protected/severalInheritors.java b/j2k/testData/fileOrElement/protected/severalInheritors.java new file mode 100644 index 00000000000..ce3bf78bd70 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/severalInheritors.java @@ -0,0 +1,16 @@ +package test; + +public class BaseProtectedConstructor { + protected void foo() { + + } +} + +class MiddleSamePackage extends BaseProtectedConstructor { +} + +class DerivedSamePackage extends MiddleSamePackage { + void usage() { + foo(); + } +} diff --git a/j2k/testData/fileOrElement/protected/severalInheritors.kt b/j2k/testData/fileOrElement/protected/severalInheritors.kt new file mode 100644 index 00000000000..8f73e1db1f0 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/severalInheritors.kt @@ -0,0 +1,15 @@ +package test + +open class BaseProtectedConstructor { + protected fun foo() { + + } +} + +internal open class MiddleSamePackage : BaseProtectedConstructor() + +internal class DerivedSamePackage : MiddleSamePackage() { + fun usage() { + foo() + } +} diff --git a/j2k/testData/fileOrElement/protected/superInSamePackage.java b/j2k/testData/fileOrElement/protected/superInSamePackage.java new file mode 100644 index 00000000000..249353641be --- /dev/null +++ b/j2k/testData/fileOrElement/protected/superInSamePackage.java @@ -0,0 +1,21 @@ +package test; + +class BaseSuperSamePackage { + public void usage1() { + DerivedSuperSamePackage derived = new DerivedSuperSamePackage(); + derived.foo(); + int i = derived.i; + } +} + +class DerivedSuperSamePackage extends BaseSuperSamePackage { + protected DerivedSuperSamePackage() { + + } + + protected void foo() { + + } + + protected int i = 1; +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/protected/superInSamePackage.kt b/j2k/testData/fileOrElement/protected/superInSamePackage.kt new file mode 100644 index 00000000000..1180f965bcf --- /dev/null +++ b/j2k/testData/fileOrElement/protected/superInSamePackage.kt @@ -0,0 +1,18 @@ +package test + +internal open class BaseSuperSamePackage { + fun usage1() { + val derived = DerivedSuperSamePackage() + derived.foo() + val i = derived.i + } +} + +internal class DerivedSuperSamePackage : BaseSuperSamePackage() { + + fun foo() { + + } + + var i = 1 +} \ No newline at end of file diff --git a/j2k/testData/fileOrElement/protected/usages.java b/j2k/testData/fileOrElement/protected/usages.java new file mode 100644 index 00000000000..2e97b744336 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/usages.java @@ -0,0 +1,35 @@ +package test; + +public class BaseProtectedConstructor { + protected void usageInConstructor() { + + } + + protected int usageInPropertyInitializer() { + return 1; + } + + protected void usageInStaticInit() { + + } + + protected void usageInMethod() { + + } +} + +class DerivedSamePackage { + DerivedSamePackage() { + new BaseProtectedConstructor().usageInConstructor(); + } + + private int i = new BaseProtectedConstructor().usageInPropertyInitializer(); + + static { + new BaseProtectedConstructor().usageInStaticInit(); + } + + void usage() { + new BaseProtectedConstructor().usageInMethod(); + } +} diff --git a/j2k/testData/fileOrElement/protected/usages.kt b/j2k/testData/fileOrElement/protected/usages.kt new file mode 100644 index 00000000000..5718c13b067 --- /dev/null +++ b/j2k/testData/fileOrElement/protected/usages.kt @@ -0,0 +1,38 @@ +package test + +class BaseProtectedConstructor { + fun usageInConstructor() { + + } + + fun usageInPropertyInitializer(): Int { + return 1 + } + + fun usageInStaticInit() { + + } + + fun usageInMethod() { + + } +} + +internal class DerivedSamePackage { + init { + BaseProtectedConstructor().usageInConstructor() + } + + private val i = BaseProtectedConstructor().usageInPropertyInitializer() + + fun usage() { + BaseProtectedConstructor().usageInMethod() + } + + companion object { + + init { + BaseProtectedConstructor().usageInStaticInit() + } + } +} diff --git a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage1.java b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage1.java new file mode 100644 index 00000000000..5013847e0fb --- /dev/null +++ b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage1.java @@ -0,0 +1,13 @@ +package test; + +public class BaseOtherPackage { + protected BaseOtherPackage() { + + } + + protected void foo() { + + } + + protected int i = 1; +} diff --git a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage1.kt b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage1.kt new file mode 100644 index 00000000000..428ec445c04 --- /dev/null +++ b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage1.kt @@ -0,0 +1,10 @@ +package test + +open class BaseOtherPackage protected constructor() { + + protected fun foo() { + + } + + protected var i = 1 +} diff --git a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java new file mode 100644 index 00000000000..13e6d3b16f0 --- /dev/null +++ b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.java @@ -0,0 +1,11 @@ +package test2; + +import test.*; + +public class DerivedOtherPackage extends BaseOtherPackage { + public void usage1() { + BaseOtherPackage base = new BaseOtherPackage(); + base.foo(); + int i = base.i; + } +} \ No newline at end of file diff --git a/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt new file mode 100644 index 00000000000..1c6304e4ab7 --- /dev/null +++ b/j2k/testData/multiFile/ProtectedVisibility/InheritorOtherPackage2.kt @@ -0,0 +1,11 @@ +package test2 + +import test.* + +class DerivedOtherPackage : BaseOtherPackage() { + fun usage1() { + val base = BaseOtherPackage() + base.foo() + val i = base.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 new file mode 100644 index 00000000000..2945ee7c6a8 --- /dev/null +++ b/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt @@ -0,0 +1,11 @@ +package test3 + +import test.* + +class DerivedOtherPackageKotlin : BaseOtherPackage() { + fun usage1() { + val base = BaseOtherPackage() + base.foo() + val i = base.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 new file mode 100644 index 00000000000..72cf39e94a8 --- /dev/null +++ b/j2k/testData/multiFile/ProtectedVisibility/external/KotlinFile.kt.expected @@ -0,0 +1,11 @@ +package test3 + +import test.* + +class DerivedOtherPackageKotlin : BaseOtherPackage() { + fun usage1() { + val base = BaseOtherPackage() + base.foo() + val i = base.i + } +} diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java index 9e88c4fe783..37ad44dfafb 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java @@ -3821,6 +3821,57 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo } } + @TestMetadata("j2k/testData/fileOrElement/protected") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Protected extends AbstractJavaToKotlinConverterForWebDemoTest { + public void testAllFilesPresentInProtected() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("j2k/testData/fileOrElement/protected"), Pattern.compile("^(.+)\\.java$"), true); + } + + @TestMetadata("constructorProperty.java") + public void testConstructorProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/constructorProperty.java"); + doTest(fileName); + } + + @TestMetadata("inheritorsSamePackage.java") + public void testInheritorsSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/inheritorsSamePackage.java"); + doTest(fileName); + } + + @TestMetadata("onlyProtectedConstructor.java") + public void testOnlyProtectedConstructor() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/onlyProtectedConstructor.java"); + doTest(fileName); + } + + @TestMetadata("protectedInSamePackage.java") + public void testProtectedInSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/protectedInSamePackage.java"); + doTest(fileName); + } + + @TestMetadata("severalInheritors.java") + public void testSeveralInheritors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/severalInheritors.java"); + doTest(fileName); + } + + @TestMetadata("superInSamePackage.java") + public void testSuperInSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/superInSamePackage.java"); + doTest(fileName); + } + + @TestMetadata("usages.java") + public void testUsages() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/usages.java"); + doTest(fileName); + } + } + @TestMetadata("j2k/testData/fileOrElement/rawGenerics") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterMultiFileTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterMultiFileTestGenerated.java index 7e5edd92018..1f8b77168bf 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterMultiFileTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterMultiFileTestGenerated.java @@ -47,6 +47,12 @@ public class JavaToKotlinConverterMultiFileTestGenerated extends AbstractJavaToK doTest(fileName); } + @TestMetadata("ProtectedVisibility") + public void testProtectedVisibility() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/multiFile/ProtectedVisibility/"); + doTest(fileName); + } + @TestMetadata("ToCompanionObject") public void testToCompanionObject() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/multiFile/ToCompanionObject/"); diff --git a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java index 1810c50e12f..97aa8f1aa78 100644 --- a/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java +++ b/j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java @@ -3821,6 +3821,57 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo } } + @TestMetadata("j2k/testData/fileOrElement/protected") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Protected extends AbstractJavaToKotlinConverterSingleFileTest { + public void testAllFilesPresentInProtected() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("j2k/testData/fileOrElement/protected"), Pattern.compile("^(.+)\\.java$"), true); + } + + @TestMetadata("constructorProperty.java") + public void testConstructorProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/constructorProperty.java"); + doTest(fileName); + } + + @TestMetadata("inheritorsSamePackage.java") + public void testInheritorsSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/inheritorsSamePackage.java"); + doTest(fileName); + } + + @TestMetadata("onlyProtectedConstructor.java") + public void testOnlyProtectedConstructor() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/onlyProtectedConstructor.java"); + doTest(fileName); + } + + @TestMetadata("protectedInSamePackage.java") + public void testProtectedInSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/protectedInSamePackage.java"); + doTest(fileName); + } + + @TestMetadata("severalInheritors.java") + public void testSeveralInheritors() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/severalInheritors.java"); + doTest(fileName); + } + + @TestMetadata("superInSamePackage.java") + public void testSuperInSamePackage() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/superInSamePackage.java"); + doTest(fileName); + } + + @TestMetadata("usages.java") + public void testUsages() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("j2k/testData/fileOrElement/protected/usages.java"); + doTest(fileName); + } + } + @TestMetadata("j2k/testData/fileOrElement/rawGenerics") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)