From 5b72c127bdba8ad82011dee3cac3862f40f101b9 Mon Sep 17 00:00:00 2001 From: Nikita Bobko Date: Fri, 17 Nov 2023 12:35:34 +0100 Subject: [PATCH] [FIR, IR] Commonize isFakeOverride check for AbstractExpectActualAnnotationMatchChecker This commit fixes fakeOverrides.fir.kt test that got broken in the previous commit Previously isFakeOverride was only checked on IR backend. Now this check is moved to AbstractExpectActualAnnotationMatchChecker which is used by both: frontend and backend. That's why frontend no longer reports false positive ACTUAL_ANNOTATIONS_NOT_MATCH_EXPECT Review: https://jetbrains.team/p/kt/reviews/13094/timeline --- .../FirActualAnnotationsMatchExpectChecker.kt | 9 +++++++-- .../FirExpectActualDeclarationChecker.kt | 12 ++++++------ .../mpp/FirExpectActualMatchingContextImpl.kt | 9 +++++---- .../actualizer/IrExpectActualMatchingContext.kt | 4 ++-- .../IrExpectActualAnnotationMatchingChecker.kt | 9 ++------- .../AbstractExpectActualAnnotationMatchChecker.kt | 13 +++++++++---- .../calls/mpp/ExpectActualMatchingContext.kt | 8 +------- .../fakeOverrides.fir.kt | 2 +- .../fakeOverrides.ll.kt | 2 +- 9 files changed, 34 insertions(+), 34 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirActualAnnotationsMatchExpectChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirActualAnnotationsMatchExpectChecker.kt index 018f0056667..b65fbed09b1 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirActualAnnotationsMatchExpectChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirActualAnnotationsMatchExpectChecker.kt @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.fir.declarations.utils.isActual import org.jetbrains.kotlin.fir.expectActualMatchingContextFactory import org.jetbrains.kotlin.fir.expressions.FirAnnotation import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.resolve.calls.mpp.AbstractExpectActualAnnotationMatchChecker /** @@ -33,19 +34,23 @@ internal object FirActualAnnotationsMatchExpectChecker : FirBasicDeclarationChec val actualSymbol = declaration.symbol val expectSymbol = actualSymbol.getSingleMatchedExpectForActualOrNull() ?: return - checkAnnotationsMatch(expectSymbol, actualSymbol, context, reporter) + + val actualContainingClass = context.containingDeclarations.lastOrNull()?.symbol as? FirRegularClassSymbol + val expectContainingClass = actualContainingClass?.getSingleMatchedExpectForActualOrNull() as? FirRegularClassSymbol + checkAnnotationsMatch(expectSymbol, actualSymbol, expectContainingClass, context, reporter) } @OptIn(InternalDiagnosticFactoryMethod::class) private fun checkAnnotationsMatch( expectSymbol: FirBasedSymbol<*>, actualSymbol: FirBasedSymbol<*>, + expectContainingClass: FirRegularClassSymbol?, context: CheckerContext, reporter: DiagnosticReporter, ) { val matchingContext = context.session.expectActualMatchingContextFactory.create(context.session, context.scopeSession) val incompatibility = - AbstractExpectActualAnnotationMatchChecker.areAnnotationsCompatible(expectSymbol, actualSymbol, matchingContext) ?: return + AbstractExpectActualAnnotationMatchChecker.areAnnotationsCompatible(expectSymbol, actualSymbol, expectContainingClass, matchingContext) ?: return val actualAnnotationTargetSourceElement = (incompatibility.actualAnnotationTargetElement as FirSourceElement).element reporter.report( 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 467feff92c2..c565948bac7 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 @@ -128,13 +128,13 @@ object FirExpectActualDeclarationChecker : FirBasicDeclarationChecker() { val matchingCompatibilityToMembersMap = symbol.expectForActual ?: return val expectedSingleCandidate = matchingCompatibilityToMembersMap[ExpectActualMatchingCompatibility.MatchedSuccessfully]?.singleOrNull() + val expectActualMatchingContext = context.session.expectActualMatchingContextFactory.create( + context.session, context.scopeSession, + allowedWritingMemberExpectForActualMapping = true, + ) + val actualContainingClass = context.containingDeclarations.lastOrNull()?.symbol as? FirRegularClassSymbol + val expectContainingClass = actualContainingClass?.getSingleMatchedExpectForActualOrNull() as? FirRegularClassSymbol val checkingCompatibility = if (expectedSingleCandidate != null) { - val expectActualMatchingContext = context.session.expectActualMatchingContextFactory.create( - context.session, context.scopeSession, - allowedWritingMemberExpectForActualMapping = true, - ) - val actualContainingClass = context.containingDeclarations.lastOrNull()?.symbol as? FirRegularClassSymbol - val expectContainingClass = actualContainingClass?.getSingleMatchedExpectForActualOrNull() as? FirRegularClassSymbol getCheckingCompatibility( symbol, expectedSingleCandidate, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt index 944a905f3b2..ad838911c0f 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt @@ -363,15 +363,16 @@ class FirExpectActualMatchingContextImpl private constructor( return !isSam } - override fun CallableSymbolMarker.shouldSkipMatching(containingExpectClass: RegularClassSymbolMarker): Boolean { + override fun CallableSymbolMarker.isFakeOverride(containingExpectClass: RegularClassSymbolMarker?): Boolean { + if (containingExpectClass == null) { + return false + } val symbol = asSymbol() val classSymbol = containingExpectClass.asSymbol() if (symbol !is FirConstructorSymbol && symbol.dispatchReceiverType?.classId != classSymbol.classId) { - // Skip fake overrides return true } - return symbol.isSubstitutionOrIntersectionOverride // Skip fake overrides - || !symbol.isExpect // Skip non-expect declarations like equals, hashCode, toString and any inherited declarations from non-expect super types + return symbol.isSubstitutionOrIntersectionOverride } override val CallableSymbolMarker.hasStableParameterNames: Boolean diff --git a/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt b/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt index 9f0bd003793..a6c8d0e16c9 100644 --- a/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt +++ b/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt @@ -454,8 +454,8 @@ internal abstract class IrExpectActualMatchingContext( return !asIr().isFun } - override fun CallableSymbolMarker.shouldSkipMatching(containingExpectClass: RegularClassSymbolMarker): Boolean { - return false + override fun CallableSymbolMarker.isFakeOverride(containingExpectClass: RegularClassSymbolMarker?): Boolean { + return asIr().isFakeOverride } override val CallableSymbolMarker.hasStableParameterNames: Boolean diff --git a/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/checker/IrExpectActualAnnotationMatchingChecker.kt b/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/checker/IrExpectActualAnnotationMatchingChecker.kt index 77a92d38b88..e71162f93f1 100644 --- a/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/checker/IrExpectActualAnnotationMatchingChecker.kt +++ b/compiler/ir/ir.actualization/src/main/kotlin/org/jetbrains/kotlin/backend/common/actualizer/checker/IrExpectActualAnnotationMatchingChecker.kt @@ -25,13 +25,8 @@ internal object IrExpectActualAnnotationMatchingChecker : IrExpectActualChecker if (expectSymbol is IrTypeParameterSymbol) { continue } - // If the expect declaration is fake-override and is incompatible, then it means that it was overridden on actual. - // In such case, regular rules for annotations on overridden declarations apply. - if (expectSymbol.isFakeOverride) { - continue - } - val incompatibility = - AbstractExpectActualAnnotationMatchChecker.areAnnotationsCompatible(expectSymbol, actualSymbol, matchingContext) ?: continue + val incompatibility = AbstractExpectActualAnnotationMatchChecker + .areAnnotationsCompatible(expectSymbol, actualSymbol, containingExpectClass = null, matchingContext) ?: continue val reportOn = getTypealiasSymbolIfActualizedViaTypealias(expectSymbol.owner as IrDeclaration, classActualizationInfo) ?: getContainingActualClassIfFakeOverride(actualSymbol) diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt index d83e4c2d6e8..53e3da7c786 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt @@ -11,7 +11,6 @@ import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.name.StandardClassIds import org.jetbrains.kotlin.resolve.checkers.OptInNames -import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualCompatibility import org.jetbrains.kotlin.utils.zipIfSizesAreEqual import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualMatchingCompatibility import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualAnnotationsIncompatibilityType as IncompatibilityType @@ -53,19 +52,21 @@ object AbstractExpectActualAnnotationMatchChecker { fun areAnnotationsCompatible( expectSymbol: DeclarationSymbolMarker, actualSymbol: DeclarationSymbolMarker, + containingExpectClass: RegularClassSymbolMarker?, // Only necessary for the frontend. IR doesn't use it context: ExpectActualMatchingContext<*>, ): Incompatibility? = with(context) { - areAnnotationsCompatible(expectSymbol, actualSymbol) + areAnnotationsCompatible(expectSymbol, actualSymbol, containingExpectClass) } context (ExpectActualMatchingContext<*>) private fun areAnnotationsCompatible( expectSymbol: DeclarationSymbolMarker, actualSymbol: DeclarationSymbolMarker, + containingExpectClass: RegularClassSymbolMarker?, ): Incompatibility? { return when (expectSymbol) { is CallableSymbolMarker -> { - areCallableAnnotationsCompatible(expectSymbol, actualSymbol as CallableSymbolMarker) + areCallableAnnotationsCompatible(expectSymbol, actualSymbol as CallableSymbolMarker, containingExpectClass) } is RegularClassSymbolMarker -> { areClassAnnotationsCompatible(expectSymbol, actualSymbol as ClassLikeSymbolMarker) @@ -78,7 +79,11 @@ object AbstractExpectActualAnnotationMatchChecker { private fun areCallableAnnotationsCompatible( expectSymbol: CallableSymbolMarker, actualSymbol: CallableSymbolMarker, + containingExpectClass: RegularClassSymbolMarker?, ): Incompatibility? { + // If the expect declaration is fake-override and is incompatible, then it means that it was overridden on actual. + // In such case, regular rules for annotations on overridden declarations apply. + if (expectSymbol.isFakeOverride(containingExpectClass)) return null commonForClassAndCallableChecks(expectSymbol, actualSymbol)?.let { return it } areAnnotationsOnValueParametersCompatible(expectSymbol, actualSymbol)?.let { return it } @@ -316,7 +321,7 @@ object AbstractExpectActualAnnotationMatchChecker { // Check also incompatible members if only one is found ?: expectToCompatibilityMap.keys.singleOrNull() ?: continue - areAnnotationsCompatible(expectMember, actualMember)?.let { return it } + areAnnotationsCompatible(expectMember, actualMember, expectClass)?.let { return it } } return null } diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt index e6e702ddb79..294a186c385 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt @@ -156,13 +156,7 @@ interface ExpectActualMatchingContext : TypeSystemC fun RegularClassSymbolMarker.isNotSamInterface(): Boolean - /* - * Determines should some declaration from expect class scope be checked - * - FE 1.0: skip fake overrides - * - FIR: skip fake overrides - * - IR: skip nothing - */ - fun CallableSymbolMarker.shouldSkipMatching(containingExpectClass: RegularClassSymbolMarker): Boolean + fun CallableSymbolMarker.isFakeOverride(containingExpectClass: RegularClassSymbolMarker?): Boolean val CallableSymbolMarker.hasStableParameterNames: Boolean diff --git a/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.fir.kt index 23ce12502c4..7d5775e79c2 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.fir.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.fir.kt @@ -20,7 +20,7 @@ expect class FakeOverrideActual : I { // MODULE: m1-jvm()()(m1-common) // FILE: jvm.kt -actual class FakeOverrideExpect : A() { +actual class FakeOverrideExpect : A() { override fun noAnnotationOnActual() {} } diff --git a/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.ll.kt b/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.ll.kt index a0b208ac00f..7fa00c05328 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.ll.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/actualAnnotationsNotMatchExpect/fakeOverrides.ll.kt @@ -20,7 +20,7 @@ expect class FakeOverrideActual : I { // MODULE: m1-jvm()()(m1-common) // FILE: jvm.kt -actual class FakeOverrideExpect : A() { +actual class FakeOverrideExpect : A() { override fun noAnnotationOnActual() {} }