From b79e7761a5084dac320cc2250577bbfca2124fa7 Mon Sep 17 00:00:00 2001 From: Arsen Nagdalian Date: Thu, 18 Aug 2022 14:31:41 +0300 Subject: [PATCH 1/4] Support hidden fields access in code generation --- .../objects/HiddenFieldAccessModifiersTest.kt | 3 +- .../objects/HiddenFieldExampleTest.kt | 3 +- .../constructor/builtin/ReflectionBuiltins.kt | 8 +- .../constructor/builtin/UtilMethodBuiltins.kt | 4 +- .../model/constructor/context/CgContext.kt | 6 + .../tree/CgCallableAccessManager.kt | 256 ++++++++++++++---- .../constructor/tree/CgFieldStateManager.kt | 5 +- .../constructor/tree/CgMethodConstructor.kt | 4 +- .../constructor/tree/CgVariableConstructor.kt | 5 +- .../util/CgStatementConstructor.kt | 61 +++++ .../framework/codegen/model/util/DslUtil.kt | 11 - .../codegen/model/util/ExecutableIdUtil.kt | 2 +- .../codegen/model/visitor/CgKotlinRenderer.kt | 46 +++- .../codegen/model/visitor/UtilMethods.kt | 83 ++---- 14 files changed, 337 insertions(+), 160 deletions(-) diff --git a/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldAccessModifiersTest.kt b/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldAccessModifiersTest.kt index 0c29e632dd..84820fb0d2 100644 --- a/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldAccessModifiersTest.kt +++ b/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldAccessModifiersTest.kt @@ -7,8 +7,7 @@ import org.utbot.testcheckers.eq internal class HiddenFieldAccessModifiersTest : UtValueTestCaseChecker(testClass = HiddenFieldAccessModifiersExample::class) { @Test fun testCheckSuperFieldEqualsOne() { - // TODO: currently, codegen can't handle tests with field hiding (see #646) - withEnabledTestingCodeGeneration(testCodeGeneration = false) { + withEnabledTestingCodeGeneration(testCodeGeneration = true) { check( HiddenFieldAccessModifiersExample::checkSuperFieldEqualsOne, eq(3), diff --git a/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldExampleTest.kt b/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldExampleTest.kt index 4a7e6af07c..8f8ebf7771 100644 --- a/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldExampleTest.kt +++ b/utbot-framework-test/src/test/kotlin/org/utbot/examples/objects/HiddenFieldExampleTest.kt @@ -21,8 +21,7 @@ internal class HiddenFieldExampleTest : UtValueTestCaseChecker(testClass = Hidde @Test fun testCheckSuccField() { - // TODO: currently, codegen can't handle tests with field hiding (see #646) - withEnabledTestingCodeGeneration(testCodeGeneration = false) { + withEnabledTestingCodeGeneration(testCodeGeneration = true) { check( HiddenFieldExample::checkSuccField, eq(5), diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/ReflectionBuiltins.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/ReflectionBuiltins.kt index dd5acb019f..0120e8d0fc 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/ReflectionBuiltins.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/ReflectionBuiltins.kt @@ -21,10 +21,10 @@ import java.lang.reflect.InvocationTargetException internal val reflectionBuiltins: Set get() = setOf( - setAccessible, invoke, newInstance, get, forName, + setAccessible, invoke, newInstance, getMethodId, forName, getDeclaredMethod, getDeclaredConstructor, allocateInstance, getClass, getDeclaredField, isEnumConstant, getFieldName, - equals, getSuperclass, set, newArrayInstance, + equals, getSuperclass, setMethodId, newArrayInstance, setArrayElement, getArrayElement, getTargetException, ) @@ -49,7 +49,7 @@ internal val newInstance = methodId( arguments = arrayOf(Array::class.id) ) -internal val get = methodId( +internal val getMethodId = methodId( classId = Field::class.id, name = "get", returnType = objectClassId, @@ -132,7 +132,7 @@ internal val getSuperclass = methodId( returnType = Class::class.id ) -internal val set = methodId( +internal val setMethodId = methodId( classId = Field::class.id, name = "set", returnType = voidClassId, diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/UtilMethodBuiltins.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/UtilMethodBuiltins.kt index 4afe1d8cc8..ea3665583c 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/UtilMethodBuiltins.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/builtin/UtilMethodBuiltins.kt @@ -69,7 +69,7 @@ internal abstract class UtilMethodProvider(val utilClassId: ClassId) { get() = utilClassId.utilMethodId( name = "setField", returnType = voidClassId, - arguments = arrayOf(objectClassId, stringClassId, objectClassId) + arguments = arrayOf(objectClassId, stringClassId, stringClassId, objectClassId) ) val setStaticFieldMethodId: MethodId @@ -83,7 +83,7 @@ internal abstract class UtilMethodProvider(val utilClassId: ClassId) { get() = utilClassId.utilMethodId( name = "getFieldValue", returnType = objectClassId, - arguments = arrayOf(objectClassId, stringClassId) + arguments = arrayOf(objectClassId, stringClassId, stringClassId) ) val getStaticFieldValueMethodId: MethodId diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/context/CgContext.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/context/CgContext.kt index c9b0dbd76e..62ea4e64f7 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/context/CgContext.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/context/CgContext.kt @@ -162,6 +162,9 @@ internal interface CgContextOwner { // java.lang.reflect.Executable is a superclass of both of these types. var declaredExecutableRefs: PersistentMap + // Variables of java.lang.reflect.Field type declared in the current name scope + var declaredFieldRefs: PersistentMap + // generated this instance for method under test var thisInstance: CgValue? @@ -308,6 +311,7 @@ internal interface CgContextOwner { val prevVariableNames = existingVariableNames val prevDeclaredClassRefs = declaredClassRefs val prevDeclaredExecutableRefs = declaredExecutableRefs + val prevDeclaredFieldRefs = declaredFieldRefs val prevValueByModel = IdentityHashMap(valueByModel) val prevValueByModelId = valueByModelId.toMutableMap() return try { @@ -316,6 +320,7 @@ internal interface CgContextOwner { existingVariableNames = prevVariableNames declaredClassRefs = prevDeclaredClassRefs declaredExecutableRefs = prevDeclaredExecutableRefs + declaredFieldRefs = prevDeclaredFieldRefs valueByModel = prevValueByModel valueByModelId = prevValueByModelId } @@ -416,6 +421,7 @@ internal data class CgContext( override var existingVariableNames: PersistentSet = persistentSetOf(), override var declaredClassRefs: PersistentMap = persistentMapOf(), override var declaredExecutableRefs: PersistentMap = persistentMapOf(), + override var declaredFieldRefs: PersistentMap = persistentMapOf(), override var thisInstance: CgValue? = null, override val methodArguments: MutableList = mutableListOf(), override val codeGenerationErrors: MutableMap> = mutableMapOf(), diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt index b70fd7af1d..7113f0fe16 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt @@ -8,14 +8,15 @@ import org.utbot.framework.codegen.model.constructor.builtin.anyOfClass import org.utbot.framework.codegen.model.constructor.builtin.forName import org.utbot.framework.codegen.model.constructor.builtin.getDeclaredConstructor import org.utbot.framework.codegen.model.constructor.builtin.getDeclaredMethod +import org.utbot.framework.codegen.model.constructor.builtin.getMethodId import org.utbot.framework.codegen.model.constructor.builtin.getTargetException import org.utbot.framework.codegen.model.constructor.builtin.invoke import org.utbot.framework.codegen.model.constructor.builtin.newInstance import org.utbot.framework.codegen.model.constructor.builtin.setAccessible 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.tree.CgCallableAccessManagerImpl.FieldAccessorSuitability.* import org.utbot.framework.codegen.model.constructor.util.CgComponents -import org.utbot.framework.codegen.model.constructor.util.classCgClassId import org.utbot.framework.codegen.model.constructor.util.getAmbiguousOverloadsOf import org.utbot.framework.codegen.model.constructor.util.importIfNeeded import org.utbot.framework.codegen.model.constructor.util.isUtil @@ -25,10 +26,11 @@ import org.utbot.framework.codegen.model.tree.CgAssignment import org.utbot.framework.codegen.model.tree.CgConstructorCall import org.utbot.framework.codegen.model.tree.CgExecutableCall import org.utbot.framework.codegen.model.tree.CgExpression -import org.utbot.framework.codegen.model.tree.CgGetJavaClass +import org.utbot.framework.codegen.model.tree.CgFieldAccess import org.utbot.framework.codegen.model.tree.CgMethodCall import org.utbot.framework.codegen.model.tree.CgSpread import org.utbot.framework.codegen.model.tree.CgStatement +import org.utbot.framework.codegen.model.tree.CgStaticFieldAccess import org.utbot.framework.codegen.model.tree.CgThisInstance import org.utbot.framework.codegen.model.tree.CgValue import org.utbot.framework.codegen.model.tree.CgVariable @@ -36,17 +38,19 @@ import org.utbot.framework.codegen.model.util.at import org.utbot.framework.codegen.model.util.isAccessibleFrom import org.utbot.framework.codegen.model.util.nullLiteral import org.utbot.framework.codegen.model.util.resolve +import org.utbot.framework.plugin.api.BuiltinClassId import org.utbot.framework.plugin.api.BuiltinMethodId import org.utbot.framework.plugin.api.ClassId import org.utbot.framework.plugin.api.ConstructorId import org.utbot.framework.plugin.api.ExecutableId +import org.utbot.framework.plugin.api.FieldId import org.utbot.framework.plugin.api.MethodId import org.utbot.framework.plugin.api.UtExplicitlyThrownException import org.utbot.framework.plugin.api.util.exceptions import org.utbot.framework.plugin.api.util.id import org.utbot.framework.plugin.api.util.isArray -import org.utbot.framework.plugin.api.util.isPrimitive import org.utbot.framework.plugin.api.util.isSubtypeOf +import org.utbot.framework.plugin.api.util.jClass import org.utbot.framework.plugin.api.util.method import org.utbot.framework.plugin.api.util.objectArrayClassId import org.utbot.framework.plugin.api.util.objectClassId @@ -69,6 +73,12 @@ interface CgCallableAccessManager { operator fun ConstructorId.invoke(vararg args: Any?): CgExecutableCall operator fun CgIncompleteMethodCall.invoke(vararg args: Any?): CgMethodCall + + // non-static fields + operator fun CgExpression.get(fieldId: FieldId): CgExpression + + // static fields + operator fun ClassId.get(fieldId: FieldId): CgStaticFieldAccess } internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableAccessManager, @@ -106,6 +116,26 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA return methodCall } + override operator fun CgExpression.get(fieldId: FieldId): CgExpression { + return when (val suitability = fieldId.accessSuitability(this)) { + // receiver (a.k.a. caller) is suitable, so we access field directly + is Suitable -> CgFieldAccess(this, fieldId) + // receiver has to be type cast, and then we can access the field + is RequiresTypeCast -> { + CgFieldAccess( + caller = typeCast(suitability.targetType, this), + fieldId + ) + } + // we can access the field only via reflection + is ReflectionOnly -> fieldId.accessWithReflection(this) + } + } + + override operator fun ClassId.get(fieldId: FieldId): CgStaticFieldAccess { + return CgStaticFieldAccess(fieldId) + } + private fun newMethodCall(methodId: MethodId) { if (isUtil(methodId)) requiredUtilMethods += methodId importIfNeeded(methodId) @@ -142,13 +172,15 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } } - private infix fun CgExpression?.canBeReceiverOf(executable: MethodId): Boolean = - when { - // TODO: rewrite by using CgMethodId, etc. - outerMostTestClass == executable.classId && this isThisInstanceOf outerMostTestClass -> true - executable.isStatic -> true - else -> this?.type?.isSubtypeOf(executable.classId) ?: false + private infix fun CgExpression.canBeReceiverOf(executable: MethodId): Boolean { + return when { + // method of the current test class can be called on its 'this' instance + currentTestClass == executable.classId && this isThisInstanceOf currentTestClass -> true + // method of a class can be called on an object of this class or any of its subtypes + this.type isSubtypeOf executable.classId -> true + else -> false } + } private infix fun CgExpression.canBeArgOf(type: ClassId): Boolean { // TODO: SAT-1210 support generics so that we wouldn't need to check specific cases such as this one @@ -224,10 +256,115 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA /** * @return true if a method can be called with the given arguments without reflection */ - private fun MethodId.canBeCalledWith(caller: CgExpression?, args: List): Boolean = - (isUtil(this) || isAccessibleFrom(testClassPackageName)) - && caller canBeReceiverOf this - && args canBeArgsOf this + private fun MethodId.canBeCalledWith(caller: CgExpression?, args: List): Boolean { + // check method accessibility + if (!isAccessibleFrom(testClassPackageName)) { + return false + } + + // check arguments suitability + if (!(args canBeArgsOf this)) { + return false + } + + // If method is static, then it must not have a caller. + if (this.isStatic) { + require(caller == null) { "Caller expression of a static method call must be null" } + return true + } + + // If method is from current test class, then it may or may not have a caller + if (this.classId == currentTestClass) { + return caller?.canBeReceiverOf(this) ?: true + } + + requireNotNull(caller) { "Method must have a caller, unless it is the method of the current test class or a static method" } + return caller canBeReceiverOf this + } + + private fun FieldId.accessSuitability(accessor: CgExpression?): FieldAccessorSuitability { + // if field is static + if (this.isStatic) { + require(accessor == null) { "Accessor expression of a static field access must be null" } + return Suitable + } + + // if field is declared in the current test class + if (this.declaringClass == currentTestClass) { + return when { + // field of the current class can be accessed without explicit accessor + accessor == null -> Suitable + // field of the current class can be accessed with `this` reference + accessor isThisInstanceOf currentTestClass -> Suitable + // field of the current class can be accessed by the instance of this class + accessor.type isSubtypeOf currentTestClass -> Suitable + // in any other case, we have to use reflection + else -> ReflectionOnly + } + } + + requireNotNull(accessor) { + "Field access must have a non-null accessor, unless it is the field of the current test class or a static field" + } + + if (this.declaringClass == accessor.type) { + // if the field was declared in class `T`, and the accessor is __exactly__ of this type (not a subtype), + // then we can safely access the field + return Suitable + } + + val fieldDeclaringClassType = this.declaringClass + val accessorType = accessor.type + + if (fieldDeclaringClassType is BuiltinClassId || accessorType is BuiltinClassId) { + // The rest of the logic of this method processes hidden fields. + // We cannot check the fields of builtin classes, so we assume that we work correctly with them, + // because they are well known library classes (e.g. Mockito) that are unlikely to have hidden fields. + return Suitable + } + + val fieldName = this.name + + if (accessorType isSubtypeOf fieldDeclaringClassType || fieldDeclaringClassType isSubtypeOf accessorType) { + if (fieldName.isNameOfFieldsInBothClasses(accessorType, fieldDeclaringClassType)) { + // classes are in an inheritance relation, and they both have a field with the same name, + // which means that field shadowing is found + return when { + // if we can access the declaring class of field, + // then we can use a type cast the accessor to it + fieldDeclaringClassType isAccessibleFrom testClassPackageName -> { + RequiresTypeCast(fieldDeclaringClassType) + } + // otherwise, we can only use reflection + else -> ReflectionOnly + } + } + // if no field shadowing is found, we can just access the field directly + return Suitable + } + + // Accessor type is not subtype or supertype of the field's declaring class. + // So the only remaining option to access the field is to use reflection. + return ReflectionOnly + } + + /** + * Determine if each of the classes [left] and [right] has their own field with name specified by the [String] receiver. + */ + private fun String.isNameOfFieldsInBothClasses(left: ClassId, right: ClassId): Boolean { + // make sure that non-builtin class ids are passed to this method + require(left !is BuiltinClassId && right !is BuiltinClassId) { + "The given class ids must not be builtin, because this method works with their jClass" + } + + val leftClass = left.jClass + val rightClass = right.jClass + + val leftField = leftClass.declaredFields.find { it.name == this } + val rightField = rightClass.declaredFields.find { it.name == this } + + return leftField != null && rightField != null + } /** * @return true if a constructor can be called with the given arguments without reflection @@ -268,38 +405,6 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA if (ancestors.isNotEmpty()) typeCast(targetType, arg) else arg } - private fun ExecutableId.toExecutableVariable(args: List): CgVariable { - val declaringClass = statementConstructor.newVar(Class::class.id) { classId[forName](classId.name) } - val argTypes = (args zip parameters).map { (arg, paramType) -> - val baseName = when (arg) { - is CgVariable -> "${arg.name}Type" - else -> "${paramType.prettifiedName.decapitalize()}Type" - } - statementConstructor.newVar(classCgClassId, baseName) { - if (paramType.isPrimitive) { - CgGetJavaClass(paramType) - } else { - Class::class.id[forName](paramType.name) - } - } - } - - return when (this) { - is MethodId -> { - val name = this.name + "Method" - statementConstructor.newVar(java.lang.reflect.Method::class.id, name) { - declaringClass[getDeclaredMethod](this.name, *argTypes.toTypedArray()) - } - } - is ConstructorId -> { - val name = this.classId.prettifiedName.decapitalize() + "Constructor" - statementConstructor.newVar(java.lang.reflect.Constructor::class.id, name) { - declaringClass[getDeclaredConstructor](*argTypes.toTypedArray()) - } - } - } - } - /** * Receives a list of [CgExpression]. * Transforms it into a list of [CgExpression] where: @@ -318,14 +423,18 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } } - private fun MethodId.callWithReflection(caller: CgExpression?, args: List): CgMethodCall { - containsReflectiveCall = true - val method = declaredExecutableRefs[this] - ?: toExecutableVariable(args).also { - declaredExecutableRefs = declaredExecutableRefs.put(this, it) + private fun FieldId.accessWithReflection(accessor: CgExpression?): CgMethodCall { + val field = declaredFieldRefs[this] + ?: statementConstructor.createFieldVariable(this).also { + declaredFieldRefs = declaredFieldRefs.put(this, it) +it[setAccessible](true) } + return field[getMethodId](accessor) + } + private fun MethodId.callWithReflection(caller: CgExpression?, args: List): CgMethodCall { + containsReflectiveCall = true + val method = getExecutableRefVariable(args) val arguments = args.guardedForReflectiveCall().toTypedArray() val argumentsArrayVariable = convertVarargToArray(method, arguments) @@ -334,18 +443,21 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA private fun ConstructorId.callWithReflection(args: List): CgExecutableCall { containsReflectiveCall = true - val constructor = declaredExecutableRefs[this] - ?: this.toExecutableVariable(args).also { - declaredExecutableRefs = declaredExecutableRefs.put(this, it) - +it[setAccessible](true) - } - + val constructor = getExecutableRefVariable(args) val arguments = args.guardedForReflectiveCall().toTypedArray() val argumentsArrayVariable = convertVarargToArray(constructor, arguments) return constructor[newInstance](argumentsArrayVariable) } + private fun ExecutableId.getExecutableRefVariable(arguments: List): CgVariable { + return declaredExecutableRefs[this] + ?: statementConstructor.createExecutableVariable(this, arguments).also { + declaredExecutableRefs = declaredExecutableRefs.put(this, it) + +it[setAccessible](true) + } + } + private fun convertVarargToArray(reflectionCallVariable: CgVariable, arguments: Array): CgVariable { val argumentsArrayVariable = variableConstructor.newVar( baseType = objectArrayClassId, @@ -378,9 +490,9 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA return when (methodId) { getEnumConstantByNameMethodId -> setOf(java.lang.IllegalAccessException::class.id) getStaticFieldValueMethodId, + setStaticFieldMethodId -> setOf(java.lang.IllegalAccessException::class.id, java.lang.NoSuchFieldException::class.id) getFieldValueMethodId, - setStaticFieldMethodId, - setFieldMethodId -> setOf(java.lang.IllegalAccessException::class.id, java.lang.NoSuchFieldException::class.id) + setFieldMethodId -> setOf(java.lang.ClassNotFoundException::class.id, java.lang.IllegalAccessException::class.id, java.lang.NoSuchFieldException::class.id) createInstanceMethodId -> setOf(Exception::class.id) getUnsafeInstanceMethodId -> setOf(java.lang.ClassNotFoundException::class.id, java.lang.NoSuchFieldException::class.id, java.lang.IllegalAccessException::class.id) createArrayMethodId -> setOf(java.lang.ClassNotFoundException::class.id) @@ -395,4 +507,32 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } } } + + /** + * This sealed class describes different extents of suitability (or matching) + * between an expression (in the role of a field accessor) and a field. + * + * In other words, this class and its inheritors describe if a given object (accessor) + * can be used to access a field, and if so, then are there any additional actions required (like type cast). + */ + private sealed class FieldAccessorSuitability { + /** + * Field can be accessed by a given accessor directly + */ + object Suitable : FieldAccessorSuitability() + + /** + * Field can be accessed by a given accessor, but it has to be type cast to the [targetType] + */ + class RequiresTypeCast(val targetType: ClassId) : FieldAccessorSuitability() + + /** + * Field can only be accessed by a given accessor via reflection. + * For example, if the accessor's type is inaccessible from the current package, + * so we cannot declare a variable of this type or perform a type cast. + * But there may be other cases. For example, we also cannot use + * anonymous classes' names in the code, so reflection may be required. + */ + object ReflectionOnly : FieldAccessorSuitability() + } } \ No newline at end of file diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgFieldStateManager.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgFieldStateManager.kt index 82b3fbb70c..59e4c9561e 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgFieldStateManager.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgFieldStateManager.kt @@ -17,7 +17,6 @@ import org.utbot.framework.codegen.model.tree.CgGetJavaClass import org.utbot.framework.codegen.model.tree.CgValue import org.utbot.framework.codegen.model.tree.CgVariable import org.utbot.framework.codegen.model.util.at -import org.utbot.framework.codegen.model.util.get import org.utbot.framework.codegen.model.util.isAccessibleFrom import org.utbot.framework.codegen.model.util.stringLiteral import org.utbot.framework.fields.ArrayElementAccess @@ -230,8 +229,8 @@ internal class CgFieldStateManagerImpl(val context: CgContext) val name = if (index == path.lastIndex) customName else getFieldVariableName(prev, passedPath) val expression = when (val newElement = path[index++]) { is FieldAccess -> { - val field = newElement.field - utilsClassId[getFieldValue](prev, stringLiteral(field.name)) + val fieldId = newElement.field + utilsClassId[getFieldValue](prev, fieldId.declaringClass.name, fieldId.name) } is ArrayElementAccess -> { Array::class.id[getArrayElement](prev, newElement.index) diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt index fee6dc125d..84a00a6d33 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt @@ -67,14 +67,12 @@ import org.utbot.framework.codegen.model.tree.convertDocToCg import org.utbot.framework.codegen.model.tree.toStatement import org.utbot.framework.codegen.model.util.canBeSetIn import org.utbot.framework.codegen.model.util.equalTo -import org.utbot.framework.codegen.model.util.get import org.utbot.framework.codegen.model.util.inc import org.utbot.framework.codegen.model.util.isAccessibleFrom import org.utbot.framework.codegen.model.util.length import org.utbot.framework.codegen.model.util.lessThan import org.utbot.framework.codegen.model.util.nullLiteral import org.utbot.framework.codegen.model.util.resolve -import org.utbot.framework.codegen.model.util.stringLiteral import org.utbot.framework.fields.ExecutionStateAnalyzer import org.utbot.framework.fields.FieldPath import org.utbot.framework.plugin.api.BuiltinClassId @@ -1048,7 +1046,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c if (variable.type.hasField(this) && isAccessibleFrom(testClassPackageName)) { if (jField.isStatic) CgStaticFieldAccess(this) else CgFieldAccess(variable, this) } else { - utilsClassId[getFieldValue](variable, stringLiteral(name)) + utilsClassId[getFieldValue](variable, this.declaringClass.name, this.name) } /** diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt index 6154bee0fa..a029ac6f3e 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt @@ -24,7 +24,6 @@ import org.utbot.framework.codegen.model.tree.CgValue import org.utbot.framework.codegen.model.tree.CgVariable import org.utbot.framework.codegen.model.util.at import org.utbot.framework.codegen.model.util.canBeSetIn -import org.utbot.framework.codegen.model.util.get import org.utbot.framework.codegen.model.util.inc import org.utbot.framework.codegen.model.util.isAccessibleFrom import org.utbot.framework.codegen.model.util.lessThan @@ -146,7 +145,7 @@ internal class CgVariableConstructor(val context: CgContext) : fieldAccess `=` variableForField } else { // composite models must not have info about static fields, hence only non-static fields are set here - +utilsClassId[setField](obj, fieldId.name, variableForField) + +utilsClassId[setField](obj, fieldId.declaringClass.name, fieldId.name, variableForField) } } return obj @@ -352,7 +351,7 @@ internal class CgVariableConstructor(val context: CgContext) : val init = if (classId.isAccessibleFrom(testClassPackageName)) { CgGetJavaClass(classId) } else { - classId[forName](classId.name) + Class::class.id[forName](classId.name) } return newVar(Class::class.id, baseName) { init } diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt index f3e1ae412a..f2b28e1781 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt @@ -57,9 +57,17 @@ 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.constructor.builtin.getDeclaredConstructor +import org.utbot.framework.codegen.model.constructor.builtin.getDeclaredField +import org.utbot.framework.codegen.model.constructor.builtin.getDeclaredMethod import org.utbot.framework.codegen.model.tree.CgArrayInitializer +import org.utbot.framework.codegen.model.tree.CgGetJavaClass import org.utbot.framework.codegen.model.tree.CgIsInstance +import org.utbot.framework.plugin.api.ConstructorId +import org.utbot.framework.plugin.api.FieldId +import org.utbot.framework.plugin.api.MethodId import org.utbot.framework.plugin.api.util.classClassId +import org.utbot.framework.plugin.api.util.isPrimitive import java.lang.reflect.Constructor import java.lang.reflect.Method import kotlin.reflect.KFunction @@ -113,6 +121,19 @@ interface CgStatementConstructor { fun doWhileLoop(condition: CgExpression, statements: () -> Unit) fun forEachLoop(init: CgForEachLoopBuilder.() -> Unit) + /** + * Create a variable of type [java.lang.reflect.Field] by the given [FieldId]. + */ + fun createFieldVariable(fieldId: FieldId): CgVariable + + + /** + * Given an [executableId] that represents method or constructor and a list of arguments for it, + * create a variable of type [java.lang.reflect.Method] or [java.lang.reflect.Constructor]. + * This created variable is returned. + */ + fun createExecutableVariable(executableId: ExecutableId, arguments: List): CgVariable + fun tryBlock(init: () -> Unit): CgTryCatch fun tryBlock(init: () -> Unit, resources: List?): CgTryCatch fun CgTryCatch.catch(exception: ClassId, init: (CgVariable) -> Unit): CgTryCatch @@ -284,6 +305,46 @@ internal class CgStatementConstructorImpl(context: CgContext) : currentBlock += buildCgForEachLoop(init) } + override fun createFieldVariable(fieldId: FieldId): CgVariable { + val declaringClass = newVar(Class::class.id) { Class::class.id[forName](fieldId.declaringClass.name) } + val name = fieldId.name + "Field" + return newVar(java.lang.reflect.Field::class.id, name) { + declaringClass[getDeclaredField](fieldId.name) + } + } + + override fun createExecutableVariable(executableId: ExecutableId, arguments: List): CgVariable { + val declaringClass = newVar(Class::class.id) { Class::class.id[forName](executableId.classId.name) } + val argTypes = (arguments zip executableId.parameters).map { (argument, parameterType) -> + val baseName = when (argument) { + is CgVariable -> "${argument.name}Type" + else -> "${parameterType.prettifiedName.decapitalize()}Type" + } + newVar(classCgClassId, baseName) { + if (parameterType.isPrimitive) { + CgGetJavaClass(parameterType) + } else { + Class::class.id[forName](parameterType.name) + } + } + } + + return when (executableId) { + is MethodId -> { + val name = executableId.name + "Method" + newVar(java.lang.reflect.Method::class.id, name) { + declaringClass[getDeclaredMethod](executableId.name, *argTypes.toTypedArray()) + } + } + is ConstructorId -> { + val name = executableId.classId.prettifiedName.decapitalize() + "Constructor" + newVar(java.lang.reflect.Constructor::class.id, name) { + declaringClass[getDeclaredConstructor](*argTypes.toTypedArray()) + } + } + } + } + override fun tryBlock(init: () -> Unit): CgTryCatch = tryBlock(init, null) override fun tryBlock(init: () -> Unit, resources: List?): CgTryCatch = diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/DslUtil.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/DslUtil.kt index b351855f7a..80583ef6ea 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/DslUtil.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/DslUtil.kt @@ -67,17 +67,6 @@ fun charLiteral(c: Char) = CgLiteral(charClassId, c) fun stringLiteral(string: String) = CgLiteral(stringClassId, string) -// Field access - -// non-static fields -operator fun CgExpression.get(fieldId: FieldId): CgFieldAccess = - CgFieldAccess(this, fieldId) - -// static fields -// TODO: unused receiver -operator fun ClassId.get(fieldId: FieldId): CgStaticFieldAccess = - CgStaticFieldAccess(fieldId) - // Get array length /** diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/ExecutableIdUtil.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/ExecutableIdUtil.kt index e5150759ab..855ee227bf 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/ExecutableIdUtil.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/ExecutableIdUtil.kt @@ -8,7 +8,7 @@ import org.utbot.framework.plugin.api.ExecutableId * * @param packageName name of the package we check accessibility from */ -fun ExecutableId.isAccessibleFrom(packageName: String): Boolean { +infix fun ExecutableId.isAccessibleFrom(packageName: String): Boolean { val isAccessibleFromPackageByModifiers = isPublic || (classId.packageName == packageName && (isPackagePrivate || isProtected)) return classId.isAccessibleFrom(packageName) && isAccessibleFromPackageByModifiers diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/CgKotlinRenderer.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/CgKotlinRenderer.kt index 1e5a107218..1a12c7d92a 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/CgKotlinRenderer.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/CgKotlinRenderer.kt @@ -226,29 +226,49 @@ internal class CgKotlinRenderer(context: CgRendererContext, printer: CgPrinter = element.right.accept(this) } - override fun visit(element: CgTypeCast) { - element.expression.accept(this) + /** + * Sometimes we can omit rendering type cast and simply render its [CgTypeCast.expression] instead. + * This method checks if the type cast can be omitted. + * + * For example, type cast can be omitted when a primitive wrapper is cast to its corresponding primitive (or vice versa), + * because in Kotlin there are no primitive types as opposed to Java. + * + * Also, sometimes we can omit type cast when the [CgTypeCast.targetType] is the same as the type of [CgTypeCast.expression]. + */ + private fun isCastNeeded(element: CgTypeCast): Boolean { + val targetType = element.targetType + val expressionType = element.expression.type + + val isPrimitiveToWrapperCast = targetType.isPrimitiveWrapper && expressionType.isPrimitive + val isWrapperToPrimitiveCast = targetType.isPrimitive && expressionType.isPrimitiveWrapper + val isNullLiteral = element.expression == nullLiteral() + + if (!isNullLiteral && element.isSafetyCast && (isPrimitiveToWrapperCast || isWrapperToPrimitiveCast)) { + return false + } + // perform type cast only if target type is not equal to expression type // but cast from nullable to not nullable should be performed // TODO SAT-1445 actually this safetyCast check looks like hack workaround and possibly does not work // so it should be carefully tested one day - if (!element.isSafetyCast || element.expression.type != element.targetType) { - // except for the case when a wrapper is cast to its primitive or vice versa - - val isCastFromPrimitiveToWrapper = element.targetType.isPrimitiveWrapper && - element.expression.type.isPrimitive - val isCastFromWrapperToPrimitive = element.targetType.isPrimitive && - element.expression.type.isPrimitiveWrapper - val isNullLiteral = element.expression == nullLiteral() - if (!isNullLiteral && element.isSafetyCast && (isCastFromPrimitiveToWrapper || isCastFromWrapperToPrimitive)) { - return - } + return !element.isSafetyCast || expressionType != targetType + } + + override fun visit(element: CgTypeCast) { + if (!isCastNeeded(element)) { + element.expression.accept(this) + } else { + print("(") + + element.expression.accept(this) if (element.isSafetyCast) print(" as? ") else print(" as ") print(getKotlinClassString(element.targetType)) renderTypeParameters(element.targetType.typeParameters) val initNullable = element.type.isNullable if (element.targetType.isNullable || initNullable) print("?") + + print(")") } } diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt index 4634590e6e..135b509949 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt @@ -152,47 +152,31 @@ private fun getFieldValue(visibility: Visibility, language: CodegenLanguage): St when (language) { CodegenLanguage.JAVA -> { """ - ${visibility by language}static Object getFieldValue(Object obj, String fieldName) throws IllegalAccessException, NoSuchFieldException { - Class clazz = obj.getClass(); - java.lang.reflect.Field field; - do { - try { - field = clazz.getDeclaredField(fieldName); - field.setAccessible(true); - java.lang.reflect.Field modifiersField = java.lang.reflect.Field.class.getDeclaredField("modifiers"); - modifiersField.setAccessible(true); - modifiersField.setInt(field, field.getModifiers() & ~java.lang.reflect.Modifier.FINAL); - - return field.get(obj); - } catch (NoSuchFieldException e) { - clazz = clazz.getSuperclass(); - } - } while (clazz != null); - - throw new NoSuchFieldException("Field '" + fieldName + "' not found on class " + obj.getClass()); + ${visibility by language}static Object getFieldValue(Object obj, String fieldClassName, String fieldName) throws ClassNotFoundException, IllegalAccessException, NoSuchFieldException { + Class clazz = Class.forName(fieldClassName); + java.lang.reflect.Field field = clazz.getDeclaredField(fieldName); + + field.setAccessible(true); + java.lang.reflect.Field modifiersField = java.lang.reflect.Field.class.getDeclaredField("modifiers"); + modifiersField.setAccessible(true); + modifiersField.setInt(field, field.getModifiers() & ~java.lang.reflect.Modifier.FINAL); + + return field.get(obj); } """ } CodegenLanguage.KOTLIN -> { """ - ${visibility by language}fun getFieldValue(any: kotlin.Any, fieldName: String): kotlin.Any? { - var clazz: Class<*>? = any.javaClass - var field: java.lang.reflect.Field - do { - try { - field = clazz!!.getDeclaredField(fieldName) - field.isAccessible = true - val modifiersField: java.lang.reflect.Field = java.lang.reflect.Field::class.java.getDeclaredField("modifiers") - modifiersField.isAccessible = true - modifiersField.setInt(field, field.modifiers and java.lang.reflect.Modifier.FINAL.inv()) - - return field.get(any) - } catch (e: NoSuchFieldException) { - clazz = clazz!!.superclass - } - } while (clazz != null) + ${visibility by language}fun getFieldValue(any: kotlin.Any, fieldClassName: String, fieldName: String): kotlin.Any? { + val clazz: Class<*> = Class.forName(fieldClassName) + val field: java.lang.reflect.Field = clazz.getDeclaredField(fieldName) + + field.isAccessible = true + val modifiersField: java.lang.reflect.Field = java.lang.reflect.Field::class.java.getDeclaredField("modifiers") + modifiersField.isAccessible = true + modifiersField.setInt(field, field.modifiers and java.lang.reflect.Modifier.FINAL.inv()) - throw NoSuchFieldException("Field '" + fieldName + "' not found on class " + any.javaClass) + return field.get(any) } """ } @@ -253,18 +237,9 @@ private fun setField(visibility: Visibility, language: CodegenLanguage): String when (language) { CodegenLanguage.JAVA -> { """ - ${visibility by language}static void setField(Object object, String fieldName, Object fieldValue) throws NoSuchFieldException, IllegalAccessException { - Class clazz = object.getClass(); - java.lang.reflect.Field field; - - do { - try { - field = clazz.getDeclaredField(fieldName); - } catch (Exception e) { - clazz = clazz.getSuperclass(); - field = null; - } - } while (field == null); + ${visibility by language}static void setField(Object object, String fieldClassName, String fieldName, Object fieldValue) throws ClassNotFoundException, NoSuchFieldException, IllegalAccessException { + Class clazz = Class.forName(fieldClassName); + java.lang.reflect.Field field = clazz.getDeclaredField(fieldName); java.lang.reflect.Field modifiersField = java.lang.reflect.Field.class.getDeclaredField("modifiers"); modifiersField.setAccessible(true); @@ -277,17 +252,9 @@ private fun setField(visibility: Visibility, language: CodegenLanguage): String } CodegenLanguage.KOTLIN -> { """ - ${visibility by language}fun setField(any: kotlin.Any, fieldName: String, fieldValue: kotlin.Any?) { - var clazz: Class<*> = any.javaClass - var field: java.lang.reflect.Field? - do { - try { - field = clazz.getDeclaredField(fieldName) - } catch (e: Exception) { - clazz = clazz.superclass - field = null - } - } while (field == null) + ${visibility by language}fun setField(any: kotlin.Any, fieldClassName: String, fieldName: String, fieldValue: kotlin.Any?) { + val clazz: Class<*> = Class.forName(fieldClassName) + val field: java.lang.reflect.Field = clazz.getDeclaredField(fieldName) val modifiersField: java.lang.reflect.Field = java.lang.reflect.Field::class.java.getDeclaredField("modifiers") modifiersField.isAccessible = true From 3503382f5de566a73d371959d85b6d533aea65d2 Mon Sep 17 00:00:00 2001 From: Arsen Nagdalian Date: Mon, 5 Sep 2022 21:40:37 +0300 Subject: [PATCH 2/4] Fix field access check --- .../tree/CgCallableAccessManager.kt | 164 ++++++++++++------ 1 file changed, 113 insertions(+), 51 deletions(-) diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt index 7113f0fe16..39cf16d30c 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt @@ -41,6 +41,7 @@ import org.utbot.framework.codegen.model.util.resolve import org.utbot.framework.plugin.api.BuiltinClassId import org.utbot.framework.plugin.api.BuiltinMethodId import org.utbot.framework.plugin.api.ClassId +import org.utbot.framework.plugin.api.CodegenLanguage import org.utbot.framework.plugin.api.ConstructorId import org.utbot.framework.plugin.api.ExecutableId import org.utbot.framework.plugin.api.FieldId @@ -132,9 +133,7 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } } - override operator fun ClassId.get(fieldId: FieldId): CgStaticFieldAccess { - return CgStaticFieldAccess(fieldId) - } + override operator fun ClassId.get(fieldId: FieldId): CgStaticFieldAccess = CgStaticFieldAccess(fieldId) private fun newMethodCall(methodId: MethodId) { if (isUtil(methodId)) requiredUtilMethods += methodId @@ -172,15 +171,14 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } } - private infix fun CgExpression.canBeReceiverOf(executable: MethodId): Boolean { - return when { + private infix fun CgExpression.canBeReceiverOf(executable: MethodId): Boolean = + when { // method of the current test class can be called on its 'this' instance currentTestClass == executable.classId && this isThisInstanceOf currentTestClass -> true // method of a class can be called on an object of this class or any of its subtypes this.type isSubtypeOf executable.classId -> true else -> false } - } private infix fun CgExpression.canBeArgOf(type: ClassId): Boolean { // TODO: SAT-1210 support generics so that we wouldn't need to check specific cases such as this one @@ -257,25 +255,28 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA * @return true if a method can be called with the given arguments without reflection */ private fun MethodId.canBeCalledWith(caller: CgExpression?, args: List): Boolean { - // check method accessibility + // Check method accessibility. if (!isAccessibleFrom(testClassPackageName)) { return false } - // check arguments suitability + // Check arguments suitability. if (!(args canBeArgsOf this)) { return false } - // If method is static, then it must not have a caller. - if (this.isStatic) { - require(caller == null) { "Caller expression of a static method call must be null" } + // If method is static, then it may not have a caller. + if (this.isStatic && caller == null) { return true } - // If method is from current test class, then it may or may not have a caller - if (this.classId == currentTestClass) { - return caller?.canBeReceiverOf(this) ?: true + if (this.isStatic && caller != null && codegenLanguage == CodegenLanguage.KOTLIN) { + error("In Kotlin, unlike Java, static methods cannot be called on an object") + } + + // If method is from current test class, then it may not have a caller. + if (this.classId == currentTestClass && caller == null) { + return true } requireNotNull(caller) { "Method must have a caller, unless it is the method of the current test class or a static method" } @@ -283,12 +284,20 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } private fun FieldId.accessSuitability(accessor: CgExpression?): FieldAccessorSuitability { - // if field is static - if (this.isStatic) { - require(accessor == null) { "Accessor expression of a static field access must be null" } + // Check field accessibility. + if (!isAccessibleFrom(testClassPackageName)) { + return ReflectionOnly + } + + // If field is static, then it may not have an accessor. + if (this.isStatic && accessor == null) { return Suitable } + if (this.isStatic && accessor != null && codegenLanguage == CodegenLanguage.KOTLIN) { + error("In Kotlin, unlike Java, static fields cannot be accessed by an object") + } + // if field is declared in the current test class if (this.declaringClass == currentTestClass) { return when { @@ -317,53 +326,106 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA val accessorType = accessor.type if (fieldDeclaringClassType is BuiltinClassId || accessorType is BuiltinClassId) { - // The rest of the logic of this method processes hidden fields. - // We cannot check the fields of builtin classes, so we assume that we work correctly with them, - // because they are well known library classes (e.g. Mockito) that are unlikely to have hidden fields. return Suitable } - val fieldName = this.name - - if (accessorType isSubtypeOf fieldDeclaringClassType || fieldDeclaringClassType isSubtypeOf accessorType) { - if (fieldName.isNameOfFieldsInBothClasses(accessorType, fieldDeclaringClassType)) { - // classes are in an inheritance relation, and they both have a field with the same name, - // which means that field shadowing is found - return when { - // if we can access the declaring class of field, - // then we can use a type cast the accessor to it - fieldDeclaringClassType isAccessibleFrom testClassPackageName -> { - RequiresTypeCast(fieldDeclaringClassType) - } - // otherwise, we can only use reflection - else -> ReflectionOnly + // The rest of the logic of this method processes hidden fields. + // We cannot check the fields of builtin classes, so we assume that we work correctly with them, + // because they are well known library classes (e.g. Mockito) that are unlikely to have hidden fields. + + return when { + accessorType isSubtypeOf fieldDeclaringClassType -> { + if (this isFieldHiddenIn accessorType.jClass) { + // We know that declaring class of field is accessible, + // because it was checked in `isAccessibleFrom` call at the start of this method, + // so we can safely do a type cast. + RequiresTypeCast(fieldDeclaringClassType) + } else { + Suitable } } - // if no field shadowing is found, we can just access the field directly - return Suitable + fieldDeclaringClassType isSubtypeOf accessorType -> { + // We know that declaring class of field is accessible, + // because it was checked in `isAccessibleFrom` call at the start of this method, + // so we can safely do a type cast. + RequiresTypeCast(fieldDeclaringClassType) + } + // Accessor type is not subtype or supertype of the field's declaring class. + // So the only remaining option to access the field is to use reflection. + else -> ReflectionOnly } - - // Accessor type is not subtype or supertype of the field's declaring class. - // So the only remaining option to access the field is to use reflection. - return ReflectionOnly } /** - * Determine if each of the classes [left] and [right] has their own field with name specified by the [String] receiver. + * Check if the field represented by @receiver is hidden when accessed from an instance of [subclass]. + * + * For example, given classes: + * ``` + * class A { int x; } + * class B extends A { int x; } + * ``` + * we can say that field `x` of class `A` is hidden when we are dealing with instances of a subclass `B`: + * ``` + * B b = new B(); + * b.x = 10; + * ``` + * There is no way to access field `x` of class `A` from variable `b` without using type casts or reflection. + * + * So the result of [isFieldHiddenIn] for field `x` of class `A` and a subclass `B` would be `true`. + * + * **NOTE** that there can be more complicated cases. For example, interfaces can also have fields (they are always static). + * Fields of an interface will be available to the classes and interfaces that inherit from it. + * That means that when checking if a field is hidden we have to take **both** classes and interfaces into account. + * + * **However**, there is an important detail. We **do not** consider superclasses and superinterfaces + * of the type where the field (represented by @receiver) was declared. Consider the following example: + * ``` + * class A { int x; } + * class B extends A { int x; } + * class C extends B { } + * ``` + * If we are checking if the field `x` of class `B` is hidden when accessed by an instance of class `C`, + * then [isFieldHiddenIn] will return `false`, because the field `x` of class `A` does not stand in the way + * and is itself hidden by `B.x`. That is why we **can** access `B.x` from a variable of type `C`. + * + * Lastly, another **important** example: + * ``` + * class A { int x; } + * interface I1 { int x = 10; } + * class B extends A implements I1 { } + * ``` + * Unlike previous examples, here we have class `B` which has different "branches" of supertypes. + * On the one hand we have superclass `A` and on the other we have interface `I1`. + * `A` and `I1` do not have any relationship with each other, but `B` **can** access fields `x` from both of them. + * However, it **must** be done with a type cast (to `A` or `I1`), because otherwise accessing field `x` will + * be ambiguous. Method [isFieldHiddenIn] **does consider** such cases as well. + * + * **These examples show** that when checking if a field is hidden we **must** consider all supertypes + * in all "branches" **up to** the type where the field is declared (**exclusively**). + * Then we check if any of these types has a field with the same name. + * If such field is found, then the field is hidden, otherwise - not. */ - private fun String.isNameOfFieldsInBothClasses(left: ClassId, right: ClassId): Boolean { - // make sure that non-builtin class ids are passed to this method - require(left !is BuiltinClassId && right !is BuiltinClassId) { - "The given class ids must not be builtin, because this method works with their jClass" + private infix fun FieldId.isFieldHiddenIn(subclass: Class<*>): Boolean { + // supertypes (classes and interfaces) from subclass (inclusive) up to superclass (exclusive) + val supertypes = sequence { + var types = generateSequence(subclass) { it.superclass } + .takeWhile { it != declaringClass.jClass } + .toList() + while (types.isNotEmpty()) { + yieldAll(types) + types = types.flatMap { it.interfaces.toList() } + } } - val leftClass = left.jClass - val rightClass = right.jClass - - val leftField = leftClass.declaredFields.find { it.name == this } - val rightField = rightClass.declaredFields.find { it.name == this } + // check if any of the collected supertypes declare a field with the same name + val fieldHidingTypes = supertypes.toList() + .filter { type -> + val fieldNames = type.declaredFields.map { it.name } + this.name in fieldNames + } - return leftField != null && rightField != null + // if we found at least one type that hides the field, then the field is hidden + return fieldHidingTypes.isNotEmpty() } /** From 0280e3d496fe9ac88e012f3236857ab5e4a0d8ca Mon Sep 17 00:00:00 2001 From: Arsen Nagdalian Date: Mon, 5 Sep 2022 21:43:24 +0300 Subject: [PATCH 3/4] Fix review comments --- .../utbot/framework/plugin/api/util/IdUtil.kt | 3 +++ .../constructor/tree/CgVariableConstructor.kt | 3 ++- .../util/CgStatementConstructor.kt | 22 ++++++++++--------- .../codegen/model/util/FieldIdUtil.kt | 3 +-- .../codegen/model/visitor/UtilMethods.kt | 3 +-- 5 files changed, 19 insertions(+), 15 deletions(-) diff --git a/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/util/IdUtil.kt b/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/util/IdUtil.kt index c758bf9bbb..c4d7060e92 100644 --- a/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/util/IdUtil.kt +++ b/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/util/IdUtil.kt @@ -144,6 +144,9 @@ val floatWrapperClassId = java.lang.Float::class.id val doubleWrapperClassId = java.lang.Double::class.id val classClassId = java.lang.Class::class.id +val fieldClassId = java.lang.reflect.Field::class.id +val methodClassId = java.lang.reflect.Method::class.id +val constructorClassId = java.lang.reflect.Constructor::class.id // We consider void wrapper as primitive wrapper // because voidClassId is considered primitive here diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt index a029ac6f3e..e54ba0d9c4 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgVariableConstructor.kt @@ -45,6 +45,7 @@ import org.utbot.framework.plugin.api.UtNullModel import org.utbot.framework.plugin.api.UtPrimitiveModel import org.utbot.framework.plugin.api.UtReferenceModel import org.utbot.framework.plugin.api.UtVoidModel +import org.utbot.framework.plugin.api.util.classClassId import org.utbot.framework.plugin.api.util.defaultValueModel import org.utbot.framework.plugin.api.util.jField import org.utbot.framework.plugin.api.util.findFieldByIdOrNull @@ -351,7 +352,7 @@ internal class CgVariableConstructor(val context: CgContext) : val init = if (classId.isAccessibleFrom(testClassPackageName)) { CgGetJavaClass(classId) } else { - Class::class.id[forName](classId.name) + classClassId[forName](classId.name) } return newVar(Class::class.id, baseName) { init } diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt index f2b28e1781..5bdaabe09e 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/util/CgStatementConstructor.kt @@ -50,7 +50,6 @@ import org.utbot.framework.plugin.api.ClassId import org.utbot.framework.plugin.api.ExecutableId import org.utbot.framework.plugin.api.UtModel import org.utbot.framework.plugin.api.util.executable -import org.utbot.framework.plugin.api.util.id import org.utbot.framework.plugin.api.util.isArray import org.utbot.framework.plugin.api.util.isNotSubtypeOf import org.utbot.framework.plugin.api.util.isSubtypeOf @@ -67,7 +66,10 @@ import org.utbot.framework.plugin.api.ConstructorId import org.utbot.framework.plugin.api.FieldId import org.utbot.framework.plugin.api.MethodId import org.utbot.framework.plugin.api.util.classClassId +import org.utbot.framework.plugin.api.util.constructorClassId +import org.utbot.framework.plugin.api.util.fieldClassId import org.utbot.framework.plugin.api.util.isPrimitive +import org.utbot.framework.plugin.api.util.methodClassId import java.lang.reflect.Constructor import java.lang.reflect.Method import kotlin.reflect.KFunction @@ -306,15 +308,15 @@ internal class CgStatementConstructorImpl(context: CgContext) : } override fun createFieldVariable(fieldId: FieldId): CgVariable { - val declaringClass = newVar(Class::class.id) { Class::class.id[forName](fieldId.declaringClass.name) } + val declaringClass = newVar(classClassId) { classClassId[forName](fieldId.declaringClass.name) } val name = fieldId.name + "Field" - return newVar(java.lang.reflect.Field::class.id, name) { + return newVar(fieldClassId, name) { declaringClass[getDeclaredField](fieldId.name) } } override fun createExecutableVariable(executableId: ExecutableId, arguments: List): CgVariable { - val declaringClass = newVar(Class::class.id) { Class::class.id[forName](executableId.classId.name) } + val declaringClass = newVar(classClassId) { classClassId[forName](executableId.classId.name) } val argTypes = (arguments zip executableId.parameters).map { (argument, parameterType) -> val baseName = when (argument) { is CgVariable -> "${argument.name}Type" @@ -324,7 +326,7 @@ internal class CgStatementConstructorImpl(context: CgContext) : if (parameterType.isPrimitive) { CgGetJavaClass(parameterType) } else { - Class::class.id[forName](parameterType.name) + classClassId[forName](parameterType.name) } } } @@ -332,13 +334,13 @@ internal class CgStatementConstructorImpl(context: CgContext) : return when (executableId) { is MethodId -> { val name = executableId.name + "Method" - newVar(java.lang.reflect.Method::class.id, name) { + newVar(methodClassId, name) { declaringClass[getDeclaredMethod](executableId.name, *argTypes.toTypedArray()) } } is ConstructorId -> { val name = executableId.classId.prettifiedName.decapitalize() + "Constructor" - newVar(java.lang.reflect.Constructor::class.id, name) { + newVar(constructorClassId, name) { declaringClass[getDeclaredConstructor](*argTypes.toTypedArray()) } } @@ -467,9 +469,9 @@ internal class CgStatementConstructorImpl(context: CgContext) : // utils private fun classRefOrNull(type: ClassId, expr: CgExpression): ClassId? { - if (type == Class::class.id && expr is CgGetClass) return expr.classId + if (type == classClassId && expr is CgGetClass) return expr.classId - if (type == Class::class.id && expr is CgExecutableCall && expr.executableId == forName) { + if (type == classClassId && expr is CgExecutableCall && expr.executableId == forName) { val name = (expr.arguments.getOrNull(0) as? CgLiteral)?.value as? String if (name != null) { @@ -487,7 +489,7 @@ internal class CgStatementConstructorImpl(context: CgContext) : ExpressionWithType(enumClass, access) } else { val enumClassVariable = newVar(classCgClassId) { - Class::class.id[forName](enumClass.name) + classClassId[forName](enumClass.name) } ExpressionWithType(objectClassId, utilsClassId[getEnumConstantByName](enumClassVariable, constant)) diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt index e81a9ccdd5..ad54a9b5b9 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt @@ -1,7 +1,6 @@ package org.utbot.framework.codegen.model.util import org.utbot.framework.plugin.api.FieldId -import org.utbot.framework.plugin.api.util.id /** * For now we will count field accessible if it is not private and its class is also accessible, @@ -10,7 +9,7 @@ import org.utbot.framework.plugin.api.util.id * * @param packageName name of the package we check accessibility from */ -fun FieldId.isAccessibleFrom(packageName: String): Boolean { +infix fun FieldId.isAccessibleFrom(packageName: String): Boolean { val isClassAccessible = declaringClass.isAccessibleFrom(packageName) val isAccessibleByVisibility = isPublic || (declaringClass.packageName == packageName && (isPackagePrivate || isProtected)) val isAccessibleFromPackageByModifiers = isAccessibleByVisibility && !isSynthetic diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt index 135b509949..c9dae09d08 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/visitor/UtilMethods.kt @@ -9,8 +9,8 @@ import org.utbot.framework.plugin.api.ClassId import org.utbot.framework.plugin.api.CodegenLanguage import org.utbot.framework.plugin.api.MethodId import org.utbot.framework.plugin.api.MockFramework +import org.utbot.framework.plugin.api.util.fieldClassId import org.utbot.framework.plugin.api.util.id -import java.lang.reflect.Field import java.lang.reflect.Modifier import java.util.Arrays import java.util.Objects @@ -815,7 +815,6 @@ private fun TestClassUtilMethodProvider.regularImportsByUtilMethod( id: MethodId, codegenLanguage: CodegenLanguage ): List { - val fieldClassId = Field::class.id return when (id) { getUnsafeInstanceMethodId -> listOf(fieldClassId) createInstanceMethodId -> listOf(java.lang.reflect.InvocationTargetException::class.id) From caba5d10a165006873c0c7734a1878038a9910d1 Mon Sep 17 00:00:00 2001 From: Arsen Nagdalian Date: Wed, 7 Sep 2022 13:15:20 +0300 Subject: [PATCH 4/4] Fix review comments #2 --- .../tree/CgCallableAccessManager.kt | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt index 39cf16d30c..0e14d5b46a 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgCallableAccessManager.kt @@ -5,9 +5,6 @@ import org.utbot.framework.codegen.Junit5 import org.utbot.framework.codegen.TestNg import org.utbot.framework.codegen.model.constructor.builtin.any import org.utbot.framework.codegen.model.constructor.builtin.anyOfClass -import org.utbot.framework.codegen.model.constructor.builtin.forName -import org.utbot.framework.codegen.model.constructor.builtin.getDeclaredConstructor -import org.utbot.framework.codegen.model.constructor.builtin.getDeclaredMethod import org.utbot.framework.codegen.model.constructor.builtin.getMethodId import org.utbot.framework.codegen.model.constructor.builtin.getTargetException import org.utbot.framework.codegen.model.constructor.builtin.invoke @@ -271,7 +268,7 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } if (this.isStatic && caller != null && codegenLanguage == CodegenLanguage.KOTLIN) { - error("In Kotlin, unlike Java, static methods cannot be called on an object") + error("In Kotlin, unlike Java, static methods cannot be called on an instance: $this") } // If method is from current test class, then it may not have a caller. @@ -279,7 +276,7 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA return true } - requireNotNull(caller) { "Method must have a caller, unless it is the method of the current test class or a static method" } + requireNotNull(caller) { "Method must have a caller, unless it is the method of the current test class or a static method: $this" } return caller canBeReceiverOf this } @@ -295,7 +292,7 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } if (this.isStatic && accessor != null && codegenLanguage == CodegenLanguage.KOTLIN) { - error("In Kotlin, unlike Java, static fields cannot be accessed by an object") + error("In Kotlin, unlike Java, static fields cannot be accessed by an object: $this") } // if field is declared in the current test class @@ -313,7 +310,7 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA } requireNotNull(accessor) { - "Field access must have a non-null accessor, unless it is the field of the current test class or a static field" + "Field access must have a non-null accessor, unless it is the field of the current test class or a static field: $this" } if (this.declaringClass == accessor.type) { @@ -359,6 +356,20 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA /** * Check if the field represented by @receiver is hidden when accessed from an instance of [subclass]. * + * Brief description: this method collects all types from hierarchy in between [subclass] (inclusive) + * up to the class where the field is declared (exclusive) and checks if any of these classes declare + * a field with the same name (thus hiding the given field). For examples and more details read documentation below. + * + * The **contract** of this method is as follows: + * [subclass] must be a subclass of `this.declaringClass` (and they **must not** be the same class). + * That is because if they are equal (we try to access field from instance of its own class), + * then the field is not hidden. And if [subclass] is actually not a subclass, but a superclass of a class + * where the field is declared, then this check makes no sense (superclass cannot hide field of a subclass). + * Lastly, if these classes are not related in terms of inheritance, then there must be some error, + * because such checks also make no sense. + * + * **Examples**. + * * For example, given classes: * ``` * class A { int x; } @@ -406,6 +417,11 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA * If such field is found, then the field is hidden, otherwise - not. */ private infix fun FieldId.isFieldHiddenIn(subclass: Class<*>): Boolean { + // see the documentation of this method for more details on this requirement + require(subclass.id != this.declaringClass) { + "A given subclass must not be equal to the declaring class of the field: $subclass" + } + // supertypes (classes and interfaces) from subclass (inclusive) up to superclass (exclusive) val supertypes = sequence { var types = generateSequence(subclass) { it.superclass }