Skip to content

Avoid false NPE tests for Spring classes #2052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions docs/SpeculativeFieldNonNullability.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,31 @@ is desirable, as it increases the coverage, but it has a downside. It is possibl
most of generated branches would be `NPE` branches, while useful paths could be lost due to timeout.

Beyond that, in many cases the `null` value of a field can't be generated using the public API
of the class. This is particularly true for final fields, especially in system classes.
of the class.

- First of all, this is particularly true for final fields, especially in system classes.
it is also often true for non-public fields from standard library and third-party libraries (even setters often do not
allow `null` values). Automatically generated tests assign `null` values to fields using reflection,
but these tests may be uninformative as the corresponding `NPE` branches would never occur
in the real code that limits itself to the public API.

- After that, field may be declared with some annotation that shows that null value is actually impossible.
For example, in Spring applications `@InjectMocks` and `@Mock` annotations on the fields of class under test
mean that these fields always have value, so `NPE` branches for them would never occur in real code.


## The solution

To discard irrelevant `NPE` branches, we can speculatively mark fields we as non-nullable even they
do not have an explicit `@NotNull` annotation. In particular, we can use this approach to final and non-public
fields of system classes, as they are usually correctly initialized and are not equal `null`.
do not have an explicit `@NotNull` annotation.

- In particular, we can use this approach to final and non-public
fields of system classes, as they are usually correctly initialized and are not equal `null`
- For Spring application, we use this approach for the fields of class
under test not obtained from Spring bean definitions

At the same time, we can't always add the "not null" hard constraint for the field: it would break
At the same time, for non-Spring classes,
we can't always add the "not null" hard constraint for the field: it would break
some special cases like `Optional<T>` class, which uses the `null` value of its final field
as a marker of an empty value.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.utbot.engine.util.trusted
package org.utbot.framework

import org.utbot.framework.TrustedLibraries
import soot.SootClass

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,15 @@ import kotlin.contracts.ExperimentalContracts
import kotlin.contracts.contract
import org.utbot.common.isAbstract
import org.utbot.common.isStatic
import org.utbot.framework.isFromTrustedLibrary
import org.utbot.framework.plugin.api.TypeReplacementMode.*
import org.utbot.framework.plugin.api.util.allDeclaredFieldIds
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.isSubtypeOf
import org.utbot.framework.plugin.api.util.objectClassId
import org.utbot.framework.plugin.api.util.utContext
import org.utbot.framework.process.OpenModulesContainer
import soot.SootField

const val SYMBOLIC_NULL_ADDR: Int = 0

Expand Down Expand Up @@ -1229,6 +1234,26 @@ open class ApplicationContext(
* if it is guided with some additional information.
*/
open fun replaceTypeIfNeeded(type: RefType): ClassId? = null

/**
* Sets the restrictions on speculative not null
* constraints in current application context.
*
* @see docs/SpeculativeFieldNonNullability.md for more information.
*/
open fun avoidSpeculativeNotNullChecks(field: SootField): Boolean =
UtSettings.maximizeCoverageUsingReflection || !field.declaringClass.isFromTrustedLibrary()

/**
* Checks whether accessing [field] (with a method invocation or field access) speculatively
* cannot produce [NullPointerException] (according to its finality or accessibility).
*
* @see docs/SpeculativeFieldNonNullability.md for more information.
*/
open fun speculativelyCannotProduceNullPointerException(
field: SootField,
classUnderTest: ClassId,
): Boolean = field.isFinal || !field.isPublic
}

/**
Expand All @@ -1244,16 +1269,35 @@ class SpringApplicationContext(
private val beanQualifiedNames: List<String> = emptyList(),
private val shouldUseImplementors: Boolean,
): ApplicationContext(mockInstalled, staticsMockingIsConfigured) {

private val springInjectedClasses: List<ClassId> by lazy {
beanQualifiedNames
.map { fqn -> utContext.classLoader.loadClass(fqn) }
.filterNot { it.isAbstract || it.isInterface || it.isLocalClass || it.isMemberClass && !it.isStatic }
.map { it.id }
}

override val typeReplacementMode: TypeReplacementMode
get() = if (shouldUseImplementors) KnownImplementor else NoImplementors
private var areInjectedClassesInitialized : Boolean = false

// Classes representing concrete types that are actually used in Spring application
private val springInjectedClasses: Set<ClassId>
get() {
if (!areInjectedClassesInitialized) {
springInjectedClassesStorage += beanQualifiedNames
.map { fqn -> utContext.classLoader.loadClass(fqn) }
.filterNot { it.isAbstract || it.isInterface || it.isLocalClass || it.isMemberClass && !it.isStatic }
.mapTo(mutableSetOf()) { it.id }

// This is done to be sure that this storage is not empty after the first class loading iteration.
// So, even if all loaded classes were filtered out, we will not try to load them again.
areInjectedClassesInitialized = true
}

return springInjectedClassesStorage
}

// This is a service field to model the lazy behavior of [springInjectedClasses].
// Do not call it outside the getter.
//
// Actually, we should just call [springInjectedClasses] with `by lazy`, but we had problems
// with a strange `kotlin.UNINITIALIZED_VALUE` in `speculativelyCannotProduceNullPointerException` method call.
private val springInjectedClassesStorage = mutableSetOf<ClassId>()

override val typeReplacementMode: TypeReplacementMode =
if (shouldUseImplementors) KnownImplementor else NoImplementors

/**
* Replaces an interface type with its implementor type
Expand All @@ -1265,6 +1309,20 @@ class SpringApplicationContext(
} else {
null
}

override fun avoidSpeculativeNotNullChecks(field: SootField): Boolean = false

/**
* In Spring applications we can mark as speculatively not null
* fields if they are mocked and injecting into class under test so on.
*
* Fields are not mocked if their actual type is obtained from [springInjectedClasses].
*
*/
override fun speculativelyCannotProduceNullPointerException(
field: SootField,
classUnderTest: ClassId,
): Boolean = field.fieldId in classUnderTest.allDeclaredFieldIds && field.declaringClass.id !in springInjectedClasses
}

val RefType.isAbstractType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import org.utbot.framework.plugin.api.MethodId
import org.utbot.framework.plugin.api.UtModel
import org.utbot.framework.plugin.api.UtNullModel
import org.utbot.framework.plugin.api.UtPrimitiveModel
import org.utbot.framework.plugin.api.id
import soot.SootField
import java.lang.reflect.Constructor
import java.lang.reflect.Executable
import java.lang.reflect.Field
Expand Down Expand Up @@ -451,6 +453,9 @@ val ClassId.allDeclaredFieldIds: Sequence<FieldId>
.flatMap { it.declaredFields.asSequence() }
.map { it.fieldId }

val SootField.fieldId: FieldId
get() = FieldId(declaringClass.id, name)

// FieldId utils
val FieldId.safeJField: Field?
get() = declaringClass.jClass.declaredFields.firstOrNull { it.name == name }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.utbot.framework.plugin.api.UtModel
import org.utbot.framework.plugin.api.UtNullModel
import org.utbot.framework.plugin.api.UtPrimitiveModel
import org.utbot.framework.plugin.api.getIdOrThrow
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.objectArrayClassId
import org.utbot.framework.plugin.api.util.objectClassId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.utbot.framework.plugin.api.UtAssembleModel
import org.utbot.framework.plugin.api.UtExecutableCallModel
import org.utbot.framework.plugin.api.UtModel
import org.utbot.framework.plugin.api.UtReferenceModel
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.methodId
import soot.SootClass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.utbot.framework.util.graph
import org.utbot.framework.plugin.api.id
import org.utbot.framework.plugin.api.util.booleanClassId
import org.utbot.framework.plugin.api.util.constructorId
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.methodId
import org.utbot.framework.plugin.api.util.objectClassId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,6 @@ val JimpleLocal.variable: LocalVariable
val Type.defaultSymValue: UtExpression
get() = toSort().defaultValue

val SootField.fieldId: FieldId
get() = FieldId(declaringClass.id, name)

val SootField.isEnumConstant: Boolean
get() = name in declaringClass.id.enumConstants.orEmpty().map { enum -> enum.name }

Expand Down
1 change: 1 addition & 0 deletions utbot-framework/src/main/kotlin/org/utbot/engine/Memory.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import org.utbot.engine.types.STRING_TYPE
import org.utbot.engine.types.SeqType
import org.utbot.engine.types.TypeResolver
import org.utbot.framework.plugin.api.id
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.isEnum
import soot.ArrayType
import soot.CharType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import org.utbot.framework.UtSettings
import org.utbot.framework.plugin.api.visible.UtStreamConsumingException
import org.utbot.framework.plugin.api.UtStreamConsumingFailure
import org.utbot.framework.plugin.api.util.constructor.ValueConstructor
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.isStatic

// hack
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.utbot.framework.plugin.api.UtNullModel
import org.utbot.framework.plugin.api.id
import org.utbot.framework.plugin.api.util.constructorId
import org.utbot.framework.plugin.api.util.defaultValueModel
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.intClassId
import org.utbot.framework.plugin.api.util.objectClassId
import org.utbot.framework.plugin.api.util.stringClassId
Expand Down
26 changes: 8 additions & 18 deletions utbot-framework/src/main/kotlin/org/utbot/engine/Traverser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,16 @@ import org.utbot.engine.types.OBJECT_TYPE
import org.utbot.engine.types.SECURITY_FIELD_SIGNATURE
import org.utbot.engine.types.TypeRegistry
import org.utbot.engine.types.TypeResolver
import org.utbot.engine.util.trusted.isFromTrustedLibrary
import org.utbot.engine.util.statics.concrete.associateEnumSootFieldsWithConcreteValues
import org.utbot.engine.util.statics.concrete.isEnumAffectingExternalStatics
import org.utbot.engine.util.statics.concrete.isEnumValuesFieldName
import org.utbot.engine.util.statics.concrete.makeEnumNonStaticFieldsUpdates
import org.utbot.engine.util.statics.concrete.makeEnumStaticFieldsUpdates
import org.utbot.engine.util.statics.concrete.makeSymbolicValuesFromEnumConcreteValues
import org.utbot.framework.UtSettings
import org.utbot.framework.UtSettings.maximizeCoverageUsingReflection
import org.utbot.framework.UtSettings.preferredCexOption
import org.utbot.framework.UtSettings.substituteStaticsWithSymbolicVariable
import org.utbot.framework.isFromTrustedLibrary
import org.utbot.framework.plugin.api.ApplicationContext
import org.utbot.framework.plugin.api.ClassId
import org.utbot.framework.plugin.api.ExecutableId
Expand All @@ -128,6 +127,7 @@ import org.utbot.framework.plugin.api.classId
import org.utbot.framework.plugin.api.id
import org.utbot.framework.plugin.api.isAbstractType
import org.utbot.framework.plugin.api.util.executable
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.findFieldByIdOrNull
import org.utbot.framework.plugin.api.util.jField
import org.utbot.framework.plugin.api.util.jClass
Expand Down Expand Up @@ -2322,30 +2322,20 @@ class Traverser(
}

/**
* Marks the [createdField] as speculatively not null if the [field] is considering as
* not producing [NullPointerException].
* Marks the [createdField] as speculatively not null if the [field] is considering
* as not producing [NullPointerException].
*
* @see [SootField.speculativelyCannotProduceNullPointerException], [markAsSpeculativelyNotNull], [isFromTrustedLibrary].
* See more detailed documentation in [ApplicationContext] mentioned methods.
*/
private fun checkAndMarkLibraryFieldSpeculativelyNotNull(field: SootField, createdField: SymbolicValue) {
if (maximizeCoverageUsingReflection || !field.declaringClass.isFromTrustedLibrary()) {
if (applicationContext.avoidSpeculativeNotNullChecks(field) ||
!applicationContext.speculativelyCannotProduceNullPointerException(field, methodUnderTest.classId)) {
return
}

if (field.speculativelyCannotProduceNullPointerException()) {
markAsSpeculativelyNotNull(createdField.addr)
}
markAsSpeculativelyNotNull(createdField.addr)
}

/**
* Checks whether accessing [this] field (with a method invocation or field access) speculatively can produce
* [NullPointerException] (according to its finality or accessibility).
*
* @see docs/SpeculativeFieldNonNullability.md for more information.
*/
@Suppress("KDocUnresolvedReference")
private fun SootField.speculativelyCannotProduceNullPointerException(): Boolean = isFinal || !isPublic

private fun createArray(pName: String, type: ArrayType): ArrayValue {
val addr = UtAddrExpression(mkBVConst(pName, UtIntSort))
return createArray(addr, type, useConcreteType = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.utbot.engine.symbolic.SymbolicStateUpdate
import org.utbot.engine.symbolic.asHardConstraint
import org.utbot.engine.types.TypeResolver
import org.utbot.framework.plugin.api.FieldId
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.plugin.api.util.jField
import soot.SootClass
import soot.SootField
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package org.utbot.framework.modifications

import org.utbot.engine.fieldId
import org.utbot.framework.plugin.api.ClassId
import org.utbot.framework.plugin.api.ExecutableId
import org.utbot.framework.plugin.api.FieldId
import org.utbot.framework.plugin.api.id
import org.utbot.framework.plugin.api.util.fieldId
import org.utbot.framework.util.executableId
import soot.Scene
import soot.SootMethod
Expand Down