From 98ec13b51a6153106b2a119e13d56d28a9175f9c Mon Sep 17 00:00:00 2001 From: Nikita Bobko Date: Wed, 2 Aug 2023 18:27:54 +0200 Subject: [PATCH] [FE] Type-safety refactoring: extract main logic of areCompatibleCallables into two functions Review: https://jetbrains.team/p/kt/reviews/11039/timeline Extract main logic of `areCompatibleCallables` into two functions: `areStrongIncompatibleCallables` and `areWeakIncompatibleCallables`. The main point is that `areStrongIncompatibleCallables` & `areWeakIncompatibleCallables` have very specific return types. This commit doesn't change any logic. The commit makes the API more type-safe ensuring that bugs like in previous commit (KT-60902) won't happen again --- .../FirExpectActualDeclarationChecker.kt | 5 +- .../ExpectedActualDeclarationChecker.kt | 5 +- ...bstractExpectActualCompatibilityChecker.kt | 69 ++++++++++------ .../ExpectActualCompatibility.kt | 81 +++++++++---------- 4 files changed, 85 insertions(+), 75 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectActualDeclarationChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectActualDeclarationChecker.kt index 209dcdaf03f..2b40156ae55 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectActualDeclarationChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirExpectActualDeclarationChecker.kt @@ -274,12 +274,11 @@ object FirExpectActualDeclarationChecker : FirBasicDeclarationChecker() { } fun Map, *>.allStrongIncompatibilities(): Boolean { - return keys.all { it is Incompatible && it.kind == IncompatibilityKind.STRONG } + return keys.all { it is Incompatible.StrongIncompatible } } private fun ExpectActualCompatibility>.isCompatibleOrWeakCompatible(): Boolean { - return this is Compatible || - this is Incompatible && kind == IncompatibilityKind.WEAK + return this is Compatible || this is Incompatible.WeakIncompatible } // we don't require `actual` modifier on diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt index d4a8cfef57c..5ea28b35bca 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/ExpectedActualDeclarationChecker.kt @@ -505,11 +505,10 @@ class ExpectedActualDeclarationChecker( companion object { fun Map, Collection>.allStrongIncompatibilities(): Boolean = - this.keys.all { it is Incompatible && it.kind == IncompatibilityKind.STRONG } + this.keys.all { it is Incompatible.StrongIncompatible } internal fun ExpectActualCompatibility.isCompatibleOrWeakCompatible() = - this is Compatible || - this is Incompatible && kind == IncompatibilityKind.WEAK + this is Compatible || this is Incompatible.WeakIncompatible } } diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt index 789bcc6ed31..96dff05dc11 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt @@ -298,8 +298,18 @@ object AbstractExpectActualCompatibilityChecker { return ExpectActualCompatibility.Compatible } - // ALL THE FOLLOWING ARE STRONG INCOMPATIBILITIES + // We must prioritize to return STRONG incompatible over WEAK incompatible (because STRONG incompatibility allows to search for overloads) + return getCallablesStrongIncompatibility(expectDeclaration, actualDeclaration, parentSubstitutor) + ?: getCallablesWeakIncompatibility(expectDeclaration, actualDeclaration, expectContainingClass, actualContainingClass) + ?: ExpectActualCompatibility.Compatible + } + context(ExpectActualMatchingContext<*>) + private fun getCallablesStrongIncompatibility( + expectDeclaration: CallableSymbolMarker, + actualDeclaration: CallableSymbolMarker, + parentSubstitutor: TypeSubstitutorMarker?, + ): Incompatible.StrongIncompatible<*>? { if (expectDeclaration is FunctionSymbolMarker != actualDeclaration is FunctionSymbolMarker) { return Incompatible.CallableKind } @@ -347,9 +357,22 @@ object AbstractExpectActualCompatibilityChecker { } } - areStrongIncompatibleTypeParameters(expectedTypeParameters, actualTypeParameters, substitutor)?.let { return it } + getTypeParametersStrongIncompatibility(expectedTypeParameters, actualTypeParameters, substitutor)?.let { return it } - // ALL THE FOLLOWING ARE WEAK INCOMPATIBILITIES + return null + } + + context(ExpectActualMatchingContext<*>) + private fun getCallablesWeakIncompatibility( + expectDeclaration: CallableSymbolMarker, + actualDeclaration: CallableSymbolMarker, + expectContainingClass: RegularClassSymbolMarker?, + actualContainingClass: RegularClassSymbolMarker?, + ): Incompatible.WeakIncompatible<*>? { + val expectedTypeParameters = expectDeclaration.typeParameters + val actualTypeParameters = actualDeclaration.typeParameters + val expectedValueParameters = expectDeclaration.valueParameters + val actualValueParameters = actualDeclaration.valueParameters if (actualDeclaration.hasStableParameterNames && !equalsBy(expectedValueParameters, actualValueParameters) { it.name }) { return Incompatible.ParameterNames @@ -376,7 +399,7 @@ object AbstractExpectActualCompatibilityChecker { return Incompatible.Visibility } - areWeakIncompatibleTypeParameters(expectedTypeParameters, actualTypeParameters)?.let { return it } + getTypeParametersWeakIncompatibility(expectedTypeParameters, actualTypeParameters)?.let { return it } if (shouldCheckAbsenceOfDefaultParamsInActual) { // "Default parameters in actual" check is required only for functions, because only functions can have parameters @@ -409,15 +432,11 @@ object AbstractExpectActualCompatibilityChecker { } when { - expectDeclaration is FunctionSymbolMarker && actualDeclaration is FunctionSymbolMarker -> areCompatibleFunctions( - expectDeclaration, - actualDeclaration - ).let { if (it != ExpectActualCompatibility.Compatible) return it } + expectDeclaration is FunctionSymbolMarker && actualDeclaration is FunctionSymbolMarker -> + getFunctionsIncompatibility(expectDeclaration, actualDeclaration)?.let { return it } - expectDeclaration is PropertySymbolMarker && actualDeclaration is PropertySymbolMarker -> areCompatibleProperties( - expectDeclaration, - actualDeclaration - ).let { if (it != ExpectActualCompatibility.Compatible) return it } + expectDeclaration is PropertySymbolMarker && actualDeclaration is PropertySymbolMarker -> + getPropertiesIncompatibility(expectDeclaration, actualDeclaration)?.let { return it } expectDeclaration is EnumEntrySymbolMarker && actualDeclaration is EnumEntrySymbolMarker -> { // do nothing, entries are matched only by name @@ -426,7 +445,7 @@ object AbstractExpectActualCompatibilityChecker { else -> error("Unsupported declarations: $expectDeclaration, $actualDeclaration") } - return ExpectActualCompatibility.Compatible + return null } context(ExpectActualMatchingContext<*>) @@ -539,16 +558,16 @@ object AbstractExpectActualCompatibilityChecker { substitutor: TypeSubstitutorMarker, ): ExpectActualCompatibility<*> = // We must prioritize to return STRONG incompatible over WEAK incompatible (because STRONG incompatibility allows to search for overloads) - areStrongIncompatibleTypeParameters(expectTypeParameterSymbols, actualTypeParameterSymbols, substitutor) - ?: areWeakIncompatibleTypeParameters(expectTypeParameterSymbols, actualTypeParameterSymbols) + getTypeParametersStrongIncompatibility(expectTypeParameterSymbols, actualTypeParameterSymbols, substitutor) + ?: getTypeParametersWeakIncompatibility(expectTypeParameterSymbols, actualTypeParameterSymbols) ?: ExpectActualCompatibility.Compatible context(ExpectActualMatchingContext<*>) - private fun areStrongIncompatibleTypeParameters( + private fun getTypeParametersStrongIncompatibility( expectTypeParameterSymbols: List, actualTypeParameterSymbols: List, substitutor: TypeSubstitutorMarker, - ): Incompatible<*>? { + ): Incompatible.StrongIncompatible<*>? { for (i in expectTypeParameterSymbols.indices) { val expectBounds = expectTypeParameterSymbols[i].bounds val actualBounds = actualTypeParameterSymbols[i].bounds @@ -564,10 +583,10 @@ object AbstractExpectActualCompatibilityChecker { } context(ExpectActualMatchingContext<*>) - private fun areWeakIncompatibleTypeParameters( + private fun getTypeParametersWeakIncompatibility( expectTypeParameterSymbols: List, actualTypeParameterSymbols: List, - ): Incompatible<*>? { + ): Incompatible.WeakIncompatible<*>? { if (!equalsBy(expectTypeParameterSymbols, actualTypeParameterSymbols) { it.variance }) { return Incompatible.TypeParameterVariance } @@ -585,10 +604,10 @@ object AbstractExpectActualCompatibilityChecker { } context(ExpectActualMatchingContext<*>) - private fun areCompatibleFunctions( + private fun getFunctionsIncompatibility( expectFunction: CallableSymbolMarker, actualFunction: CallableSymbolMarker, - ): ExpectActualCompatibility<*> { + ): Incompatible.WeakIncompatible<*>? { if (!equalBy(expectFunction, actualFunction) { f -> f.isSuspend }) { return Incompatible.FunctionModifiersDifferent } @@ -601,20 +620,20 @@ object AbstractExpectActualCompatibilityChecker { return Incompatible.FunctionModifiersNotSubset } - return ExpectActualCompatibility.Compatible + return null } context(ExpectActualMatchingContext<*>) - private fun areCompatibleProperties( + private fun getPropertiesIncompatibility( expected: PropertySymbolMarker, actual: PropertySymbolMarker, - ): ExpectActualCompatibility<*> { + ): Incompatible.WeakIncompatible<*>? { return when { !equalBy(expected, actual) { p -> p.isVar } -> Incompatible.PropertyKind !equalBy(expected, actual) { p -> p.isLateinit } -> Incompatible.PropertyLateinitModifier expected.isConst && !actual.isConst -> Incompatible.PropertyConstModifier !arePropertySettersWithCompatibleVisibilities(expected, actual) -> Incompatible.PropertySetterVisibility - else -> ExpectActualCompatibility.Compatible + else -> null } } diff --git a/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt b/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt index 5a4ed1a72e4..8475338142a 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectActualCompatibility.kt @@ -9,90 +9,83 @@ import kotlin.contracts.ExperimentalContracts import kotlin.contracts.contract sealed class ExpectActualCompatibility { - // For IncompatibilityKind.STRONG `actual` declaration is considered as overload and error reports on expected declaration - enum class IncompatibilityKind { - WEAK, STRONG - } // Note that the reason is used in the diagnostic output, see PlatformIncompatibilityDiagnosticRenderer - sealed class Incompatible( - val reason: String?, - val kind: IncompatibilityKind = IncompatibilityKind.WEAK - ) : ExpectActualCompatibility() { + sealed class Incompatible(val reason: String?) : ExpectActualCompatibility() { + + sealed class WeakIncompatible(reason: String?) : Incompatible(reason) + + // For StrongIncompatible `actual` declaration is considered as overload and error reports on expected declaration + sealed class StrongIncompatible(reason: String?) : Incompatible(reason) + // Callables - object CallableKind : Incompatible( - "callable kinds are different (function vs property)", - IncompatibilityKind.STRONG - ) + object CallableKind : StrongIncompatible("callable kinds are different (function vs property)") - object ParameterShape : Incompatible( - "parameter shapes are different (extension vs non-extension)", - IncompatibilityKind.STRONG - ) + object ParameterShape : StrongIncompatible("parameter shapes are different (extension vs non-extension)") - object ParameterCount : Incompatible("number of value parameters is different", IncompatibilityKind.STRONG) - object TypeParameterCount : Incompatible("number of type parameters is different", IncompatibilityKind.STRONG) + object ParameterCount : StrongIncompatible("number of value parameters is different") + object TypeParameterCount : StrongIncompatible("number of type parameters is different") - object ParameterTypes : Incompatible("parameter types are different", IncompatibilityKind.STRONG) - object ReturnType : Incompatible("return type is different", IncompatibilityKind.STRONG) + object ParameterTypes : StrongIncompatible("parameter types are different") + object ReturnType : StrongIncompatible("return type is different") - object ParameterNames : Incompatible("parameter names are different") - object TypeParameterNames : Incompatible("names of type parameters are different") + object ParameterNames : WeakIncompatible("parameter names are different") + object TypeParameterNames : WeakIncompatible("names of type parameters are different") - object ValueParameterVararg : Incompatible("some value parameter is vararg in one declaration and non-vararg in the other") - object ValueParameterNoinline : Incompatible( + object ValueParameterVararg : WeakIncompatible("some value parameter is vararg in one declaration and non-vararg in the other") + object ValueParameterNoinline : WeakIncompatible( "some value parameter is noinline in one declaration and not noinline in the other" ) - object ValueParameterCrossinline : Incompatible( + object ValueParameterCrossinline : WeakIncompatible( "some value parameter is crossinline in one declaration and not crossinline in the other" ) // Functions - object FunctionModifiersDifferent : Incompatible("modifiers are different (suspend)") - object FunctionModifiersNotSubset : Incompatible( + object FunctionModifiersDifferent : WeakIncompatible("modifiers are different (suspend)") + object FunctionModifiersNotSubset : WeakIncompatible( "some modifiers on expected declaration are missing on the actual one (infix, inline, operator)" ) object ActualFunctionWithDefaultParameters : - Incompatible("actual function cannot have default argument values, they should be declared in the expected function") + WeakIncompatible("actual function cannot have default argument values, they should be declared in the expected function") // Properties - object PropertyKind : Incompatible("property kinds are different (val vs var)") - object PropertyLateinitModifier : Incompatible("modifiers are different (lateinit)") - object PropertyConstModifier : Incompatible("modifiers are different (const)") - object PropertySetterVisibility : Incompatible("setter visibility is different") + object PropertyKind : WeakIncompatible("property kinds are different (val vs var)") + object PropertyLateinitModifier : WeakIncompatible("modifiers are different (lateinit)") + object PropertyConstModifier : WeakIncompatible("modifiers are different (const)") + object PropertySetterVisibility : WeakIncompatible("setter visibility is different") // Classifiers - object ClassKind : Incompatible("class kinds are different (class, interface, object, enum, annotation)") + object ClassKind : WeakIncompatible("class kinds are different (class, interface, object, enum, annotation)") - object ClassModifiers : Incompatible("modifiers are different (companion, inner, inline)") + object ClassModifiers : WeakIncompatible("modifiers are different (companion, inner, inline)") - object FunInterfaceModifier : Incompatible("actual declaration for fun expect interface is not a functional interface") + object FunInterfaceModifier : WeakIncompatible("actual declaration for fun expect interface is not a functional interface") - object Supertypes : Incompatible("some supertypes are missing in the actual declaration") + object Supertypes : WeakIncompatible("some supertypes are missing in the actual declaration") class ClassScopes( val unfulfilled: List, Collection>>> - ) : Incompatible("some expected members have no actual ones") + ) : WeakIncompatible("some expected members have no actual ones") - object EnumEntries : Incompatible("some entries from expected enum are missing in the actual enum") + object EnumEntries : WeakIncompatible("some entries from expected enum are missing in the actual enum") // Common - object Modality : Incompatible("modality is different") - object Visibility : Incompatible("visibility is different") + object Modality : WeakIncompatible("modality is different") + object Visibility : WeakIncompatible("visibility is different") - object TypeParameterUpperBounds : Incompatible("upper bounds of type parameters are different", IncompatibilityKind.STRONG) - object TypeParameterVariance : Incompatible("declaration-site variances of type parameters are different") - object TypeParameterReified : Incompatible( + object TypeParameterUpperBounds : StrongIncompatible("upper bounds of type parameters are different") + object TypeParameterVariance : WeakIncompatible("declaration-site variances of type parameters are different") + object TypeParameterReified : WeakIncompatible( "some type parameter is reified in one declaration and non-reified in the other" ) - object Unknown : Incompatible(null) + object Unknown : WeakIncompatible(null) } object Compatible : ExpectActualCompatibility()