I think the answer here is "This is an unfortunate...

# android

[Don Phillips]

I think the answer here is "This is an unfortunate limitation with no workaround", but hoping I'm wrong.

I'm attempting to write an Android Lint check to validate my retrofit response models are properly annotated for Moshi. I'm doing this by using UAST/PSI to look for methods annotated with retrofit annotations, and then I use the KotlinUMethod to find the return type of the method, resolve it into a UClass (I think, but unimportant for this question) and then from there I can see if the class of the type is annotated with Moshi annotations properly.

Now that we wrote a custom Retrofit CallAdapter to allow us to adapt a Retrofit Call<T> to a kotlin.Result<T>, my lint check is returning a false negative. If you look at the attached screenshot of the debugger, you'll see that for the highlighted method definition in the left pane, node.returnType.canonicalText is returning java.lang.Object rather than kotlin.Result ,and the false negative happens b/c Retrofit technically can handle Object natively, but I don't want that here. however, if I use the node to grab the sourcePsi prop and drill down to the source definition of the return value, I can see the Result bit there in the source that was parsed.

From what I've found so far, I'm basically stuck b/c kotlin.Result is an inline value class, soo the true type is lost when UAST/PSI resolves the source code. And I can't think of a good way to drop into the sourcePsi and look at the source directly because it's all strings, seems brittle AF

So I ask, is there any way for me to know that KotlinUMethod.returnType is a kotlin.Result<T> ?

[gmz]

are you sure you need this for the cases in which resultTest() isn't a suspend fun? I'm asking because a) I'd expect a suspend fun for something like this, b) that changes everything: https://cs.android.com/android-studio/platform/tools/base/+/mirror-goog-studio-main:lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/CheckResultDetector.kt;l=135-141;drc=c670bb919484bbf611c085c04c36287276c018d4

[gmz]

If not, I too would like to know if there's a way to retrieve the original class, because I remember I needed something similar in the past and had to resort to some ugly string-based solution

[gmz]

but I guess you are already doing something similar to what I linked in getSuspendFunctionReturnTypeAsPsiClassReference()

[gmz]

And now that I think of it, retrofit uses reflection to get the return type, so it probably does not see Result<String>, but Object (disclaimer: I did not try it), so would this even work as expected without the suspend modifier?

[Don Phillips]

Here's the extension method for suspending functions:

fun UMethod.getSuspendFunctionReturnTypeAsPsiClassReference(): PsiClassReferenceType? =
    // Suspending function signatures are rewritten by the compiler to a regular function that returns
    // Unit, and adds a parameter named $continuation to the function's parameter list. This continuation
    // parameter has a type of kotlin.coroutines.Continuation<? super T>, where T is the return type of
    // the original suspend function definition.
    uastParameters.find { uParameter ->
        // Type of $completion parameter is kotlin.coroutines.Continuation
        val uParameterType = uParameter.type
        if (!uParameterType.canonicalText.startsWith("kotlin.coroutines.Continuation")) return@find false

        // Continuation parameter's type parameter must be "? super (return type of suspend fun)"
        val continuationTypeParameter = uParameterType as? PsiClassReferenceType ?: return@find false
        val continuationWildcardTypeParameter =
            continuationTypeParameter.parameters[0] as? PsiWildcardType
                ?: return@find false

        // I don't think this is possible, but just in case, make sure the super bound != null
        continuationWildcardTypeParameter.superBound != PsiTypes.nullType()
    }?.let { continuationParameter ->
        // Extract the super bound type from the Continuation parameter
        val continuationParameterWildcardTypeParameter =
            // No null checking b/c we know it's there from the previous find step
            (continuationParameter.type as PsiClassReferenceType).parameters[0] as PsiWildcardType
        continuationParameterWildcardTypeParameter.superBound as PsiClassReferenceType
    }

And from what I've found, this somehow finds the type just fine. I didn't show that case in the screenshot, but debugging that problem showed that the type of the continuation parameter was Result? super X. I haven't dug deeply into that part yet.

You may also be right about these test cases not being valid. TBH I speedrun'd learning linting writing this check and it was a lot to grok and I probably have some invalid test cases here.

[Don Phillips]

But I am not sure it needs to be a suspend fun at all times, because I am 99% sure this call adapter we wrote for Result works in concert w/ the call adapter for suspending functions

[gmz]

Your code does roughly what the code I linked does, so yeah, I'm pretty sure it works.

But I am not sure it needs to be a suspend fun at all times, because I am 99% sure this call adapter we wrote for Result works in concert w/ the call adapter for suspending functions

Retrofit does more or less what your lint check is doing, it gets the return type of each method using reflection. It then uses that information to find a call adapters that can handle it.
This is why I think this won't work if you use a regular function, retrofit should only see Object, given that Result does not exist at runtime.
It works with suspend functions because the type gets encoded in the Continuation and it can get it from there, just like your lint check does.

That said, if you don't make that a suspend fun, then the only way to get a Result from it is by making that a blocking call, which is unexpected.

[gmz]

for reference: https://github.com/square/retrofit/blob/12ff091305b3c832b198bd3960749bc10e5656bd/retrofit/src/main/java/retrofit2/HttpServiceMethod.java#L47-L50

[Don Phillips]

Aye. I think I see what you're saying.

The only way to do it blocking would to make the function signature something like fun getFoo(): Call<Result<Foo>> .

[Don Phillips]

Either way, I think I'm pivoting on this entirely. Where I asked this question on Reddit, a guy said you can just configure your Retrofit instance when building it w/ validateEagerly(true) and then when you call retrofit.create(FooService::class.java) it'll make sure everything is good, including making sure it can get a json converter.

I just tested this, and it seems to work. Create a unit test, with a retrofit instance with a Moshi converter factory with only generated adapters, Try to create a service where Foo isn't annotated w/ @JsonClass(generateAdapter = true) in the method sig, test fails with an exception from Moshi saying it can't find the adapter

[gmz]

yes, that works too, although I'm not sure it catches all the issues. The nice thing about lint is that it warns you as you write code

[gmz]

or maybe it does and I'm just considering some weird edge cases such as code working fine in debug builds, but not in release builds

[gmz]

e.g. R8 stripping the generic type because the return type is not used

[Don Phillips]

Yeah I feel you. Unfortunately my situation:

1. I'm the only one on the team that's attempted writing a lint rule
2. We never have the time to surface the lint results nicely, so most devs ignore them anyways
3. The team generally hates lint
So if I can avoid lint, it's unfortunately better

[Don Phillips]

def appreciate your help though!

[David Rawson]

Not sure how stable it is, but I was able to get the KtTypeElement with:

((node.sourcePsi.lastChild as KtTypeReference).lastChild as KtTypeElement)

From there, you can get the type arguments (String from Result<String>) and so on.

[gmz]

Thanks! It seems that from KtTypeReference I can retrieve the original class, for example: (method.sourcePsi?.lastChild as? KtTypeReference)?.getType()?.getKotlinTypeFqName(false) (but I've just noticed that getType is internal)