[Commonizer] More accurate verification of annotations in tests

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/mergedtree/ir/CirAnnotation.kt

@@ -6,11 +6,9 @@
 package org.jetbrains.kotlin.descriptors.commonizer.mergedtree.ir
 
 import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
-import org.jetbrains.kotlin.descriptors.commonizer.utils.Interner
+import org.jetbrains.kotlin.descriptors.commonizer.utils.*
-import org.jetbrains.kotlin.descriptors.commonizer.utils.hashCode
+import org.jetbrains.kotlin.descriptors.commonizer.utils.checkConstantSupportedInCommonization
-import org.jetbrains.kotlin.descriptors.commonizer.utils.appendHashCode
 import org.jetbrains.kotlin.descriptors.commonizer.utils.intern
-import org.jetbrains.kotlin.descriptors.commonizer.utils.isUnderStandardKotlinPackages
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.resolve.constants.*
@@ -21,7 +19,11 @@ class CirAnnotation private constructor(original: AnnotationDescriptor) {
 
     init {
         allValueArguments.forEach { (name, constantValue) ->
-            checkSupportedInCommonization(constantValue) { "${original::class.java}, $original[$name]" }
+            checkConstantSupportedInCommonization(
+                constantValue = constantValue,
+                constantName = name,
+                owner = original
+            )
         }
     }
 
@@ -51,31 +53,3 @@ class CirAnnotation private constructor(original: AnnotationDescriptor) {
         fun create(original: AnnotationDescriptor): CirAnnotation = interner.intern(CirAnnotation(original))
     }
 }
-
-internal fun checkSupportedInCommonization(constantValue: ConstantValue<*>, location: () -> String) {
-    @Suppress("TrailingComma")
-    return when (constantValue) {
-        is StringValue,
-        is IntegerValueConstant<*>,
-        is UnsignedValueConstant<*>,
-        is BooleanValue,
-        is NullValue,
-        is DoubleValue,
-        is FloatValue,
-        is EnumValue -> {
-            // OK
-        }
-        is AnnotationValue -> {
-            if (constantValue.value.fqName?.isUnderStandardKotlinPackages != true)
-                error("Only ${constantValue::class.java} const values from Kotlin standard packages are supported, $constantValue at ${location()}")
-
-            Unit
-        }
-        is ArrayValue -> {
-            constantValue.value.forEachIndexed { index, innerConstantValue ->
-                checkSupportedInCommonization(innerConstantValue) { "${location()}[$index]" }
-            }
-        }
-        else -> error("Unsupported const value type: ${constantValue::class.java}, $constantValue at ${location()}")
-    }
-}

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/mergedtree/ir/CirProperty.kt

@@ -10,6 +10,7 @@ import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.descriptors.commonizer.mergedtree.ir.CirGetter.Companion.toGetter
 import org.jetbrains.kotlin.descriptors.commonizer.mergedtree.ir.CirSetter.Companion.toSetter
 import org.jetbrains.kotlin.descriptors.commonizer.utils.Interner
+import org.jetbrains.kotlin.descriptors.commonizer.utils.checkConstantSupportedInCommonization
 import org.jetbrains.kotlin.resolve.constants.ConstantValue
 
 interface CirProperty : CirFunctionOrProperty {
@@ -58,7 +59,10 @@ class CirPropertyImpl(original: PropertyDescriptor) : CirFunctionOrPropertyImpl<
 
     init {
         compileTimeInitializer?.let { compileTimeInitializer ->
-            checkSupportedInCommonization(compileTimeInitializer) { "${original::class.java}, $original" }
+            checkConstantSupportedInCommonization(
+                constantValue = compileTimeInitializer,
+                owner = original
+            )
         }
     }
 }

native/commonizer/src/org/jetbrains/kotlin/descriptors/commonizer/utils/constants.kt

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.descriptors.commonizer.utils
+
+import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.resolve.constants.*
+
+internal fun checkConstantSupportedInCommonization(
+    constantValue: ConstantValue<*>,
+    constantName: Name? = null,
+    owner: Any,
+    onError: (String) -> Nothing = ::error
+) {
+    checkConstantSupportedInCommonization(
+        constantValue = constantValue,
+        location = { "${owner::class.java}, $owner" + constantName?.asString()?.let { "[$it]" } },
+        onError = onError
+    )
+}
+
+private fun checkConstantSupportedInCommonization(
+    constantValue: ConstantValue<*>,
+    location: () -> String,
+    onError: (String) -> Nothing
+) {
+    @Suppress("TrailingComma")
+    when (constantValue) {
+        is StringValue,
+        is IntegerValueConstant<*>,
+        is UnsignedValueConstant<*>,
+        is BooleanValue,
+        is NullValue,
+        is DoubleValue,
+        is FloatValue,
+        is EnumValue -> {
+            // OK
+        }
+        is AnnotationValue -> {
+            if (constantValue.value.fqName?.isUnderStandardKotlinPackages != true)
+                onError("Only ${constantValue::class.java} const values from Kotlin standard packages are supported, $constantValue at ${location()}")
+        }
+        is ArrayValue -> {
+            constantValue.value.forEachIndexed { index, innerConstantValue ->
+                checkConstantSupportedInCommonization(
+                    constantValue = innerConstantValue,
+                    location = { "${location()}[$index]" },
+                    onError = onError
+                )
+            }
+        }
+        else -> onError("Unsupported const value type: ${constantValue::class.java}, $constantValue at ${location()}")
+    }
+}

native/commonizer/tests/org/jetbrains/kotlin/descriptors/commonizer/utils/ComparingDeclarationsVisitor.kt

@@ -6,9 +6,12 @@
 package org.jetbrains.kotlin.descriptors.commonizer.utils
 
 import org.jetbrains.kotlin.descriptors.*
+import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor
 import org.jetbrains.kotlin.descriptors.annotations.Annotations
 import org.jetbrains.kotlin.descriptors.commonizer.mergedtree.*
 import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.resolve.constants.ConstantValue
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
 import org.jetbrains.kotlin.resolve.scopes.MemberScope
 import org.jetbrains.kotlin.types.KotlinType
@@ -394,14 +397,52 @@ internal class ComparingDeclarationsVisitor(
         visitAnnotations(expected.annotations, actual.annotations, context.nextLevel("Receiver parameter annotations"))
     }
 
-
     private fun visitAnnotations(expected: Annotations?, actual: Annotations?, context: Context) {
         if (expected === actual || (expected?.isEmpty() != false && actual?.isEmpty() != false)) return
 
-        val expectedAnnotationFqNames: Set<FqName?> = expected?.mapTo(HashSet()) { it.fqName } ?: emptySet()
+        fun AnnotationDescriptor.getMandatoryFqName(): FqName = fqName ?: context.fail("No FQ name for annotation $this")
-        val actualAnnotationFqNames: Set<FqName?> = actual?.mapTo(HashSet()) { it.fqName } ?: emptySet()
 
-        context.assertSetsEqual(expectedAnnotationFqNames, actualAnnotationFqNames, "annotations")
+        val expectedAnnotations: Map<FqName, AnnotationDescriptor> = expected?.associateBy { it.getMandatoryFqName() }.orEmpty()
+        val actualAnnotations: Map<FqName, AnnotationDescriptor> = actual?.associateBy { it.getMandatoryFqName() }.orEmpty()
+
+        context.assertSetsEqual(expectedAnnotations.keys, actualAnnotations.keys, "annotation FQ names")
+
+        for (annotationFqName in expectedAnnotations.keys) {
+            val expectedAnnotation = expectedAnnotations.getValue(annotationFqName)
+            val actualAnnotation = actualAnnotations.getValue(annotationFqName)
+
+            visitAnnotation(expectedAnnotation, actualAnnotation, context.nextLevel("Annotation $annotationFqName"))
+        }
+    }
+
+    private fun visitAnnotation(expected: AnnotationDescriptor, actual: AnnotationDescriptor, context: Context) {
+        visitType(expected.type, actual.type, context.nextLevel("annotation type"))
+
+        val expectedValueArguments: Map<Name, ConstantValue<*>> = expected.allValueArguments
+        val actualValueArguments: Map<Name, ConstantValue<*>> = actual.allValueArguments
+
+        context.assertSetsEqual(expectedValueArguments.keys, actualValueArguments.keys, "annotation value argument names")
+
+        for (name in expectedValueArguments.keys) {
+            val expectedValueArgument = expectedValueArguments.getValue(name)
+            checkConstantSupportedInCommonization(
+                constantValue = expectedValueArgument,
+                constantName = name,
+                owner = expected,
+                onError = { context.fail(it) }
+            )
+
+            val actualValueArgument = actualValueArguments.getValue(name)
+            checkConstantSupportedInCommonization(
+                constantValue = actualValueArgument,
+                constantName = name,
+                owner = actual,
+                onError = { context.fail(it) }
+            )
+
+            context.assertEquals(expectedValueArgument::class, actualValueArgument::class, "annotation value argument classe")
+            context.assertEquals(expectedValueArgument.value, actualValueArgument.value, "annotation value argument value")
+        }
     }
 
     private fun visitType(expected: KotlinType?, actual: KotlinType?, context: Context) {
@@ -452,8 +493,7 @@ internal class ComparingDeclarationsVisitor(
             fail(
                 buildString {
                     append("Comparing $subject:\n")
-                    append("$expected is not equal to $actual\n")
+                    append("\"$expected\" is not equal to \"$actual\"\n")
-                    append(this@assertEquals.toString())
                 }
             )
     }
@@ -472,17 +512,27 @@ internal class ComparingDeclarationsVisitor(
         if (expectedMinusActual.isNotEmpty() || actualMinusExpected.isNotEmpty())
             fail(
                 buildString {
-                    append("Comparing $subject:\n")
+                    appendLine("Comparing $subject:")
-                    append("$expected is not equal to $actual\n")
+                    appendLine("$expected is not equal to $actual")
-                    append("Expected size: ${expected.size}\n")
+                    appendLine("Expected size: ${expected.size}")
-                    append("Actual size: ${actual.size}\n")
+                    appendLine("Actual size: ${actual.size}")
-                    append("Expected minus actual: $expectedMinusActual\n")
+                    appendLine("Expected minus actual: $expectedMinusActual")
-                    append("Actual minus expected: $actualMinusExpected\n")
+                    appendLine("Actual minus expected: $actualMinusExpected")
-                    append(this@assertSetsEqual.toString())
                 }
             )
     }
 
+    private fun Context.fail(message: String): Nothing {
+        kotlin.test.fail(
+            buildString {
+                if (message.isNotEmpty()) {
+                    if (message.last() != '\n') appendLine(message) else append(message)
+                }
+                append(this@fail.toString())
+            }
+        )
+    }
+
     override fun visitPackageViewDescriptor(expected: PackageViewDescriptor, context: Context) =
         fail("Comparison of package views not supported")