From 061aa63a7318a552b15726bc299386816a4a5d1b Mon Sep 17 00:00:00 2001 From: Toshiaki Kameyama Date: Fri, 28 Sep 2018 13:17:05 +0300 Subject: [PATCH] "Make private" intention: fix some false positive cases --- .../kotlin/idea/core/psiModificationUtils.kt | 12 ++++++++ .../MemberVisibilityCanBePrivateInspection.kt | 7 ++--- .../changeVisibility/private/actual.kt | 11 +++++++ .../changeVisibility/private/actual2.kt | 11 +++++++ .../changeVisibility/private/expect.kt | 7 +++++ .../changeVisibility/private/expect2.kt | 7 +++++ .../changeVisibility/private/inAnnotation.kt | 3 ++ .../changeVisibility/private/interface.kt | 4 +++ ...OfferToImplementMembersForExpectedClass.kt | 1 - .../intentions/IntentionTestGenerated.java | 30 +++++++++++++++++++ 10 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 idea/testData/intentions/changeVisibility/private/actual.kt create mode 100644 idea/testData/intentions/changeVisibility/private/actual2.kt create mode 100644 idea/testData/intentions/changeVisibility/private/expect.kt create mode 100644 idea/testData/intentions/changeVisibility/private/expect2.kt create mode 100644 idea/testData/intentions/changeVisibility/private/inAnnotation.kt create mode 100644 idea/testData/intentions/changeVisibility/private/interface.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 d4436dd2d64..59199888593 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 @@ -29,6 +29,7 @@ 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.idea.util.isExpectDeclaration import org.jetbrains.kotlin.lexer.KtModifierKeywordToken import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.Name @@ -282,6 +283,17 @@ 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 + + if (this is KtDeclaration) { + if (hasActualModifier() || isExpectDeclaration()) return false + val containingClassOrObject = containingClassOrObject ?: return true + if (containingClassOrObject is KtClass && + (containingClassOrObject.isInterface() || containingClassOrObject.isAnnotation()) + ) { + return false + } + } + return true } diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanBePrivateInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanBePrivateInspection.kt index aa7b93c06b8..2ef9de4c933 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanBePrivateInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/MemberVisibilityCanBePrivateInspection.kt @@ -33,6 +33,7 @@ import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility import org.jetbrains.kotlin.descriptors.EffectiveVisibility import org.jetbrains.kotlin.descriptors.effectiveVisibility import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall +import org.jetbrains.kotlin.idea.core.canBePrivate import org.jetbrains.kotlin.idea.core.isInheritable import org.jetbrains.kotlin.idea.core.isOverridable import org.jetbrains.kotlin.idea.core.toDescriptor @@ -42,7 +43,6 @@ import org.jetbrains.kotlin.idea.search.isCheapEnoughToSearchConsideringOperator import org.jetbrains.kotlin.idea.search.usagesSearch.dataClassComponentFunction import org.jetbrains.kotlin.idea.search.usagesSearch.descriptor import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope -import org.jetbrains.kotlin.idea.util.isExpectDeclaration import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.* @@ -77,7 +77,6 @@ class MemberVisibilityCanBePrivateInspection : AbstractKotlinInspection() { private fun canBePrivate(declaration: KtNamedDeclaration): Boolean { if (declaration.hasModifier(KtTokens.PRIVATE_KEYWORD) || declaration.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return false if (declaration.annotationEntries.isNotEmpty()) return false - if (declaration.hasActualModifier() || declaration.isExpectDeclaration()) return false val descriptor = (declaration.toDescriptor() as? DeclarationDescriptorWithVisibility) ?: return false when (descriptor.effectiveVisibility()) { @@ -85,8 +84,6 @@ class MemberVisibilityCanBePrivateInspection : AbstractKotlinInspection() { } val classOrObject = declaration.containingClassOrObject ?: return false - if (classOrObject.isAnnotation()) return false - val inheritable = classOrObject is KtClass && classOrObject.isInheritable() if (!inheritable && declaration.hasModifier(KtTokens.PROTECTED_KEYWORD)) return false //reported by ProtectedInFinalInspection if (declaration.isOverridable()) return false @@ -100,6 +97,8 @@ class MemberVisibilityCanBePrivateInspection : AbstractKotlinInspection() { ) ) return false + if (!declaration.canBePrivate()) return false + // properties can be referred by component1/component2, which is too expensive to search, don't analyze them if (declaration is KtParameter && declaration.dataClassComponentFunction() != null) return false diff --git a/idea/testData/intentions/changeVisibility/private/actual.kt b/idea/testData/intentions/changeVisibility/private/actual.kt new file mode 100644 index 00000000000..54287045319 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/actual.kt @@ -0,0 +1,11 @@ +// SKIP_ERRORS_BEFORE +// SKIP_ERRORS_AFTER + +// IS_APPLICABLE: false +expect class C { + fun test() +} + +actual class C { + actual fun test() {} +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/actual2.kt b/idea/testData/intentions/changeVisibility/private/actual2.kt new file mode 100644 index 00000000000..747dc950cc1 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/actual2.kt @@ -0,0 +1,11 @@ +// SKIP_ERRORS_BEFORE +// SKIP_ERRORS_AFTER + +// IS_APPLICABLE: false +expect class C { + fun test() +} + +actual class C { + actual fun test() {} +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/expect.kt b/idea/testData/intentions/changeVisibility/private/expect.kt new file mode 100644 index 00000000000..a049d5362a4 --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/expect.kt @@ -0,0 +1,7 @@ +// SKIP_ERRORS_BEFORE +// SKIP_ERRORS_AFTER + +// IS_APPLICABLE: false +expect class C { + fun test() +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/expect2.kt b/idea/testData/intentions/changeVisibility/private/expect2.kt new file mode 100644 index 00000000000..5408659216a --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/expect2.kt @@ -0,0 +1,7 @@ +// SKIP_ERRORS_BEFORE +// SKIP_ERRORS_AFTER + +// IS_APPLICABLE: false +expect class C { + fun test() +} \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/inAnnotation.kt b/idea/testData/intentions/changeVisibility/private/inAnnotation.kt new file mode 100644 index 00000000000..849da9fb60e --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/inAnnotation.kt @@ -0,0 +1,3 @@ +// IS_APPLICABLE: false + +annotation class Ann(val x: Int) \ No newline at end of file diff --git a/idea/testData/intentions/changeVisibility/private/interface.kt b/idea/testData/intentions/changeVisibility/private/interface.kt new file mode 100644 index 00000000000..4cd500e6b9e --- /dev/null +++ b/idea/testData/intentions/changeVisibility/private/interface.kt @@ -0,0 +1,4 @@ +// IS_APPLICABLE: false +interface Interface { + val x: String +} \ No newline at end of file diff --git a/idea/testData/quickfix/override/dontOfferToImplementMembersForExpectedClass.kt b/idea/testData/quickfix/override/dontOfferToImplementMembersForExpectedClass.kt index e918eaca7dc..a32638870a2 100644 --- a/idea/testData/quickfix/override/dontOfferToImplementMembersForExpectedClass.kt +++ b/idea/testData/quickfix/override/dontOfferToImplementMembersForExpectedClass.kt @@ -1,7 +1,6 @@ // "Implement members" "false" // ACTION: Create test // ACTION: Make internal -// ACTION: Make private // ACTION: Move 'A' to separate file interface I { diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index bee4dd8bb87..8a4212adc4e 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -3539,6 +3539,16 @@ public class IntentionTestGenerated extends AbstractIntentionTest { KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); } + @TestMetadata("actual.kt") + public void testActual() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/actual.kt"); + } + + @TestMetadata("actual2.kt") + public void testActual2() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/actual2.kt"); + } + public void testAllFilesPresentInPrivate() throws Exception { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/changeVisibility/private"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); } @@ -3548,6 +3558,16 @@ public class IntentionTestGenerated extends AbstractIntentionTest { runTest("idea/testData/intentions/changeVisibility/private/annotated.kt"); } + @TestMetadata("expect.kt") + public void testExpect() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/expect.kt"); + } + + @TestMetadata("expect2.kt") + public void testExpect2() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/expect2.kt"); + } + @TestMetadata("hasModifier1.kt") public void testHasModifier1() throws Exception { runTest("idea/testData/intentions/changeVisibility/private/hasModifier1.kt"); @@ -3568,6 +3588,16 @@ public class IntentionTestGenerated extends AbstractIntentionTest { runTest("idea/testData/intentions/changeVisibility/private/hasModifier4.kt"); } + @TestMetadata("inAnnotation.kt") + public void testInAnnotation() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/inAnnotation.kt"); + } + + @TestMetadata("interface.kt") + public void testInterface() throws Exception { + runTest("idea/testData/intentions/changeVisibility/private/interface.kt"); + } + @TestMetadata("jvmField.kt") public void testJvmField() throws Exception { runTest("idea/testData/intentions/changeVisibility/private/jvmField.kt");