From 794127a38be15b374efc5b16f5e2f70872d03be1 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Mon, 7 Sep 2015 18:58:25 +0300 Subject: [PATCH] Change Signature: Add/remove constructor keyword on primary constructor when changing modifier list --- .../kotlin/psi/JetPrimaryConstructor.kt | 24 +++++++++++++++---- .../jetbrains/kotlin/psi/addRemoveModifier.kt | 10 +++++++- .../changeSignature/JetChangeInfo.kt | 2 +- .../usages/JetCallableDefinitionUsage.java | 20 ++++------------ .../noModifierListPrimaryConstructor.kt.after | 2 +- .../AddConstructorVisibilityAfter.kt | 2 +- .../ChangeConstructorVisibilityAfter.kt | 2 +- ...ePrimaryConstructorPrivateNoParamsAfter.kt | 1 + ...PrimaryConstructorPrivateNoParamsBefore.kt | 1 + .../MakePrimaryConstructorPublicAfter.kt | 1 + .../MakePrimaryConstructorPublicBefore.kt | 1 + .../JetChangeSignatureTest.java | 12 ++++++++++ 12 files changed, 53 insertions(+), 25 deletions(-) create mode 100644 idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsAfter.kt create mode 100644 idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsBefore.kt create mode 100644 idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicAfter.kt create mode 100644 idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicBefore.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/JetPrimaryConstructor.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/JetPrimaryConstructor.kt index 1194566bc4b..5fbd552b980 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/JetPrimaryConstructor.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/JetPrimaryConstructor.kt @@ -17,9 +17,10 @@ package org.jetbrains.kotlin.psi import com.intellij.lang.ASTNode +import com.intellij.psi.PsiElement import org.jetbrains.kotlin.lexer.JetModifierKeywordToken import org.jetbrains.kotlin.lexer.JetTokens -import org.jetbrains.kotlin.psi.addRemoveModifier.* +import org.jetbrains.kotlin.psi.addRemoveModifier.addModifier import org.jetbrains.kotlin.psi.stubs.KotlinPlaceHolderStub import org.jetbrains.kotlin.psi.stubs.elements.JetStubElementTypes @@ -31,16 +32,29 @@ public class JetPrimaryConstructor : JetConstructor { override fun getContainingClassOrObject() = getParent() as JetClassOrObject + private fun getOrCreateConstructorKeyword(): PsiElement { + return getConstructorKeyword() ?: addBefore(JetPsiFactory(this).createConstructorKeyword(), valueParameterList!!) + } + override fun addModifier(modifier: JetModifierKeywordToken) { - val modifierList = getModifierList() + val modifierList = modifierList if (modifierList != null) { addModifier(modifierList, modifier, JetTokens.PUBLIC_KEYWORD) + if (this.modifierList == null) { + getConstructorKeyword()?.delete() + } } else { if (modifier == JetTokens.PUBLIC_KEYWORD) return - val parameterList = getValueParameterList()!! - val newModifierList = JetPsiFactory(getProject()).createModifierList(modifier) - addBefore(newModifierList, parameterList) + val newModifierList = JetPsiFactory(this).createModifierList(modifier) + addBefore(newModifierList, getOrCreateConstructorKeyword()) + } + } + + override fun removeModifier(modifier: JetModifierKeywordToken) { + super.removeModifier(modifier) + if (modifierList == null) { + getConstructorKeyword()?.delete() } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/addRemoveModifier.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/addRemoveModifier.kt index 78f655f8f65..a00966518ec 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/addRemoveModifier.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/addRemoveModifier.kt @@ -66,6 +66,9 @@ internal fun addModifier(modifierList: JetModifierList, modifier: JetModifierKey if (modifier == defaultVisibilityModifier) { // do not insert explicit 'internal' keyword (or 'public' for primary constructor) //TODO: code style option modifierToReplace?.delete() + if (modifierList.firstChild == null) { + modifierList.delete() + } return } @@ -98,7 +101,12 @@ internal fun addModifier(modifierList: JetModifierList, modifier: JetModifierKey } internal fun removeModifier(owner: JetModifierListOwner, modifier: JetModifierKeywordToken) { - owner.getModifierList()?.getModifier(modifier)?.delete() + owner.getModifierList()?.let { + it.getModifier(modifier)?.delete() + if (it.firstChild == null) { + it.delete() + } + } } private val MODIFIERS_TO_REPLACE = mapOf( diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt index 8f40513bb8a..23892710a0f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeInfo.kt @@ -194,7 +194,7 @@ public open class JetChangeInfo( buffer.append(name) if (newVisibility != defaultVisibility) { - buffer.append(' ').append(newVisibility).append(' ') + buffer.append(' ').append(newVisibility).append(" constructor ") } } else { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.java b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.java index a6a0428cb75..323db7564ef 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.java +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/changeSignature/usages/JetCallableDefinitionUsage.java @@ -293,18 +293,10 @@ public class JetCallableDefinitionUsage extends JetUsageIn } else { if (element instanceof JetClass) { - PsiElement anchor = ((JetClass) element).getTypeParameterList(); - - if (anchor == null) { - anchor = ((JetClass) element).getNameIdentifier(); - } - if (anchor != null) { - JetPrimaryConstructor constructor = - (JetPrimaryConstructor) element.addAfter(psiFactory.createPrimaryConstructor(), anchor); - JetParameterList oldParameterList = constructor.getValueParameterList(); - assert oldParameterList != null : "primary constructor from factory has parameter list"; - newParameterList = (JetParameterList) oldParameterList.replace(newParameterList); - } + JetPrimaryConstructor constructor = ((JetClass) element).createPrimaryConstructorIfAbsent(); + JetParameterList oldParameterList = constructor.getValueParameterList(); + assert oldParameterList != null : "primary constructor from factory has parameter list"; + newParameterList = (JetParameterList) oldParameterList.replace(newParameterList); } else if (isLambda) { //noinspection ConstantConditions @@ -357,9 +349,7 @@ public class JetCallableDefinitionUsage extends JetUsageIn ((JetCallableDeclaration)element).addModifier(newVisibilityToken); } else if (element instanceof JetClass) { - JetPrimaryConstructor constructor = ((JetClass) element).getPrimaryConstructor(); - assert constructor != null : "Primary constructor should be created before changing visibility"; - constructor.addModifier(newVisibilityToken); + ((JetClass) element).createPrimaryConstructorIfAbsent().addModifier(newVisibilityToken); } else throw new AssertionError("Invalid element: " + PsiUtilPackage.getElementTextWithContext(element)); } diff --git a/idea/testData/intentions/changeVisibility/private/noModifierListPrimaryConstructor.kt.after b/idea/testData/intentions/changeVisibility/private/noModifierListPrimaryConstructor.kt.after index bc74be1bd91..12e01401d09 100644 --- a/idea/testData/intentions/changeVisibility/private/noModifierListPrimaryConstructor.kt.after +++ b/idea/testData/intentions/changeVisibility/private/noModifierListPrimaryConstructor.kt.after @@ -1,2 +1,2 @@ // INTENTION_TEXT: Make primary constructor private -class C private (val v: Int) \ No newline at end of file +class C private constructor(val v: Int) \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/AddConstructorVisibilityAfter.kt b/idea/testData/refactoring/changeSignature/AddConstructorVisibilityAfter.kt index 34a111ffaa2..0405ff86111 100644 --- a/idea/testData/refactoring/changeSignature/AddConstructorVisibilityAfter.kt +++ b/idea/testData/refactoring/changeSignature/AddConstructorVisibilityAfter.kt @@ -1,4 +1,4 @@ -class C1 protected (val x: Any) {} +class C1 protected constructor(val x: Any) {} fun f() { val c = C1(12); diff --git a/idea/testData/refactoring/changeSignature/ChangeConstructorVisibilityAfter.kt b/idea/testData/refactoring/changeSignature/ChangeConstructorVisibilityAfter.kt index 542c05bc799..12243ef4bda 100644 --- a/idea/testData/refactoring/changeSignature/ChangeConstructorVisibilityAfter.kt +++ b/idea/testData/refactoring/changeSignature/ChangeConstructorVisibilityAfter.kt @@ -1 +1 @@ -class C1 protected (){} +class C1 protected constructor(){} diff --git a/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsAfter.kt b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsAfter.kt new file mode 100644 index 00000000000..22c981f23f5 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsAfter.kt @@ -0,0 +1 @@ +class X private constructor() \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsBefore.kt b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsBefore.kt new file mode 100644 index 00000000000..b5704a46450 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPrivateNoParamsBefore.kt @@ -0,0 +1 @@ +class X \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicAfter.kt b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicAfter.kt new file mode 100644 index 00000000000..a01f5fe11c9 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicAfter.kt @@ -0,0 +1 @@ +class X (n: Int) \ No newline at end of file diff --git a/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicBefore.kt b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicBefore.kt new file mode 100644 index 00000000000..1a9f6573730 --- /dev/null +++ b/idea/testData/refactoring/changeSignature/MakePrimaryConstructorPublicBefore.kt @@ -0,0 +1 @@ +class X private constructor(n: Int) \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.java index 10a86e7726b..4f4b58797b9 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/changeSignature/JetChangeSignatureTest.java @@ -1306,6 +1306,18 @@ public class JetChangeSignatureTest extends KotlinCodeInsightTestCase { doTest(changeInfo); } + public void testMakePrimaryConstructorPrivateNoParams() throws Exception { + JetChangeInfo changeInfo = getChangeInfo(); + changeInfo.setNewVisibility(Visibilities.PRIVATE); + doTest(changeInfo); + } + + public void testMakePrimaryConstructorPublic() throws Exception { + JetChangeInfo changeInfo = getChangeInfo(); + changeInfo.setNewVisibility(Visibilities.PUBLIC); + doTest(changeInfo); + } + private List editors = null; private static final String[] EXTENSIONS = {".kt", ".java"};