Create actual: add type accessibility checker

#KT-28740 Fixed
This commit is contained in:
Dmitry Gridin
2019-09-06 20:49:23 +07:00
parent fcaa897b9f
commit 57d25fc5f5
16 changed files with 130 additions and 34 deletions
@@ -22,6 +22,7 @@ import org.jetbrains.kotlin.psi.KtTypeParameter
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.KotlinType
import org.jetbrains.kotlin.types.TypeProjection import org.jetbrains.kotlin.types.TypeProjection
import org.jetbrains.kotlin.types.isError
import org.jetbrains.kotlin.types.typeUtil.isNullableAny import org.jetbrains.kotlin.types.typeUtil.isNullableAny
import org.jetbrains.kotlin.utils.addToStdlib.safeAs import org.jetbrains.kotlin.utils.addToStdlib.safeAs
@@ -126,7 +127,8 @@ private fun DeclarationDescriptor.collectAllTypes(): Sequence<FqName?> {
} }
} }
private fun KotlinType.collectAllTypes(): Sequence<FqName?> = sequenceOf(fqName) + arguments.asSequence() private fun KotlinType.collectAllTypes(): Sequence<FqName?> = if (isError) sequenceOf(null)
else sequenceOf(fqName) + arguments.asSequence()
.map(TypeProjection::getType) .map(TypeProjection::getType)
.flatMap(KotlinType::collectAllTypes) .flatMap(KotlinType::collectAllTypes)
@@ -57,7 +57,7 @@ abstract class AbstractCreateDeclarationFix<D : KtNamedDeclaration>(
factory.generateIt(project, TypeAccessibilityChecker.create(project, module), element) ?: return@runWhenSmart factory.generateIt(project, TypeAccessibilityChecker.create(project, module), element) ?: return@runWhenSmart
} catch (e: KotlinTypeInaccessibleException) { } catch (e: KotlinTypeInaccessibleException) {
if (editor != null) { if (editor != null) {
showErrorHint(project, editor, "Cannot generate expected $elementType: " + e.message, e.message) showErrorHint(project, editor, escapeXml("Cannot generate $elementType: " + e.message), "Inaccessible type")
} }
return@runWhenSmart return@runWhenSmart
} }
@@ -30,6 +30,7 @@ import org.jetbrains.kotlin.idea.core.toDescriptor
import org.jetbrains.kotlin.idea.quickfix.KotlinQuickFixAction import org.jetbrains.kotlin.idea.quickfix.KotlinQuickFixAction
import org.jetbrains.kotlin.idea.quickfix.KotlinSingleIntentionActionFactory import org.jetbrains.kotlin.idea.quickfix.KotlinSingleIntentionActionFactory
import org.jetbrains.kotlin.idea.quickfix.TypeAccessibilityChecker import org.jetbrains.kotlin.idea.quickfix.TypeAccessibilityChecker
import org.jetbrains.kotlin.idea.refactoring.getExpressionShortText
import org.jetbrains.kotlin.idea.util.module import org.jetbrains.kotlin.idea.util.module
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer
@@ -58,14 +59,20 @@ class AddActualFix(
val module = element.module ?: return val module = element.module ?: return
val checker = TypeAccessibilityChecker.create(project, module) val checker = TypeAccessibilityChecker.create(project, module)
val errors = linkedMapOf<KtDeclaration, KotlinTypeInaccessibleException>()
for (missedDeclaration in missedDeclarationPointers.mapNotNull { it.element }) { for (missedDeclaration in missedDeclarationPointers.mapNotNull { it.element }) {
val actualDeclaration = when (missedDeclaration) { val actualDeclaration = try {
is KtClassOrObject -> factory.generateClassOrObject(project, false, missedDeclaration, checker) when (missedDeclaration) {
is KtFunction, is KtProperty -> missedDeclaration.toDescriptor()?.safeAs<CallableMemberDescriptor>()?.let { is KtClassOrObject -> factory.generateClassOrObject(project, false, missedDeclaration, checker)
generateCallable(project, false, missedDeclaration, it, element, checker = checker) is KtFunction, is KtProperty -> missedDeclaration.toDescriptor()?.safeAs<CallableMemberDescriptor>()?.let {
} generateCallable(project, false, missedDeclaration, it, element, checker = checker)
else -> null }
} ?: continue else -> null
} ?: continue
} catch (e: KotlinTypeInaccessibleException) {
errors += missedDeclaration to e
continue
}
if (actualDeclaration is KtPrimaryConstructor) { if (actualDeclaration is KtPrimaryConstructor) {
if (element.primaryConstructor == null) if (element.primaryConstructor == null)
@@ -74,6 +81,16 @@ class AddActualFix(
element.addDeclaration(actualDeclaration).clean() element.addDeclaration(actualDeclaration).clean()
} }
} }
if (errors.isNotEmpty()) {
val message = errors.entries.joinToString(
separator = "\n",
prefix = "Some types are not accessible:\n"
) { (declaration, error) ->
getExpressionShortText(declaration) + " -> " + error.message
}
showInaccessibleDeclarationError(element, message, editor)
}
} }
companion object : KotlinSingleIntentionActionFactory() { companion object : KotlinSingleIntentionActionFactory() {
@@ -32,6 +32,7 @@ import org.jetbrains.kotlin.idea.quickfix.TypeAccessibilityChecker
import org.jetbrains.kotlin.idea.util.actualsForExpected import org.jetbrains.kotlin.idea.util.actualsForExpected
import org.jetbrains.kotlin.platform.TargetPlatform import org.jetbrains.kotlin.platform.TargetPlatform
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.getSuperNames
import org.jetbrains.kotlin.psi.psiUtil.hasExpectModifier import org.jetbrains.kotlin.psi.psiUtil.hasExpectModifier
sealed class CreateActualFix<D : KtNamedDeclaration>( sealed class CreateActualFix<D : KtNamedDeclaration>(
@@ -89,7 +90,10 @@ class CreateActualClassFix(
klass: KtClassOrObject, klass: KtClassOrObject,
actualModule: Module, actualModule: Module,
actualPlatform: TargetPlatform actualPlatform: TargetPlatform
) : CreateActualFix<KtClassOrObject>(klass, actualModule, actualPlatform, { project, checker, element -> ) : CreateActualFix<KtClassOrObject>(klass, actualModule, actualPlatform, block@{ project, checker, element ->
checker.findAndApplyExistingClasses(element.collectDeclarationsForAddActualModifier().toList())
if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
generateClassOrObject(project, false, element, checker = checker) generateClassOrObject(project, false, element, checker = checker)
}) })
@@ -97,7 +101,9 @@ class CreateActualCallableMemberFix(
declaration: KtCallableDeclaration, declaration: KtCallableDeclaration,
actualModule: Module, actualModule: Module,
actualPlatform: TargetPlatform actualPlatform: TargetPlatform
) : CreateActualFix<KtCallableDeclaration>(declaration, actualModule, actualPlatform, { project, checker, element -> ) : CreateActualFix<KtCallableDeclaration>(declaration, actualModule, actualPlatform, block@{ project, checker, element ->
if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
val descriptor = element.toDescriptor() as? CallableMemberDescriptor val descriptor = element.toDescriptor() as? CallableMemberDescriptor
descriptor?.let { generateCallable(project, false, element, descriptor, checker = checker) } descriptor?.let { generateCallable(project, false, element, descriptor, checker = checker) }
}) })
@@ -113,7 +113,7 @@ class CreateExpectedClassFix(
outerExpectedClass: KtClassOrObject?, outerExpectedClass: KtClassOrObject?,
commonModule: Module commonModule: Module
) : CreateExpectedFix<KtClassOrObject>(klass, outerExpectedClass, commonModule, block@{ project, checker, element -> ) : CreateExpectedFix<KtClassOrObject>(klass, outerExpectedClass, commonModule, block@{ project, checker, element ->
val originalElements = element.collectDeclarations(withSelf = false).toList() val originalElements = element.collectDeclarationsForAddActualModifier(withSelf = false).toList()
val existingClasses = checker.findAndApplyExistingClasses(originalElements + klass) val existingClasses = checker.findAndApplyExistingClasses(originalElements + klass)
if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null if (!checker.isCorrectAndHaveNonPrivateModifier(element, true)) return@block null
@@ -155,19 +155,6 @@ class CreateExpectedClassFix(
generateClassOrObject(project, true, element, checker) generateClassOrObject(project, true, element, checker)
}) })
private fun TypeAccessibilityChecker.findAndApplyExistingClasses(elements: Collection<KtNamedDeclaration>): HashSet<String> {
var classes = elements.filterIsInstance<KtClassOrObject>()
while (true) {
val existingNames = classes.mapNotNull { it.fqName?.asString() }.toHashSet()
existingTypeNames = existingNames
val newExistingClasses = classes.filter { isCorrectAndHaveNonPrivateModifier(it) }
if (classes.size == newExistingClasses.size) return existingNames
classes = newExistingClasses
}
}
private fun showUnknownTypeInDeclarationDialog( private fun showUnknownTypeInDeclarationDialog(
project: Project, project: Project,
declarationsWithNonExistentClasses: Collection<KtNamedDeclaration> declarationsWithNonExistentClasses: Collection<KtNamedDeclaration>
@@ -231,7 +218,7 @@ private class Member(val prefix: String, element: KtElement, descriptor: Declara
} }
} }
private fun KtClassOrObject.collectDeclarations(withSelf: Boolean = true): Sequence<KtNamedDeclaration> { fun KtClassOrObject.collectDeclarationsForAddActualModifier(withSelf: Boolean = true): Sequence<KtNamedDeclaration> {
val thisSequence: Sequence<KtNamedDeclaration> = if (withSelf) sequenceOf(this) else emptySequence() val thisSequence: Sequence<KtNamedDeclaration> = if (withSelf) sequenceOf(this) else emptySequence()
val primaryConstructorSequence: Sequence<KtNamedDeclaration> = primaryConstructorParameters.asSequence() + primaryConstructor.let { val primaryConstructorSequence: Sequence<KtNamedDeclaration> = primaryConstructorParameters.asSequence() + primaryConstructor.let {
if (it != null) sequenceOf(it) else emptySequence() if (it != null) sequenceOf(it) else emptySequence()
@@ -240,7 +227,7 @@ private fun KtClassOrObject.collectDeclarations(withSelf: Boolean = true): Seque
return thisSequence + primaryConstructorSequence + declarations.asSequence().flatMap { return thisSequence + primaryConstructorSequence + declarations.asSequence().flatMap {
if (it.canAddActualModifier()) if (it.canAddActualModifier())
when (it) { when (it) {
is KtClassOrObject -> it.collectDeclarations() is KtClassOrObject -> it.collectDeclarationsForAddActualModifier()
is KtNamedDeclaration -> sequenceOf(it) is KtNamedDeclaration -> sequenceOf(it)
else -> emptySequence() else -> emptySequence()
} }
@@ -5,6 +5,7 @@
package org.jetbrains.kotlin.idea.quickfix.expectactual package org.jetbrains.kotlin.idea.quickfix.expectactual
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.module.Module import com.intellij.openapi.module.Module
import com.intellij.openapi.project.Project import com.intellij.openapi.project.Project
import com.intellij.openapi.util.text.StringUtil import com.intellij.openapi.util.text.StringUtil
@@ -265,7 +266,7 @@ internal fun generateCallable(
generatedClass: KtClassOrObject? = null, generatedClass: KtClassOrObject? = null,
checker: TypeAccessibilityChecker checker: TypeAccessibilityChecker
): KtCallableDeclaration { ): KtCallableDeclaration {
if (generateExpect) descriptor.checkAccessibility(checker) descriptor.checkAccessibility(checker)
val memberChooserObject = create( val memberChooserObject = create(
originalDeclaration, descriptor, descriptor, originalDeclaration, descriptor, descriptor,
if (generateExpect || descriptor.modality == Modality.ABSTRACT) NO_BODY else EMPTY_OR_TEMPLATE if (generateExpect || descriptor.modality == Modality.ABSTRACT) NO_BODY else EMPTY_OR_TEMPLATE
@@ -372,8 +373,10 @@ private fun AnnotationDescriptor.isValidInModule(checker: TypeAccessibilityCheck
} }
class KotlinTypeInaccessibleException(fqNames: Collection<FqName?>) : Exception() { class KotlinTypeInaccessibleException(fqNames: Collection<FqName?>) : Exception() {
override val message: String = override val message: String = "${StringUtil.pluralize(
"${StringUtil.pluralize("Type", fqNames.size)} ${fqNames.joinToString()} is not accessible from common code" "Type",
fqNames.size
)} ${TypeAccessibilityChecker.typesToString(fqNames)} is not accessible from target module"
} }
fun KtNamedDeclaration.isAlwaysActual(): Boolean = safeAs<KtParameter>()?.parent?.parent?.safeAs<KtPrimaryConstructor>() fun KtNamedDeclaration.isAlwaysActual(): Boolean = safeAs<KtParameter>()?.parent?.parent?.safeAs<KtPrimaryConstructor>()
@@ -399,14 +402,27 @@ fun TypeAccessibilityChecker.isCorrectAndHaveNonPrivateModifier(declaration: KtN
return false return false
} }
private fun showInaccessibleDeclarationError(element: KtNamedDeclaration, message: String) { fun showInaccessibleDeclarationError(element: KtNamedDeclaration, message: String, editor: Editor? = element.findExistingEditor()) {
element.findExistingEditor()?.let { editor -> editor?.let {
showErrorHint(element.project, editor, message, "Inaccessible declaration") showErrorHint(element.project, editor, escapeXml(message), "Inaccessible declaration")
} }
} }
fun TypeAccessibilityChecker.Companion.typesToString(types: Collection<FqName?>, separator: CharSequence = "\n"): String { fun TypeAccessibilityChecker.Companion.typesToString(types: Collection<FqName?>, separator: CharSequence = "\n"): String {
return types.toSet().joinToString(separator = separator) { return types.toSet().joinToString(separator = separator) {
it?.shortName()?.asString() ?: "Unknown type" it?.shortName()?.asString() ?: "<Unknown>"
}
}
fun TypeAccessibilityChecker.findAndApplyExistingClasses(elements: Collection<KtNamedDeclaration>): HashSet<String> {
var classes = elements.filterIsInstance<KtClassOrObject>()
while (true) {
val existingNames = classes.mapNotNull { it.fqName?.asString() }.toHashSet()
existingTypeNames = existingNames
val newExistingClasses = classes.filter { isCorrectAndHaveNonPrivateModifier(it) }
if (classes.size == newExistingClasses.size) return existingNames
classes = newExistingClasses
} }
} }
@@ -0,0 +1,7 @@
// "Create actual class for module testModule_JVM (JVM)" "true"
// SHOULD_FAIL_WITH: Cannot generate class: Type &lt;Unknown&gt; is not accessible from target module
// DISABLE-ERRORS
expect class Foo<caret> {
var bar
}
@@ -0,0 +1,8 @@
expect class F<T> {
val a: List<T>
val b: T
fun <F> c(t: T): F
class M<V> {
val v: K
}
}
@@ -0,0 +1,5 @@
// "Add missing actual members" "true"
// SHOULD_FAIL_WITH: Some types are not accessible:,class M {...} -&gt; Type &lt;Unknown&gt; is not accessible from target module
// DISABLE-ERRORS
actual class <caret>F<T>
@@ -0,0 +1,14 @@
// "Add missing actual members" "true"
// SHOULD_FAIL_WITH: Some types are not accessible:,class M {...} -&gt; Type &lt;Unknown&gt; is not accessible from target module
// DISABLE-ERRORS
actual class <caret>F<T> {
actual val a: List<T>
get() = TODO("not implemented") //To change initializer of created properties use File | Settings | File Templates.
actual val b: T
get() = TODO("not implemented") //To change initializer of created properties use File | Settings | File Templates.
actual fun <F> c(t: T): F {
TODO("not implemented") //To change body of created functions use File | Settings | File Templates.
}
}
@@ -0,0 +1,5 @@
expect class F {
val a
val b: Int
val c: Gdssmem
}
@@ -0,0 +1,5 @@
// "Add missing actual members" "true"
// SHOULD_FAIL_WITH: Some types are not accessible:,val a -&gt; Type &lt;Unknown&gt; is not accessible from target module,val c: Gdssmem -&gt; Type &lt;Unknown&gt; is not accessible from target module
// DISABLE-ERRORS
actual class <caret>F
@@ -0,0 +1,8 @@
// "Add missing actual members" "true"
// SHOULD_FAIL_WITH: Some types are not accessible:,val a -&gt; Type &lt;Unknown&gt; is not accessible from target module,val c: Gdssmem -&gt; Type &lt;Unknown&gt; is not accessible from target module
// DISABLE-ERRORS
actual class <caret>F {
actual val b: Int
get() = TODO("not implemented") //To change initializer of created properties use File | Settings | File Templates.
}
@@ -91,6 +91,7 @@ abstract class AbstractQuickFixMultiModuleTest : AbstractMultiModuleTest(), Quic
TestCase.fail(getTestName(true)) TestCase.fail(getTestName(true))
} else { } else {
Assert.assertEquals("Wrong exception message", expectedErrorMessage, e.message) Assert.assertEquals("Wrong exception message", expectedErrorMessage, e.message)
compareToExpected(dirPath)
} }
} }
}, "", "") }, "", "")
@@ -56,6 +56,11 @@ public class QuickFixMultiModuleTestGenerated extends AbstractQuickFixMultiModul
runTest("idea/testData/multiModuleQuickFix/accessibilityChecker/classUpperBounds/"); runTest("idea/testData/multiModuleQuickFix/accessibilityChecker/classUpperBounds/");
} }
@TestMetadata("errorType")
public void testErrorType() throws Exception {
runTest("idea/testData/multiModuleQuickFix/accessibilityChecker/errorType/");
}
@TestMetadata("memberFunction") @TestMetadata("memberFunction")
public void testMemberFunction() throws Exception { public void testMemberFunction() throws Exception {
runTest("idea/testData/multiModuleQuickFix/accessibilityChecker/memberFunction/"); runTest("idea/testData/multiModuleQuickFix/accessibilityChecker/memberFunction/");
@@ -184,11 +189,21 @@ public class QuickFixMultiModuleTestGenerated extends AbstractQuickFixMultiModul
runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/companionAbsence/"); runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/companionAbsence/");
} }
@TestMetadata("membersWithIncorrectType")
public void testMembersWithIncorrectType() throws Exception {
runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/membersWithIncorrectType/");
}
@TestMetadata("primaryConstructorAbsence") @TestMetadata("primaryConstructorAbsence")
public void testPrimaryConstructorAbsence() throws Exception { public void testPrimaryConstructorAbsence() throws Exception {
runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/primaryConstructorAbsence/"); runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/primaryConstructorAbsence/");
} }
@TestMetadata("propertyWithIncorrectType")
public void testPropertyWithIncorrectType() throws Exception {
runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/propertyWithIncorrectType/");
}
@TestMetadata("secondaryConstructorAbsence") @TestMetadata("secondaryConstructorAbsence")
public void testSecondaryConstructorAbsence() throws Exception { public void testSecondaryConstructorAbsence() throws Exception {
runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/secondaryConstructorAbsence/"); runTest("idea/testData/multiModuleQuickFix/addMissingActualMembers/secondaryConstructorAbsence/");