Avoid false NPE tests for Spring classes #2034 (#2052)

Author: EgorkaKulikov

docs/SpeculativeFieldNonNullability.md

@@ -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.
 

utbot-framework/src/main/kotlin/org/utbot/engine/util/trusted/TrustedPackages.kt renamed to utbot-framework-api/src/main/kotlin/org/utbot/framework/TrustedPackages.kt

@@ -1,6 +1,5 @@
-package org.utbot.engine.util.trusted
+package org.utbot.framework
 
-import org.utbot.framework.TrustedLibraries
 import soot.SootClass
 
 /**

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

@@ -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
 
@@ -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
 }
 
 /**
@@ -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
@@ -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

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

@@ -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
@@ -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 }

utbot-framework/src/main/kotlin/org/utbot/engine/ArrayObjectWrappers.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/engine/CollectionIteratorWrappers.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/engine/CollectionWrappers.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/engine/Extensions.kt

@@ -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 }
 

utbot-framework/src/main/kotlin/org/utbot/engine/Memory.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/engine/Resolver.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/engine/ThreadWrappers.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/engine/Traverser.kt

@@ -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
@@ -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
@@ -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)

utbot-framework/src/main/kotlin/org/utbot/engine/util/statics/concrete/EnumConcreteUtils.kt

@@ -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

utbot-framework/src/main/kotlin/org/utbot/framework/modifications/ExecutablesAnalyzer.kt

@@ -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