From 0325ffaa011890afea0b0dc57e48a8516aeb6d42 Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Thu, 19 Oct 2023 17:10:56 +0300 Subject: [PATCH] [FIR] Properly handle nested annotations on compiler-required annotation phase There was a case when we visited the same declaration multiple times because of forward-referencing and visiting with designation So because of this there were two changes required: - remove assertion about visiting the same declaration twice (it's fine since we don't actually resolve annotations twice) - not skipping resolution of class children if annotations on this class are already resolved ^KT-61388 Fixed KT-62854 --- ...cCompilerTestFirTestdataTestGenerated.java | 6 ++ ...osticCompilerFirTestDataTestGenerated.java | 6 ++ ...daCompilerRequiredNestedAnnotation.fir.txt | 28 +++++++++ .../kindaCompilerRequiredNestedAnnotation.kt | 13 +++++ .../FirLightTreeDiagnosticsTestGenerated.java | 6 ++ .../FirPsiDiagnosticTestGenerated.java | 6 ++ ...erRequiredAnnotationsResolveTransformer.kt | 57 +++++++++++++++++-- 7 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.fir.txt create mode 100644 compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFirTestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFirTestdataTestGenerated.java index 59afdaf4306..ab942875d20 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFirTestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFirTestdataTestGenerated.java @@ -767,6 +767,12 @@ public class DiagnosticCompilerTestFirTestdataTestGenerated extends AbstractDiag KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/resolve/annotations"), Pattern.compile("^([^.]+)\\.kt$"), null, true); } + @Test + @TestMetadata("kindaCompilerRequiredNestedAnnotation.kt") + public void testKindaCompilerRequiredNestedAnnotation() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt"); + } + @Test @TestMetadata("kt43936.kt") public void testKt43936() throws Exception { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFirTestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFirTestDataTestGenerated.java index df80514aec2..e0456dbfd58 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFirTestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFirTestDataTestGenerated.java @@ -767,6 +767,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFirTestDataTestGenerated KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/resolve/annotations"), Pattern.compile("^([^.]+)\\.kt$"), null, true); } + @Test + @TestMetadata("kindaCompilerRequiredNestedAnnotation.kt") + public void testKindaCompilerRequiredNestedAnnotation() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt"); + } + @Test @TestMetadata("kt43936.kt") public void testKt43936() throws Exception { diff --git a/compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.fir.txt b/compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.fir.txt new file mode 100644 index 00000000000..80b47348743 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.fir.txt @@ -0,0 +1,28 @@ +FILE: a.kt + @R|SinceKotlin|() public final class Some : R|kotlin/Any| { + public constructor(): R|Some| { + super() + } + + } +FILE: b.kt + @R|SinceKotlin.SinceKotlin|() public final class Other : R|kotlin/Any| { + public constructor(): R|Other| { + super() + } + + } +FILE: SinceKotlin.kt + public final annotation class SinceKotlin : R|kotlin/Annotation| { + public constructor(): R|SinceKotlin| { + super() + } + + public final annotation class SinceKotlin : R|kotlin/Annotation| { + public constructor(): R|SinceKotlin.SinceKotlin| { + super() + } + + } + + } diff --git a/compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt b/compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt new file mode 100644 index 00000000000..7a264760f9d --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt @@ -0,0 +1,13 @@ +// ISSUE: KT-61388 +// FILE: a.kt +@SinceKotlin +class Some + +// FILE: b.kt +@SinceKotlin.SinceKotlin +class Other + +// FILE: SinceKotlin.kt +annotation class SinceKotlin { + annotation class SinceKotlin +} diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeDiagnosticsTestGenerated.java index a2bc2e4cd9f..aa572e49e07 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeDiagnosticsTestGenerated.java @@ -767,6 +767,12 @@ public class FirLightTreeDiagnosticsTestGenerated extends AbstractFirLightTreeDi KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/resolve/annotations"), Pattern.compile("^([^.]+)\\.kt$"), null, true); } + @Test + @TestMetadata("kindaCompilerRequiredNestedAnnotation.kt") + public void testKindaCompilerRequiredNestedAnnotation() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt"); + } + @Test @TestMetadata("kt43936.kt") public void testKt43936() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiDiagnosticTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiDiagnosticTestGenerated.java index 85df81acfb7..aff4944958c 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiDiagnosticTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiDiagnosticTestGenerated.java @@ -767,6 +767,12 @@ public class FirPsiDiagnosticTestGenerated extends AbstractFirPsiDiagnosticTest KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/fir/analysis-tests/testData/resolve/annotations"), Pattern.compile("^([^.]+)\\.kt$"), null, true); } + @Test + @TestMetadata("kindaCompilerRequiredNestedAnnotation.kt") + public void testKindaCompilerRequiredNestedAnnotation() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/annotations/kindaCompilerRequiredNestedAnnotation.kt"); + } + @Test @TestMetadata("kt43936.kt") public void testKt43936() throws Exception { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/plugin/FirCompilerRequiredAnnotationsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/plugin/FirCompilerRequiredAnnotationsResolveTransformer.kt index 6e17a34b442..ce9d706fe6e 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/plugin/FirCompilerRequiredAnnotationsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/plugin/FirCompilerRequiredAnnotationsResolveTransformer.kt @@ -31,10 +31,56 @@ import org.jetbrains.kotlin.fir.visitors.transformSingle import org.jetbrains.kotlin.fir.withFileAnalysisExceptionWrapping import org.jetbrains.kotlin.name.FqName + +/** + * Little explanation the logic of this phase + * + * - this transformer visits all declarations and tries to resolve each annotation that looks like compiler-required by short name + * - if there are any meta-annotations registered by plugins, then each annotation is considered as "potentially compiler-required" + * - transformer resolves: + * - annotation types + * - annotation call arguments, if annotation classId mentioned in FirAnnotationsPlatformSpecificSupportComponent.requiredAnnotationsWithArguments + * - if annotation is considered as compiler-required after the resolution, the resolved type is saved in the annotation with + * FirAnnotationResolvePhase.CompilerRequiredAnnotations phase + * - if annotation potentially can be annotated with meta-annotation, the phase performs a designated jump to corresponding annotation class + * to resolve annotations on it. This jump resolves annotations only on last declaration from designation path + * + * Example: + * ``` + * // FILE: plugin.kt + * annotation class MetaAnnotation + * + * // FILE: a.kt + * @A // (1) + * class Some + * + * // FILE: b.kt + * @A.B // (2) + * class Other + * + * // FILE: SinceKotlin.kt + * @MetaAnnotation // (3) + * annotation class A { + * @MetaAnnotation // (4) + * annotation class B + * + * @MetaAnnotation // (5) + * annotation class C + * } + * ``` + * + * 1. visit class `Some`, resolve (1) + * 2. there are meta-annotations -> jump to class `A` and resolve (3) + * 3. visit class `Other`, resolved (2) + * 4. there are meta-annotations -> jump to class `A.B` and resolve (4) + * 5. visit class `A`, skip annotations on `A` since they are already resolved + * 6. visit class `A.B`, skip annotations on `A.B` since they are already resolved + * 7. visit class `A.C`, resolve (5) + */ class FirCompilerRequiredAnnotationsResolveProcessor( session: FirSession, scopeSession: ScopeSession -) : FirGlobalResolveProcessor(session, scopeSession, FirResolvePhase.COMPILER_REQUIRED_ANNOTATIONS) { +) : FirGlobalResolveProcessor(session, scopeSession, COMPILER_REQUIRED_ANNOTATIONS) { override fun process(files: Collection) { val computationSession = CompilerRequiredAnnotationsComputationSession() @@ -150,9 +196,7 @@ open class CompilerRequiredAnnotationsComputationSession { } fun recordThatAnnotationsAreResolved(declaration: FirAnnotationContainer) { - if (!declarationsWithResolvedAnnotations.add(declaration)) { - error("Annotations are resolved twice") - } + declarationsWithResolvedAnnotations.add(declaration) } fun resolveAnnotationsOnAnnotationIfNeeded(symbol: FirRegularClassSymbol, scopeSession: ScopeSession) { @@ -198,6 +242,11 @@ class FirSpecificAnnotationResolveTransformer( computationSession: CompilerRequiredAnnotationsComputationSession ) : AbstractFirSpecificAnnotationResolveTransformer(session, scopeSession, computationSession) { override fun shouldTransformDeclaration(declaration: FirDeclaration): Boolean { + /* + * Even if annotations on class are resolved, annotations on nested declarations might be not resolved yet + * It may happen if we visited a top-level class with designated transformer with this class as target of designation + */ + if (declaration is FirRegularClass) return true @OptIn(PrivateForInline::class) return !computationSession.annotationsAreResolved(declaration, treatNonSourceDeclarationsAsResolved = true) }