Insufficient keep rules for R8 in full mode

[christofferqa]

The keep rules in https://github.com/square/retrofit/blob/master/retrofit/src/main/resources/META-INF/proguard/retrofit2.pro are insufficient for R8 in full mode.

See for example https://issuetracker.google.com/issues/120168590#comment3 and https://issuetracker.google.com/issues/122940365#comment9.

As a simple example, consider the 'SimpleService' sample:
https://github.com/square/retrofit/blob/master/samples/src/main/java/com/example/retrofit/SimpleService.java

The interface 'GitHub' is never instantiated in the code (except via reflection), and it is not explicitly kept by a keep rule, so R8 can safely replace all values in the program that have type 'GitHub' with constant null.

[JakeWharton]

Can the use of a Proxy to create instances be detected? The Class literal flows directly into a method which calls Proxy.newProxyInstance in the common case.

[christofferqa]

R8 does not have any interprocedural tracking of class literals at this point. Even with such tracking, the keep rules would still be insufficient depending on the program.

It should be possible to keep the relevant interfaces if they are used by the program with a rule inspired by the following one.

-if interface * { @retrofit2.http.* <methods>; }
-keep,allowobfuscation interface <1>

[JakeWharton]

I guess that's not that terrible because the methods can still be removed. You might wind up with an empty interface or two if you have unused ones coming from libraries.

[JakeWharton]

I've got another full mode problem. R8 replaces interfaces used as return types in service interface methods with a subtype which then breaks reflective lookup.

Is there a way to issue a -keep on all types used in return types of an interface with a Retrofit annotation?

[christofferqa]

You should be able to use the following rule:

-if interface * { @retrofit2.http.* *** *(...); }
-keep,allowobfuscation interface <3>

This is assuming that the return type is an interface. If the return type could also be a class or enum type, then the following rules are unfortunately also needed:

-if interface * { @retrofit2.http.* *** *(...); }
-keep,allowobfuscation class <3>

-if interface * { @retrofit2.http.* *** *(...); }
-keep,allowobfuscation enum <3>

It would be easy to extend the grammar a bit, though. I am considering if this rule should not be written as:

-if interface * { @retrofit2.http.* *** *(...); }
-keep,allowobfuscation basetype(<3>)

This rule should then suffice to handle the cases where the return type is an array, class, enum, and interface. This would no longer be compatible with Proguard, though.

[JakeWharton]

The duplication is okay since Retrofit is the only one that needs to ship those rules. But I can't get those rules to actually work. I tried with just -keep as well. R8 still rewrites the return types.

Here's my input:

@GET("_s/getsuggestions?p=/&s=irina&c=3")
fun list(): Deferred<Map<String, List<Item>>>

However, upon more investigation as to what's happening, R8 is performing vertical class merging of the kotlinx.coroutines.Deferred interface into kotlinx.coroutines.CompletableDeferred but it's also (correctly) updating the Retrofit Converter to look for CompletableDeferred so this should be working. The problem is that the interface isn't having its @dalvik.annotation.Signature annotation values updated so it still refers to kotlinx.coroutines.Deferred.

.method public abstract list()Lkotlinx/coroutines/CompletableDeferred;
    .annotation system Ldalvik/annotation/Signature;
        value = {
            "()",
            "Lkotlinx/coroutines/Deferred<",
            "Ljava/util/Map<",
            "Ljava/lang/String;",
            "Ljava/util/List<",
            "Lcom/jakewharton/sdksearch/api/dac/Item;",
            ">;>;>;"
        }
    .end annotation

    .annotation runtime Lretrofit2/http/GET;
        value = "_s/getsuggestions?p=/&s=irina&c=3"
    .end annotation
.end method

When using reflection on the parameterized return type, this annotation is checked for the type which is then loaded.

Caused by: java.lang.ClassNotFoundException: Didn't find class "kotlinx.coroutines.Deferred" on path: DexPathList[[zip file "/data/app/com.jakewharton.sdksearch-YFsxa6XLTfSDhK4wx_eH3g==/base.apk"],nativeLibraryDirectories=[/data/app/com.jakewharton.sdksearch-YFsxa6XLTfSDhK4wx_eH3g==/lib/x86, /system/lib]]
        at dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:134)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:379)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:312)
        at java.lang.Class.classForName(Class.java:-2)
        at java.lang.Class.forName(Class.java:453)
        at libcore.reflect.ParameterizedTypeImpl.getRawType(ParameterizedTypeImpl.java:65)
        at libcore.reflect.ParameterizedTypeImpl.getRawType(ParameterizedTypeImpl.java:24)
        at retrofit2.Utils.getRawType(Utils.java:5)
        at retrofit2.CallAdapter$Factory.getRawType(CallAdapter.java:1)
        at com.jakewharton.retrofit2.adapter.kotlin.coroutines.CoroutineCallAdapterFactory.get(CoroutineCallAdapterFactory.kt:2)

This seems like an R8 bug, right? It should be updating the signature annotation when it changes the types.

[christofferqa]

I can't get those rules to actually work

Have you tried with a recent version of R8, e.g., 'com.android.tools:r8:1.4.39'? I verified that a variant of the rule worked on a small test. If it doesn't work with 1.4.39, I will try to create a small repro that uses Retrofit.

(You may also try to add the rule -keep,allowobfuscation @interface retrofit2.http.*.)

This seems like an R8 bug, right?

It is. Would you mind filing an issue on the R8 bug tracker?

[JakeWharton]

I'm stuck on AGP 3.3.x for unrelated reasons which I'm guessing uses R8 1.3.x. I'll try with the latest R8.

[JakeWharton]

I couldn't get far enough to build a repro because there appears to be a regression in -if rule handling which I filed at https://issuetracker.google.com/issues/124267873 and CC'd you on.

[JakeWharton]

...and unblocked again. Back working on it.

[JakeWharton]

I can't get it to reproduce in a small project. I have filed https://issuetracker.google.com/issues/124357885 with a link to a branch of this project using AGP 3.5.0-alpha03 which reproduces the issue reliably though.

[JakeWharton]

Fixed upstream 🎉

[buntupana]

Hi @JakeWharton , I've just updated to gradle 8 and I'm getting this error when using R8 in full mode

IllegalArgumentException: Response must include generic type (e.g., Response<String>)

That happen in every call the app trying to do, when R8 full mode is disabled it's working well.

[mirekkukla]

I'm likewise seeing this become a problem again after I've updated to gradle 8 (I'm on the latest retrofit version v2.9.0). In my case the exception reads:

java.lang.IllegalArgumentException: Unable to create call adapter for interface wc.g

If I disable full mode everything works as expected.

Revisiting the workaround suggested for the original manifestation of this bug works, ie if I add the following rule everything works as expected even in full model:

-if interface * { @retrofit2.http.* *** *(...); }
-keep,allowobfuscation interface <3>

UPDATE: adding the "Call" rule added in this comit likewise resolves the problem.

-keep,allowobfuscation,allowshrinking interface retrofit2.Call

It seems said commit has yet to be released.

[tprochazka]

Now, when R8 full mode is officially available for everyone, a new version of RetroFit that fix could be really released.