From 0fd700de21b9d99246f2d244c19dc1df0fa4f548 Mon Sep 17 00:00:00 2001 From: Roman Efremov Date: Wed, 20 Sep 2023 11:09:51 +0200 Subject: [PATCH] [FIR] Fix case with lazy resolve in expect-actual annotation checker Unresolved annotation arguments were treated as absent arguments, which lead to false-positive reports. Add assert and test for that and fix. MR: KT-MR-12245 ^KT-60671 Fixed --- ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ ...endMPPDiagnosticsWithPsiTestGenerated.java | 6 +++ .../mpp/FirExpectActualMatchingContextImpl.kt | 42 +++++++++++++++---- .../IrExpectActualMatchingContext.kt | 4 +- ...tractExpectActualAnnotationMatchChecker.kt | 5 ++- .../calls/mpp/ExpectActualMatchingContext.kt | 6 ++- ...pectActualAnnotationsWithLazyResolve.ll.kt | 6 ++- ...pealiasToJavaWithAnnotationArgument.fir.kt | 19 +++++++++ .../typealiasToJavaWithAnnotationArgument.kt | 19 +++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ 10 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.fir.kt create mode 100644 compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java index bf7a1dbdd97..2f6e51756e9 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java @@ -1652,6 +1652,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/sourceRetentionAnnotationsWhenTypealias.kt"); } + @Test + @TestMetadata("typealiasToJavaWithAnnotationArgument.kt") + public void testTypealiasToJavaWithAnnotationArgument() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt"); + } + @Test @TestMetadata("typealiasedAnnotation.kt") public void testTypealiasedAnnotation() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java index 9f8eb73d032..90e4fd05920 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java @@ -1652,6 +1652,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/sourceRetentionAnnotationsWhenTypealias.kt"); } + @Test + @TestMetadata("typealiasToJavaWithAnnotationArgument.kt") + public void testTypealiasToJavaWithAnnotationArgument() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt"); + } + @Test @TestMetadata("typealiasedAnnotation.kt") public void testTypealiasedAnnotation() throws Exception { 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 6f92dbcd9c9..56c4c5e6eff 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 @@ -11,14 +11,13 @@ import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.fir.* import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.isActual -import org.jetbrains.kotlin.fir.expressions.FirAnnotation -import org.jetbrains.kotlin.fir.expressions.FirConstExpression -import org.jetbrains.kotlin.fir.expressions.FirExpression +import org.jetbrains.kotlin.fir.expressions.* import org.jetbrains.kotlin.fir.resolve.* import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor import org.jetbrains.kotlin.fir.scopes.* import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol import org.jetbrains.kotlin.fir.symbols.impl.* +import org.jetbrains.kotlin.fir.symbols.resolvedAnnotationsWithArguments import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.mpp.* import org.jetbrains.kotlin.name.CallableId @@ -364,6 +363,13 @@ class FirExpectActualMatchingContextImpl private constructor( } private fun areFirAnnotationsEqual(annotation1: FirAnnotation, annotation2: FirAnnotation): Boolean { + fun FirAnnotation.hasResolvedArguments(): Boolean { + return resolved || (this is FirAnnotationCall && arguments.isEmpty()) + } + + check(annotation1.hasResolvedArguments() && annotation2.hasResolvedArguments()) { + "By this time compared annotations are expected to have resolved arguments" + } if (!areCompatibleExpectActualTypes(annotation1.resolvedType, annotation2.resolvedType)) { return false } @@ -503,25 +509,37 @@ class FirExpectActualMatchingContextImpl private constructor( override fun TypeRefMarker.getClassId(): ClassId? = (this as FirResolvedTypeRef).type.fullyExpandedType(actualSession).classId override fun checkAnnotationsOnTypeRefAndArguments( + expectContainingSymbol: DeclarationSymbolMarker, + actualContainingSymbol: DeclarationSymbolMarker, expectTypeRef: TypeRefMarker, actualTypeRef: TypeRefMarker, checker: ExpectActualMatchingContext.AnnotationsCheckerCallback, ) { check(expectTypeRef is FirResolvedTypeRef && actualTypeRef is FirResolvedTypeRef) - checkAnnotationsOnTypeRefAndArgumentsImpl(expectTypeRef, actualTypeRef, checker) + checkAnnotationsOnTypeRefAndArgumentsImpl( + expectContainingSymbol.asSymbol(), actualContainingSymbol.asSymbol(), + expectTypeRef, actualTypeRef, checker + ) } private fun checkAnnotationsOnTypeRefAndArgumentsImpl( + expectContainingSymbol: FirBasedSymbol<*>, + actualContainingSymbol: FirBasedSymbol<*>, expectTypeRef: FirTypeRef?, actualTypeRef: FirTypeRef?, checker: ExpectActualMatchingContext.AnnotationsCheckerCallback, ) { - fun FirAnnotationContainer.getAnnotations() = annotations.map(::AnnotationCallInfoImpl) + fun FirAnnotationContainer.getAnnotations(anchor: FirBasedSymbol<*>): List { + return resolvedAnnotationsWithArguments(anchor).map(::AnnotationCallInfoImpl) + } if (expectTypeRef == null || actualTypeRef == null) return if (expectTypeRef is FirErrorTypeRef || actualTypeRef is FirErrorTypeRef) return - checker.check(expectTypeRef.getAnnotations(), actualTypeRef.getAnnotations(), FirSourceElement(actualTypeRef.source)) + checker.check( + expectTypeRef.getAnnotations(expectContainingSymbol), actualTypeRef.getAnnotations(actualContainingSymbol), + FirSourceElement(actualTypeRef.source) + ) val expectDelegatedTypeRef = (expectTypeRef as? FirResolvedTypeRef)?.delegatedTypeRef ?: return val actualDelegatedTypeRef = (actualTypeRef as? FirResolvedTypeRef?)?.delegatedTypeRef ?: return @@ -538,22 +556,30 @@ class FirExpectActualMatchingContextImpl private constructor( if (expectTypeArgument !is FirTypeProjectionWithVariance || actualTypeArgument !is FirTypeProjectionWithVariance) { continue } - checkAnnotationsOnTypeRefAndArgumentsImpl(expectTypeArgument.typeRef, actualTypeArgument.typeRef, checker) + checkAnnotationsOnTypeRefAndArgumentsImpl( + expectContainingSymbol, actualContainingSymbol, + expectTypeArgument.typeRef, actualTypeArgument.typeRef, checker + ) } } } expectDelegatedTypeRef is FirFunctionTypeRef && actualDelegatedTypeRef is FirFunctionTypeRef -> { checkAnnotationsOnTypeRefAndArgumentsImpl( + expectContainingSymbol, actualContainingSymbol, expectDelegatedTypeRef.receiverTypeRef, actualDelegatedTypeRef.receiverTypeRef, checker, ) checkAnnotationsOnTypeRefAndArgumentsImpl( + expectContainingSymbol, actualContainingSymbol, expectDelegatedTypeRef.returnTypeRef, actualDelegatedTypeRef.returnTypeRef, checker, ) val expectParams = expectDelegatedTypeRef.parameters val actualParams = actualDelegatedTypeRef.parameters for ((expectParam, actualParam) in expectParams.zipIfSizesAreEqual(actualParams).orEmpty()) { - checkAnnotationsOnTypeRefAndArgumentsImpl(expectParam.returnTypeRef, actualParam.returnTypeRef, checker) + checkAnnotationsOnTypeRefAndArgumentsImpl( + expectContainingSymbol, actualContainingSymbol, + expectParam.returnTypeRef, actualParam.returnTypeRef, checker + ) } } } diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt index 0ff8188713c..bd7103b43e9 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt @@ -544,6 +544,8 @@ internal abstract class IrExpectActualMatchingContext( override fun TypeRefMarker.getClassId(): ClassId? = (this as IrType).getClass()?.classId override fun checkAnnotationsOnTypeRefAndArguments( + expectContainingSymbol: DeclarationSymbolMarker, + actualContainingSymbol: DeclarationSymbolMarker, expectTypeRef: TypeRefMarker, actualTypeRef: TypeRefMarker, checker: ExpectActualMatchingContext.AnnotationsCheckerCallback @@ -560,7 +562,7 @@ internal abstract class IrExpectActualMatchingContext( for ((expectArg, actualArg) in expectTypeRef.arguments.zip(actualTypeRef.arguments)) { val expectArgType = expectArg.typeOrNull ?: continue val actualArgType = actualArg.typeOrNull ?: continue - checkAnnotationsOnTypeRefAndArguments(expectArgType, actualArgType, checker) + checkAnnotationsOnTypeRefAndArguments(expectContainingSymbol, actualContainingSymbol, expectArgType, actualArgType, checker) } } } 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 18d64bf6f9f..ff48164ef0d 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 @@ -232,7 +232,10 @@ object AbstractExpectActualAnnotationMatchChecker { var firstIncompatibility: Incompatibility? = null - checkAnnotationsOnTypeRefAndArguments(expectTypeRef, actualTypeRef) { expectAnnotations, actualAnnotations, actualTypeRefSource -> + checkAnnotationsOnTypeRefAndArguments( + expectDeclarationSymbol, actualDeclarationSymbol, + expectTypeRef, actualTypeRef + ) { expectAnnotations, actualAnnotations, actualTypeRefSource -> if (firstIncompatibility == null) { firstIncompatibility = areAnnotationListsCompatible(expectAnnotations, actualAnnotations, actualDeclarationSymbol) ?.let { Incompatibility(expectDeclarationSymbol, actualDeclarationSymbol, actualTypeRefSource, type = it) } 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 99859b8369e..24da91a390c 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 @@ -248,6 +248,10 @@ interface ExpectActualMatchingContext : TypeSystemC * **Example**: for type `@Ann1 List<@Ann2 Map<@Ann3 Int, @Ann4 String>>`, there are 4 types to check in [checker]. */ fun checkAnnotationsOnTypeRefAndArguments( - expectTypeRef: TypeRefMarker, actualTypeRef: TypeRefMarker, checker: AnnotationsCheckerCallback, + expectContainingSymbol: DeclarationSymbolMarker, + actualContainingSymbol: DeclarationSymbolMarker, + expectTypeRef: TypeRefMarker, + actualTypeRef: TypeRefMarker, + checker: AnnotationsCheckerCallback, ) } diff --git a/compiler/testData/diagnostics/tests/multimodule/expectActualAnnotationsWithLazyResolve.ll.kt b/compiler/testData/diagnostics/tests/multimodule/expectActualAnnotationsWithLazyResolve.ll.kt index 45498814753..1d70085ee82 100644 --- a/compiler/testData/diagnostics/tests/multimodule/expectActualAnnotationsWithLazyResolve.ll.kt +++ b/compiler/testData/diagnostics/tests/multimodule/expectActualAnnotationsWithLazyResolve.ll.kt @@ -1,5 +1,7 @@ // LL_FIR_DIVERGENCE -// The reason is bug, which will be fixed in next commit +// Difference to FIR is false-positive report of MISSING_DEPENDENCY_SUPERCLASS. +// This is because JVM Enum with `java.io.Serializable` superclass resolved instead of common builtin Enum. +// Must be fixed with KT-61757. // LL_FIR_DIVERGENCE // Test for ACTUAL_ANNOTATIONS_NOT_MATCH_EXPECT diagnostic when annotations arguments are lazily resolved. @@ -25,7 +27,7 @@ expect fun withEmptyArguments_positive() // MODULE: main()()(common) // TARGET_PLATFORM: JVM -actual fun onType_negative(): @Ann("") Any = Any() +actual fun onType_negative(): @Ann("") Any = Any() actual fun onType_positive(): @Ann("incorrect") Any = Any() @Ann("") diff --git a/compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.fir.kt new file mode 100644 index 00000000000..22ab5dd7bc4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.fir.kt @@ -0,0 +1,19 @@ +// This test is for the case when expect annotation is FirAnnotationCall and actual annotation is not FirAnnotationCall +// MODULE: common +// TARGET_PLATFORM: Common +expect annotation class Ann(val p: Int) + +@Ann(p = 1) +expect class Foo + +// MODULE: main()()(common) +// TARGET_PLATFORM: JVM +// FILE: Foo.kt +actual annotation class Ann actual constructor(actual val p: Int) + +actual typealias Foo = FooImpl + +// FILE: FooImpl.java +@Ann(p = 2) +public class FooImpl { +} diff --git a/compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt b/compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt new file mode 100644 index 00000000000..f4ed8d806be --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt @@ -0,0 +1,19 @@ +// This test is for the case when expect annotation is FirAnnotationCall and actual annotation is not FirAnnotationCall +// MODULE: common +// TARGET_PLATFORM: Common +expect annotation class Ann(val p: Int) + +@Ann(p = 1) +expect class Foo + +// MODULE: main()()(common) +// TARGET_PLATFORM: JVM +// FILE: Foo.kt +actual annotation class Ann actual constructor(actual val p: Int) + +actual typealias Foo = FooImpl + +// FILE: FooImpl.java +@Ann(p = 2) +public class FooImpl { +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 7b7c55eb266..917af0a3304 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -24833,6 +24833,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/sourceRetentionAnnotationsWhenTypealias.kt"); } + @Test + @TestMetadata("typealiasToJavaWithAnnotationArgument.kt") + public void testTypealiasToJavaWithAnnotationArgument() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/hmpp/multiplatformCompositeAnalysis/annotationMatching/typealiasToJavaWithAnnotationArgument.kt"); + } + @Test @TestMetadata("typealiasedAnnotation.kt") public void testTypealiasedAnnotation() throws Exception {