KT-43205 Ignore annotations in CallableClsStubBuilder when needed

Kotlin compiler can add `@Deprecated` annotations to the fields of
private companion objects, and if those annotations are not supposed to
be shown in decompiled code and used, the field is marked with
`HAS_ANNOTATIONS=false` flag (see KT-25009)

However, it was not taken into account in stubs building process, which
led to the 'Stubs vs PSI mismatch' exceptions

^KT-43205 Fixed

Also, restore the order of nested typealiases and classes (see KT-41859)

We didn't want to bump the version of the stubs when we fixed this
issue; now we have an opportunity to restore the order back to
match the `MemberComparator`

Also, some refactoring is done to underscore that
`createPackageDeclarationsStubs` is suitable only for packages, not
for any declarations container

compiler/psi/src/org/jetbrains/kotlin/psi/stubs/KotlinStubVersions.kt

@@ -28,8 +28,7 @@ object KotlinStubVersions {
     // Binary stub version should be increased if stub format (org.jetbrains.kotlin.psi.stubs.impl) is changed
     //      or changes are made to the core stub building code (org.jetbrains.kotlin.idea.decompiler.stubBuilder).
     // Increasing this version will lead to reindexing of all binary files that are potentially kotlin binaries (including all class files).
-    // TODO: Before updating this, please see the kdoc at DeserializedMemberScope.computeDescriptors and KT-41859
+    private const val BINARY_STUB_VERSION = 73
-    private const val BINARY_STUB_VERSION = 72
 
     // Classfile stub version should be increased if changes are made to classfile stub building subsystem (org.jetbrains.kotlin.idea.decompiler.classFile)
     // Increasing this version will lead to reindexing of all classfiles.

core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/descriptors/DeserializedMemberScope.kt

@@ -99,24 +99,13 @@ abstract class DeserializedMemberScope protected constructor(
         return impl.getContributedVariables(name, location)
     }
 
-    /**
-     * N.B. Currently the order of declarations here and in the [MemberComparator] is intentionally different:
-     * [MemberComparator] places classes first and typealiases second.
-     *
-     * However, `ClassClsStubBuilder` places typealiases first and classes second. This leads to
-     * Stub vs Psi mismatch error, when we have a class which have both inner typealiases and inner classes.
-     * This is unintentional and should be fixed.
-     *
-     * We do not want to update stubs versions prematurely, so we temporary mitigate the issue by making sure that
-     * order in stubs (in `ClassClsStubBuilder`) and here (in [DeserializedMemberScope]) is the same. As soon as the
-     * opportunity to bump the stubs version arises, we should fix the bug in the `ClassClsStubBuilder`, and
-     * fix the order here accordingly.
-     */
     protected fun computeDescriptors(
         kindFilter: DescriptorKindFilter,
         nameFilter: (Name) -> Boolean,
         location: LookupLocation
     ): Collection<DeclarationDescriptor> {
+        //NOTE: descriptors should be in the same order they were serialized in
+        // see MemberComparator
         val result = ArrayList<DeclarationDescriptor>(0)
 
         if (kindFilter.acceptsKinds(DescriptorKindFilter.SINGLETON_CLASSIFIERS_MASK)) {
@@ -125,14 +114,6 @@ abstract class DeserializedMemberScope protected constructor(
 
         impl.addFunctionsAndPropertiesTo(result, kindFilter, nameFilter, location)
 
-        if (kindFilter.acceptsKinds(DescriptorKindFilter.TYPE_ALIASES_MASK)) {
-            for (typeAliasName in impl.typeAliasNames) {
-                if (nameFilter(typeAliasName)) {
-                    result.addIfNotNull(impl.getTypeAliasByName(typeAliasName))
-                }
-            }
-        }
-
         if (kindFilter.acceptsKinds(DescriptorKindFilter.CLASSIFIERS_MASK)) {
             for (className in classNames) {
                 if (nameFilter(className)) {
@@ -141,6 +122,14 @@ abstract class DeserializedMemberScope protected constructor(
             }
         }
 
+        if (kindFilter.acceptsKinds(DescriptorKindFilter.TYPE_ALIASES_MASK)) {
+            for (typeAliasName in impl.typeAliasNames) {
+                if (nameFilter(typeAliasName)) {
+                    result.addIfNotNull(impl.getTypeAliasByName(typeAliasName))
+                }
+            }
+        }
+
         return result.compact()
     }
 

idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/common/KotlinMetadataStubBuilder.kt

@@ -47,7 +47,7 @@ open class KotlinMetadataStubBuilder(
                 val context = components.createContext(nameResolver, packageFqName, TypeTable(packageProto.typeTable))
 
                 val fileStub = createFileStub(packageFqName, isScript = false)
-                createDeclarationsStubs(
+                createPackageDeclarationsStubs(
                     fileStub, context,
                     ProtoContainer.Package(packageFqName, context.nameResolver, context.typeTable, source = null),
                     packageProto

idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/stubBuilder/CallableClsStubBuilder.kt

@@ -26,15 +26,14 @@ import org.jetbrains.kotlin.serialization.deserialization.AnnotatedCallableKind
 import org.jetbrains.kotlin.serialization.deserialization.ProtoContainer
 import org.jetbrains.kotlin.serialization.deserialization.getName
 
-fun createDeclarationsStubs(
+fun createPackageDeclarationsStubs(
     parentStub: StubElement<out PsiElement>,
     outerContext: ClsStubBuilderContext,
-    protoContainer: ProtoContainer,
+    protoContainer: ProtoContainer.Package,
     packageProto: ProtoBuf.Package
 ) {
-    createDeclarationsStubs(
+    createDeclarationsStubs(parentStub, outerContext, protoContainer, packageProto.functionList, packageProto.propertyList)
-        parentStub, outerContext, protoContainer, packageProto.functionList, packageProto.propertyList, packageProto.typeAliasList
+    createTypeAliasesStubs(parentStub, outerContext, protoContainer, packageProto.typeAliasList)
-    )
 }
 
 fun createDeclarationsStubs(
@@ -43,7 +42,6 @@ fun createDeclarationsStubs(
     protoContainer: ProtoContainer,
     functionProtos: List<ProtoBuf.Function>,
     propertyProtos: List<ProtoBuf.Property>,
-    typeAliasesProtos: List<ProtoBuf.TypeAlias>
 ) {
     for (propertyProto in propertyProtos) {
         if (!shouldSkip(propertyProto.flags, outerContext.nameResolver.getName(propertyProto.name))) {
@@ -55,7 +53,14 @@ fun createDeclarationsStubs(
             FunctionClsStubBuilder(parentStub, outerContext, protoContainer, functionProto).build()
         }
     }
+}
 
+fun createTypeAliasesStubs(
+    parentStub: StubElement<out PsiElement>,
+    outerContext: ClsStubBuilderContext,
+    protoContainer: ProtoContainer,
+    typeAliasesProtos: List<ProtoBuf.TypeAlias>
+) {
     for (typeAliasProto in typeAliasesProtos) {
         createTypeAliasStub(parentStub, typeAliasProto, protoContainer, outerContext)
     }
@@ -139,7 +144,7 @@ private class FunctionClsStubBuilder(
                 .map { ClassIdWithTarget(it, AnnotationUseSiteTarget.RECEIVER) }
         }
 
-    override val returnType: ProtoBuf.Type?
+    override val returnType: ProtoBuf.Type
         get() = functionProto.returnType(c.typeTable)
 
     override fun createValueParameterList() {
@@ -153,6 +158,9 @@ private class FunctionClsStubBuilder(
             listOf(VISIBILITY, OPERATOR, INFIX, EXTERNAL_FUN, INLINE, TAILREC, SUSPEND) + modalityModifier
         )
 
+        // If function is marked as having no annotations, we don't create stubs for it
+        if (!Flags.HAS_ANNOTATIONS.get(functionProto.flags)) return
+
         val annotationIds = c.components.annotationLoader.loadCallableAnnotations(
             protoContainer, functionProto, AnnotatedCallableKind.FUNCTION
         )
@@ -192,7 +200,7 @@ private class PropertyClsStubBuilder(
             .loadExtensionReceiverParameterAnnotations(protoContainer, propertyProto, AnnotatedCallableKind.PROPERTY_GETTER)
             .map { ClassIdWithTarget(it, AnnotationUseSiteTarget.RECEIVER) }
 
-    override val returnType: ProtoBuf.Type?
+    override val returnType: ProtoBuf.Type
         get() = propertyProto.returnType(c.typeTable)
 
     override fun createValueParameterList() {
@@ -207,6 +215,9 @@ private class PropertyClsStubBuilder(
             listOf(VISIBILITY, LATEINIT, EXTERNAL_PROPERTY) + constModifier + modalityModifier
         )
 
+        // If field is marked as having no annotations, we don't create stubs for it
+        if (!Flags.HAS_ANNOTATIONS.get(propertyProto.flags)) return
+
         val propertyAnnotations =
             c.components.annotationLoader.loadCallableAnnotations(protoContainer, propertyProto, AnnotatedCallableKind.PROPERTY)
         val backingFieldAnnotations =
@@ -260,6 +271,9 @@ private class ConstructorClsStubBuilder(
     override fun createModifierListStub() {
         val modifierListStubImpl = createModifierListStubForDeclaration(callableStub, constructorProto.flags, listOf(VISIBILITY))
 
+        // If constructor is marked as having no annotations, we don't create stubs for it
+        if (!Flags.HAS_ANNOTATIONS.get(constructorProto.flags)) return
+
         val annotationIds = c.components.annotationLoader.loadCallableAnnotations(
             protoContainer, constructorProto, AnnotatedCallableKind.FUNCTION
         )

idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/stubBuilder/ClassClsStubBuilder.kt

@@ -178,6 +178,7 @@ private class ClassClsStubBuilder(
         createCompanionObjectStub(classBody)
         createCallableMemberStubs(classBody)
         createInnerAndNestedClasses(classBody)
+        createTypeAliasesStubs(classBody)
     }
 
     private fun createCompanionObjectStub(classBody: KotlinPlaceHolderStubImpl<KtClassBody>) {
@@ -219,10 +220,7 @@ private class ClassClsStubBuilder(
             }
         }
 
-        // FIXME using this function breaks the order of `MemberComparator` (see KT-41859)
+        createDeclarationsStubs(classBody, c, thisAsProtoContainer, classProto.functionList, classProto.propertyList)
-        createDeclarationsStubs(
-            classBody, c, thisAsProtoContainer, classProto.functionList, classProto.propertyList, classProto.typeAliasList
-        )
     }
 
     private fun isClass(): Boolean {
@@ -245,6 +243,10 @@ private class ClassClsStubBuilder(
         }
     }
 
+    private fun createTypeAliasesStubs(classBody: KotlinPlaceHolderStubImpl<KtClassBody>) {
+        createTypeAliasesStubs(classBody, c, thisAsProtoContainer, classProto.typeAliasList)
+    }
+
     private fun createNestedClassStub(classBody: StubElement<out PsiElement>, nestedClassId: ClassId) {
         val (nameResolver, classProto, _, sourceElement) =
             c.components.classDataFinder.findClassData(nestedClassId)

idea/idea-analysis/src/org/jetbrains/kotlin/idea/decompiler/stubBuilder/clsStubBuilding.kt

@@ -50,7 +50,7 @@ fun createPackageFacadeStub(
 ): KotlinFileStubImpl {
     val fileStub = KotlinFileStubForIde.forFile(packageFqName, isScript = false)
     setupFileStub(fileStub, packageFqName)
-    createDeclarationsStubs(
+    createPackageDeclarationsStubs(
         fileStub, c, ProtoContainer.Package(packageFqName, c.nameResolver, c.typeTable, source = null), packageProto
     )
     return fileStub
@@ -68,7 +68,7 @@ fun createFileFacadeStub(
         packageFqName, c.nameResolver, c.typeTable,
         JvmPackagePartSource(JvmClassName.byClassId(ClassId.topLevel(facadeFqName)), null, packageProto, c.nameResolver)
     )
-    createDeclarationsStubs(fileStub, c, container, packageProto)
+    createPackageDeclarationsStubs(fileStub, c, container, packageProto)
     return fileStub
 }
 
@@ -90,7 +90,7 @@ fun createMultifileClassStub(
             packageFqName, partContext.nameResolver, partContext.typeTable,
             JvmPackagePartSource(partFile, packageProto, nameResolver)
         )
-        createDeclarationsStubs(fileStub, partContext, container, packageProto)
+        createPackageDeclarationsStubs(fileStub, partContext, container, packageProto)
     }
     return fileStub
 }

idea/testData/decompiler/decompiledTextJvm/PrivateConstField.expected.kt

@@ -0,0 +1,17 @@
+// IntelliJ API Decompiler stub source generated from a class file
+// Implementation of methods is not available
+
+package test
+
+public final class PrivateConstField public constructor() {
+    private companion object {
+        public const final val CONST: kotlin.Int /* compiled code */
+
+        @test.A @field:java.lang.Deprecated public const final val CONST_WITH_ANNOTATION: kotlin.Int /* compiled code */
+
+        public final val field: kotlin.Int /* compiled code */
+
+        @test.A @field:java.lang.Deprecated public final val fieldWithAnnotation: kotlin.Int /* compiled code */
+    }
+}
+

idea/testData/decompiler/decompiledTextJvm/PrivateConstField/PrivateConstField.kt

@@ -0,0 +1,16 @@
+package test
+
+@Retention(AnnotationRetention.RUNTIME)
+annotation class A
+
+class PrivateConstField {
+    private companion object {
+        const val CONST: Int = 10
+        @A
+        const val CONST_WITH_ANNOTATION: Int = 10
+
+        val field: Int = 10
+        @A
+        val fieldWithAnnotation: Int = 10
+    }
+}

idea/testData/decompiler/stubBuilder/PrivateConstField/PrivateConstField.kt

@@ -0,0 +1,16 @@
+package test
+
+@Retention(AnnotationRetention.RUNTIME)
+annotation class A
+
+class PrivateConstField {
+    private companion object {
+        const val CONST: Int = 10
+        @A
+        const val CONST_WITH_ANNOTATION: Int = 10
+
+        val field: Int = 10
+        @A
+        val fieldWithAnnotation: Int = 10
+    }
+}

idea/testData/decompiler/stubBuilder/PrivateConstField/PrivateConstField.txt

@@ -0,0 +1,75 @@
+PsiJetFileStubImpl[package=test]
+  PACKAGE_DIRECTIVE
+    REFERENCE_EXPRESSION[referencedName=test]
+  IMPORT_LIST
+  CLASS[fqName=test.PrivateConstField, isEnumEntry=false, isInterface=false, isLocal=false, isTopLevel=true, name=PrivateConstField, superNames=[]]
+    MODIFIER_LIST[public final]
+    PRIMARY_CONSTRUCTOR
+      MODIFIER_LIST[public]
+      VALUE_PARAMETER_LIST
+    CLASS_BODY
+      OBJECT_DECLARATION[fqName=test.PrivateConstField.Companion, isCompanion=true, isLocal=false, isObjectLiteral=false, isTopLevel=false, name=Companion, superNames=[]]
+        MODIFIER_LIST[private companion]
+        CLASS_BODY
+          PROPERTY[fqName=test.PrivateConstField.Companion.CONST, hasDelegate=false, hasDelegateExpression=false, hasInitializer=false, hasReturnTypeRef=true, isExtension=false, isTopLevel=false, isVar=false, name=CONST]
+            MODIFIER_LIST[public final const]
+            TYPE_REFERENCE
+              USER_TYPE
+                USER_TYPE
+                  REFERENCE_EXPRESSION[referencedName=kotlin]
+                REFERENCE_EXPRESSION[referencedName=Int]
+          PROPERTY[fqName=test.PrivateConstField.Companion.CONST_WITH_ANNOTATION, hasDelegate=false, hasDelegateExpression=false, hasInitializer=false, hasReturnTypeRef=true, isExtension=false, isTopLevel=false, isVar=false, name=CONST_WITH_ANNOTATION]
+            MODIFIER_LIST[public final const]
+              ANNOTATION_ENTRY[hasValueArguments=false, shortName=A]
+                CONSTRUCTOR_CALLEE
+                  TYPE_REFERENCE
+                    USER_TYPE
+                      USER_TYPE
+                        REFERENCE_EXPRESSION[referencedName=test]
+                      REFERENCE_EXPRESSION[referencedName=A]
+              ANNOTATION_ENTRY[hasValueArguments=false, shortName=Deprecated]
+                ANNOTATION_TARGET[useSiteTarget=FIELD]
+                CONSTRUCTOR_CALLEE
+                  TYPE_REFERENCE
+                    USER_TYPE
+                      USER_TYPE
+                        USER_TYPE
+                          REFERENCE_EXPRESSION[referencedName=java]
+                        REFERENCE_EXPRESSION[referencedName=lang]
+                      REFERENCE_EXPRESSION[referencedName=Deprecated]
+            TYPE_REFERENCE
+              USER_TYPE
+                USER_TYPE
+                  REFERENCE_EXPRESSION[referencedName=kotlin]
+                REFERENCE_EXPRESSION[referencedName=Int]
+          PROPERTY[fqName=test.PrivateConstField.Companion.field, hasDelegate=false, hasDelegateExpression=false, hasInitializer=false, hasReturnTypeRef=true, isExtension=false, isTopLevel=false, isVar=false, name=field]
+            MODIFIER_LIST[public final]
+            TYPE_REFERENCE
+              USER_TYPE
+                USER_TYPE
+                  REFERENCE_EXPRESSION[referencedName=kotlin]
+                REFERENCE_EXPRESSION[referencedName=Int]
+          PROPERTY[fqName=test.PrivateConstField.Companion.fieldWithAnnotation, hasDelegate=false, hasDelegateExpression=false, hasInitializer=false, hasReturnTypeRef=true, isExtension=false, isTopLevel=false, isVar=false, name=fieldWithAnnotation]
+            MODIFIER_LIST[public final]
+              ANNOTATION_ENTRY[hasValueArguments=false, shortName=A]
+                CONSTRUCTOR_CALLEE
+                  TYPE_REFERENCE
+                    USER_TYPE
+                      USER_TYPE
+                        REFERENCE_EXPRESSION[referencedName=test]
+                      REFERENCE_EXPRESSION[referencedName=A]
+              ANNOTATION_ENTRY[hasValueArguments=false, shortName=Deprecated]
+                ANNOTATION_TARGET[useSiteTarget=FIELD]
+                CONSTRUCTOR_CALLEE
+                  TYPE_REFERENCE
+                    USER_TYPE
+                      USER_TYPE
+                        USER_TYPE
+                          REFERENCE_EXPRESSION[referencedName=java]
+                        REFERENCE_EXPRESSION[referencedName=lang]
+                      REFERENCE_EXPRESSION[referencedName=Deprecated]
+            TYPE_REFERENCE
+              USER_TYPE
+                USER_TYPE
+                  REFERENCE_EXPRESSION[referencedName=kotlin]
+                REFERENCE_EXPRESSION[referencedName=Int]

idea/testData/decompiler/stubBuilder/TypeAliases/TypeAliases.txt

@@ -95,6 +95,18 @@ PsiJetFileStubImpl[package=test]
             USER_TYPE
               REFERENCE_EXPRESSION[referencedName=kotlin]
             REFERENCE_EXPRESSION[referencedName=Unit]
+      CLASS[fqName=test.TypeAliases.OrderA, isEnumEntry=false, isInterface=false, isLocal=false, isTopLevel=false, name=OrderA, superNames=[]]
+        MODIFIER_LIST[public final]
+        PRIMARY_CONSTRUCTOR
+          MODIFIER_LIST[public]
+          VALUE_PARAMETER_LIST
+        CLASS_BODY
+      CLASS[fqName=test.TypeAliases.OrderB, isEnumEntry=false, isInterface=false, isLocal=false, isTopLevel=false, name=OrderB, superNames=[]]
+        MODIFIER_LIST[public final]
+        PRIMARY_CONSTRUCTOR
+          MODIFIER_LIST[public]
+          VALUE_PARAMETER_LIST
+        CLASS_BODY
       TYPEALIAS[fqName=test.TypeAliases.B, isTopLevel=false, name=B]
         MODIFIER_LIST[public]
         TYPE_REFERENCE
@@ -139,15 +151,3 @@ PsiJetFileStubImpl[package=test]
                 TYPE_REFERENCE
                   USER_TYPE
                     REFERENCE_EXPRESSION[referencedName=F]
-      CLASS[fqName=test.TypeAliases.OrderA, isEnumEntry=false, isInterface=false, isLocal=false, isTopLevel=false, name=OrderA, superNames=[]]
-        MODIFIER_LIST[public final]
-        PRIMARY_CONSTRUCTOR
-          MODIFIER_LIST[public]
-          VALUE_PARAMETER_LIST
-        CLASS_BODY
-      CLASS[fqName=test.TypeAliases.OrderB, isEnumEntry=false, isInterface=false, isLocal=false, isTopLevel=false, name=OrderB, superNames=[]]
-        MODIFIER_LIST[public final]
-        PRIMARY_CONSTRUCTOR
-          MODIFIER_LIST[public]
-          VALUE_PARAMETER_LIST
-        CLASS_BODY

idea/tests/org/jetbrains/kotlin/idea/decompiler/stubBuilder/ClsStubBuilderTestGenerated.java

@@ -163,6 +163,11 @@ public class ClsStubBuilderTestGenerated extends AbstractClsStubBuilderTest {
         runTest("idea/testData/decompiler/stubBuilder/Objects/");
     }
 
+    @TestMetadata("PrivateConstField")
+    public void testPrivateConstField() throws Exception {
+        runTest("idea/testData/decompiler/stubBuilder/PrivateConstField/");
+    }
+
     @TestMetadata("PrivateToThis")
     public void testPrivateToThis() throws Exception {
         runTest("idea/testData/decompiler/stubBuilder/PrivateToThis/");

idea/tests/org/jetbrains/kotlin/idea/decompiler/textBuilder/JvmDecompiledTextTestGenerated.java

@@ -53,6 +53,11 @@ public class JvmDecompiledTextTestGenerated extends AbstractJvmDecompiledTextTes
         runTest("idea/testData/decompiler/decompiledTextJvm/ParameterName/");
     }
 
+    @TestMetadata("PrivateConstField")
+    public void testPrivateConstField() throws Exception {
+        runTest("idea/testData/decompiler/decompiledTextJvm/PrivateConstField/");
+    }
+
     @TestMetadata("TestKt")
     public void testTestKt() throws Exception {
         runTest("idea/testData/decompiler/decompiledTextJvm/TestKt/");
@@ -128,6 +133,19 @@ public class JvmDecompiledTextTestGenerated extends AbstractJvmDecompiledTextTes
         }
     }
 
+    @TestMetadata("idea/testData/decompiler/decompiledTextJvm/PrivateConstField")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class PrivateConstField extends AbstractJvmDecompiledTextTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInPrivateConstField() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/testData/decompiler/decompiledTextJvm/PrivateConstField"), Pattern.compile("^([^\\.]+)$"), null, true);
+        }
+    }
+
     @TestMetadata("idea/testData/decompiler/decompiledTextJvm/TestKt")
     @TestDataPath("$PROJECT_ROOT")
     @RunWith(JUnit3RunnerWithInners.class)