Refactor code related to block construction (#571)

* Refactor code related to block construction

Code generation relies on CgContext for storing information about the currently available variables, imports, statements, etc. The context also stores info about code block currently being generated (currentBlock).

It is possible to collect statements into some list and then update the currentBlock with them, but it is discouraged, because this way it is very easy to occacionally lose some of the statements. That's because some statements will go to the currentBlock, but then will be overriden on update from the new list of statements.

Hence, it is recommended to always work with the statements using currentBlock and DSL methods that also use it. Such methods can be found in CgStatementConstructor.kt, see method `ifStatement` for example.

This PR makes more use of currentBlock instead of using separate lists. That is done in order to prevent possible future bugs.

* Refactor data provider method construction

This idea of this refactoring is the same as in the previous commit. Instead of collecting statements into some lists we now collect them right into the currentBlock.

* Fix ClassWithEnumTest.testOrdinal test

The problem was in the parameterized tests. There were multiple bugs: wrong class ids (class id's name must not include generic parameters, but they did), wrong method id for Arguments.arguments() method (its argument must be vararg, but it wasn't). Also, when a method accepts a vararg and is called via reflection, an array of vararg arguments should be additionally wrapped into an array of Objects (e.g. new Object[]{new Integer[]{1, 2, 3}}). However, when calling this method without reflection there must not be any such wrappers.

* Fix Arguments#arguments method call

- Make Arguments#arguments method id static. This allowed us to call this method without reflection. Previously we used reflection, because the method id was non-static by mistake.
- Construct an array of arguments for Arguments#arguments call. This way we don't have to worry about multiple cases of vararg method calls, but we should think about them later.

* Limit the maximum length of an array initializer + use guards for both array allocation and initialization

* Rework type parameters for some class ids

Earlier type parameters were hardcoded in the class name, simple name, etc. Then they were removed from names completely, but that lead to compilation errors for Kotlin, where type parameters must always be specified. So, this commit adds type parameters, but instead of writing them into names, we add them as a separate property 'typeParameters' of BuiltinClassId.

Author: ArsenHD

utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/Api.kt

@@ -753,6 +753,7 @@ open class ClassId @JvmOverloads constructor(
  */
 class BuiltinClassId(
     name: String,
+    elementClassId: ClassId? = null,
     override val canonicalName: String,
     override val simpleName: String,
     // by default we assume that the class is not a member class
@@ -769,6 +770,7 @@ class BuiltinClassId(
     override val isInner: Boolean = false,
     override val isNested: Boolean = false,
     override val isSynthetic: Boolean = false,
+    override val typeParameters: TypeParameters = TypeParameters(),
     override val allMethods: Sequence<MethodId> = emptySequence(),
     override val allConstructors: Sequence<ConstructorId> = emptySequence(),
     override val outerClass: Class<*>? = null,
@@ -777,7 +779,7 @@ class BuiltinClassId(
             -1, 0 -> ""
             else -> canonicalName.substring(0, index)
         },
-) : ClassId(name = name, isNullable = isNullable) {
+) : ClassId(name = name, isNullable = isNullable, elementClassId = elementClassId) {
     init {
         BUILTIN_CLASSES_BY_NAMES[name] = this
     }

utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/util/SignatureUtil.kt

@@ -1,5 +1,6 @@
 package org.utbot.framework.plugin.api.util
 
+import org.utbot.framework.plugin.api.ClassId
 import org.utbot.framework.plugin.api.ExecutableId
 import java.lang.reflect.Constructor
 import java.lang.reflect.Executable
@@ -39,6 +40,23 @@ fun Constructor<*>.bytecodeSignature() = buildString {
 
 fun Class<*>.bytecodeSignature(): String = id.jvmName
 
+/**
+ * Method [Class.getName] works differently for arrays than for other types.
+ * - When an element of an array is a reference type (e.g. `java.lang.Object`),
+ * the array of `java.lang.Object` will have name `[Ljava.lang.Object;`.
+ * - When an element of an array is a primitive type (e.g. `int`),
+ * the array of `int` will have name `[I`.
+ *
+ * So, this property returns the name of the given class in the format of an array element type name.
+ * Basically, it performs conversions for primitives and reference types (e.g. `int` -> `I`, `java.lang.Object` -> `Ljava.lang.Object;`.
+ */
+val ClassId.arrayLikeName: String
+    get() = when {
+        isPrimitive -> primitiveTypeJvmNameOrNull()!!
+        isRefType -> "L$name;"
+        else -> name
+    }
+
 fun String.toReferenceTypeBytecodeSignature(): String {
     val packageName = this
             .takeIf { "." in this }

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/context/CgContext.kt

@@ -99,7 +99,7 @@ internal interface CgContextOwner {
     val collectedExceptions: MutableSet<ClassId>
 
     // annotations required by the current method being built
-    val collectedTestMethodAnnotations: MutableSet<CgAnnotation>
+    val collectedMethodAnnotations: MutableSet<CgAnnotation>
 
     // imports required by the test class being built
     val collectedImports: MutableSet<Import>
@@ -259,7 +259,7 @@ internal interface CgContextOwner {
     }
 
     fun addAnnotation(annotation: CgAnnotation) {
-        if (collectedTestMethodAnnotations.add(annotation)) {
+        if (collectedMethodAnnotations.add(annotation)) {
             importIfNeeded(annotation.classId) // TODO: check how JUnit annotations are loaded
         }
     }
@@ -391,7 +391,7 @@ internal data class CgContext(
     override val collectedTestClassInterfaces: MutableSet<ClassId> = mutableSetOf(),
     override val collectedTestClassAnnotations: MutableSet<CgAnnotation> = mutableSetOf(),
     override val collectedExceptions: MutableSet<ClassId> = mutableSetOf(),
-    override val collectedTestMethodAnnotations: MutableSet<CgAnnotation> = mutableSetOf(),
+    override val collectedMethodAnnotations: MutableSet<CgAnnotation> = mutableSetOf(),
     override val collectedImports: MutableSet<Import> = mutableSetOf(),
     override val importedStaticMethods: MutableSet<MethodId> = mutableSetOf(),
     override val importedClasses: MutableSet<ClassId> = mutableSetOf(),

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt

@@ -6,19 +6,19 @@ import org.utbot.framework.codegen.model.constructor.context.CgContext
 import org.utbot.framework.codegen.model.constructor.context.CgContextOwner
 import org.utbot.framework.codegen.model.constructor.util.CgComponents
 import org.utbot.framework.codegen.model.constructor.util.CgStatementConstructor
+import org.utbot.framework.codegen.model.constructor.util.MAX_ARRAY_INITIALIZER_SIZE
+import org.utbot.framework.codegen.model.constructor.util.arrayInitializer
 import org.utbot.framework.codegen.model.constructor.util.get
 import org.utbot.framework.codegen.model.constructor.util.isDefaultValueOf
 import org.utbot.framework.codegen.model.constructor.util.isNotDefaultValueOf
 import org.utbot.framework.codegen.model.constructor.util.typeCast
 import org.utbot.framework.codegen.model.tree.CgAllocateArray
-import org.utbot.framework.codegen.model.tree.CgAllocateInitializedArray
 import org.utbot.framework.codegen.model.tree.CgDeclaration
 import org.utbot.framework.codegen.model.tree.CgEnumConstantAccess
 import org.utbot.framework.codegen.model.tree.CgExpression
 import org.utbot.framework.codegen.model.tree.CgFieldAccess
 import org.utbot.framework.codegen.model.tree.CgGetJavaClass
 import org.utbot.framework.codegen.model.tree.CgLiteral
-import org.utbot.framework.codegen.model.tree.CgNotNullAssertion
 import org.utbot.framework.codegen.model.tree.CgStaticFieldAccess
 import org.utbot.framework.codegen.model.tree.CgValue
 import org.utbot.framework.codegen.model.tree.CgVariable
@@ -222,10 +222,21 @@ internal class CgVariableConstructor(val context: CgContext) :
             arrayModel.stores.getOrDefault(it, arrayModel.constModel)
         }
 
-        val canInitWithValues = elementModels.all { it is UtPrimitiveModel } || elementModels.all { it is UtNullModel }
+        val allPrimitives = elementModels.all { it is UtPrimitiveModel }
+        val allNulls = elementModels.all { it is UtNullModel }
+        // we can use array initializer if all elements are primitives or all of them are null,
+        // and the size of an array is not greater than the fixed maximum size
+        val canInitWithValues = (allPrimitives || allNulls) && elementModels.size <= MAX_ARRAY_INITIALIZER_SIZE
 
         val initializer = if (canInitWithValues) {
-            CgAllocateInitializedArray(arrayModel)
+            val elements = elementModels.map { model ->
+                when (model) {
+                    is UtPrimitiveModel -> model.value.resolve()
+                    is UtNullModel -> null.resolve()
+                    else -> error("Non primitive or null model $model is unexpected in array initializer")
+                }
+            }
+            arrayInitializer(arrayModel.classId, elementType, elements)
         } else {
             CgAllocateArray(arrayModel.classId, elementType, arrayModel.length)
         }

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt

@@ -162,12 +162,12 @@ internal abstract class TestFrameworkManager(val context: CgContext)
             name = timeoutArgumentName,
             value = timeoutMs.resolve()
         )
-        val testAnnotation = collectedTestMethodAnnotations.singleOrNull { it.classId == testFramework.testAnnotationId }
+        val testAnnotation = collectedMethodAnnotations.singleOrNull { it.classId == testFramework.testAnnotationId }
 
         if (testAnnotation is CgMultipleArgsAnnotation) {
             testAnnotation.arguments += timeout
         } else {
-            collectedTestMethodAnnotations += CgMultipleArgsAnnotation(
+            collectedMethodAnnotations += CgMultipleArgsAnnotation(
                 testFramework.testAnnotationId,
                 mutableListOf(timeout)
             )
@@ -180,7 +180,7 @@ internal abstract class TestFrameworkManager(val context: CgContext)
     // because other test frameworks do not support such feature.
     open fun addDisplayName(name: String) {
         val displayName = CgSingleArgAnnotation(Junit5.displayNameClassId, stringLiteral(name))
-        collectedTestMethodAnnotations += CgCommentedAnnotation(displayName)
+        collectedMethodAnnotations += CgCommentedAnnotation(displayName)
     }
 
     protected fun ClassId.toExceptionClass(): CgExpression =
@@ -241,7 +241,7 @@ internal class TestNgManager(context: CgContext) : TestFrameworkManager(context)
             value = reason.resolve()
         )
 
-        val testAnnotation = collectedTestMethodAnnotations.singleOrNull { it.classId == testFramework.testAnnotationId }
+        val testAnnotation = collectedMethodAnnotations.singleOrNull { it.classId == testFramework.testAnnotationId }
         if (testAnnotation is CgMultipleArgsAnnotation) {
             testAnnotation.arguments += disabledAnnotationArgument
 
@@ -271,7 +271,7 @@ internal class TestNgManager(context: CgContext) : TestFrameworkManager(context)
                 testAnnotation.arguments += descriptionTestAnnotationArgument
             }
         } else {
-            collectedTestMethodAnnotations += CgMultipleArgsAnnotation(
+            collectedMethodAnnotations += CgMultipleArgsAnnotation(
                 testFramework.testAnnotationId,
                 mutableListOf(disabledAnnotationArgument, descriptionTestAnnotationArgument)
             )
@@ -291,19 +291,19 @@ internal class Junit4Manager(context: CgContext) : TestFrameworkManager(context)
             name = "expected",
             value = classLiteralAnnotationArgument(exception, codegenLanguage)
         )
-        val testAnnotation = collectedTestMethodAnnotations.singleOrNull { it.classId == testFramework.testAnnotationId }
+        val testAnnotation = collectedMethodAnnotations.singleOrNull { it.classId == testFramework.testAnnotationId }
         if (testAnnotation is CgMultipleArgsAnnotation) {
             testAnnotation.arguments += expected
         } else {
-            collectedTestMethodAnnotations += CgMultipleArgsAnnotation(testFramework.testAnnotationId, mutableListOf(expected))
+            collectedMethodAnnotations += CgMultipleArgsAnnotation(testFramework.testAnnotationId, mutableListOf(expected))
         }
         block()
     }
 
     override fun disableTestMethod(reason: String) {
         require(testFramework is Junit4) { "According to settings, JUnit4 was expected, but got: $testFramework" }
 
-        collectedTestMethodAnnotations += CgMultipleArgsAnnotation(
+        collectedMethodAnnotations += CgMultipleArgsAnnotation(
             testFramework.ignoreAnnotationClassId,
             mutableListOf(
                 CgNamedAnnotationArgument(
@@ -339,7 +339,7 @@ internal class Junit5Manager(context: CgContext) : TestFrameworkManager(context)
 
     override fun addDisplayName(name: String) {
         require(testFramework is Junit5) { "According to settings, JUnit5 was expected, but got: $testFramework" }
-        collectedTestMethodAnnotations += statementConstructor.annotation(testFramework.displayNameClassId, name)
+        collectedMethodAnnotations += statementConstructor.annotation(testFramework.displayNameClassId, name)
     }
 
     override fun setTestExecutionTimeout(timeoutMs: Long) {
@@ -358,7 +358,7 @@ internal class Junit5Manager(context: CgContext) : TestFrameworkManager(context)
         )
         importIfNeeded(testFramework.timeunitClassId)
 
-        collectedTestMethodAnnotations += CgMultipleArgsAnnotation(
+        collectedMethodAnnotations += CgMultipleArgsAnnotation(
             Junit5.timeoutClassId,
             timeoutAnnotationArguments
         )
@@ -367,7 +367,7 @@ internal class Junit5Manager(context: CgContext) : TestFrameworkManager(context)
     override fun disableTestMethod(reason: String) {
         require(testFramework is Junit5) { "According to settings, JUnit5 was expected, but got: $testFramework" }
 
-        collectedTestMethodAnnotations += CgMultipleArgsAnnotation(
+        collectedMethodAnnotations += CgMultipleArgsAnnotation(
             testFramework.disabledAnnotationClassId,
             mutableListOf(
                 CgNamedAnnotationArgument(

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/TestFrameworkManager.kt

@@ -31,7 +31,6 @@ import org.utbot.framework.codegen.model.tree.CgParameterDeclaration
 import org.utbot.framework.codegen.model.tree.CgReturnStatement
 import org.utbot.framework.codegen.model.tree.CgSingleArgAnnotation
 import org.utbot.framework.codegen.model.tree.CgSingleLineComment
-import org.utbot.framework.codegen.model.tree.CgStatement
 import org.utbot.framework.codegen.model.tree.CgThrowStatement
 import org.utbot.framework.codegen.model.tree.CgTryCatch
 import org.utbot.framework.codegen.model.tree.CgVariable
@@ -40,7 +39,6 @@ import org.utbot.framework.codegen.model.tree.buildCgForEachLoop
 import org.utbot.framework.codegen.model.tree.buildDeclaration
 import org.utbot.framework.codegen.model.tree.buildDoWhileLoop
 import org.utbot.framework.codegen.model.tree.buildForLoop
-import org.utbot.framework.codegen.model.tree.buildSimpleBlock
 import org.utbot.framework.codegen.model.tree.buildTryCatch
 import org.utbot.framework.codegen.model.tree.buildWhileLoop
 import org.utbot.framework.codegen.model.util.buildExceptionHandler
@@ -59,6 +57,7 @@ import org.utbot.framework.plugin.api.util.isSubtypeOf
 import org.utbot.framework.plugin.api.util.objectArrayClassId
 import org.utbot.framework.plugin.api.util.objectClassId
 import fj.data.Either
+import org.utbot.framework.codegen.model.tree.CgArrayInitializer
 import java.lang.reflect.Constructor
 import java.lang.reflect.Method
 import kotlin.reflect.KFunction
@@ -106,7 +105,7 @@ interface CgStatementConstructor {
     infix fun CgExpression.`=`(value: Any?)
     infix fun CgExpression.and(other: CgExpression): CgLogicalAnd
     infix fun CgExpression.or(other: CgExpression): CgLogicalOr
-    fun ifStatement(condition: CgExpression, trueBranch: () -> Unit): CgIfStatement
+    fun ifStatement(condition: CgExpression, trueBranch: () -> Unit, falseBranch: (() -> Unit)? = null): CgIfStatement
     fun forLoop(init: CgForLoopBuilder.() -> Unit)
     fun whileLoop(condition: CgExpression, statements: () -> Unit)
     fun doWhileLoop(condition: CgExpression, statements: () -> Unit)
@@ -117,7 +116,7 @@ interface CgStatementConstructor {
     fun CgTryCatch.catch(exception: ClassId, init: (CgVariable) -> Unit): CgTryCatch
     fun CgTryCatch.finally(init: () -> Unit): CgTryCatch
 
-    fun innerBlock(init: () -> Unit, additionalStatements: List<CgStatement>): CgInnerBlock
+    fun innerBlock(init: () -> Unit): CgInnerBlock
 
 //    fun CgTryCatchBuilder.statements(init: () -> Unit)
 //    fun CgTryCatchBuilder.handler(exception: ClassId, init: (CgVariable) -> Unit)
@@ -176,6 +175,7 @@ internal class CgStatementConstructorImpl(context: CgContext) :
         val (type, expr) = when (baseExpr) {
             is CgEnumConstantAccess -> guardEnumConstantAccess(baseExpr)
             is CgAllocateArray -> guardArrayAllocation(baseExpr)
+            is CgArrayInitializer -> guardArrayInitializer(baseExpr)
             is CgExecutableCall -> guardExecutableCall(baseType, baseExpr)
             else -> guardExpression(baseType, baseExpr)
         }
@@ -250,8 +250,10 @@ internal class CgStatementConstructorImpl(context: CgContext) :
     override fun CgExpression.or(other: CgExpression): CgLogicalOr =
         CgLogicalOr(this, other)
 
-    override fun ifStatement(condition: CgExpression, trueBranch: () -> Unit): CgIfStatement {
-        return CgIfStatement(condition, block(trueBranch)).also {
+    override fun ifStatement(condition: CgExpression, trueBranch: () -> Unit, falseBranch: (() -> Unit)?): CgIfStatement {
+        val trueBranchBlock = block(trueBranch)
+        val falseBranchBlock = falseBranch?.let { block(it) }
+        return CgIfStatement(condition, trueBranchBlock, falseBranchBlock).also {
             currentBlock += it
         }
     }
@@ -300,12 +302,10 @@ internal class CgStatementConstructorImpl(context: CgContext) :
         return this.copy(finally = finallyBlock)
     }
 
-    override fun innerBlock(
-        init: () -> Unit,
-        additionalStatements: List<CgStatement>,
-    ): CgInnerBlock = buildSimpleBlock {
-        statements = mutableListOf<CgStatement>() + block(init) + additionalStatements
-    }
+    override fun innerBlock(init: () -> Unit): CgInnerBlock =
+        CgInnerBlock(block(init)).also {
+            currentBlock += it
+        }
 
     override fun comment(text: String): CgComment =
         CgSingleLineComment(text).also {
@@ -421,13 +421,21 @@ internal class CgStatementConstructorImpl(context: CgContext) :
     }
 
     private fun guardArrayAllocation(allocation: CgAllocateArray): ExpressionWithType {
+        return guardArrayCreation(allocation.type, allocation.size, allocation)
+    }
+
+    private fun guardArrayInitializer(initializer: CgArrayInitializer): ExpressionWithType {
+        return guardArrayCreation(initializer.type, initializer.size, initializer)
+    }
+
+    private fun guardArrayCreation(arrayType: ClassId, arraySize: Int, initialization: CgExpression): ExpressionWithType {
         // TODO: check if this is the right way to check array type accessibility
-        return if (allocation.type.isAccessibleFrom(testClassPackageName)) {
-            ExpressionWithType(allocation.type, allocation)
+        return if (arrayType.isAccessibleFrom(testClassPackageName)) {
+            ExpressionWithType(arrayType, initialization)
         } else {
             ExpressionWithType(
                 objectArrayClassId,
-                testClassThisInstance[createArray](allocation.elementType.name, allocation.size)
+                testClassThisInstance[createArray](arrayType.elementClassId!!.name, arraySize)
             )
         }
     }

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt

@@ -42,6 +42,9 @@ import org.utbot.framework.plugin.api.util.shortClassId
 import org.utbot.framework.plugin.api.util.underlyingType
 import kotlinx.collections.immutable.PersistentList
 import kotlinx.collections.immutable.PersistentSet
+import org.utbot.framework.codegen.model.tree.CgAllocateInitializedArray
+import org.utbot.framework.codegen.model.tree.CgArrayInitializer
+import org.utbot.framework.plugin.api.util.arrayLikeName
 
 internal data class EnvironmentFieldStateCache(
     val thisInstance: FieldStateCache,
@@ -153,6 +156,8 @@ private fun FieldPath.toStringList(): List<String> =
 internal fun infiniteInts(): Sequence<Int> =
     generateSequence(1) { it + 1 }
 
+internal const val MAX_ARRAY_INITIALIZER_SIZE = 10
+
 /**
  * Checks if we have already imported a class with such simple name.
  * If so, we cannot import [type] (because it will be used with simple name and will be clashed with already imported)
@@ -216,6 +221,39 @@ internal fun CgContextOwner.typeCast(
     return CgTypeCast(targetType, expression, isSafetyCast)
 }
 
+@Suppress("unused")
+internal fun newArrayOf(elementType: ClassId, values: List<CgExpression>): CgAllocateInitializedArray {
+    val arrayType = arrayTypeOf(elementType)
+    return CgAllocateInitializedArray(arrayInitializer(arrayType, elementType, values))
+}
+
+internal fun arrayInitializer(arrayType: ClassId, elementType: ClassId, values: List<CgExpression>): CgArrayInitializer =
+    CgArrayInitializer(arrayType, elementType, values)
+
+/**
+ * For a given [elementType] returns a [ClassId] of an array with elements of this type.
+ * For example, for an id of `int` the result will be an id of `int[]`.
+ *
+ * @param elementType the element type of the returned array class id
+ * @param isNullable a flag whether returned array is nullable or not
+ */
+internal fun arrayTypeOf(elementType: ClassId, isNullable: Boolean = false): ClassId {
+    val arrayIdName = "[${elementType.arrayLikeName}"
+    return when (elementType) {
+        is BuiltinClassId -> BuiltinClassId(
+            name = arrayIdName,
+            canonicalName = "${elementType.canonicalName}[]",
+            simpleName = "${elementType.simpleName}[]",
+            isNullable = isNullable
+        )
+        else -> ClassId(
+            name = arrayIdName,
+            elementClassId = elementType,
+            isNullable = isNullable
+        )
+    }
+}
+
 @Suppress("unused")
 internal fun CgContextOwner.getJavaClass(classId: ClassId): CgGetClass {
     importIfNeeded(classId)