Decrease visibility fix is now active for exposed visibility errors #KT-11920 Fixed

idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt

@@ -21,6 +21,10 @@ import com.intellij.psi.PsiWhiteSpace
 import com.intellij.psi.impl.source.codeStyle.CodeEditUtil
 import com.intellij.psi.util.PsiTreeUtil
 import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
+import org.jetbrains.kotlin.descriptors.ClassDescriptor
+import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
+import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
 import org.jetbrains.kotlin.lexer.KtTokens
@@ -166,6 +170,20 @@ fun KtClass.getOrCreateCompanionObject() : KtObjectDeclaration {
     return addDeclaration(KtPsiFactory(this).createCompanionObject())
 }
 
+fun KtDeclaration.toDescriptor(): DeclarationDescriptor? {
+    val bindingContext = analyze()
+    // TODO: temporary code
+    if (this is KtPrimaryConstructor) {
+        return (this.getContainingClassOrObject().resolveToDescriptor() as ClassDescriptor).unsubstitutedPrimaryConstructor
+    }
+
+    val descriptor = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, this]
+    if (descriptor is ValueParameterDescriptor) {
+        return bindingContext[BindingContext.VALUE_PARAMETER_AS_PROPERTY, descriptor]
+    }
+    return descriptor
+}
+
 //TODO: code style option whether to insert redundant 'public' keyword or not
 fun KtModifierListOwner.setVisibility(visibilityModifier: KtModifierKeywordToken) {
     if (this is KtDeclaration) {

idea/src/org/jetbrains/kotlin/idea/intentions/ChangeVisibilityModifierIntention.kt

@@ -20,17 +20,15 @@ import com.intellij.codeInsight.intention.HighPriorityAction
 import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.util.TextRange
 import org.jetbrains.kotlin.descriptors.*
-import org.jetbrains.kotlin.idea.caches.resolve.analyze
-import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 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.lexer.KtModifierKeywordToken
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.startOffset
 import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier
-import org.jetbrains.kotlin.resolve.BindingContext
 
 open class ChangeVisibilityModifierIntention protected constructor(
         val modifier: KtModifierKeywordToken
@@ -71,20 +69,6 @@ open class ChangeVisibilityModifierIntention protected constructor(
             defaultRange
     }
 
-    private fun KtDeclaration.toDescriptor(): DeclarationDescriptor? {
-        val bindingContext = analyze()
-        // TODO: temporary code
-        if (this is KtPrimaryConstructor) {
-            return (this.getContainingClassOrObject().resolveToDescriptor() as ClassDescriptor).unsubstitutedPrimaryConstructor
-        }
-
-        val descriptor = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, this]
-        if (descriptor is ValueParameterDescriptor) {
-            return bindingContext[BindingContext.VALUE_PARAMETER_AS_PROPERTY, descriptor]
-        }
-        return descriptor
-    }
-
     override fun applyTo(element: KtDeclaration, editor: Editor?) {
         element.setVisibility(modifier)
     }

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

@@ -24,6 +24,7 @@ import com.intellij.psi.PsiFile
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility
 import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.descriptors.Visibility
+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.lexer.KtModifierKeywordToken
@@ -58,6 +59,13 @@ open class ChangeVisibilityFix(
     protected class ChangeToInternalFix(element: KtModifierListOwner, elementName: String) :
             ChangeVisibilityFix(element, elementName, KtTokens.INTERNAL_KEYWORD)
 
+    protected class ChangeToPrivateFix(element: KtModifierListOwner, elementName: String) :
+            ChangeVisibilityFix(element, elementName, KtTokens.PRIVATE_KEYWORD) {
+
+        override fun isAvailable(project: Project, editor: Editor?, file: PsiFile) =
+                super.isAvailable(project, editor, file) && element.canBePrivate()
+    }
+
     companion object {
         fun create(
                 declaration: KtModifierListOwner,
@@ -69,6 +77,7 @@ open class ChangeVisibilityFix(
             val name = descriptor.name.asString()
 
             return when (targetVisibility) {
+                Visibilities.PRIVATE ->   ChangeToPrivateFix(declaration, name)
                 Visibilities.INTERNAL ->  ChangeToInternalFix(declaration, name)
                 Visibilities.PROTECTED -> ChangeToProtectedFix(declaration, name)
                 Visibilities.PUBLIC ->    ChangeToPublicFix(declaration, name)

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

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2010-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.idea.quickfix
+
+import com.intellij.codeInsight.intention.IntentionAction
+import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility
+import org.jetbrains.kotlin.descriptors.DescriptorWithRelation
+import org.jetbrains.kotlin.descriptors.EffectiveVisibility
+import org.jetbrains.kotlin.descriptors.EffectiveVisibility.Permissiveness.LESS
+import org.jetbrains.kotlin.descriptors.Visibilities
+import org.jetbrains.kotlin.descriptors.Visibilities.*
+import org.jetbrains.kotlin.diagnostics.Diagnostic
+import org.jetbrains.kotlin.diagnostics.DiagnosticFactory3
+import org.jetbrains.kotlin.idea.core.toDescriptor
+import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.psi.KtModifierListOwner
+import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
+import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils
+
+object DecreaseExposingVisibilityFactory  : KotlinSingleIntentionActionFactory() {
+    override fun createAction(diagnostic: Diagnostic): IntentionAction? {
+        @Suppress("UNCHECKED_CAST")
+        val factory = diagnostic.factory as DiagnosticFactory3<*, EffectiveVisibility, DescriptorWithRelation, EffectiveVisibility>
+        val exposedDiagnostic = factory.cast(diagnostic)
+        val exposedDescriptor = exposedDiagnostic.b.descriptor as? DeclarationDescriptorWithVisibility ?: return null
+        val exposedDeclaration = DescriptorToSourceUtils.getSourceFromDescriptor(exposedDescriptor) as? KtModifierListOwner ?: return null
+        val exposedVisibility = exposedDiagnostic.c
+        val exposingVisibility = exposedDiagnostic.a
+        val boundVisibility = when (exposedVisibility.relation(exposingVisibility)) {
+            LESS -> exposedVisibility.toVisibility()
+            else -> PRIVATE
+        }
+        val exposingDeclaration = diagnostic.psiElement.getParentOfType<KtDeclaration>(true) ?: return null
+        val targetVisibility = when (boundVisibility) {
+            PUBLIC -> return null
+            PROTECTED -> if (exposedDeclaration.parent == exposingDeclaration.parent) PROTECTED else PRIVATE
+            else -> boundVisibility
+        }
+        val exposingDescriptor = exposingDeclaration.toDescriptor() as? DeclarationDescriptorWithVisibility ?: return null
+        if (!Visibilities.isVisibleIgnoringReceiver(exposedDescriptor, exposingDescriptor)) return null
+        return ChangeVisibilityFix.create(exposingDeclaration, exposingDescriptor, targetVisibility)
+    }
+}

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

@@ -21,8 +21,7 @@ import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility
 import org.jetbrains.kotlin.descriptors.DescriptorWithRelation
 import org.jetbrains.kotlin.descriptors.EffectiveVisibility
 import org.jetbrains.kotlin.descriptors.EffectiveVisibility.Permissiveness.LESS
-import org.jetbrains.kotlin.descriptors.Visibilities.PROTECTED
+import org.jetbrains.kotlin.descriptors.Visibilities.*
-import org.jetbrains.kotlin.descriptors.Visibilities.PUBLIC
 import org.jetbrains.kotlin.diagnostics.Diagnostic
 import org.jetbrains.kotlin.diagnostics.DiagnosticFactory3
 import org.jetbrains.kotlin.psi.KtDeclaration
@@ -45,6 +44,7 @@ object IncreaseExposedVisibilityFactory : KotlinSingleIntentionActionFactory() {
         }
         val exposingDeclaration = diagnostic.psiElement.getParentOfType<KtDeclaration>(true)
         val targetVisibility = when (boundVisibility) {
+            PRIVATE -> return null
             PROTECTED -> if (exposedDeclaration.parent == exposingDeclaration?.parent) PROTECTED else PUBLIC
             else -> boundVisibility
         }

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

@@ -153,13 +153,10 @@ class QuickFixRegistrar : QuickFixContributor {
         INVISIBLE_MEMBER.registerFactory(MakeVisibleFactory)
         INVISIBLE_SETTER.registerFactory(MakeVisibleFactory)
 
-        EXPOSED_FUNCTION_RETURN_TYPE.registerFactory(IncreaseExposedVisibilityFactory)
+        for (exposed in listOf(EXPOSED_FUNCTION_RETURN_TYPE, EXPOSED_PARAMETER_TYPE, EXPOSED_PROPERTY_TYPE, EXPOSED_RECEIVER_TYPE,
-        EXPOSED_PARAMETER_TYPE.registerFactory(IncreaseExposedVisibilityFactory)
+                               EXPOSED_SUPER_CLASS, EXPOSED_SUPER_INTERFACE, EXPOSED_TYPE_PARAMETER_BOUND)) {
-        EXPOSED_PROPERTY_TYPE.registerFactory(IncreaseExposedVisibilityFactory)
+            exposed.registerFactory(IncreaseExposedVisibilityFactory, DecreaseExposingVisibilityFactory)
-        EXPOSED_RECEIVER_TYPE.registerFactory(IncreaseExposedVisibilityFactory)
+        }
-        EXPOSED_SUPER_CLASS.registerFactory(IncreaseExposedVisibilityFactory)
-        EXPOSED_SUPER_INTERFACE.registerFactory(IncreaseExposedVisibilityFactory)
-        EXPOSED_TYPE_PARAMETER_BOUND.registerFactory(IncreaseExposedVisibilityFactory)
 
         REDUNDANT_NULLABLE.registerFactory(RemoveNullableFix.Factory(RemoveNullableFix.NullableKind.REDUNDANT))
         NULLABLE_SUPERTYPE.registerFactory(RemoveNullableFix.Factory(RemoveNullableFix.NullableKind.SUPERTYPE))

idea/testData/quickfix/decreaseVisibility/exposedParameterType.kt

@@ -0,0 +1,14 @@
+// "Make foo private" "false"
+// ACTION: Convert parameter to receiver
+// ACTION: Make Nested internal
+// ACTION: Remove parameter 'arg'
+// ERROR: 'internal' function exposes its 'private' parameter type argument Nested
+// ERROR: Cannot access 'Nested': it is 'private' in 'Outer'
+
+class Outer {
+    private class Nested
+}
+
+class Generic<T>
+
+internal fun foo(<caret>arg: Generic<Outer.Nested>) {}

idea/testData/quickfix/decreaseVisibility/exposedPropertyType.kt

@@ -0,0 +1,7 @@
+// "Make foo private" "true"
+
+private data class Data(val x: Int)
+
+class First {
+    val <caret>foo = Data(13)
+}

idea/testData/quickfix/decreaseVisibility/exposedPropertyType.kt.after

@@ -0,0 +1,7 @@
+// "Make foo private" "true"
+
+private data class Data(val x: Int)
+
+class First {
+    private val foo = Data(13)
+}

idea/testData/quickfix/decreaseVisibility/exposedReceiverType.kt

@@ -0,0 +1,12 @@
+// "Make foo private" "false"
+// ACTION: Convert receiver to parameter
+// ACTION: Make Private protected
+// ERROR: 'protected (in My)' member exposes its 'private' receiver type argument Private
+
+class Receiver<T>
+
+abstract class My {
+    private class Private
+    // abstract never can be private
+    abstract protected fun <caret>Receiver<Private>.foo()
+}

idea/testData/quickfix/decreaseVisibility/exposedReturnType.kt

@@ -0,0 +1,7 @@
+// "Make bar private" "true"
+
+private data class Data(val x: Int)
+
+class First {
+    internal fun <caret>bar(x: Int) = Data(x)
+}

idea/testData/quickfix/decreaseVisibility/exposedReturnType.kt.after

@@ -0,0 +1,7 @@
+// "Make bar private" "true"
+
+private data class Data(val x: Int)
+
+class First {
+    private fun bar(x: Int) = Data(x)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperClass.kt

@@ -0,0 +1,7 @@
+// "Make First private" "true"
+
+class Outer {
+    private open class Data(val x: Int)
+
+    protected class First : <caret>Data(42)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperClass.kt.after

@@ -0,0 +1,7 @@
+// "Make First private" "true"
+
+class Outer {
+    private open class Data(val x: Int)
+
+    private class First : Data(42)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperClassProtectedBase.kt

@@ -0,0 +1,7 @@
+// "Make First private" "true"
+
+private open class Data(val x: Int)
+
+class Outer {
+    protected class First : <caret>Data(42)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperClassProtectedBase.kt.after

@@ -0,0 +1,7 @@
+// "Make First private" "true"
+
+private open class Data(val x: Int)
+
+class Outer {
+    private class First : Data(42)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperClassProtectedInAnother.kt

@@ -0,0 +1,9 @@
+// "Make First private" "true"
+
+class Other {
+    internal open class Data(val x: Int)
+}
+
+class Another {
+    protected class First : Other.<caret>Data(42)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperClassProtectedInAnother.kt.after

@@ -0,0 +1,9 @@
+// "Make First private" "true"
+
+class Other {
+    internal open class Data(val x: Int)
+}
+
+class Another {
+    private class First : Other.Data(42)
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperInterface.kt

@@ -0,0 +1,11 @@
+// "Make Derived internal" "true"
+
+import Outer.Base
+
+internal class Outer {
+    interface Base
+}
+
+class Container {
+    interface Derived : <caret>Base
+}

idea/testData/quickfix/decreaseVisibility/exposedSuperInterface.kt.after

@@ -0,0 +1,11 @@
+// "Make Derived internal" "true"
+
+import Outer.Base
+
+internal class Outer {
+    interface Base
+}
+
+class Container {
+    internal interface Derived : Base
+}

idea/testData/quickfix/decreaseVisibility/exposedTypeParameterBound.kt

@@ -0,0 +1,5 @@
+// "Make User internal" "true"
+
+internal open class InternalString
+
+class User<T : <caret>User<T, InternalString>, R>

idea/testData/quickfix/decreaseVisibility/exposedTypeParameterBound.kt.after

@@ -0,0 +1,5 @@
+// "Make User internal" "true"
+
+internal open class InternalString
+
+internal class User<T : User<T, InternalString>, R>

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

@@ -3492,6 +3492,69 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
         }
     }
 
+    @TestMetadata("idea/testData/quickfix/decreaseVisibility")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class DecreaseVisibility extends AbstractQuickFixTest {
+        public void testAllFilesPresentInDecreaseVisibility() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/quickfix/decreaseVisibility"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true);
+        }
+
+        @TestMetadata("exposedParameterType.kt")
+        public void testExposedParameterType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedParameterType.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedPropertyType.kt")
+        public void testExposedPropertyType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedPropertyType.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedReceiverType.kt")
+        public void testExposedReceiverType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedReceiverType.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedReturnType.kt")
+        public void testExposedReturnType() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedReturnType.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedSuperClass.kt")
+        public void testExposedSuperClass() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedSuperClass.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedSuperClassProtectedBase.kt")
+        public void testExposedSuperClassProtectedBase() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedSuperClassProtectedBase.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedSuperClassProtectedInAnother.kt")
+        public void testExposedSuperClassProtectedInAnother() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedSuperClassProtectedInAnother.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedSuperInterface.kt")
+        public void testExposedSuperInterface() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedSuperInterface.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("exposedTypeParameterBound.kt")
+        public void testExposedTypeParameterBound() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/decreaseVisibility/exposedTypeParameterBound.kt");
+            doTest(fileName);
+        }
+    }
+
     @TestMetadata("idea/testData/quickfix/deprecatedSymbolUsage")
     @TestDataPath("$PROJECT_ROOT")
     @RunWith(JUnit3RunnerWithInners.class)