From 2b32c9d2f95852299e103b82fb8cbccf8f45156e Mon Sep 17 00:00:00 2001 From: Dmitry Gridin Date: Wed, 7 Aug 2019 16:43:50 +0300 Subject: [PATCH] ExpectActualUtils: add type arguments accessibility check for constructors --- .../AddValVarToConstructorParameterAction.kt | 4 +- .../expectactual/CreateExpectedFix.kt | 38 +++++++++---------- .../expectactual/ExpectActualUtils.kt | 28 +++++++++++--- .../kotlin/idea/util/expressionExt.kt | 2 +- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/AddValVarToConstructorParameterAction.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/AddValVarToConstructorParameterAction.kt index d7e3b031721..a736916cb90 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/AddValVarToConstructorParameterAction.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/AddValVarToConstructorParameterAction.kt @@ -16,8 +16,8 @@ import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.idea.quickfix.KotlinQuickFixAction import org.jetbrains.kotlin.idea.quickfix.KotlinSingleIntentionActionFactory import org.jetbrains.kotlin.idea.refactoring.ValVarExpression -import org.jetbrains.kotlin.idea.util.allowedValOrVar import org.jetbrains.kotlin.idea.util.isExpectDeclaration +import org.jetbrains.kotlin.idea.util.mustHaveValOrVar import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType @@ -32,7 +32,7 @@ interface AddValVarToConstructorParameterAction { fun canInvoke(element: KtParameter): Boolean { return element.valOrVarKeyword == null && ((element.parent as? KtParameterList)?.parent as? KtPrimaryConstructor) - ?.takeIf { it.allowedValOrVar() || !it.isExpectDeclaration() } != null + ?.takeIf { it.mustHaveValOrVar() || !it.isExpectDeclaration() } != null } operator fun invoke(element: KtParameter, editor: Editor?) { diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/CreateExpectedFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/CreateExpectedFix.kt index 7e16d95b6a0..34218f86378 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/CreateExpectedFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/CreateExpectedFix.kt @@ -29,7 +29,6 @@ import org.jetbrains.kotlin.idea.inspections.findExistingEditor import org.jetbrains.kotlin.idea.quickfix.KotlinIntentionActionsFactory import org.jetbrains.kotlin.idea.refactoring.introduce.showErrorHint import org.jetbrains.kotlin.idea.refactoring.renderTrimmed -import org.jetbrains.kotlin.idea.util.allowedValOrVar import org.jetbrains.kotlin.idea.util.application.executeWriteCommand import org.jetbrains.kotlin.idea.util.liftToExpected import org.jetbrains.kotlin.idea.util.module @@ -38,6 +37,7 @@ import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer import org.jetbrains.kotlin.psi.psiUtil.hasActualModifier +import org.jetbrains.kotlin.utils.addToStdlib.safeAs import java.util.* sealed class CreateExpectedFix( @@ -116,28 +116,29 @@ class CreateExpectedClassFix( outerExpectedClass: KtClassOrObject?, commonModule: Module ) : CreateExpectedFix(klass, outerExpectedClass, commonModule, block@{ project, element -> - val originalElements = element.collectDeclarations(false).toList() - val existingClasses = findClasses(originalElements + klass) + val originalElements = element.collectDeclarations(withSelf = false).toList() + val existingClasses = findClasses(originalElements + klass, commonModule) if (!element.checkTypeAccessibilityInModule(commonModule, existingClasses)) { showUnknownTypesError(element) return@block null } - val (members, declarationsWithNonExistentClasses) = originalElements.filterNot(KtNamedDeclaration::isAlwaysActual).partition { + val (members, declarationsWithNonExistentClasses) = originalElements.partition { it.checkTypeAccessibilityInModule(commonModule, existingClasses) } if (!showUnknownTypesDialog(project, declarationsWithNonExistentClasses)) return@block null + val membersForSelection = members.filterNot(KtNamedDeclaration::isAlwaysActual) val selectedElements = when { - members.all(KtDeclaration::hasActualModifier) -> originalElements - declarationsWithNonExistentClasses - ApplicationManager.getApplication().isUnitTestMode -> members.filter(KtDeclaration::hasActualModifier) + membersForSelection.all(KtDeclaration::hasActualModifier) -> membersForSelection + ApplicationManager.getApplication().isUnitTestMode -> membersForSelection.filter(KtDeclaration::hasActualModifier) else -> { val prefix = klass.fqName?.asString()?.plus(".") ?: "" - chooseMembers(project, members, prefix) ?: return@block null + chooseMembers(project, membersForSelection, prefix) ?: return@block null } - }.let { selectedElements -> - val selectedClasses = findClasses(selectedElements + klass) + }.plus(members.filter(KtNamedDeclaration::isAlwaysActual)).let { selectedElements -> + val selectedClasses = findClasses(selectedElements + klass, commonModule) if (selectedClasses != existingClasses) { if (!element.checkTypeAccessibilityInModule(commonModule, selectedClasses)) { showUnknownTypesError(element) @@ -157,15 +158,19 @@ class CreateExpectedClassFix( if (originalElements.isNotEmpty()) { project.executeWriteCommand("Repair actual members") { - repairActualModifiers(originalElements, selectedElements) + repairActualModifiers(originalElements, selectedElements.toSet()) } } generateClassOrObject(project, true, element, listOfNotNull(outerExpectedClass)) }) -private fun findClasses(elements: List): HashSet { - return elements.filterIsInstance().mapNotNull { it.fqName?.asString() }.toHashSet() +private tailrec fun findClasses(elements: List, module: Module): HashSet { + val classes = elements.filterIsInstance() + val existingNames = classes.mapNotNull { it.fqName?.asString() }.toHashSet() + val newExistingClasses = classes.filter { it.checkTypeAccessibilityInModule(module, existingNames) } + return if (classes.size == newExistingClasses.size) existingNames + else findClasses(newExistingClasses, module) } private fun showUnknownTypesDialog(project: Project, declarationsWithNonExistentClasses: List): Boolean = @@ -258,7 +263,7 @@ private fun repairActualModifiers( } else for (original in originalElements) { - if (original.isAlwaysActual() || original in selectedElements) + if (original in selectedElements) original.makeActualWithParents() else original.makeNotActual() @@ -267,15 +272,10 @@ private fun repairActualModifiers( private tailrec fun KtDeclaration.makeActualWithParents() { makeActual() + safeAs()?.parent?.parent?.safeAs()?.takeUnless(KtDeclaration::hasActualModifier)?.makeActual() containingClassOrObject?.takeUnless(KtDeclaration::hasActualModifier)?.makeActualWithParents() } -private fun KtNamedDeclaration.isAlwaysActual(): Boolean = when (this) { - is KtPrimaryConstructor -> this - is KtParameter -> (parent as? KtParameterList)?.parent as? KtPrimaryConstructor - else -> null -}?.allowedValOrVar() ?: false - class CreateExpectedPropertyFix( property: KtNamedDeclaration, targetExpectedClass: KtClassOrObject?, diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/ExpectActualUtils.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/ExpectActualUtils.kt index 042e9d1d5e8..6e1caed5650 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/ExpectActualUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/expectactual/ExpectActualUtils.kt @@ -325,19 +325,32 @@ fun DeclarationDescriptor.checkTypeAccessibilityInModule( module: Module, existingClasses: Set = emptySet() ): Boolean { - val existingClassesWithTypeParameters = if (this is ClassifierDescriptorWithTypeParameters) - existingClasses + declaredTypeParameters.map { it.fqNameOrNull()?.asString() ?: return false }.toSet() - else - existingClasses + val existingClassesWithTypeParameters = additionalClasses(existingClasses) return collectAllTypes().all { it?.canFindClassInModule(project, module, existingClassesWithTypeParameters) == true } } +private tailrec fun DeclarationDescriptor.additionalClasses(existingClasses: Set = emptySet()): Set = when (this) { + is ClassifierDescriptorWithTypeParameters -> { + val myParameters = existingClasses + declaredTypeParameters.map { it.fqNameOrNull()?.asString() ?: return emptySet() } + val containingDeclaration = containingDeclaration + if (isInner) containingDeclaration.additionalClasses(myParameters) else myParameters + } + is CallableDescriptor -> containingDeclaration.additionalClasses(existingClasses + typeParameters.map { + it.fqNameOrNull()?.asString() ?: return emptySet() + }) + else -> + existingClasses +} + private fun DeclarationDescriptor.collectAllTypes(): Sequence { return when (this) { - is ClassDescriptor -> declaredTypeParameters.asSequence().flatMap(DeclarationDescriptor::collectAllTypes) + is ClassConstructorDescriptor -> valueParameters.asSequence().map(ValueParameterDescriptor::getType).flatMap(KotlinType::collectAllTypes) + is ClassDescriptor -> if (isInline) unsubstitutedPrimaryConstructor?.collectAllTypes().orEmpty() else { + emptySequence() + } + declaredTypeParameters.asSequence().flatMap(DeclarationDescriptor::collectAllTypes) is CallableDescriptor -> { val returnType = returnType ?: return sequenceOf(null) - returnType.collectAllTypes() + allParameters.asSequence().map { it.type }.flatMap(KotlinType::collectAllTypes) + returnType.collectAllTypes() + allParameters.asSequence().map(ParameterDescriptor::getType).flatMap(KotlinType::collectAllTypes) } is TypeParameterDescriptor -> { val upperBounds = upperBounds @@ -352,6 +365,9 @@ private fun KotlinType.collectAllTypes(): Sequence = sequenceOf(fqName) .map(TypeProjection::getType) .flatMap(KotlinType::collectAllTypes) +fun KtNamedDeclaration.isAlwaysActual(): Boolean = safeAs()?.parent?.parent?.safeAs() + ?.mustHaveValOrVar() ?: false + fun KtNamedDeclaration.checkTypeAccessibilityInModule(module: Module, existingClasses: Set = emptySet()): Boolean { return !hasPrivateModifier() && descriptor?.checkTypeAccessibilityInModule(project, module, existingClasses) == true } diff --git a/idea/src/org/jetbrains/kotlin/idea/util/expressionExt.kt b/idea/src/org/jetbrains/kotlin/idea/util/expressionExt.kt index 23c4d570715..ebdfad03b1c 100644 --- a/idea/src/org/jetbrains/kotlin/idea/util/expressionExt.kt +++ b/idea/src/org/jetbrains/kotlin/idea/util/expressionExt.kt @@ -31,7 +31,7 @@ fun KtModifierListOwner.hasInlineModifier() = hasModifier(KtTokens.INLINE_KEYWOR fun KtModifierListOwner.hasPrivateModifier() = hasModifier(KtTokens.PRIVATE_KEYWORD) -fun KtPrimaryConstructor.allowedValOrVar(): Boolean = containingClass()?.let { +fun KtPrimaryConstructor.mustHaveValOrVar(): Boolean = containingClass()?.let { it.isAnnotation() || it.hasInlineModifier() } ?: false