Skip to content

Commit

Permalink
Fix false-positive canBeSetIn result for private fields with getters …
Browse files Browse the repository at this point in the history
…in Kotlin
  • Loading branch information
volivan239 committed Sep 24, 2022
1 parent c45244f commit 31ddce3
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ 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.isAccessibleFrom
import org.utbot.framework.codegen.model.util.canBeReadFrom
import org.utbot.framework.codegen.model.util.nullLiteral
import org.utbot.framework.codegen.model.util.resolve
import org.utbot.framework.plugin.api.BuiltinConstructorId
Expand Down Expand Up @@ -293,7 +294,7 @@ internal class CgCallableAccessManagerImpl(val context: CgContext) : CgCallableA

private fun FieldId.accessSuitability(accessor: CgExpression?): FieldAccessorSuitability {
// Check field accessibility.
if (!isAccessibleFrom(context)) {
if (!canBeReadFrom(context)) {
return ReflectionOnly
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ 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.isAccessibleFrom
import org.utbot.framework.codegen.model.util.canBeReadFrom
import org.utbot.framework.codegen.model.util.stringLiteral
import org.utbot.framework.fields.ArrayElementAccess
import org.utbot.framework.fields.FieldAccess
Expand Down Expand Up @@ -182,7 +183,7 @@ internal class CgFieldStateManagerImpl(val context: CgContext)
for ((index, fieldPathElement) in path.withIndex()) {
when (fieldPathElement) {
is FieldAccess -> {
if (!fieldPathElement.field.isAccessibleFrom(context)) {
if (!fieldPathElement.field.canBeReadFrom(context)) {
lastAccessibleIndex = index - 1
break
}
Expand Down Expand Up @@ -246,7 +247,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.isAccessibleFrom(context)) {
val firstAccessor = if (owner.isAccessibleFrom(testClassPackageName) && firstField.canBeReadFrom(context)) {
owner[firstField]
} else {
// TODO: there is a function getClassOf() for these purposes, but it is not accessible from here for now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ import org.utbot.framework.codegen.model.tree.buildParameterizedTestDataProvider
import org.utbot.framework.codegen.model.tree.buildTestMethod
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.canBeSetFrom
import org.utbot.framework.codegen.model.util.equalTo
import org.utbot.framework.codegen.model.util.inc
import org.utbot.framework.codegen.model.util.isAccessibleFrom
import org.utbot.framework.codegen.model.util.canBeReadFrom
import org.utbot.framework.codegen.model.util.length
import org.utbot.framework.codegen.model.util.lessThan
import org.utbot.framework.codegen.model.util.nullLiteral
Expand Down Expand Up @@ -218,7 +218,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c
val accessibleStaticFields = statics.accessibleFields()
for ((field, _) in accessibleStaticFields) {
val declaringClass = field.declaringClass
val fieldAccessible = field.isAccessibleFrom(context)
val fieldAccessible = field.canBeReadFrom(context)

// prevValue is nullable if not accessible because of getStaticFieldValue(..) : Any?
val prevValue = newVar(
Expand All @@ -243,7 +243,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c
val accessibleStaticFields = statics.accessibleFields()
for ((field, model) in accessibleStaticFields) {
val declaringClass = field.declaringClass
val fieldAccessible = field.canBeSetIn(context)
val fieldAccessible = field.canBeSetFrom(context)

val fieldValue = if (isParametrized) {
currentMethodParameters[CgParameterKind.Statics(model)]
Expand All @@ -264,7 +264,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c

private fun recoverStaticFields() {
for ((field, prevValue) in prevStaticFieldValues.accessibleFields()) {
if (field.canBeSetIn(context)) {
if (field.canBeSetFrom(context)) {
field.declaringClass[field] `=` prevValue
} else {
val declaringClass = getClassOf(field.declaringClass)
Expand Down Expand Up @@ -1039,7 +1039,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c
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) && isAccessibleFrom(context)) {
if (variable.type.hasField(this) && canBeReadFrom(context)) {
if (jField.isStatic) CgStaticFieldAccess(this) else CgFieldAccess(variable, this)
} else {
utilsClassId[getFieldValue](variable, this.declaringClass.name, this.name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import org.utbot.framework.codegen.model.tree.CgStaticFieldAccess
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.canBeSetFrom
import org.utbot.framework.codegen.model.util.fieldThisIsGetterFor
import org.utbot.framework.codegen.model.util.fieldThisIsSetterFor
import org.utbot.framework.codegen.model.util.inc
Expand Down Expand Up @@ -193,7 +193,7 @@ internal 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.canBeSetIn(context)) {
if (canFieldBeDirectlySetByVariableAndFieldTypeRestrictions && fieldId.canBeSetFrom(context)) {
// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import org.utbot.framework.codegen.model.constructor.builtin.anyLong
import org.utbot.framework.codegen.model.constructor.builtin.anyOfClass
import org.utbot.framework.codegen.model.constructor.builtin.anyShort
import org.utbot.framework.codegen.model.constructor.builtin.argumentMatchersClassId
import org.utbot.framework.codegen.model.constructor.builtin.forName
import org.utbot.framework.codegen.model.constructor.builtin.mockMethodId
import org.utbot.framework.codegen.model.constructor.builtin.mockedConstructionContextClassId
import org.utbot.framework.codegen.model.constructor.builtin.mockitoClassId
Expand All @@ -24,7 +23,6 @@ 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.CgStatementConstructorImpl
import org.utbot.framework.codegen.model.constructor.util.classCgClassId
import org.utbot.framework.codegen.model.constructor.util.hasAmbiguousOverloadsOf
import org.utbot.framework.codegen.model.tree.CgAnonymousFunction
import org.utbot.framework.codegen.model.tree.CgAssignment
Expand All @@ -33,7 +31,6 @@ import org.utbot.framework.codegen.model.tree.CgConstructorCall
import org.utbot.framework.codegen.model.tree.CgDeclaration
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.CgLiteral
import org.utbot.framework.codegen.model.tree.CgMethodCall
import org.utbot.framework.codegen.model.tree.CgParameterDeclaration
Expand Down Expand Up @@ -63,7 +60,6 @@ import org.utbot.framework.plugin.api.util.byteClassId
import org.utbot.framework.plugin.api.util.charClassId
import org.utbot.framework.plugin.api.util.doubleClassId
import org.utbot.framework.plugin.api.util.floatClassId
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.intClassId
import org.utbot.framework.plugin.api.util.longClassId
import org.utbot.framework.plugin.api.util.shortClassId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import org.utbot.framework.codegen.StaticImport
import org.utbot.framework.codegen.model.constructor.context.CgContextOwner
import org.utbot.framework.codegen.model.tree.CgClassId
import org.utbot.framework.codegen.model.tree.CgExpression
import org.utbot.framework.codegen.model.tree.CgGetClass
import org.utbot.framework.codegen.model.tree.CgGetJavaClass
import org.utbot.framework.codegen.model.tree.CgTypeCast
import org.utbot.framework.codegen.model.tree.CgValue
import org.utbot.framework.codegen.model.tree.CgVariable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import org.utbot.framework.plugin.api.util.voidClassId
* @param context context in which code is generated (it is needed because the method needs to know package and language)
*/
// TODO: change parameter from packageName: String to context: CgContext in ClassId.isAccessibleFrom and ExecutableId.isAccessibleFrom ?
internal infix fun FieldId.isAccessibleFrom(context: CgContext): Boolean {
if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
private fun FieldId.isAccessibleFrom(context: CgContext): Boolean {
/*if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
// Here we call field accessible iff its getter is accessible, checks for setter are made in FieldId.canBeSetIn
if (!isStatic && declaringClass.allMethods.contains(getter) && getter.isAccessibleFrom(context.testClassPackageName))
if (!isStatic && isAccessibleViaGetterFrom(context))
return true
}
}*/
val packageName = context.testClassPackageName
val isClassAccessible = declaringClass.isAccessibleFrom(packageName)
val isAccessibleByVisibility = isPublic || (declaringClass.packageName == packageName && (isPackagePrivate || isProtected))
Expand All @@ -28,21 +28,34 @@ internal infix fun FieldId.isAccessibleFrom(context: CgContext): Boolean {
return isClassAccessible && isAccessibleFromPackageByModifiers
}

private fun FieldId.canBeReadViaGetterFrom(context: CgContext): Boolean =
declaringClass.allMethods.contains(getter) && getter.isAccessibleFrom(context.testClassPackageName)

internal infix fun FieldId.canBeReadFrom(context: CgContext): Boolean {
if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
if (!isStatic && canBeReadViaGetterFrom(context))
return true
}

return isAccessibleFrom(context)
}

private fun FieldId.canBeSetViaSetterFrom(context: CgContext): Boolean =
declaringClass.allMethods.contains(setter) && setter.isAccessibleFrom(context.testClassPackageName)

/**
* Whether or not a field can be set without reflection
*/
internal fun FieldId.canBeSetIn(context: CgContext): Boolean {
if (!isAccessibleFrom(context)) {
return false
}

internal fun FieldId.canBeSetFrom(context: CgContext): Boolean {
if (context.codegenLanguage == CodegenLanguage.KOTLIN) {
if (!isStatic && declaringClass.allMethods.contains(setter) && setter.isAccessibleFrom(context.testClassPackageName)) {
// Kotlin will allow direct write access if both getter and setter is defined (even if the field is final)
// TODO: add comment about final public and final private fields
if (!isAccessibleFrom(context) && !isStatic && canBeReadViaGetterFrom(context) && canBeSetViaSetterFrom(context)) {
return true
}
}

return !isFinal
return isAccessibleFrom(context) && !isFinal
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static void setStaticBoxedInt(@NotNull Integer staticBoxedInt) {
}

@SuppressWarnings("NullableProblems")
public @NotNull Integer getBoxedInt() {
public Integer getBoxedInt() {
return boxedInt;
}
}

0 comments on commit 31ddce3

Please sign in to comment.