Pull Members Up: Always insert spaces between keywords

#KT-10971 Fixed

compiler/frontend/src/org/jetbrains/kotlin/parsing/KotlinParserDefinition.kt

@@ -36,6 +36,7 @@ import org.jetbrains.kotlin.kdoc.lexer.KDocTokens
 import org.jetbrains.kotlin.kdoc.parser.KDocElementType
 import org.jetbrains.kotlin.kdoc.psi.impl.KDocLink
 import org.jetbrains.kotlin.lexer.KotlinLexer
+import org.jetbrains.kotlin.lexer.KtKeywordToken
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtWhenEntry
@@ -73,11 +74,16 @@ class KotlinParserDefinition : ParserDefinition {
 
     override fun spaceExistanceTypeBetweenTokens(left: ASTNode, right: ASTNode): ParserDefinition.SpaceRequirements {
         val rightTokenType = right.elementType
+
         // get/set from a new line
         if (rightTokenType == KtTokens.GET_KEYWORD || rightTokenType == KtTokens.SET_KEYWORD) {
             return MUST_LINE_BREAK
         }
+
         val leftTokenType = left.elementType
+
+        if (leftTokenType is KtKeywordToken && rightTokenType is KtKeywordToken) return MUST
+
         // When entry from a new line
         val rightWhenEntry = right.psi.getNonStrictParentOfType<KtWhenEntry>()
         if (rightWhenEntry != null) {
@@ -86,6 +92,7 @@ class KotlinParserDefinition : ParserDefinition {
                 return MUST_LINE_BREAK
             }
         }
+
         // Default
         return MAY
     }

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

@@ -272,7 +272,7 @@ class KotlinPullUpHelper(
 
     private fun liftToProtected(declaration: KtNamedDeclaration, ignoreUsages: Boolean = false) {
         if (!declaration.hasModifier(KtTokens.PRIVATE_KEYWORD)) return
-        if (ignoreUsages || willBeUsedInSourceClass(declaration)) declaration.addModifierWithSpace(KtTokens.PROTECTED_KEYWORD)
+        if (ignoreUsages || willBeUsedInSourceClass(declaration)) declaration.addModifier(KtTokens.PROTECTED_KEYWORD)
     }
 
     override fun setCorrectVisibility(info: MemberInfoBase<PsiMember>) {
@@ -358,7 +358,7 @@ class KotlinPullUpHelper(
             member.delete()
         }
         else {
-            member.addModifierWithSpace(KtTokens.OVERRIDE_KEYWORD)
+            member.addModifier(KtTokens.OVERRIDE_KEYWORD)
             (member as? KtNamedFunction)?.valueParameters?.forEach { it.dropDefaultValue() }
         }
     }

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

@@ -69,12 +69,6 @@ fun doAddCallableMember(
     return addMemberToTarget(memberCopy, targetClass) as KtCallableDeclaration
 }
 
-// TODO: Formatting rules don't apply here for some reason
-fun KtNamedDeclaration.addModifierWithSpace(modifier: KtModifierKeywordToken) {
-    addModifier(modifier)
-    addAfter(KtPsiFactory(this).createWhiteSpace(), modifierList)
-}
-
 // TODO: Formatting rules don't apply here for some reason
 fun KtNamedDeclaration.addAnnotationWithSpace(annotationEntry: KtAnnotationEntry): KtAnnotationEntry {
     val result = addAnnotationEntry(annotationEntry)
@@ -84,7 +78,7 @@ fun KtNamedDeclaration.addAnnotationWithSpace(annotationEntry: KtAnnotationEntry
 
 fun KtClass.makeAbstract() {
     if (!isInterface()) {
-        addModifierWithSpace(KtTokens.ABSTRACT_KEYWORD)
+        addModifier(KtTokens.ABSTRACT_KEYWORD)
     }
 }
 
@@ -105,7 +99,7 @@ fun makeAbstract(member: KtCallableDeclaration,
                  substitutor: TypeSubstitutor,
                  targetClass: KtClass) {
     if (!targetClass.isInterface()) {
-        member.addModifierWithSpace(KtTokens.ABSTRACT_KEYWORD)
+        member.addModifier(KtTokens.ABSTRACT_KEYWORD)
     }
 
     val builtIns = originalMemberDescriptor.builtIns

idea/src/org/jetbrains/kotlin/idea/refactoring/pushDown/KotlinPushDownProcessor.kt

@@ -139,25 +139,25 @@ class KotlinPushDownProcessor(
                     val targetMember = targetMemberDescriptor?.source?.getPsi() as? KtCallableDeclaration
                     targetMember?.apply {
                         if (memberDescriptor.modality != Modality.ABSTRACT && memberInfo.isToAbstract) {
-                            addModifierWithSpace(KtTokens.OVERRIDE_KEYWORD)
+                            addModifier(KtTokens.OVERRIDE_KEYWORD)
                         }
                         else if (memberDescriptor.overriddenDescriptors.isEmpty()) {
                             removeModifier(KtTokens.OVERRIDE_KEYWORD)
                         }
                         else {
-                            addModifierWithSpace(KtTokens.OVERRIDE_KEYWORD)
+                            addModifier(KtTokens.OVERRIDE_KEYWORD)
                         }
                     } ?: addMemberToTarget(member, targetClass).apply {
                         if (this@KotlinPushDownProcessor.context.sourceClassDescriptor.kind == ClassKind.INTERFACE) {
                             if (targetClassDescriptor.kind != ClassKind.INTERFACE && memberDescriptor.modality == Modality.ABSTRACT) {
-                                addModifierWithSpace(KtTokens.ABSTRACT_KEYWORD)
+                                addModifier(KtTokens.ABSTRACT_KEYWORD)
                             }
                         }
                         if (memberDescriptor.modality != Modality.ABSTRACT && memberInfo.isToAbstract) {
                             if (hasModifier(KtTokens.PRIVATE_KEYWORD)) {
-                                addModifierWithSpace(KtTokens.PROTECTED_KEYWORD)
+                                addModifier(KtTokens.PROTECTED_KEYWORD)
                             }
-                            addModifierWithSpace(KtTokens.OVERRIDE_KEYWORD)
+                            addModifier(KtTokens.OVERRIDE_KEYWORD)
                         }
                     }
                 }
@@ -194,7 +194,7 @@ class KotlinPushDownProcessor(
 
                     if (memberDescriptor.modality != Modality.ABSTRACT && memberInfo.isToAbstract) {
                         if (member.hasModifier(KtTokens.PRIVATE_KEYWORD)) {
-                            member.addModifierWithSpace(KtTokens.PROTECTED_KEYWORD)
+                            member.addModifier(KtTokens.PROTECTED_KEYWORD)
                         }
                         makeAbstract(member, memberDescriptor, TypeSubstitutor.EMPTY, context.sourceClass)
                         member.typeReference?.addToShorteningWaitSet()

idea/testData/refactoring/pullUp/k2k/spaceAfterModifier.kt

@@ -0,0 +1,6 @@
+open class Upper
+
+open class <caret>Lower : Upper() {
+    // INFO: {"checked": "true", "toAbstract": "true"}
+    protected val moving: Int = 0
+}

idea/testData/refactoring/pullUp/k2k/spaceAfterModifier.kt.after

@@ -0,0 +1,9 @@
+abstract class Upper {
+    // INFO: {"checked": "true", "toAbstract": "true"}
+    protected abstract val moving: Int
+}
+
+open class Lower : Upper() {
+    // INFO: {"checked": "true", "toAbstract": "true"}
+    protected override val moving: Int = 0
+}

idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java

@@ -223,6 +223,12 @@ public class PullUpTestGenerated extends AbstractPullUpTest {
             doKotlinTest(fileName);
         }
 
+        @TestMetadata("spaceAfterModifier.kt")
+        public void testSpaceAfterModifier() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/spaceAfterModifier.kt");
+            doKotlinTest(fileName);
+        }
+
         @TestMetadata("superToThis.kt")
         public void testSuperToThis() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/superToThis.kt");