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

#KT-33930 Fixed
2019-10-02 12:11:27 +07:00
parent 9da6dcf840
commit eaa07dac17
19 changed files with 82 additions and 20 deletions
@@ -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) }
@@ -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 {
@@ -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 {
-    if (declaration.hasPrivateModifier()) {
-        if (showErrorHint) showInaccessibleDeclarationError(declaration, "The declaration has a private modifier")
-        return false
-    }
+fun TypeAccessibilityChecker.isCorrectAndHaveAccessibleModifiers(declaration: KtNamedDeclaration, showErrorHint: Boolean = false): Boolean {
+    if (declaration.anyInaccessibleModifier(INACCESSIBLE_MODIFIERS, showErrorHint)) 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
@@ -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 += " ..."
@@ -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"
 }
@@ -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"
 }
@@ -1,2 +1,5 @@
 These declarations cannot be transformed:
 fun a() = TODO()
+fun b() = &quot;string&quot;
+actual lateinit var c: Int
+const val d = &quot;const&quot;
@@ -0,0 +1 @@
+// DISABLE-ERRORS
@@ -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"
@@ -0,0 +1 @@
+// DISABLE-ERRORS
@@ -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"
@@ -0,0 +1 @@
+// DISABLE-ERRORS
@@ -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
@@ -0,0 +1 @@
+// DISABLE-ERRORS
@@ -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"
@@ -4,7 +4,6 @@
 annotation class CommonAnnotation
 expect class My {
    tailrec fun foo(arg: Int): Int
-    var some: Boolean
    @CommonAnnotation
    fun initialize()

@@ -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() {
@@ -0,0 +1,2 @@
+These declarations cannot be transformed:
+actual lateinit var some: Boolean
@@ -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/");