KTIJ-30908 [kotlin] Do not propose ChangeVisibilityFix on KtParameter unless it has val/var

Also, skip all the private declarations on the way to the parent declaration N.B. `exposedConstructorParameterType_val.kt` doesn't work in K1, because there is a more advanced check which checks if the changed visibility modifier will actually make the problem go away (see `ExposedVisibilityChecker.checkDeclarationWithVisibility`). In the case of the constructor value parameter, the exposure error will stay, but it can be fixed by also making the constructor private with a second invocation of the quickfix. ^KTIJ-30908 Fixed GitOrigin-RevId: ff4207a3dc71b7c185112aa128a13e10c38fd895
2026-09-27 10:03:11 +07:00 · 2024-08-08 11:45:18 +00:00
parent edeb1cc677
commit 1e49336fc1
12 changed files with 170 additions and 13 deletions
@@ -7,6 +7,7 @@ import com.intellij.modcommand.ModPsiUpdater
 import com.intellij.modcommand.Presentation
 import com.intellij.psi.PsiElement
 import com.intellij.psi.util.parentOfTypes
+import com.intellij.psi.util.parentsOfType
 import com.intellij.util.containers.addIfNotNull
 import org.jetbrains.kotlin.analysis.api.KaSession
 import org.jetbrains.kotlin.analysis.api.analyze
@@ -50,7 +51,6 @@ import org.jetbrains.kotlin.psi.KtPropertyAccessor
 import org.jetbrains.kotlin.psi.KtTypeParameter
 import org.jetbrains.kotlin.psi.psiUtil.containingClass
 import org.jetbrains.kotlin.psi.psiUtil.isPrivate
-import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
 import org.jetbrains.kotlin.psi.psiUtil.visibilityModifierType
 import org.jetbrains.kotlin.utils.addToStdlib.safeAs

@@ -316,13 +316,18 @@ object ChangeVisibilityFixFactories {
                Visibilities.Private to Visibilities.Public
            }

-        val userDeclaration = element.parentsWithSelf.firstOrNull { it is KtDeclaration && it !is KtTypeParameter } as? KtDeclaration
+        val userDeclaration = element.parentsOfType<KtDeclaration>(withSelf = true)
+            .filterNot { it is KtTypeParameter }
+            .filterNot { it is KtParameter && !it.hasValOrVar() }
+            .filterNot { it.isPrivate() }
+            .firstOrNull()
+
        val exposedDeclaration = restrictingSymbol.psi
        val protectedAllowed = exposedDeclaration?.parent == userDeclaration?.parent

        val modCommandActions = arrayListOf<ChangeVisibilityModCommandAction>()

-        if (userDeclaration != null && !userDeclaration.isPrivate()) {
+        if (userDeclaration != null) {
            if ((restrictingSymbol as? KaDeclarationSymbol)?.isVisible(element) == true) {
                addFixToTargetVisibility(
                    symbol = restrictingSymbol,
@@ -7198,11 +7198,31 @@ public abstract class HighLevelQuickFixTestGenerated extends AbstractHighLevelQu
            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
        }

+        @TestMetadata("exposedConstructorParameterType.kt")
+        public void testExposedConstructorParameterType() throws Exception {
+            runTest("../../../idea/tests/testData/quickfix/decreaseVisibility/exposedConstructorParameterType.kt");
+        }
+
+        @TestMetadata("exposedConstructorParameterType_privateVal.kt")
+        public void testExposedConstructorParameterType_privateVal() throws Exception {
+            runTest("../../../idea/tests/testData/quickfix/decreaseVisibility/exposedConstructorParameterType_privateVal.kt");
+        }
+
+        @TestMetadata("exposedConstructorParameterType_val.kt")
+        public void testExposedConstructorParameterType_val() throws Exception {
+            runTest("../../../idea/tests/testData/quickfix/decreaseVisibility/exposedConstructorParameterType_val.kt");
+        }
+
        @TestMetadata("exposedParameterType.kt")
        public void testExposedParameterType() throws Exception {
            runTest("../../../idea/tests/testData/quickfix/decreaseVisibility/exposedParameterType.kt");
        }

+        @TestMetadata("exposedParameterType_privateType.kt")
+        public void testExposedParameterType_privateType() throws Exception {
+            runTest("../../../idea/tests/testData/quickfix/decreaseVisibility/exposedParameterType_privateType.kt");
+        }
+
        @TestMetadata("exposedPropertyType.kt")
        public void testExposedPropertyType() throws Exception {
            runTest("../../../idea/tests/testData/quickfix/decreaseVisibility/exposedPropertyType.kt");
@@ -7599,11 +7599,31 @@ public abstract class K1QuickFixTestGenerated extends AbstractK1QuickFixTest {
            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
        }

+        @TestMetadata("exposedConstructorParameterType.kt")
+        public void testExposedConstructorParameterType() throws Exception {
+            runTest("testData/quickfix/decreaseVisibility/exposedConstructorParameterType.kt");
+        }
+
+        @TestMetadata("exposedConstructorParameterType_privateVal.kt")
+        public void testExposedConstructorParameterType_privateVal() throws Exception {
+            runTest("testData/quickfix/decreaseVisibility/exposedConstructorParameterType_privateVal.kt");
+        }
+
+        @TestMetadata("exposedConstructorParameterType_val.kt")
+        public void testExposedConstructorParameterType_val() throws Exception {
+            runTest("testData/quickfix/decreaseVisibility/exposedConstructorParameterType_val.kt");
+        }
+
        @TestMetadata("exposedParameterType.kt")
        public void testExposedParameterType() throws Exception {
            runTest("testData/quickfix/decreaseVisibility/exposedParameterType.kt");
        }

+        @TestMetadata("exposedParameterType_privateType.kt")
+        public void testExposedParameterType_privateType() throws Exception {
+            runTest("testData/quickfix/decreaseVisibility/exposedParameterType_privateType.kt");
+        }
+
        @TestMetadata("exposedPropertyType.kt")
        public void testExposedPropertyType() throws Exception {
            runTest("testData/quickfix/decreaseVisibility/exposedPropertyType.kt");
@@ -0,0 +1,15 @@
+// "Make '<init>' private" "true"
+// ACTION: Add 'val' or 'var' to parameter 'arg'
+// ACTION: Convert to secondary constructor
+// ACTION: Create test
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make '<init>' private
+// ACTION: Make 'PrivateType' public
+// ACTION: Remove parameter 'arg'
+
+private class PrivateType
+
+class Foo(<caret>arg: PrivateType) {}
+
+// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToPrivateFix
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,15 @@
+// "Make '<init>' private" "true"
+// ACTION: Add 'val' or 'var' to parameter 'arg'
+// ACTION: Convert to secondary constructor
+// ACTION: Create test
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make '<init>' private
+// ACTION: Make 'PrivateType' public
+// ACTION: Remove parameter 'arg'
+
+private class PrivateType
+
+class Foo private constructor(<caret>arg: PrivateType) {}
+
+// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToPrivateFix
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,14 @@
+// "Make '<init>' private" "true"
+// ACTION: Convert to secondary constructor
+// ACTION: Create test
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make 'PrivateType' public
+// ACTION: Move to class body
+// IGNORE_K1
+// ERROR: 'public' function exposes its 'private-in-file' parameter type PrivateType
+
+private class PrivateType
+
+class Foo(private val <caret>arg: PrivateType) {}
+
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,14 @@
+// "Make '<init>' private" "true"
+// ACTION: Convert to secondary constructor
+// ACTION: Create test
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make 'PrivateType' public
+// ACTION: Move to class body
+// IGNORE_K1
+// ERROR: 'public' function exposes its 'private-in-file' parameter type PrivateType
+
+private class PrivateType
+
+class Foo private constructor(private val <caret>arg: PrivateType) {}
+
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,14 @@
+// "Make 'arg' private" "true"
+// ACTION: Convert to secondary constructor
+// ACTION: Create test
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make 'PrivateType' public
+// ACTION: Move to class body
+// ERROR: 'public' function exposes its 'private-in-file' parameter type PrivateType
+// IGNORE_K1
+
+private class PrivateType
+
+class Foo(val <caret>arg: PrivateType) {}
+
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,14 @@
+// "Make 'arg' private" "true"
+// ACTION: Convert to secondary constructor
+// ACTION: Create test
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make 'PrivateType' public
+// ACTION: Move to class body
+// ERROR: 'public' function exposes its 'private-in-file' parameter type PrivateType
+// IGNORE_K1
+
+private class PrivateType
+
+class Foo(private val <caret>arg: PrivateType) {}
+
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -1,16 +1,13 @@
-// "Make 'foo' private" "false"
+// "Make 'foo' private" "true"
 // ACTION: Convert parameter to receiver
 // ACTION: Enable a trailing comma by default in the formatter
-// ACTION: Make 'Nested' internal
-// ACTION: Make 'Nested' public
+// ACTION: Make 'PrivateType' public
+// ACTION: Make 'foo' private
 // ACTION: Remove parameter 'arg'
-// ERROR: 'internal' function exposes its 'private-in-class' parameter type argument Nested
-// ERROR: Cannot access 'Nested': it is private in 'Outer'

-class Outer {
-    private class Nested
-}
+private class PrivateType

-class Generic<T>
+fun foo(<caret>arg: PrivateType) {}

-internal fun foo(<caret>arg: Generic<Outer.Nested>) {}
+// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToPrivateFix
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,13 @@
+// "Make 'foo' private" "true"
+// ACTION: Convert parameter to receiver
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make 'PrivateType' public
+// ACTION: Make 'foo' private
+// ACTION: Remove parameter 'arg'
+
+private class PrivateType
+
+private fun foo(<caret>arg: PrivateType) {}
+
+// FUS_QUICKFIX_NAME: org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToPrivateFix
+// FUS_K2_QUICKFIX_NAME: org.jetbrains.kotlin.idea.k2.codeinsight.fixes.ChangeVisibilityFixFactories$ChangeToPrivateModCommandAction
@@ -0,0 +1,16 @@
+// "Make 'foo' private" "false"
+// ACTION: Convert parameter to receiver
+// ACTION: Enable a trailing comma by default in the formatter
+// ACTION: Make 'Nested' internal
+// ACTION: Make 'Nested' public
+// ACTION: Remove parameter 'arg'
+// ERROR: 'internal' function exposes its 'private-in-class' 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>) {}