"Make private" intention: fix some false positive cases

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
 }
 

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
 

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()
+}
+
+<caret>actual class C {
+    actual fun test() {}
+}

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 {
+    <caret>actual fun test() {}
+}

idea/testData/intentions/changeVisibility/private/expect.kt

@@ -0,0 +1,7 @@
+// SKIP_ERRORS_BEFORE
+// SKIP_ERRORS_AFTER
+
+// IS_APPLICABLE: false
+<caret>expect class C {
+    fun test()
+}

idea/testData/intentions/changeVisibility/private/expect2.kt

@@ -0,0 +1,7 @@
+// SKIP_ERRORS_BEFORE
+// SKIP_ERRORS_AFTER
+
+// IS_APPLICABLE: false
+expect class C {
+    <caret>fun test()
+}

idea/testData/intentions/changeVisibility/private/inAnnotation.kt

@@ -0,0 +1,3 @@
+// IS_APPLICABLE: false
+
+annotation class Ann(va<caret>l x: Int)

idea/testData/intentions/changeVisibility/private/interface.kt

@@ -0,0 +1,4 @@
+// IS_APPLICABLE: false
+interface Interface {
+    <caret>val x: String
+}

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 {

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");