CreateExpect: shouldn't generate expect declaration from actual function with private, lateinit or const

#KT-33930 Fixed

idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/CreateActualFix.kt

@@ -32,7 +32,6 @@ import org.jetbrains.kotlin.idea.quickfix.TypeAccessibilityChecker
 import org.jetbrains.kotlin.idea.util.actualsForExpected
 import org.jetbrains.kotlin.platform.TargetPlatform
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.getSuperNames
 import org.jetbrains.kotlin.psi.psiUtil.hasExpectModifier
 
 sealed class CreateActualFix<D : KtNamedDeclaration>(
@@ -92,7 +91,7 @@ class CreateActualClassFix(
     actualPlatform: TargetPlatform
 ) : CreateActualFix<KtClassOrObject>(klass, actualModule, actualPlatform, block@{ project, checker, element ->
     checker.findAndApplyExistingClasses(element.collectDeclarationsForAddActualModifier().toList())
-    if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
+    if (!checker.isCorrectAndHaveAccessibleModifiers(element, true)) return@block null
 
     generateClassOrObject(project, false, element, checker = checker)
 })
@@ -102,7 +101,7 @@ class CreateActualCallableMemberFix(
     actualModule: Module,
     actualPlatform: TargetPlatform
 ) : CreateActualFix<KtCallableDeclaration>(declaration, actualModule, actualPlatform, block@{ project, checker, element ->
-    if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
+    if (!checker.isCorrectAndHaveAccessibleModifiers(element, true)) return@block null
 
     val descriptor = element.toDescriptor() as? CallableMemberDescriptor
     descriptor?.let { generateCallable(project, false, element, descriptor, checker = checker) }

idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/CreateExpectedFix.kt

@@ -30,7 +30,6 @@ import org.jetbrains.kotlin.idea.refactoring.getExpressionShortText
 import org.jetbrains.kotlin.idea.util.application.executeWriteCommand
 import org.jetbrains.kotlin.idea.util.liftToExpected
 import org.jetbrains.kotlin.idea.util.module
-import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
 import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer
@@ -115,10 +114,10 @@ class CreateExpectedClassFix(
 ) : CreateExpectedFix<KtClassOrObject>(klass, outerExpectedClass, commonModule, block@{ project, checker, element ->
     val originalElements = element.collectDeclarationsForAddActualModifier(withSelf = false).toList()
     val existingClasses = checker.findAndApplyExistingClasses(originalElements + klass)
-    if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
+    if (!checker.isCorrectAndHaveAccessibleModifiers(element, true)) return@block null
 
     val (members, declarationsWithNonExistentClasses) = originalElements.partition {
-        checker.isCorrectAndHaveNonPrivateModifier(it)
+        checker.isCorrectAndHaveAccessibleModifiers(it)
     }
 
     if (!showUnknownTypeInDeclarationDialog(project, declarationsWithNonExistentClasses)) return@block null
@@ -138,10 +137,10 @@ class CreateExpectedClassFix(
 
     val selectedClasses = checker.findAndApplyExistingClasses(selectedElements)
     val resultDeclarations = if (selectedClasses != existingClasses) {
-        if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
+        if (!checker.isCorrectAndHaveAccessibleModifiers(element, true)) return@block null
 
         val (resultDeclarations, withErrors) = selectedElements.partition {
-            checker.isCorrectAndHaveNonPrivateModifier(it)
+            checker.isCorrectAndHaveAccessibleModifiers(it)
         }
         if (!showUnknownTypeInDeclarationDialog(project, withErrors)) return@block null
         resultDeclarations
@@ -179,7 +178,6 @@ private fun showUnknownTypeInDeclarationDialog(
 private fun KtDeclaration.canAddActualModifier() = when (this) {
     is KtEnumEntry, is KtClassInitializer -> false
     is KtParameter -> hasValOrVar()
-    is KtProperty -> !hasModifier(KtTokens.LATEINIT_KEYWORD) && !hasModifier(KtTokens.CONST_KEYWORD)
     else -> true
 }
 
@@ -270,7 +268,7 @@ class CreateExpectedCallableMemberFix(
     targetExpectedClass: KtClassOrObject?,
     commonModule: Module
 ) : CreateExpectedFix<KtNamedDeclaration>(declaration, targetExpectedClass, commonModule, block@{ project, checker, element ->
-    if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
+    if (!checker.isCorrectAndHaveAccessibleModifiers(element, true)) return@block null
     val descriptor = element.toDescriptor() as? CallableMemberDescriptor
     checker.existingTypeNames = targetExpectedClass?.getSuperNames()?.toSet().orEmpty()
     descriptor?.let {

idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/ExpectActualUtils.kt

@@ -10,6 +10,7 @@ import com.intellij.openapi.module.Module
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.text.StringUtil
 import com.intellij.psi.JavaDirectoryService
+import com.intellij.psi.PsiElement
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
 import org.jetbrains.kotlin.descriptors.annotations.Annotations
@@ -33,6 +34,7 @@ import org.jetbrains.kotlin.idea.refactoring.introduce.showErrorHint
 import org.jetbrains.kotlin.idea.refactoring.isInterfaceClass
 import org.jetbrains.kotlin.idea.util.*
 import org.jetbrains.kotlin.idea.util.application.runWriteAction
+import org.jetbrains.kotlin.lexer.KtModifierKeywordToken
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.psi.*
@@ -385,11 +387,8 @@ fun KtNamedDeclaration.isAlwaysActual(): Boolean = safeAs<KtParameter>()?.parent
     ?.mustHaveValOrVar() ?: false
 
 
-fun TypeAccessibilityChecker.isCorrectAndHaveNonPrivateModifier(declaration: KtNamedDeclaration, showErrorHint: Boolean = false): Boolean {
+fun TypeAccessibilityChecker.isCorrectAndHaveAccessibleModifiers(declaration: KtNamedDeclaration, showErrorHint: Boolean = false): Boolean {
-    if (declaration.hasPrivateModifier()) {
+    if (declaration.anyInaccessibleModifier(INACCESSIBLE_MODIFIERS, showErrorHint)) return false
-        if (showErrorHint) showInaccessibleDeclarationError(declaration, "The declaration has a private modifier")
-        return false
-    }
 
     if (declaration is KtFunction && declaration.hasBody() && declaration.containingClassOrObject?.isInterfaceClass() == true) {
         if (showErrorHint) showInaccessibleDeclarationError(declaration, "The function declaration shouldn't have a default implementation")
@@ -409,7 +408,19 @@ fun TypeAccessibilityChecker.isCorrectAndHaveNonPrivateModifier(declaration: KtN
     return false
 }
 
-fun showInaccessibleDeclarationError(element: KtNamedDeclaration, message: String, editor: Editor? = element.findExistingEditor()) {
+private val INACCESSIBLE_MODIFIERS = listOf(KtTokens.PRIVATE_KEYWORD, KtTokens.CONST_KEYWORD, KtTokens.LATEINIT_KEYWORD)
+
+private fun KtModifierListOwner.anyInaccessibleModifier(modifiers: Collection<KtModifierKeywordToken>, showErrorHint: Boolean): Boolean {
+    for (modifier in modifiers) {
+        if (hasModifier(modifier)) {
+            if (showErrorHint) showInaccessibleDeclarationError(this, "The declaration has `$modifier` modifier")
+            return true
+        }
+    }
+    return false
+}
+
+fun showInaccessibleDeclarationError(element: PsiElement, message: String, editor: Editor? = element.findExistingEditor()) {
     editor?.let {
         showErrorHint(element.project, editor, escapeXml(message), "Inaccessible declaration")
     }
@@ -427,7 +438,7 @@ fun TypeAccessibilityChecker.findAndApplyExistingClasses(elements: Collection<Kt
         val existingNames = classes.mapNotNull { it.fqName?.asString() }.toHashSet()
         existingTypeNames = existingNames
 
-        val newExistingClasses = classes.filter { isCorrectAndHaveNonPrivateModifier(it) }
+        val newExistingClasses = classes.filter { isCorrectAndHaveAccessibleModifiers(it) }
         if (classes.size == newExistingClasses.size) return existingNames
 
         classes = newExistingClasses

idea/src/org/jetbrains/kotlin/idea/refactoring/elementSelectionUtils.kt

@@ -248,7 +248,7 @@ private fun smartSelectElement(
 }
 
 fun getExpressionShortText(element: KtElement): String {
-    val text = element.renderTrimmed()
+    val text = element.renderTrimmed().trimStart()
     val firstNewLinePos = text.indexOf('\n')
     var trimmedText = text.substring(0, if (firstNewLinePos != -1) firstNewLinePos else min(100, text.length))
     if (trimmedText.length != text.length) trimmedText += " ..."

idea/testData/multiModuleQuickFix/accessibilityChecker/memberFunction/jvm/Utils.kt

@@ -5,4 +5,7 @@ interface Some
 
 actual class A {
     actual fun <caret>a(): Some = TODO()
+    private fun b() = "string"
+    actual lateinit var c: Int
+    const val d = "const"
 }

idea/testData/multiModuleQuickFix/accessibilityChecker/memberFunction/jvm/Utils.kt.after

@@ -5,4 +5,7 @@ interface Some
 
 actual class A {
     fun <caret>a(): Some = TODO()
+    private fun b() = "string"
+    lateinit var c: Int
+    const val d = "const"
 }

idea/testData/multiModuleQuickFix/accessibilityChecker/memberFunction/log.log

@@ -1,2 +1,5 @@
 These declarations cannot be transformed:
 fun a() = TODO()
+fun b() = "string"
+actual lateinit var c: Int
+const val d = "const"

idea/testData/multiModuleQuickFix/createExpect/funWithPrivateModifier/common/My.kt

@@ -0,0 +1 @@
+// DISABLE-ERRORS

idea/testData/multiModuleQuickFix/createExpect/funWithPrivateModifier/jvm/My.kt

@@ -0,0 +1,5 @@
+// "Create expected function in common module testModule_Common" "true"
+// SHOULD_FAIL_WITH: "The declaration has `private` modifier"
+// DISABLE-ERRORS
+
+private actual fun <caret>s() = "s"

idea/testData/multiModuleQuickFix/createExpect/propertyWithConstModifier/common/My.kt

@@ -0,0 +1 @@
+// DISABLE-ERRORS

idea/testData/multiModuleQuickFix/createExpect/propertyWithConstModifier/jvm/My.kt

@@ -0,0 +1,5 @@
+// "Create expected property in common module testModule_Common" "true"
+// SHOULD_FAIL_WITH: "The declaration has `const` modifier"
+// DISABLE-ERRORS
+
+actual const val <caret>s: String = "Hello"

idea/testData/multiModuleQuickFix/createExpect/propertyWithLateinitModifier/common/My.kt

@@ -0,0 +1 @@
+// DISABLE-ERRORS

idea/testData/multiModuleQuickFix/createExpect/propertyWithLateinitModifier/jvm/My.kt

@@ -0,0 +1,5 @@
+// "Create expected property in common module testModule_Common" "true"
+// SHOULD_FAIL_WITH: "The declaration has `lateinit` modifier"
+// DISABLE-ERRORS
+
+actual lateinit var <caret>s: String

idea/testData/multiModuleQuickFix/createExpect/propertyWithPrivateModifier/common/My.kt

@@ -0,0 +1 @@
+// DISABLE-ERRORS

idea/testData/multiModuleQuickFix/createExpect/propertyWithPrivateModifier/jvm/My.kt

@@ -0,0 +1,5 @@
+// "Create expected property in common module testModule_Common" "true"
+// SHOULD_FAIL_WITH: "The declaration has `private` modifier"
+// DISABLE-ERRORS
+
+private actual val <caret>s: String = "s"

idea/testData/multiModuleQuickFix/createExpect/withAnnotations/header/My.kt.after

@@ -4,7 +4,6 @@
 annotation class CommonAnnotation
 expect class My {
     tailrec fun foo(arg: Int): Int
-    var some: Boolean
     @CommonAnnotation
     fun initialize()
 

idea/testData/multiModuleQuickFix/createExpect/withAnnotations/jvm/My.kt.after

@@ -11,7 +11,7 @@ actual class <caret>My {
     }
 
     // Here we will have an error (lateinit is not supported on both sides)
-    actual lateinit var some: Boolean
+    lateinit var some: Boolean
 
     @CommonAnnotation
     actual fun initialize() {

idea/testData/multiModuleQuickFix/createExpect/withAnnotations/log.log

@@ -0,0 +1,2 @@
+These declarations cannot be transformed:
+actual lateinit var some: Boolean

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

@@ -543,6 +543,11 @@ public class QuickFixMultiModuleTestGenerated extends AbstractQuickFixMultiModul
             runTest("idea/testData/multiModuleQuickFix/createExpect/funWithJdk/");
         }
 
+        @TestMetadata("funWithPrivateModifier")
+        public void testFunWithPrivateModifier() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/createExpect/funWithPrivateModifier/");
+        }
+
         @TestMetadata("function")
         public void testFunction() throws Exception {
             runTest("idea/testData/multiModuleQuickFix/createExpect/function/");
@@ -633,6 +638,21 @@ public class QuickFixMultiModuleTestGenerated extends AbstractQuickFixMultiModul
             runTest("idea/testData/multiModuleQuickFix/createExpect/propertyInConstructor/");
         }
 
+        @TestMetadata("propertyWithConstModifier")
+        public void testPropertyWithConstModifier() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/createExpect/propertyWithConstModifier/");
+        }
+
+        @TestMetadata("propertyWithLateinitModifier")
+        public void testPropertyWithLateinitModifier() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/createExpect/propertyWithLateinitModifier/");
+        }
+
+        @TestMetadata("propertyWithPrivateModifier")
+        public void testPropertyWithPrivateModifier() throws Exception {
+            runTest("idea/testData/multiModuleQuickFix/createExpect/propertyWithPrivateModifier/");
+        }
+
         @TestMetadata("sealedClass")
         public void testSealedClass() throws Exception {
             runTest("idea/testData/multiModuleQuickFix/createExpect/sealedClass/");