[FE] Convert specific diagnostic for actual function with default arguments into a common incompatibility
^KT-59665 Fixed Review: https://jetbrains.team/p/kt/reviews/11039/timeline It's better to have this logic in common place (AbstractExpectActualCompatibilityChecker) to avoid missing compilation errors in the future This commit fixes: 1. Missing compilation error for actual function with default arguments for 'actual typealias' KT-59665 2. Missing compilation error for actual function with default arguments for actual fake-override KT-59665 Alternative solution for KT-59665 is to create a special checker. "incompatibility" vs "special checker": Arguments for common incompatibility: - What if we had a rule that expect and actual default params must match? If so then it certainly would be an incompatibility. - Technically, we do the matching of expect and actual params (because we allow default params in common ancestors of expect and actual declarations). - It's hard to check that the actual definition doesn't use default params because `ExpectedActualResolver.findActualForExpected` filters out fake-overrides and doesn't return them. It's not clear logic for me, that I'm afraid to touch. implicitActualFakeOverride_AbstractMap.kt test breaks if you drop this weird logic - WEAK incompatibilities can be considered as "checkers". So it doesn't matter how it's implemented, as a "incompatibility" or a "checker" Arguments against common incompatibility: - Although we match expect and actual declarations to allow default params in common ancestors of expect and actual declarations, it's still can be considered that we check that the actual declaration doesn't have default params. And it doesn't feel right that we check correctness of the actual declaration in expect-actual matcher. - ~~It may change the rules of expect actual matching~~ (It's not true, because ActualFunctionWithDefaultParameters is declared as WEAK incompatibility)
This commit is contained in:
+17
-3
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.resolve.calls.mpp.ExpectActualMatchingContext
|
||||
import org.jetbrains.kotlin.resolve.calls.mpp.ExpectActualMatchingContext.AnnotationCallInfo
|
||||
import org.jetbrains.kotlin.resolve.checkers.OptInNames
|
||||
import org.jetbrains.kotlin.resolve.descriptorUtil.*
|
||||
import org.jetbrains.kotlin.resolve.findTopMostOverriddenDescriptors
|
||||
import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter
|
||||
import org.jetbrains.kotlin.resolve.scopes.MemberScope
|
||||
import org.jetbrains.kotlin.resolve.scopes.getDescriptorsFiltered
|
||||
@@ -34,9 +35,16 @@ import org.jetbrains.kotlin.utils.addToStdlib.castAll
|
||||
import org.jetbrains.kotlin.utils.addToStdlib.shouldNotBeCalled
|
||||
import org.jetbrains.kotlin.utils.keysToMap
|
||||
|
||||
class ClassicExpectActualMatchingContext(val platformModule: ModuleDescriptor) : ExpectActualMatchingContext<MemberDescriptor>,
|
||||
TypeSystemContext by ClassicTypeSystemContextForCS(platformModule.builtIns, KotlinTypeRefiner.Default)
|
||||
{
|
||||
class ClassicExpectActualMatchingContext(
|
||||
val platformModule: ModuleDescriptor,
|
||||
/**
|
||||
* You want to enable this check only in expect-actual matcher checker. And disable everywhere else (especially on backends)
|
||||
*
|
||||
* Otherwise, it won't be possible to suppress the compilation error in user code
|
||||
*/
|
||||
override val shouldCheckAbsenceOfDefaultParamsInActual: Boolean = false
|
||||
) : ExpectActualMatchingContext<MemberDescriptor>,
|
||||
TypeSystemContext by ClassicTypeSystemContextForCS(platformModule.builtIns, KotlinTypeRefiner.Default) {
|
||||
override val shouldCheckReturnTypesOfCallables: Boolean
|
||||
get() = true
|
||||
|
||||
@@ -194,6 +202,12 @@ class ClassicExpectActualMatchingContext(val platformModule: ModuleDescriptor) :
|
||||
get() = asDescriptor().isCrossinline
|
||||
override val ValueParameterSymbolMarker.hasDefaultValue: Boolean
|
||||
get() = asDescriptor().declaresDefaultValue()
|
||||
override fun FunctionSymbolMarker.allOverriddenDeclarationsRecursive(): Sequence<CallableSymbolMarker> =
|
||||
(sequenceOf(asDescriptor()) + asDescriptor().overriddenTreeAsSequence(useOriginal = true))
|
||||
// Tests work even if you don't filter out fake-overrides. Filtering fake-overrides is needed because
|
||||
// the returned descriptors are compared by `equals`. And `equals` for fake-overrides is weird.
|
||||
// I didn't manage to invent a test that would check this condition
|
||||
.filter { it.kind.isReal }
|
||||
|
||||
override fun CallableSymbolMarker.isAnnotationConstructor(): Boolean {
|
||||
val descriptor = safeAsDescriptor<ConstructorDescriptor>() ?: return false
|
||||
|
||||
+7
-4
@@ -56,9 +56,10 @@ object ExpectedActualResolver {
|
||||
|
||||
fun findExpectedForActual(
|
||||
actual: MemberDescriptor,
|
||||
moduleFilter: (ModuleDescriptor) -> Boolean = allModulesProvidingExpectsFor(actual.module)
|
||||
moduleFilter: (ModuleDescriptor) -> Boolean = allModulesProvidingExpectsFor(actual.module),
|
||||
shouldCheckAbsenceOfDefaultParamsInActual: Boolean = false,
|
||||
): Map<ExpectActualCompatibility<MemberDescriptor>, List<MemberDescriptor>>? {
|
||||
val context = ClassicExpectActualMatchingContext(actual.module)
|
||||
val context = ClassicExpectActualMatchingContext(actual.module, shouldCheckAbsenceOfDefaultParamsInActual)
|
||||
return when (actual) {
|
||||
is CallableMemberDescriptor -> {
|
||||
val container = actual.containingDeclaration
|
||||
@@ -66,7 +67,8 @@ object ExpectedActualResolver {
|
||||
is ClassifierDescriptorWithTypeParameters -> {
|
||||
// TODO: replace with 'singleOrNull' as soon as multi-module diagnostic tests are refactored
|
||||
val expectedClass =
|
||||
findExpectedForActual(container, moduleFilter)?.values?.firstOrNull()?.firstOrNull() as? ClassDescriptor
|
||||
findExpectedForActual(container, moduleFilter, shouldCheckAbsenceOfDefaultParamsInActual)?.values
|
||||
?.firstOrNull()?.firstOrNull() as? ClassDescriptor
|
||||
with(context) {
|
||||
expectedClass?.getMembersForExpectClass(actual.name)?.filterIsInstance<CallableMemberDescriptor>().orEmpty()
|
||||
}
|
||||
@@ -160,11 +162,12 @@ object ExpectedActualResolver {
|
||||
}
|
||||
}
|
||||
|
||||
// FIXME(dsavvinov): review clients, as they won't work properly in HMPP projects
|
||||
// FIXME(dsavvinov): review clients, as they won't work properly in HMPP projects. KT-61105
|
||||
@JvmOverloads
|
||||
fun MemberDescriptor.findCompatibleActualsForExpected(
|
||||
platformModule: ModuleDescriptor, moduleFilter: ModuleFilter = allModulesProvidingActualsFor(module, platformModule)
|
||||
): List<MemberDescriptor> =
|
||||
// ?.get(Compatible) is suspicious. Probably, we must check not only Compatible but Incompatible.WeakIncompatible as well
|
||||
ExpectedActualResolver.findActualForExpected(this, platformModule, moduleFilter)?.get(Compatible).orEmpty()
|
||||
|
||||
@JvmOverloads
|
||||
|
||||
Reference in New Issue
Block a user