Increase visibility: make not available in case of potential exposed visibility problems (see KT-11920)

compiler/frontend/src/org/jetbrains/kotlin/resolve/ExposedVisibilityChecker.kt

@@ -17,13 +17,15 @@
 package org.jetbrains.kotlin.resolve
 
 import org.jetbrains.kotlin.descriptors.*
+import org.jetbrains.kotlin.diagnostics.DiagnosticSink
+import org.jetbrains.kotlin.diagnostics.DiagnosticSink.DO_NOTHING
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.types.TypeUtils
 
 // Checker for all seven EXPOSED_* errors
 // All functions return true if everything is OK, or false in case of any errors
-class ExposedVisibilityChecker(private val trace: BindingTrace) {
+class ExposedVisibilityChecker(private val trace: DiagnosticSink = DO_NOTHING) {
 
     // NB: does not check any members
     fun checkClassHeader(klass: KtClassOrObject, classDescriptor: ClassDescriptor): Boolean {
@@ -35,8 +37,27 @@ class ExposedVisibilityChecker(private val trace: BindingTrace) {
         return result and checkFunction(constructor, constructorDescriptor)
     }
 
-    fun checkFunction(function: KtFunction, functionDescriptor: FunctionDescriptor): Boolean {
-        val functionVisibility = functionDescriptor.effectiveVisibility()
+    fun checkDeclarationWithVisibility(modifierListOwner: KtModifierListOwner,
+                                       descriptor: DeclarationDescriptorWithVisibility,
+                                       visibility: Visibility
+    ) : Boolean {
+        return when {
+            modifierListOwner is KtFunction &&
+            descriptor is FunctionDescriptor -> checkFunction(modifierListOwner, descriptor, visibility)
+
+            modifierListOwner is KtProperty &&
+            descriptor is PropertyDescriptor -> checkProperty(modifierListOwner, descriptor, visibility)
+
+            else -> true
+        }
+    }
+
+    fun checkFunction(function: KtFunction,
+                      functionDescriptor: FunctionDescriptor,
+                      // for checking situation with modified basic visibility
+                      visibility: Visibility = functionDescriptor.visibility
+    ): Boolean {
+        val functionVisibility = functionDescriptor.effectiveVisibility(visibility)
         var result = true
         if (function !is KtConstructor<*>) {
             val restricting = functionDescriptor.returnType?.leastPermissiveDescriptor(functionVisibility)
@@ -57,8 +78,12 @@ class ExposedVisibilityChecker(private val trace: BindingTrace) {
         return result and checkMemberReceiver(function.receiverTypeReference, functionDescriptor)
     }
 
-    fun checkProperty(property: KtProperty, propertyDescriptor: PropertyDescriptor): Boolean {
-        val propertyVisibility = propertyDescriptor.effectiveVisibility()
+    fun checkProperty(property: KtProperty,
+                      propertyDescriptor: PropertyDescriptor,
+                      // for checking situation with modified basic visibility
+                      visibility: Visibility = propertyDescriptor.visibility
+    ): Boolean {
+        val propertyVisibility = propertyDescriptor.effectiveVisibility(visibility)
         val restricting = propertyDescriptor.type.leastPermissiveDescriptor(propertyVisibility)
         var result = true
         if (restricting != null) {

core/descriptors/src/org/jetbrains/kotlin/descriptors/EffectiveVisibility.kt

@@ -281,7 +281,7 @@ private fun Set<DescriptorWithRelation>.leastPermissive(base: EffectiveVisibilit
 
 fun KotlinType.leastPermissiveDescriptor(base: EffectiveVisibility) = dependentDescriptors().leastPermissive(base)
 
-fun DeclarationDescriptorWithVisibility.effectiveVisibility(): EffectiveVisibility =
+fun DeclarationDescriptorWithVisibility.effectiveVisibility(visibility: Visibility = this.visibility): EffectiveVisibility =
         lowerBound(visibility.effectiveVisibility(this.containingDeclaration as? ClassDescriptor),
                    (this.containingDeclaration as? ClassDescriptor)?.effectiveVisibility() ?: Public)
 

idea/src/org/jetbrains/kotlin/idea/quickfix/IncreaseVisibilityFix.kt

@@ -33,6 +33,7 @@ import org.jetbrains.kotlin.psi.KtModifierListOwner
 import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
 import org.jetbrains.kotlin.resolve.DescriptorUtils
+import org.jetbrains.kotlin.resolve.ExposedVisibilityChecker
 import org.jetbrains.kotlin.resolve.descriptorUtil.module
 import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
 import org.jetbrains.kotlin.idea.core.setVisibility
@@ -62,12 +63,15 @@ class IncreaseVisibilityFix(
             val declaration = DescriptorToSourceUtils.getSourceFromDescriptor(descriptor) as? KtModifierListOwner ?: return null
 
             val module = DescriptorUtils.getContainingModule(descriptor)
-            val modifier = if (module != usageModule || descriptor.visibility != Visibilities.PRIVATE) {
-                KtTokens.PUBLIC_KEYWORD
+            val (modifier, visibility) = if (module != usageModule || descriptor.visibility != Visibilities.PRIVATE) {
+                Pair(KtTokens.PUBLIC_KEYWORD, Visibilities.PUBLIC)
             }
             else {
-                KtTokens.INTERNAL_KEYWORD
+                Pair(KtTokens.INTERNAL_KEYWORD, Visibilities.INTERNAL)
             }
+
+            if (!ExposedVisibilityChecker().checkDeclarationWithVisibility(declaration, descriptor, visibility)) return null
+
             return IncreaseVisibilityFix(declaration, descriptor.name.asString(), modifier)
         }
     }

idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt

@@ -0,0 +1,14 @@
+// "Make bar internal" "false"
+// ACTION: Convert property initializer to getter
+// ERROR: Cannot access 'bar': it is 'private' in 'First'
+
+private data class Data(val x: Int)
+
+class First {
+    // Making it internal exposes 'Data'
+    private fun bar(x: Int) = Data(x)
+}
+
+class Second(f: First) {
+    private val y = f.<caret>bar(42)
+}

idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java

@@ -4731,6 +4731,12 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
             doTest(fileName);
         }
 
+        @TestMetadata("privateMemberToInternalWithExposed.kt")
+        public void testPrivateMemberToInternalWithExposed() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/increaseVisibility/privateMemberToInternalWithExposed.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("protectedMemberToPublicSingleFile.kt")
         public void testProtectedMemberToPublicSingleFile() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/increaseVisibility/protectedMemberToPublicSingleFile.kt");