FIR: convert Java type parameter bounds before reading annotations

This avoids a crash due to circular class references through annotation
arguments.

compiler/fir/analysis-tests/legacy-fir-tests/tests-gen/org/jetbrains/kotlin/fir/java/FirTypeEnhancementTestGenerated.java

@@ -224,6 +224,11 @@ public class FirTypeEnhancementTestGenerated extends AbstractFirTypeEnhancementT
         runTest("compiler/testData/loadJava/compiledJava/RecursiveWildcardUpperBound.java");
     }
 
+    @TestMetadata("ReferenceCycleThroughAnnotation.java")
+    public void testReferenceCycleThroughAnnotation() throws Exception {
+        runTest("compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java");
+    }
+
     @TestMetadata("RemoveRedundantProjectionKind.java")
     public void testRemoveRedundantProjectionKind() throws Exception {
         runTest("compiler/testData/loadJava/compiledJava/RemoveRedundantProjectionKind.java");

compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaFacade.kt

@@ -164,9 +164,27 @@ abstract class FirJavaFacade(
         parentClassTypeParameterStackCache.remove(classSymbol)
         parentClassEffectiveVisibilityCache.remove(classSymbol)
 
-        // There's a bit of an ordering restriction here:
-        // 1. annotations should be added after the symbol is bound, as annotations can refer to the class itself;
-        // 2. type enhancement requires annotations to be already present (and supertypes can refer to type parameters).
+        // This is where the problems begin. We need to enhance nullability of super types and type parameter bounds,
+        // for which we need the annotations of this class as they may specify default nullability.
+        // However, all three - annotations, type parameter bounds, and supertypes - can refer to other classes,
+        // which will cause the type parameter bounds and supertypes of *those* classes to get enhanced first,
+        // but they may refer back to this class again - which, thanks to the magic of symbol resolver caches,
+        // will be observed in a state where we've not done the enhancement yet. For those cases, we must publish
+        // at least unenhanced resolved types, or else FIR may crash upon encountering a FirJavaTypeRef where FirResolvedTypeRef
+        // is expected.
+        // TODO: some (all?) of those loops can be avoided, e.g. we don't actually need to resolve class arguments of annotations
+        //   to determine whether they set default nullability - but without laziness, breaking those loops is somewhat hard,
+        //   as we have a nested ordering here.
+        for (typeParameter in firJavaClass.typeParameters) {
+            if (typeParameter is FirTypeParameter) {
+                typeParameter.replaceBounds(typeParameter.bounds.map {
+                    it.resolveIfJavaType(session, javaTypeParameterStack, FirJavaTypeConversionMode.TYPE_PARAMETER_BOUND)
+                })
+            }
+        }
+        // 1. Resolve annotations
+        // 2. Enhance type parameter bounds - may refer to each other, take default nullability from annotations
+        // 3. Enhance super types - may refer to type parameter bounds, take default nullability from annotations
         firJavaClass.annotations.addFromJava(session, javaClass, javaTypeParameterStack)
         val enhancement = FirSignatureEnhancement(firJavaClass, session) { emptyList() }
         enhancement.enhanceTypeParameterBounds(firJavaClass.typeParameters)

compiler/fir/java/src/org/jetbrains/kotlin/fir/java/enhancement/SignatureEnhancement.kt

@@ -333,16 +333,11 @@ class FirSignatureEnhancement(
         // (`A : B, B : A` - invalid), the cycles can still appear when type parameters use each other in argument
         // position (`A : C<B>, B : D<A>` - valid). In this case the precise enhancement of each bound depends on
         // the others' nullability, for which we need to enhance at least its head type constructor.
-        //
-        // While this is straightforward to do within a single class/method (enhance all bounds' head type
-        // constructors, then enhance fully), it's not so simple when two classes depend on each other (we need
-        // to enhance *both* classes' type parameters' bounds' heads first). This is why we replace each bound
-        // with an unenhanced version first: this ensures that the frontend at least doesn't fail.
-        //
-        // TODO: find a way to partially enhance type parameters of all classes before fully enhancing anything.
-        // TODO: should this be done in topological order on head type constructors?
-        //   I.e. for `A : B, B : C<A>` should we process `B` first?
         typeParameters.replaceBounds { _, bound ->
+            // Resolve without enhancement so we don't crash the frontend if there is a restricted cycle (`A : B, B : A`)
+            // or if we visit type parameters in the wrong order (`A : B, B : C<A>` with `A` enhanced before `B`).
+            // TODO: the second case technically produces incorrect results - the loop below should visit type parameters
+            //   in topological order, then a resolved-but-not-enhanced type will never be observable with valid code.
             bound.resolveIfJavaType(session, javaTypeParameterStack, FirJavaTypeConversionMode.TYPE_PARAMETER_BOUND)
         }
         typeParameters.replaceBounds { typeParameter, bound ->

compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.fir.txt

@@ -0,0 +1,18 @@
+public open class ReferenceCycleThroughAnnotation : R|kotlin/Any| {
+    public constructor(): R|test/ReferenceCycleThroughAnnotation|
+
+    @R|test/ReferenceCycleThroughAnnotation.C|(value = <getClass>(<getClass>(R|ft<test/ReferenceCycleThroughAnnotation.B<ft<test/ReferenceCycleThroughAnnotation.A<*>, test/ReferenceCycleThroughAnnotation.A<*>?>>, test/ReferenceCycleThroughAnnotation.B<*>?>|))) public open inner class A<T : R|kotlin/Any!|> : R|kotlin/Any| {
+        public open fun foo(): R|kotlin/Unit|
+
+        public test/ReferenceCycleThroughAnnotation.constructor<T : R|kotlin/Any!|>(): R|test/ReferenceCycleThroughAnnotation.A<T>|
+
+    }
+    public open inner class B<T : R|ft<test/ReferenceCycleThroughAnnotation.A<ft<T & Any, T?>>, test/ReferenceCycleThroughAnnotation.A<ft<T & Any, T?>>?>|> : R|kotlin/Any| {
+        public test/ReferenceCycleThroughAnnotation.constructor<T : R|ft<test/ReferenceCycleThroughAnnotation.A<ft<T & Any, T?>>, test/ReferenceCycleThroughAnnotation.A<ft<T & Any, T?>>?>|>(): R|test/ReferenceCycleThroughAnnotation.B<T>|
+
+    }
+    public final annotation class C : R|kotlin/Annotation| {
+        public constructor(value: R|kotlin/reflect/KClass<*>|): R|test/ReferenceCycleThroughAnnotation.C|
+
+    }
+}

compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java

@@ -0,0 +1,16 @@
+package test;
+
+public class ReferenceCycleThroughAnnotation {
+    @C(B.class)
+    public class A<T extends Object> {
+        public void foo() {
+        }
+    }
+
+    public class B<T extends A<T>> {
+    }
+
+    public @interface C {
+        public Class<?> value();
+    }
+}

compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.txt

@@ -0,0 +1,19 @@
+package test
+
+public open class ReferenceCycleThroughAnnotation {
+    public constructor ReferenceCycleThroughAnnotation()
+
+    @test.ReferenceCycleThroughAnnotation.C(value = test.ReferenceCycleThroughAnnotation.B::class) public open inner class A</*0*/ T : kotlin.Any!> {
+        public constructor A</*0*/ T : kotlin.Any!>()
+        public open fun foo(): kotlin.Unit
+    }
+
+    public open inner class B</*0*/ T : test.ReferenceCycleThroughAnnotation.A<T!>!> {
+        public constructor B</*0*/ T : test.ReferenceCycleThroughAnnotation.A<T!>!>()
+    }
+
+    public final annotation class C : kotlin.Annotation {
+        public constructor C(/*0*/ value: kotlin.reflect.KClass<*>)
+        public final val value: kotlin.reflect.KClass<*>
+    }
+}

compiler/tests-gen/org/jetbrains/kotlin/jvm/compiler/LoadJavaTestGenerated.java

@@ -226,6 +226,11 @@ public class LoadJavaTestGenerated extends AbstractLoadJavaTest {
             runTest("compiler/testData/loadJava/compiledJava/RecursiveWildcardUpperBound.java");
         }
 
+        @TestMetadata("ReferenceCycleThroughAnnotation.java")
+        public void testReferenceCycleThroughAnnotation() throws Exception {
+            runTest("compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java");
+        }
+
         @TestMetadata("RemoveRedundantProjectionKind.java")
         public void testRemoveRedundantProjectionKind() throws Exception {
             runTest("compiler/testData/loadJava/compiledJava/RemoveRedundantProjectionKind.java");

compiler/tests-gen/org/jetbrains/kotlin/jvm/compiler/LoadJavaWithPsiClassReadingTestGenerated.java

@@ -224,6 +224,11 @@ public class LoadJavaWithPsiClassReadingTestGenerated extends AbstractLoadJavaWi
         runTest("compiler/testData/loadJava/compiledJava/RecursiveWildcardUpperBound.java");
     }
 
+    @TestMetadata("ReferenceCycleThroughAnnotation.java")
+    public void testReferenceCycleThroughAnnotation() throws Exception {
+        runTest("compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java");
+    }
+
     @TestMetadata("RemoveRedundantProjectionKind.java")
     public void testRemoveRedundantProjectionKind() throws Exception {
         runTest("compiler/testData/loadJava/compiledJava/RemoveRedundantProjectionKind.java");

compiler/tests-gen/org/jetbrains/kotlin/jvm/compiler/ir/IrLoadJavaTestGenerated.java

@@ -227,6 +227,11 @@ public class IrLoadJavaTestGenerated extends AbstractIrLoadJavaTest {
             runTest("compiler/testData/loadJava/compiledJava/RecursiveWildcardUpperBound.java");
         }
 
+        @TestMetadata("ReferenceCycleThroughAnnotation.java")
+        public void testReferenceCycleThroughAnnotation() throws Exception {
+            runTest("compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java");
+        }
+
         @TestMetadata("RemoveRedundantProjectionKind.java")
         public void testRemoveRedundantProjectionKind() throws Exception {
             runTest("compiler/testData/loadJava/compiledJava/RemoveRedundantProjectionKind.java");

compiler/tests-gen/org/jetbrains/kotlin/jvm/compiler/javac/LoadJavaUsingJavacTestGenerated.java

@@ -226,6 +226,11 @@ public class LoadJavaUsingJavacTestGenerated extends AbstractLoadJavaUsingJavacT
             runTest("compiler/testData/loadJava/compiledJava/RecursiveWildcardUpperBound.java");
         }
 
+        @TestMetadata("ReferenceCycleThroughAnnotation.java")
+        public void testReferenceCycleThroughAnnotation() throws Exception {
+            runTest("compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java");
+        }
+
         @TestMetadata("RemoveRedundantProjectionKind.java")
         public void testRemoveRedundantProjectionKind() throws Exception {
             runTest("compiler/testData/loadJava/compiledJava/RemoveRedundantProjectionKind.java");

core/descriptors.runtime/tests/org/jetbrains/kotlin/jvm/runtime/JvmRuntimeDescriptorLoaderTestGenerated.java

@@ -3067,6 +3067,11 @@ public class JvmRuntimeDescriptorLoaderTestGenerated extends AbstractJvmRuntimeD
             runTest("compiler/testData/loadJava/compiledJava/RecursiveWildcardUpperBound.java");
         }
 
+        @TestMetadata("ReferenceCycleThroughAnnotation.java")
+        public void testReferenceCycleThroughAnnotation() throws Exception {
+            runTest("compiler/testData/loadJava/compiledJava/ReferenceCycleThroughAnnotation.java");
+        }
+
         @TestMetadata("RemoveRedundantProjectionKind.java")
         public void testRemoveRedundantProjectionKind() throws Exception {
             runTest("compiler/testData/loadJava/compiledJava/RemoveRedundantProjectionKind.java");