diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt index 91604887e54..dde653af000 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt @@ -232,7 +232,10 @@ fun KtDeclaration.implicitVisibility(): KtModifierKeywordToken? = fun KtModifierListOwner.canBePrivate() = modifierList?.hasModifier(KtTokens.ABSTRACT_KEYWORD) != true fun KtModifierListOwner.canBeProtected(): Boolean { - val parent = this.parent + val parent = when (this) { + is KtPropertyAccessor -> this.property.parent + else -> this.parent + } return when (parent) { is KtClassBody -> parent.parent is KtClass is KtParameterList -> parent.parent is KtPrimaryConstructor diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt index cd9fef153b3..04d92dcbef6 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt @@ -32,30 +32,45 @@ import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.startOffset import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier +import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType import java.lang.IllegalArgumentException open class ChangeVisibilityModifierIntention protected constructor( - val modifier: KtModifierKeywordToken + val modifier: KtModifierKeywordToken ) : SelfTargetingRangeIntention(KtDeclaration::class.java, "Make ${modifier.value}") { override fun applicabilityRange(element: KtDeclaration): TextRange? { val modifierList = element.modifierList - if (modifierList?.hasModifier(modifier) ?: false) return null + if (modifierList?.hasModifier(modifier) == true) return null val descriptor = element.toDescriptor() as? DeclarationDescriptorWithVisibility ?: return null val targetVisibility = modifier.toVisibility() if (descriptor.visibility == targetVisibility) return null - if (modifierList?.hasModifier(KtTokens.OVERRIDE_KEYWORD) ?: false) { + if (KtPsiUtil.isLocal((element as? KtPropertyAccessor)?.property ?: element)) return null + + if (modifierList?.hasModifier(KtTokens.OVERRIDE_KEYWORD) == true) { val callableDescriptor = descriptor as? CallableDescriptor ?: return null // cannot make visibility less than (or non-comparable with) any of the supers if (callableDescriptor.overriddenDescriptors .map { Visibilities.compare(it.visibility, targetVisibility) } - .any { it == null || it > 0 }) return null + .any { it == null || it > 0 }) return null } text = defaultText + if (element is KtPropertyAccessor) { + if (element.isGetter) return null + if (targetVisibility == Visibilities.PUBLIC) { + val explicitVisibility = element.modifierList?.visibilityModifierType()?.value ?: return null + text = "Remove '$explicitVisibility' modifier" + } else { + val propVisibility = (element.property.toDescriptor() as? DeclarationDescriptorWithVisibility)?.visibility ?: return null + if (propVisibility == targetVisibility) return null + val compare = Visibilities.compare(targetVisibility, propVisibility) + if (compare == null || compare > 0) return null + } + } val defaultRange = noModifierYetApplicabilityRange(element) ?: return null if (element is KtPrimaryConstructor && defaultRange.isEmpty && element.visibilityModifier() == null) { @@ -70,6 +85,7 @@ open class ChangeVisibilityModifierIntention protected constructor( override fun applyTo(element: KtDeclaration, editor: Editor?) { element.setVisibility(modifier) + if (element is KtPropertyAccessor) element.modifierList?.nextSibling?.replace(KtPsiFactory(element).createWhiteSpace()) } private fun KtModifierKeywordToken.toVisibility(): Visibility { @@ -83,13 +99,16 @@ open class ChangeVisibilityModifierIntention protected constructor( } private fun noModifierYetApplicabilityRange(declaration: KtDeclaration): TextRange? { - if (KtPsiUtil.isLocal(declaration)) return null return when (declaration) { is KtNamedFunction -> declaration.funKeyword?.textRange is KtProperty -> declaration.valOrVarKeyword.textRange + is KtPropertyAccessor -> declaration.namePlaceholder.textRange is KtClass -> declaration.getClassOrInterfaceKeyword()?.textRange is KtObjectDeclaration -> declaration.getObjectKeyword()?.textRange - is KtPrimaryConstructor -> declaration.valueParameterList?.let { TextRange.from(it.startOffset, 0) } //TODO: use constructor keyword if exist + is KtPrimaryConstructor -> declaration.valueParameterList?.let { + //TODO: use constructor keyword if exist + TextRange.from(it.startOffset, 0) + } is KtSecondaryConstructor -> declaration.getConstructorKeyword().textRange is KtParameter -> declaration.valOrVarKeyword?.textRange is KtTypeAlias -> declaration.getTypeAliasKeyword()?.textRange @@ -97,7 +116,8 @@ open class ChangeVisibilityModifierIntention protected constructor( } } - protected fun isAnnotationClassPrimaryConstructor(element: KtDeclaration) = element is KtPrimaryConstructor && (element.parent as? KtClass)?.hasModifier(KtTokens.ANNOTATION_KEYWORD) ?: false + protected fun isAnnotationClassPrimaryConstructor(element: KtDeclaration) = + element is KtPrimaryConstructor && (element.parent as? KtClass)?.hasModifier(KtTokens.ANNOTATION_KEYWORD) ?: false class Public : ChangeVisibilityModifierIntention(KtTokens.PUBLIC_KEYWORD), HighPriorityAction diff --git a/idea/testData/intentions/changeVisibility/internal/propertySetter.kt b/idea/testData/intentions/changeVisibility/internal/propertySetter.kt new file mode 100644 index 00000000000..b9075519794 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/internal/propertySetter.kt @@ -0,0 +1,4 @@ +class Test { + var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/internal/propertySetter.kt.after b/idea/testData/intentions/changeVisibility/internal/propertySetter.kt.after new file mode 100644 index 00000000000..d84b09ecc7f --- /dev/null +++ b/idea/testData/intentions/changeVisibility/internal/propertySetter.kt.after @@ -0,0 +1,4 @@ +class Test { + var foo: String = "" + internal set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/internal/propertySetterForPrivateProperty.kt b/idea/testData/intentions/changeVisibility/internal/propertySetterForPrivateProperty.kt new file mode 100644 index 00000000000..c9c600ddf6d --- /dev/null +++ b/idea/testData/intentions/changeVisibility/internal/propertySetterForPrivateProperty.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +class Test { + private var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertyGetter.kt b/idea/testData/intentions/changeVisibility/private/propertyGetter.kt new file mode 100644 index 00000000000..e53e56b8f51 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertyGetter.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +class Test { + var foo: String = "" + get() = "" +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetter.kt b/idea/testData/intentions/changeVisibility/private/propertySetter.kt new file mode 100644 index 00000000000..b9075519794 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetter.kt @@ -0,0 +1,4 @@ +class Test { + var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetter.kt.after b/idea/testData/intentions/changeVisibility/private/propertySetter.kt.after new file mode 100644 index 00000000000..e94773843ca --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetter.kt.after @@ -0,0 +1,4 @@ +class Test { + var foo: String = "" + private set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt b/idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt new file mode 100644 index 00000000000..1761b04ca70 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt @@ -0,0 +1,4 @@ +class Test { + internal var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt.after b/idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt.after new file mode 100644 index 00000000000..56d2a7a329a --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt.after @@ -0,0 +1,4 @@ +class Test { + internal var foo: String = "" + private set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetterForPrivateProperty.kt b/idea/testData/intentions/changeVisibility/private/propertySetterForPrivateProperty.kt new file mode 100644 index 00000000000..c9c600ddf6d --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetterForPrivateProperty.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +class Test { + private var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt b/idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt new file mode 100644 index 00000000000..6b4ecdd7aa3 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt @@ -0,0 +1,4 @@ +class Test { + protected var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt.after b/idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt.after new file mode 100644 index 00000000000..05b17142af3 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt.after @@ -0,0 +1,4 @@ +class Test { + protected var foo: String = "" + private set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/propertySetterInLocalClass.kt b/idea/testData/intentions/changeVisibility/private/propertySetterInLocalClass.kt new file mode 100644 index 00000000000..c56d37855cd --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/propertySetterInLocalClass.kt @@ -0,0 +1,7 @@ +// IS_APPLICABLE: false +fun test() { + class Test { + var foo: String = "" + set + } +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/protected/propertySetter.kt b/idea/testData/intentions/changeVisibility/protected/propertySetter.kt new file mode 100644 index 00000000000..b9075519794 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/protected/propertySetter.kt @@ -0,0 +1,4 @@ +class Test { + var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/protected/propertySetter.kt.after b/idea/testData/intentions/changeVisibility/protected/propertySetter.kt.after new file mode 100644 index 00000000000..e998f2eff16 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/protected/propertySetter.kt.after @@ -0,0 +1,4 @@ +class Test { + var foo: String = "" + protected set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/protected/propertySetterForPrivateProperty.kt b/idea/testData/intentions/changeVisibility/protected/propertySetterForPrivateProperty.kt new file mode 100644 index 00000000000..c9c600ddf6d --- /dev/null +++ b/idea/testData/intentions/changeVisibility/protected/propertySetterForPrivateProperty.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +class Test { + private var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt new file mode 100644 index 00000000000..4abc3593ece --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt @@ -0,0 +1,5 @@ +// INTENTION_TEXT: Remove 'private' modifier +class Test { + var foo: String = "" + private set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt.after b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt.after new file mode 100644 index 00000000000..51170f70fd8 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt.after @@ -0,0 +1,5 @@ +// INTENTION_TEXT: Remove 'private' modifier +class Test { + var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt new file mode 100644 index 00000000000..8afb13f45af --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt @@ -0,0 +1,5 @@ +// INTENTION_TEXT: Remove 'private' modifier +class Test { + private var foo: String = "" + private set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt.after b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt.after new file mode 100644 index 00000000000..cee93d66854 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt.after @@ -0,0 +1,5 @@ +// INTENTION_TEXT: Remove 'private' modifier +class Test { + private var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/propertySetter.kt b/idea/testData/intentions/changeVisibility/public/propertySetter.kt new file mode 100644 index 00000000000..dd164d4fe37 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/propertySetter.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +class Test { + var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/propertySetterForPrivateProperty.kt b/idea/testData/intentions/changeVisibility/public/propertySetterForPrivateProperty.kt new file mode 100644 index 00000000000..c9c600ddf6d --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/propertySetterForPrivateProperty.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +class Test { + private var foo: String = "" + set +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index e44e27786a5..9a3eb46d9d3 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -2813,6 +2813,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("propertySetter.kt") + public void testPropertySetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/internal/propertySetter.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterForPrivateProperty.kt") + public void testPropertySetterForPrivateProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/internal/propertySetterForPrivateProperty.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/internal/simple.kt"); @@ -2966,6 +2978,42 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("propertyGetter.kt") + public void testPropertyGetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/propertyGetter.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetter.kt") + public void testPropertySetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/propertySetter.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterForInternalProperty.kt") + public void testPropertySetterForInternalProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/propertySetterForInternalProperty.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterForPrivateProperty.kt") + public void testPropertySetterForPrivateProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/propertySetterForPrivateProperty.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterForProtectedProperty.kt") + public void testPropertySetterForProtectedProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/propertySetterForProtectedProperty.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterInLocalClass.kt") + public void testPropertySetterInLocalClass() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/propertySetterInLocalClass.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/private/simple.kt"); @@ -3029,6 +3077,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("propertySetter.kt") + public void testPropertySetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/protected/propertySetter.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterForPrivateProperty.kt") + public void testPropertySetterForPrivateProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/protected/propertySetterForPrivateProperty.kt"); + doTest(fileName); + } + @TestMetadata("simple.kt") public void testSimple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/protected/simple.kt"); @@ -3068,6 +3128,30 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("propertyPrivateSetter.kt") + public void testPropertyPrivateSetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/public/propertyPrivateSetter.kt"); + doTest(fileName); + } + + @TestMetadata("propertyPrivateSetterForPrivateProperty.kt") + public void testPropertyPrivateSetterForPrivateProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/public/propertyPrivateSetterForPrivateProperty.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetter.kt") + public void testPropertySetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/public/propertySetter.kt"); + doTest(fileName); + } + + @TestMetadata("propertySetterForPrivateProperty.kt") + public void testPropertySetterForPrivateProperty() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/public/propertySetterForPrivateProperty.kt"); + doTest(fileName); + } + @TestMetadata("publicByDefault.kt") public void testPublicByDefault() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/changeVisibility/public/publicByDefault.kt");