From c65e246e02367a83a4ceaf16d34732df3766f7ab Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 28 Sep 2018 11:46:50 +0300 Subject: [PATCH] Make private: don't suggest if property has @JvmField annotation #KT-27138 Fixed --- .../kotlin/idea/util/modifierListModifactor.kt | 4 ++++ .../kotlin/idea/core/psiModificationUtils.kt | 13 ++++++++++++- .../intentions/ChangeVisibilityModifierIntention.kt | 12 ++---------- .../intentions/changeVisibility/private/jvmField.kt | 6 ++++++ .../quickfix/addAnnotationTarget/fromLib.kt | 1 - .../quickfix/replaceJvmFieldWithConst/getter.kt | 1 - .../idea/intentions/IntentionTestGenerated.java | 5 +++++ 7 files changed, 29 insertions(+), 13 deletions(-) create mode 100644 idea/testData/intentions/changeVisibility/private/jvmField.kt diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/modifierListModifactor.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/modifierListModifactor.kt index 875723fa129..f0b1b8c9100 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/modifierListModifactor.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/modifierListModifactor.kt @@ -18,9 +18,11 @@ package org.jetbrains.kotlin.idea.util import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.ShortenReferences +import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.annotations.JVM_FIELD_ANNOTATION_FQ_NAME import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode fun KtModifierListOwner.addAnnotation( @@ -73,3 +75,5 @@ fun KtAnnotated.findAnnotation(annotationFqName: FqName): KtAnnotationEntry? { return annotationEntries.firstOrNull { entry -> context.get(BindingContext.ANNOTATION, entry)?.fqName == annotationFqName } } + +fun KtAnnotated.hasJvmFieldAnnotation(): Boolean = findAnnotation(JvmAbi.JVM_FIELD_ANNOTATION_FQ_NAME) != null 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 0d9a07ab5d8..d4436dd2d64 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 @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers +import org.jetbrains.kotlin.idea.util.hasJvmFieldAnnotation import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.Name @@ -277,7 +278,12 @@ fun KtDeclaration.implicitVisibility(): KtModifierKeywordToken? { } } -fun KtModifierListOwner.canBePrivate() = modifierList?.hasModifier(KtTokens.ABSTRACT_KEYWORD) != true +fun KtModifierListOwner.canBePrivate(): Boolean { + if (modifierList?.hasModifier(KtTokens.ABSTRACT_KEYWORD) == true) return false + if (this.isAnnotationClassPrimaryConstructor()) return false + if (this is KtProperty && this.hasJvmFieldAnnotation()) return false + return true +} fun KtModifierListOwner.canBeProtected(): Boolean { val parent = when (this) { @@ -291,6 +297,11 @@ fun KtModifierListOwner.canBeProtected(): Boolean { } } +fun KtModifierListOwner.canBeInternal(): Boolean = !isAnnotationClassPrimaryConstructor() + +private fun KtModifierListOwner.isAnnotationClassPrimaryConstructor(): Boolean = + this is KtPrimaryConstructor && (this.parent as? KtClass)?.hasModifier(KtTokens.ANNOTATION_KEYWORD) ?: false + 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 17ec0facd78..5c435d7abee 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt @@ -22,10 +22,7 @@ import com.intellij.openapi.util.TextRange import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility import org.jetbrains.kotlin.descriptors.Visibilities -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.toDescriptor +import org.jetbrains.kotlin.idea.core.* import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* @@ -105,14 +102,10 @@ open class ChangeVisibilityModifierIntention protected constructor( } } - protected fun isAnnotationClassPrimaryConstructor(element: KtDeclaration) = - element is KtPrimaryConstructor && (element.parent as? KtClass)?.hasModifier(KtTokens.ANNOTATION_KEYWORD) ?: false - class Public : ChangeVisibilityModifierIntention(KtTokens.PUBLIC_KEYWORD), HighPriorityAction class Private : ChangeVisibilityModifierIntention(KtTokens.PRIVATE_KEYWORD), HighPriorityAction { override fun applicabilityRange(element: KtDeclaration): TextRange? { - if (isAnnotationClassPrimaryConstructor(element)) return null return if (element.canBePrivate()) super.applicabilityRange(element) else null } } @@ -125,8 +118,7 @@ open class ChangeVisibilityModifierIntention protected constructor( class Internal : ChangeVisibilityModifierIntention(KtTokens.INTERNAL_KEYWORD) { override fun applicabilityRange(element: KtDeclaration): TextRange? { - if (isAnnotationClassPrimaryConstructor(element)) return null - return super.applicabilityRange(element) + return if (element.canBeInternal()) super.applicabilityRange(element) else null } } } diff --git a/idea/testData/intentions/changeVisibility/private/jvmField.kt b/idea/testData/intentions/changeVisibility/private/jvmField.kt new file mode 100644 index 00000000000..126030cdc84 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/jvmField.kt @@ -0,0 +1,6 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME +class Test { + @JvmField + val foo = 1 +} \ No newline at end of file diff --git a/idea/testData/quickfix/addAnnotationTarget/fromLib.kt b/idea/testData/quickfix/addAnnotationTarget/fromLib.kt index 791d8477675..1428a9bda9c 100644 --- a/idea/testData/quickfix/addAnnotationTarget/fromLib.kt +++ b/idea/testData/quickfix/addAnnotationTarget/fromLib.kt @@ -1,7 +1,6 @@ // "Add annotation target" "false" // WITH_RUNTIME // ACTION: Make internal -// ACTION: Make private // ACTION: Specify type explicitly // ACTION: Add use-site target 'property' // ERROR: This annotation is not applicable to target 'top level property without backing field or delegate' diff --git a/idea/testData/quickfix/replaceJvmFieldWithConst/getter.kt b/idea/testData/quickfix/replaceJvmFieldWithConst/getter.kt index 0a4984e0497..31b4f768831 100644 --- a/idea/testData/quickfix/replaceJvmFieldWithConst/getter.kt +++ b/idea/testData/quickfix/replaceJvmFieldWithConst/getter.kt @@ -2,7 +2,6 @@ // WITH_RUNTIME // ERROR: This annotation is not applicable to target 'top level property without backing field or delegate' // ACTION: Make internal -// ACTION: Make private // ACTION: Remove explicit type specification // ACTION: Add use-site target 'property' @JvmField val number: Int diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 420dbe96060..bee4dd8bb87 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -3568,6 +3568,11 @@ public class IntentionTestGenerated extends AbstractIntentionTest { runTest("idea/testData/intentions/changeVisibility/private/hasModifier4.kt"); } + @TestMetadata("jvmField.kt") + public void testJvmField() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/jvmField.kt"); + } + @TestMetadata("noModifierListAnnotation.kt") public void testNoModifierListAnnotation() throws Exception { runTest("idea/testData/intentions/changeVisibility/private/noModifierListAnnotation.kt");