From e56a3b3c303bcf8a7f4238b9f4c20f2789ba899a Mon Sep 17 00:00:00 2001 From: Andrei Klunnyi Date: Wed, 5 May 2021 14:49:16 +0300 Subject: [PATCH] KT-46146 'protected' and 'private' only for sealed constructors Prior to this commit we suggested two invalid intentions for sealed class constructors: change visibility to 'public' and 'internal'. ^KT-46146 Fixed --- .../kotlin/idea/core/psiModificationUtils.kt | 11 ++++++++-- .../ChangeVisibilityModifierIntention.kt | 5 ++++- .../idea/quickfix/ChangeVisibilityFix.kt | 13 +++++++----- .../notForSealedPrimaryConstructor.kt | 2 ++ .../notForSealedSecondaryConstructor.kt | 4 ++++ .../public/notForSealedPrimaryConstructor.kt | 2 ++ .../notForSealedSecondaryConstructor.kt | 4 ++++ .../intentions/IntentionTestGenerated.java | 20 +++++++++++++++++++ 8 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 idea/testData/intentions/changeVisibility/internal/notForSealedPrimaryConstructor.kt create mode 100644 idea/testData/intentions/changeVisibility/internal/notForSealedSecondaryConstructor.kt create mode 100644 idea/testData/intentions/changeVisibility/public/notForSealedPrimaryConstructor.kt create mode 100644 idea/testData/intentions/changeVisibility/public/notForSealedSecondaryConstructor.kt 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 b7fbb8dc167..ad45bdb6718 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 @@ -14,7 +14,6 @@ import com.intellij.psi.util.PsiTreeUtil import com.intellij.psi.util.elementType import org.jetbrains.kotlin.builtins.isFunctionOrSuspendFunctionType import org.jetbrains.kotlin.config.LanguageFeature -import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.extensions.DeclarationAttributeAltererExtension import org.jetbrains.kotlin.idea.FrontendInternals @@ -358,6 +357,8 @@ fun KtModifierListOwner.canBePrivate(): Boolean { return true } +fun KtModifierListOwner.canBePublic(): Boolean = !isSealedClassConstructor() + fun KtModifierListOwner.canBeProtected(): Boolean { val parent = when (this) { is KtPropertyAccessor -> this.property.parent @@ -376,12 +377,18 @@ fun KtModifierListOwner.canBeInternal(): Boolean { val objectDeclaration = getStrictParentOfType() ?: return false if (objectDeclaration.isCompanion() && hasJvmFieldAnnotation()) return false } - return !isAnnotationClassPrimaryConstructor() + return !isAnnotationClassPrimaryConstructor() && !isSealedClassConstructor() } private fun KtModifierListOwner.isAnnotationClassPrimaryConstructor(): Boolean = this is KtPrimaryConstructor && (this.parent as? KtClass)?.hasModifier(KtTokens.ANNOTATION_KEYWORD) ?: false +private fun KtModifierListOwner.isSealedClassConstructor(): Boolean { + if (this !is KtConstructor<*>) return false + val ktClass = getContainingClassOrObject().safeAs() ?: return false + return ktClass.isSealed() +} + fun KtClass.isInheritable(): Boolean { return when (getModalityFromDescriptor()) { KtTokens.ABSTRACT_KEYWORD, KtTokens.OPEN_KEYWORD, KtTokens.SEALED_KEYWORD -> true diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt index 9e81c4bd135..4485dbf7ff4 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt @@ -97,7 +97,10 @@ open class ChangeVisibilityModifierIntention protected constructor(val modifier: else -> null } - class Public : ChangeVisibilityModifierIntention(KtTokens.PUBLIC_KEYWORD), HighPriorityAction + class Public : ChangeVisibilityModifierIntention(KtTokens.PUBLIC_KEYWORD), HighPriorityAction { + override fun applicabilityRange(element: KtDeclaration): TextRange? = + if (element.canBePublic()) super.applicabilityRange(element) else null + } class Private : ChangeVisibilityModifierIntention(KtTokens.PRIVATE_KEYWORD), HighPriorityAction { override fun applicabilityRange(element: KtDeclaration): TextRange? = diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVisibilityFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVisibilityFix.kt index e0dc10684eb..ae14fad0567 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVisibilityFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/ChangeVisibilityFix.kt @@ -16,10 +16,7 @@ import org.jetbrains.kotlin.descriptors.DescriptorVisibility import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.DiagnosticFactory0 import org.jetbrains.kotlin.idea.KotlinBundle -import org.jetbrains.kotlin.idea.core.canBeInternal -import org.jetbrains.kotlin.idea.core.canBePrivate -import org.jetbrains.kotlin.idea.core.canBeProtected -import org.jetbrains.kotlin.idea.core.setVisibility +import org.jetbrains.kotlin.idea.core.* import org.jetbrains.kotlin.idea.inspections.RemoveRedundantSetterFix import org.jetbrains.kotlin.idea.inspections.isRedundantSetter import org.jetbrains.kotlin.idea.project.languageVersionSettings @@ -62,7 +59,13 @@ open class ChangeVisibilityFix( } protected class ChangeToPublicFix(element: KtModifierListOwner, elementName: String) : - ChangeVisibilityFix(element, elementName, KtTokens.PUBLIC_KEYWORD), HighPriorityAction + ChangeVisibilityFix(element, elementName, KtTokens.PUBLIC_KEYWORD), HighPriorityAction { + + override fun isAvailable(project: Project, editor: Editor?, file: KtFile): Boolean { + val element = element ?: return false + return element.canBePublic() + } + } protected class ChangeToProtectedFix(element: KtModifierListOwner, elementName: String) : ChangeVisibilityFix(element, elementName, KtTokens.PROTECTED_KEYWORD) { diff --git a/idea/testData/intentions/changeVisibility/internal/notForSealedPrimaryConstructor.kt b/idea/testData/intentions/changeVisibility/internal/notForSealedPrimaryConstructor.kt new file mode 100644 index 00000000000..374edd99522 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/internal/notForSealedPrimaryConstructor.kt @@ -0,0 +1,2 @@ +// IS_APPLICABLE: false +sealed class C private constructor() \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/internal/notForSealedSecondaryConstructor.kt b/idea/testData/intentions/changeVisibility/internal/notForSealedSecondaryConstructor.kt new file mode 100644 index 00000000000..85ac01d00f0 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/internal/notForSealedSecondaryConstructor.kt @@ -0,0 +1,4 @@ +// IS_APPLICABLE: false +sealed class C { + private constructor() +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/notForSealedPrimaryConstructor.kt b/idea/testData/intentions/changeVisibility/public/notForSealedPrimaryConstructor.kt new file mode 100644 index 00000000000..374edd99522 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/notForSealedPrimaryConstructor.kt @@ -0,0 +1,2 @@ +// IS_APPLICABLE: false +sealed class C private constructor() \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/public/notForSealedSecondaryConstructor.kt b/idea/testData/intentions/changeVisibility/public/notForSealedSecondaryConstructor.kt new file mode 100644 index 00000000000..85ac01d00f0 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/public/notForSealedSecondaryConstructor.kt @@ -0,0 +1,4 @@ +// IS_APPLICABLE: false +sealed class C { + private constructor() +} \ 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 7eafe9af7be..b17f732cc38 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -3402,6 +3402,16 @@ public class IntentionTestGenerated extends AbstractIntentionTest { runTest("idea/testData/intentions/changeVisibility/internal/notForAnnotationClassPrimaryConstructor.kt"); } + @TestMetadata("notForSealedPrimaryConstructor.kt") + public void testNotForSealedPrimaryConstructor() throws Exception { + runTest("idea/testData/intentions/changeVisibility/internal/notForSealedPrimaryConstructor.kt"); + } + + @TestMetadata("notForSealedSecondaryConstructor.kt") + public void testNotForSealedSecondaryConstructor() throws Exception { + runTest("idea/testData/intentions/changeVisibility/internal/notForSealedSecondaryConstructor.kt"); + } + @TestMetadata("propertySetter.kt") public void testPropertySetter() throws Exception { runTest("idea/testData/intentions/changeVisibility/internal/propertySetter.kt"); @@ -3726,6 +3736,16 @@ public class IntentionTestGenerated extends AbstractIntentionTest { runTest("idea/testData/intentions/changeVisibility/public/destructuringPropertyException.kt"); } + @TestMetadata("notForSealedPrimaryConstructor.kt") + public void testNotForSealedPrimaryConstructor() throws Exception { + runTest("idea/testData/intentions/changeVisibility/public/notForSealedPrimaryConstructor.kt"); + } + + @TestMetadata("notForSealedSecondaryConstructor.kt") + public void testNotForSealedSecondaryConstructor() throws Exception { + runTest("idea/testData/intentions/changeVisibility/public/notForSealedSecondaryConstructor.kt"); + } + @TestMetadata("primaryConstructor.kt") public void testPrimaryConstructor() throws Exception { runTest("idea/testData/intentions/changeVisibility/public/primaryConstructor.kt");