Return correct return type for Kotlin suspending functions in MethodParameter.

[konrad-kaminski]

Return type for Kotlin suspending functions (as returned by MethodParameter#getParameterType and MethodParameter#getgenericReturnType methods) is incorrect. The true return type is the generic parameter of the last parameter of the method.

This change modifies the behaviour of the aforementioned methods so that they work correctly for all cases.

Issue: SPR-16515

[sdeleuze]

Not necessary, already the default via package level @NonNullApi.

[konrad-kaminski]

Fixed.

[sdeleuze]

Not necessary, already the default via package level @NonNullApi.

[konrad-kaminski]

Fixed.

[sdeleuze]

resolve() can return null but Class<?> getReturnType(Method method) is expected to return non null values. Maybe we should use resolve(Class<?> fallback) variant or assert the returned class is not null before returning it if we are sure that will be the case here?

[konrad-kaminski]

Assertion added. This should never be null.

[sdeleuze]

After reviewing more in detail this PR, I am not sure yet Coroutines should be managed at MethodParameter level. I was initially tempted to refactor it to manage a generalized version of what we support for Optional, partially because we have SPR-16530 which requires a more generalized support of wrapper types like Option / Try / Either / Lazy. We could for example imagine a MethodParameter#nestedIfWrapped that would unwrap these wrappers including Coroutine Continuation by reusing the logic you implemented @konrad-kaminski.

But I am not sure this is the right way to support that, because Coroutines are more high-level than just a wrapper. As described here, suspend fun <T> foo(): T becomes fun <T> foo(continuation: Continuation<T>): Any?. The type information moves from the return values to the last parameter generic nested type so IMO hardcoding a clever recognition of suspending function return type is not necessarily what we always want and could lead to some unexpected behavior.

So what are the alternatives?

Coroutines are more related to Mono than to Optional so I thought it could maybe be a good idea to use ReactiveAdapter to deal with Coroutines types but an issue with current API is that ReactiveTypeDescriptor deals with Class, not with ResolvableType which could allow us to find the potentially suspending function via ResolvableType#getSource. So one option could maybe be to update ReactiveTypeDescriptor to deal with ResolvableType instead of Class (with deprecation in 5.1 of Class based API). This could allow us to find the right return type of suspending functions. Another important pro for using ReactiveAdapter here is that it could be maybe used to implement Coroutines -> Mono conversion in a more cleanly designed way than what is done in CustomControllerMethodResolver. This could maybe be a fix for SPR-15413, reused in Spring Data, etc.

Any thoughts @jhoeller @rstoyanchev @konrad-kaminski ?

[sdeleuze]

Merged via this revised commit which leverages Kotlin reflection that now supports Coroutines. Thanks @konrad-kaminski for this contribution.