Add more correct type resolving for collections and maps #2610
Conversation
Markoutte
added
comp-fuzzing
| import org.utbot.framework.plugin.api.* | ||
| import org.utbot.framework.plugin.api.util.* | ||
| import org.utbot.fuzzer.FuzzedType | ||
| import org.utbot.fuzzer.FuzzedValue | ||
| import org.utbot.fuzzer.IdGenerator | ||
| import org.utbot.fuzzer.fuzzed | ||
| import org.utbot.fuzzing.* | ||
| import org.utbot.fuzzing.spring.utils.jType |
There was a problem hiding this comment.
If TypeUtils are used here, then I think they should be moved out of spring package, similarly to how it's done in #2584
| val keyGeneric = type.generics.getOrNull(0) ?: FuzzedType(objectClassId) | ||
| val valueGeneric = type.generics.getOrNull(1) ?: FuzzedType(objectClassId) | ||
| val keyGeneric = findTypeByMethod(description, type, MethodCall.KEYS) | ||
| val valueGeneric = findTypeByMethod(description, type, MethodCall.VALUES) |
There was a problem hiding this comment.
I was expecting that you'd add something like this to TypeUtils:
private fun resolveGenericsOfSuperClass(
type: FuzzedType,
description: FuzzedDescription,
superClass: Class<*>,
): List<FuzzedType> {
return try {
check(superClass.isAssignableFrom(type.classId.jClass)) { "$superClass isn't super class of $type" }
@Suppress("UNCHECKED_CAST")
toFuzzerType(
type.jType.typeToken.getSupertype(superClass as Class<in Any>).type,
description.typeCache
).generics
} catch (e: Throwable) {
logger.error("Failed to resolve $superClass generics for $type, using unresolved type parameters")
superClass.typeParameters.map { toFuzzerType(it, description.typeCache) }
}
}And use it like this:
val (keyGeneric, valueGeneric) = resolveGenericsOfSuperClass(type, description, Map::class.java)But your approach should also work.
There was a problem hiding this comment.
Thank you, this is much better. Fixed.
| /** | ||
| * Can be used to resolve some types using [type] and some method of this type | ||
| */ | ||
| protected fun resolveTypeByMethod(description: FuzzedDescription, type: FuzzedType, method: Method): FuzzedType? { |
There was a problem hiding this comment.
In your PR this method is only used for Maps and Collections, but we also have an IteratorValueProvider, that also needs to resolve generics.
There was a problem hiding this comment.
It is not really necessary, because IteratorValueProvider accepts only Iterator class itself, so, it has no modifications that can cause a problem. But I added the same call to unify these providers.
| TypeToken.of(type.jType).getSupertype(superClass as Class<in Any>).type, | ||
| description.typeCache | ||
| ).generics | ||
| } catch (e: Throwable) { |
There was a problem hiding this comment.
I think e should be logged here, since it should never be thrown if everything goes right. Also, without logging check(superClass.isAssignableFrom(type.classId.jClass)) is pretty much useless.
Markoutte
force-pushed
the
pelevin/2571-Fuzzer-ignores-generics-of-class-StringMap
branch
from
89ddbe6 to
a9ca3e0
Compare
Markoutte
deleted the
pelevin/2571-Fuzzer-ignores-generics-of-class-StringMap
branch
Description
Fixes #2571
The generic retrieving logic for collections and maps was naive. This PR improves that logic using guava type resolving to find out correct types. For every type it tries to resolve a type of returned values when calling such methods as
Map.keySet,Map.valuesandCollection.iterator. A generic used in the returned type of those methods is a target type used for building a collection or a map.Note, that the fix reveals another problem in the code generator. Because it uses untyped model it fails to generate a compilable test. For example, the result of fuzzing
getAnyStringmethod from the issue is follow:This line
issue2571.put(((Object) "XZ"), ((Object) "abc"));has syntax error because the map expects a string, not an object. @EgorkaKulikov , please, check this one.How to test
Automated tests
New tests added:
Manual tests
All manual tests from collections samples should pass.
Self-check list