Improve isAccessible checks (#1754)

Add more checks for isAccessible fieldId method

Author: sofurihafe

utbot-framework/src/main/kotlin/org/utbot/framework/assemble/AssembleModelGenerator.kt

@@ -463,7 +463,7 @@ class AssembleModelGenerator(private val basePackageName: String) {
 
         return allModificatorsOfClass
             .mapNotNull { (fieldId, possibleModificators) ->
-                chooseModificator(fieldId, possibleModificators)?.let { fieldId to it }
+                chooseModificator(classId, fieldId, possibleModificators)?.let { fieldId to it }
             }
             .toMap()
     }
@@ -474,12 +474,13 @@ class AssembleModelGenerator(private val basePackageName: String) {
      * Note: direct accessor is more preferred than setter.
      */
     private fun chooseModificator(
+        callerClassId: ClassId,
         fieldId: FieldId,
         settersAndDirectAccessors: Set<StatementId>,
     ): StatementId? {
         val directAccessors = settersAndDirectAccessors
             .filterIsInstance<DirectFieldAccessId>()
-            .filter {it.fieldId.isAccessibleFrom(basePackageName) }
+            .filter {it.fieldId.isAccessibleFrom(basePackageName, callerClassId) }
 
         if (directAccessors.any()) {
             return directAccessors.singleOrNull()

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/services/access/CgCallableAccessManager.kt

@@ -323,26 +323,19 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA
         return caller canBeReceiverOf this
     }
 
-    private fun FieldId.accessSuitability(accessor: CgExpression?): FieldAccessorSuitability {
+    private fun FieldId.accessSuitability(accessor: CgExpression): FieldAccessorSuitability {
         // Check field accessibility.
-        if (!canBeReadFrom(context)) {
+        if (!canBeReadFrom(context, accessor.type)) {
             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) {
+        if (this.isStatic && codegenLanguage == CodegenLanguage.KOTLIN) {
             error("In Kotlin, unlike Java, static fields cannot be accessed by an object: $this")
         }
 
         // 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
@@ -352,10 +345,6 @@ 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: $this"
-        }
-
         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

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/services/access/CgFieldStateManager.kt

@@ -180,7 +180,7 @@ internal class CgFieldStateManagerImpl(val context: CgContext)
         for ((index, fieldPathElement) in path.withIndex()) {
             when (fieldPathElement) {
                 is FieldAccess -> {
-                    if (!fieldPathElement.field.canBeReadFrom(context)) {
+                    if (!fieldPathElement.field.canBeReadFrom(context, curType)) {
                         lastAccessibleIndex = index - 1
                         break
                     }
@@ -246,7 +246,7 @@ internal class CgFieldStateManagerImpl(val context: CgContext)
 
     private fun variableForStaticFieldState(owner: ClassId, fieldPath: FieldPath, customName: String?): CgVariable {
         val firstField = (fieldPath.elements.first() as FieldAccess).field
-        val firstAccessor = if (owner.isAccessibleFrom(testClassPackageName) && firstField.canBeReadFrom(context)) {
+        val firstAccessor = if (owner.isAccessibleFrom(testClassPackageName) && firstField.canBeReadFrom(context, owner)) {
             owner[firstField]
         } else {
             // TODO: there is a function getClassOf() for these purposes, but it is not accessible from here for now

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

@@ -150,7 +150,6 @@ import org.utbot.framework.plugin.api.util.doubleStreamClassId
 import org.utbot.framework.plugin.api.util.doubleStreamToArrayMethodId
 import org.utbot.framework.plugin.api.util.intStreamClassId
 import org.utbot.framework.plugin.api.util.intStreamToArrayMethodId
-import org.utbot.framework.plugin.api.util.isPackagePrivate
 import org.utbot.framework.plugin.api.util.isSubtypeOf
 import org.utbot.framework.plugin.api.util.longStreamClassId
 import org.utbot.framework.plugin.api.util.longStreamToArrayMethodId
@@ -236,7 +235,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
         val accessibleStaticFields = statics.accessibleFields()
         for ((field, _) in accessibleStaticFields) {
             val declaringClass = field.declaringClass
-            val fieldAccessible = field.canBeReadFrom(context)
+            val fieldAccessible = field.canBeReadFrom(context, declaringClass)
 
             // prevValue is nullable if not accessible because of getStaticFieldValue(..) : Any?
             val prevValue = newVar(
@@ -261,7 +260,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
         val accessibleStaticFields = statics.accessibleFields()
         for ((field, model) in accessibleStaticFields) {
             val declaringClass = field.declaringClass
-            val fieldAccessible = field.canBeSetFrom(context)
+            val fieldAccessible = field.canBeSetFrom(context, declaringClass)
 
             val fieldValue = if (isParametrized) {
                 currentMethodParameters[CgParameterKind.Statics(model)]
@@ -282,7 +281,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
 
     protected fun recoverStaticFields() {
         for ((field, prevValue) in prevStaticFieldValues.accessibleFields()) {
-            if (field.canBeSetFrom(context)) {
+            if (field.canBeSetFrom(context, field.declaringClass)) {
                 field.declaringClass[field] `=` prevValue
             } else {
                 val declaringClass = getClassOf(field.declaringClass)
@@ -1125,11 +1124,7 @@ open class CgMethodConstructor(val context: CgContext) : CgContextOwner by conte
     private fun FieldId.getAccessExpression(variable: CgVariable): CgExpression =
         // Can directly access field only if it is declared in variable class (or in its ancestors)
         // and is accessible from current package
-        if (variable.type.hasField(this)
-            //TODO: think about moving variable type checks into [isAccessibleFrom] after contest
-            && (!isPackagePrivate || variable.type.packageName == context.testClassPackageName)
-            && canBeReadFrom(context)
-        ) {
+        if (variable.type.hasField(this) && canBeReadFrom(context, variable.type)) {
             if (jField.isStatic) CgStaticFieldAccess(this) else CgFieldAccess(variable, this)
         } else {
             utilsClassId[getFieldValue](variable, this.declaringClass.name, this.name)

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

@@ -192,7 +192,7 @@ open class CgVariableConstructor(val context: CgContext) :
             // byteBuffer is field of type ByteBuffer and upper line is incorrect
             val canFieldBeDirectlySetByVariableAndFieldTypeRestrictions =
                 fieldFromVariableSpecifiedType != null && fieldFromVariableSpecifiedType.type.id == variableForField.type
-            if (canFieldBeDirectlySetByVariableAndFieldTypeRestrictions && fieldId.canBeSetFrom(context)) {
+            if (canFieldBeDirectlySetByVariableAndFieldTypeRestrictions && fieldId.canBeSetFrom(context, obj.type)) {
                 // TODO: check if it is correct to use declaringClass of a field here
                 val fieldAccess = if (field.isStatic) CgStaticFieldAccess(fieldId) else CgFieldAccess(obj, fieldId)
                 fieldAccess `=` variableForField

utbot-framework/src/main/kotlin/org/utbot/framework/codegen/util/FieldIdUtil.kt

@@ -1,6 +1,7 @@
 package org.utbot.framework.codegen.util
 
 import org.utbot.framework.codegen.domain.context.CgContext
+import org.utbot.framework.plugin.api.ClassId
 import org.utbot.framework.plugin.api.CodegenLanguage
 import org.utbot.framework.plugin.api.FieldId
 import org.utbot.framework.plugin.api.MethodId
@@ -17,10 +18,19 @@ import org.utbot.framework.plugin.api.util.voidClassId
  * which means we can access public, protected and package-private fields
  *
  * @param context context in which code is generated (it is needed because the method needs to know package and language)
+ * @param callerClassId object on which we try to access the field
  */
-fun FieldId.isAccessibleFrom(packageName: String): Boolean {
+fun FieldId.isAccessibleFrom(packageName: String, callerClassId: ClassId): Boolean {
     val isClassAccessible = declaringClass.isAccessibleFrom(packageName)
-    val isAccessibleByVisibility = isPublic || (declaringClass.packageName == packageName && (isPackagePrivate || isProtected))
+
+    /*
+    * There is a corner case which we ignore now.
+    * Protected fields are accessible in nested classes of inheritors.
+    */
+    val isAccessibleByVisibility =
+        isPublic ||
+        isPackagePrivate && callerClassId.packageName == packageName && declaringClass.packageName == packageName ||
+        isProtected && declaringClass.packageName == packageName
     val isAccessibleFromPackageByModifiers = isAccessibleByVisibility && !isSynthetic
 
     return isClassAccessible && isAccessibleFromPackageByModifiers
@@ -32,14 +42,14 @@ private fun FieldId.canBeReadViaGetterFrom(context: CgContext): Boolean =
 /**
  * Returns whether you can read field's value without reflection
  */
-internal infix fun FieldId.canBeReadFrom(context: CgContext): Boolean {
+internal fun FieldId.canBeReadFrom(context: CgContext, callerClassId: ClassId): Boolean {
     if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
         // Kotlin will allow direct field access for non-static fields with accessible getter
         if (!isStatic && canBeReadViaGetterFrom(context))
             return true
     }
 
-    return isAccessibleFrom(context.testClassPackageName)
+    return isAccessibleFrom(context.testClassPackageName, callerClassId)
 }
 
 private fun FieldId.canBeSetViaSetterFrom(context: CgContext): Boolean =
@@ -48,16 +58,16 @@ private fun FieldId.canBeSetViaSetterFrom(context: CgContext): Boolean =
 /**
  * Whether or not a field can be set without reflection
  */
-internal fun FieldId.canBeSetFrom(context: CgContext): Boolean {
+internal fun FieldId.canBeSetFrom(context: CgContext, callerClassId: ClassId): Boolean {
     if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
         // Kotlin will allow direct write access if both getter and setter is defined
         // !isAccessibleFrom(context) is important here because above rule applies to final fields only if they are not accessible in Java terms
-        if (!isAccessibleFrom(context.testClassPackageName) && !isStatic && canBeReadViaGetterFrom(context) && canBeSetViaSetterFrom(context)) {
+        if (!isAccessibleFrom(context.testClassPackageName, callerClassId) && !isStatic && canBeReadViaGetterFrom(context) && canBeSetViaSetterFrom(context)) {
             return true
         }
     }
 
-    return isAccessibleFrom(context.testClassPackageName) && !isFinal
+    return isAccessibleFrom(context.testClassPackageName, callerClassId) && !isFinal
 }
 
 /**