Extract Superclass: Fix visibility lifting when moving to interface

#KT-16281 Fixed

compiler/psi/src/org/jetbrains/kotlin/psi/psiUtil/ktPsiUtil.kt

@@ -430,6 +430,8 @@ val KtModifierListOwner.isPublic: Boolean
 fun KtModifierListOwner.visibilityModifierType(): KtModifierKeywordToken? =
     visibilityModifier()?.node?.elementType as KtModifierKeywordToken?
 
+fun KtModifierListOwner.visibilityModifierTypeOrDefault(): KtModifierKeywordToken = visibilityModifierType() ?: KtTokens.DEFAULT_VISIBILITY_KEYWORD
+
 fun KtDeclaration.modalityModifier() = modifierFromTokenSet(MODALITY_MODIFIERS)
 
 fun KtStringTemplateExpression.isPlain() = entries.all { it is KtLiteralStringTemplateEntry }

idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractClass/ui/KotlinExtractInterfaceDialog.kt

@@ -65,10 +65,8 @@ class KotlinExtractInterfaceDialog(
 
                 val member = memberInfo.member
                 return !(member.hasModifier(KtTokens.INLINE_KEYWORD) ||
-                         member.hasModifier(KtTokens.EXTERNAL_KEYWORD) ||
+                        member.hasModifier(KtTokens.EXTERNAL_KEYWORD) ||
-                         member.hasModifier(KtTokens.LATEINIT_KEYWORD) ||
+                        member.hasModifier(KtTokens.LATEINIT_KEYWORD))
-                         member.hasModifier(KtTokens.INTERNAL_KEYWORD) ||
-                         member.hasModifier(KtTokens.PROTECTED_KEYWORD))
             }
 
             override fun isAbstractEnabled(memberInfo: KotlinMemberInfo): Boolean {

idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/KotlinPullUpHelper.kt

@@ -64,6 +64,11 @@ class KotlinPullUpHelper(
     private val javaData: PullUpData,
     private val data: KotlinPullUpData
 ) : PullUpHelper<MemberInfoBase<PsiMember>> {
+    companion object {
+        private val MODIFIERS_TO_LIFT_IN_SUPERCLASS = listOf(KtTokens.PRIVATE_KEYWORD)
+        private val MODIFIERS_TO_LIFT_IN_INTERFACE = listOf(KtTokens.PRIVATE_KEYWORD, KtTokens.PROTECTED_KEYWORD, KtTokens.INTERNAL_KEYWORD)
+    }
+
     private fun KtExpression.isMovable(): Boolean {
         return accept(
             object : KtVisitor<Boolean, Nothing?>() {
@@ -271,13 +276,18 @@ class KotlinPullUpHelper(
         }
     }
 
-    private fun liftToProtected(declaration: KtNamedDeclaration, ignoreUsages: Boolean = false) {
+    private fun liftVisibility(declaration: KtNamedDeclaration, ignoreUsages: Boolean = false) {
-        if (!declaration.hasModifier(KtTokens.PRIVATE_KEYWORD)) return
+        val newModifier = if (data.isInterfaceTarget) KtTokens.PUBLIC_KEYWORD else KtTokens.PROTECTED_KEYWORD
-        if (ignoreUsages || willBeUsedInSourceClass(
+        val modifiersToLift = if (data.isInterfaceTarget) MODIFIERS_TO_LIFT_IN_INTERFACE else MODIFIERS_TO_LIFT_IN_SUPERCLASS
-                declaration,
+        val currentModifier = declaration.visibilityModifierTypeOrDefault()
-                data.sourceClass,
+        if (currentModifier !in modifiersToLift) return
-                data.membersToMove
+        if (ignoreUsages || willBeUsedInSourceClass(declaration, data.sourceClass, data.membersToMove)) {
-            )) declaration.addModifier(KtTokens.PROTECTED_KEYWORD)
+            if (newModifier != KtTokens.DEFAULT_VISIBILITY_KEYWORD) {
+                declaration.addModifier(newModifier)
+            } else {
+                declaration.removeModifier(currentModifier)
+            }
+        }
     }
 
     override fun setCorrectVisibility(info: MemberInfoBase<PsiMember>) {
@@ -285,20 +295,20 @@ class KotlinPullUpHelper(
 
         if (data.isInterfaceTarget) {
             member.removeModifier(KtTokens.PUBLIC_KEYWORD)
-            return
         }
 
-        if (member.hasModifier(KtTokens.PRIVATE_KEYWORD)) {
+        val modifiersToLift = if (data.isInterfaceTarget) MODIFIERS_TO_LIFT_IN_INTERFACE else MODIFIERS_TO_LIFT_IN_SUPERCLASS
+        if (member.visibilityModifierTypeOrDefault() in modifiersToLift) {
             member.accept(
                 object : KtVisitorVoid() {
                     override fun visitNamedDeclaration(declaration: KtNamedDeclaration) {
                         when (declaration) {
                             is KtClass -> {
-                                liftToProtected(declaration)
+                                liftVisibility(declaration)
                                 declaration.declarations.forEach { it.accept(this) }
                             }
                             is KtNamedFunction, is KtProperty -> {
-                                liftToProtected(declaration, declaration == member && info.isToAbstract)
+                                liftVisibility(declaration, declaration == member && info.isToAbstract)
                             }
                         }
                     }

idea/testData/refactoring/extractInterface/liftInternal.kt

@@ -0,0 +1,8 @@
+// NAME: IPurePrivate
+// SIBLING:
+class PurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    internal var internalVar = 0
+    // INFO: {checked: "true", toAbstract: "true"}
+    internal fun internalFun() {}
+}

idea/testData/refactoring/extractInterface/liftInternal.kt.after

@@ -0,0 +1,16 @@
+interface IPurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    var internalVar: Int
+
+    // INFO: {checked: "true", toAbstract: "true"}
+    fun internalFun()
+}
+
+// NAME: IPurePrivate
+// SIBLING:
+class PurePrivate : IPurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    override var internalVar = 0
+    // INFO: {checked: "true", toAbstract: "true"}
+    override fun internalFun() {}
+}

idea/testData/refactoring/extractInterface/liftPrivate.kt

@@ -0,0 +1,8 @@
+// NAME: IPurePrivate
+// SIBLING:
+class PurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    private var privateVar = 0
+    // INFO: {checked: "true", toAbstract: "true"}
+    private fun privateFun() {}
+}

idea/testData/refactoring/extractInterface/liftPrivate.kt.after

@@ -0,0 +1,16 @@
+interface IPurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    var privateVar: Int
+
+    // INFO: {checked: "true", toAbstract: "true"}
+    fun privateFun()
+}
+
+// NAME: IPurePrivate
+// SIBLING:
+class PurePrivate : IPurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    override var privateVar = 0
+    // INFO: {checked: "true", toAbstract: "true"}
+    override fun privateFun() {}
+}

idea/testData/refactoring/extractInterface/liftProtected.kt

@@ -0,0 +1,8 @@
+// NAME: IPurePrivate
+// SIBLING:
+class PurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    protected var protectedVar = 0
+    // INFO: {checked: "true", toAbstract: "true"}
+    protected fun protectedFun() {}
+}

idea/testData/refactoring/extractInterface/liftProtected.kt.after

@@ -0,0 +1,16 @@
+interface IPurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    var protectedVar: Int
+
+    // INFO: {checked: "true", toAbstract: "true"}
+    fun protectedFun()
+}
+
+// NAME: IPurePrivate
+// SIBLING:
+class PurePrivate : IPurePrivate {
+    // INFO: {checked: "true", toAbstract: "true"}
+    override var protectedVar = 0
+    // INFO: {checked: "true", toAbstract: "true"}
+    override fun protectedFun() {}
+}

idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java

@@ -4068,6 +4068,21 @@ public class ExtractionTestGenerated extends AbstractExtractionTest {
             runTest("idea/testData/refactoring/extractInterface/extractToExistingFile.kt");
         }
 
+        @TestMetadata("liftInternal.kt")
+        public void testLiftInternal() throws Exception {
+            runTest("idea/testData/refactoring/extractInterface/liftInternal.kt");
+        }
+
+        @TestMetadata("liftPrivate.kt")
+        public void testLiftPrivate() throws Exception {
+            runTest("idea/testData/refactoring/extractInterface/liftPrivate.kt");
+        }
+
+        @TestMetadata("liftProtected.kt")
+        public void testLiftProtected() throws Exception {
+            runTest("idea/testData/refactoring/extractInterface/liftProtected.kt");
+        }
+
         @TestMetadata("noWarningOnVisibilityInsideAbstractedMember.kt")
         public void testNoWarningOnVisibilityInsideAbstractedMember() throws Exception {
             runTest("idea/testData/refactoring/extractInterface/noWarningOnVisibilityInsideAbstractedMember.kt");