Improve default ProGuard / R8 rules and Troubleshooting Guide

[Marcono1234]

Problem solved by the feature

Improve the default ProGuard / R8 rules in META-INF/proguard/gson.pro added by #2397, and the Troubleshooting Guide.

Feature description

I was testing the new default rules with an Android app and Kotlin classes, and there might be the following areas to improve.

Troubleshooting Guide: JsonIOException: 'Abstract classes can't be instantiated!' (R8)

The Troubleshooting Guide currently suggests the following:

# Keep the no-args constructor of the deserialized class
-keepclassmembers class com.example.MyClass {
  <init>();
}

This means only the no-args constructor is kept, but not any other constructors. However, especially for Kotlin I assume it is common that classes don't have a no-args constructor because you might declare properties and the primary constructor at once, e.g. class MyClass(val s: String) (see Kotlin documentation). Not sure though if using that is a good idea in combination with Gson because it then requires JDK Unsafe to create instances, but that is a different topic.

So there are two questions:

• Should the Troubleshooting Guide rather recommend <init>(...); (all constructors), so that the users don't experience issues with R8 anymore (but implicitly depend on JDK Unsafe; using GsonBuilder.disableJdkUnsafe() would cause Gson to fail with a different exception)
• Should the Troubleshooting Guide recommend -keep instead of -keepclassmembers. In case the <init> rule for the class properly matches a constructor it might not matter. But in case <init>(); is used, but a no-args constructor does not exist, then -keep seems to at least have the side-effect that R8 does not make the class abstract (and then using JDK Unsafe an instance can be created).

Troubleshooting Guide: Recommend @Keep for Android

Instead of configuring ProGuard / R8 rules, it might be easier for Android developers to use @Keep on the corresponding class or constructor, see https://developer.android.com/studio/write/annotations#keep

Preventing R8 from making classes abstract / removing no-args constructor

It would be good if we could adjust the default rules to avoid the "Abstract classes can't be instantiated!" exception (for most cases) in the first place.

There is probably no general way to detect if a class might be used with Gson, but the most reliable variant might be to keep the constructor if any of the Gson annotations is used by a class.

This can probably be achieved with an -if rule (see also answers to this Stack Overflow question). For example:

# If class has fields with `@SerializedName` annotation keep its constructors
-if class * {
  @com.google.gson.annotations.SerializedName <fields>;
}
-keep class <1> {
  <init>(...);
}

(duplicated for all Gson annotations)

But it looks like this is not enough, maybe because there is no -keep (or -keepclasseswithmembers) rule for the class in the first place so -if somehow has no effect? You additionally need the following (duplicated for all Gson annotations probably):

-keepclasseswithmembers class * {
  @com.google.gson.annotations.SerializedName <fields>;
}

Only then the class is not made abstract and the constructor is properly kept...

(As side note: Having only the -keepclasseswithmembers ... { @SerializedName } above, but not the -if ... -keep seems to prevent R8 from making the class abstract, but still removes all constructors, so users would be dependent on JDK Unsafe then.)

Or similar to #2379 (comment) maybe the following would work as well:

-keepclasseswithmembers class * {
  <init>(...);
  @com.google.gson.annotations.SerializedName <fields>;
}

There is however probably no need for the -if rule shown in the original comment, unless that has some special effect, see #2397 (comment).

R8 makes class abstract but still keeps constructors

It also looks like the Android app build creates (for some reason) additional constructors / keeps constructors even when the class is made abstract. Due to this, the Troubleshooting Guide URLs are unfortunately not included in the exception messages (they are only added at the moment when there are no constructors).
Not really sure why these constructors exist in the first place and are kept even when the class is made abstract. In the R8 integration test of Gson this behavior was not seen.

Maybe we should remove the getDeclaredConstructors().length == 0 check if the class is abstract and always refer to the Troubleshooting Guide. The guide would have to be adjusted then though to also properly cover the case where the class was indeed abstract in the source code.

---

Any additional feedback is highly appreciated!

[keehl789]

hello,
how to use R8 for shrinking
programatically in java ?
thank you

[Marcono1234]

@keehl789, is this a general question about R8, not specifically related to Gson? If so, it might be better to ask this somewhere else where you probably get more and better answers, for example on Stack Overflow or on the R8 mailing list. But it would be good then to describe your use case a bit more in detail, so that others can give you more specific answers.
In case you ask this question somewhere else, feel free to leave a link to it here so that other users can follow up on it.

[alipov]

@Marcono1234 First of all thanks a lot for introducing consumer proguard rules!

I have added the rules to one of my projects. Everything seemed to work fine, but I ran into one issue.
Here's a sample code that makes use of Gson's TypeToken:

private List<? extends MyInterface> getData() {
    String input = "[{\"value\": \"data\"}]";
    Type type = new TypeToken<List<MyImplementation>>() { }.getType();
    return new Gson().fromJson(input, type);
}

MyImplementation class implements the MyInterface interface, and this is its only usage in code.

Upon minification, R8 removes the MyImplementation class completely, while TypeToken is being left with java.lang.Object as its generic argument:

.class Linfo/osom/typetokenminify/a;
.super Lh/a;
.source "SourceFile"

# annotations
.annotation system Ldalvik/annotation/Signature;
    value = {
        "Lh/a<",
        "Ljava/util/List<",
        "Ljava/lang/Object;",
        ">;>;"
    }
.end annotation

This, in turn, causes a runtime exception:

Caused by: java.lang.ClassCastException
    at c.a.a(SourceFile:1)
    at info.osom.typetokenminify.MainActivity.onCreate(SourceFile:163)

As a workaround, I'm currently have to explicitly keep the class:

-keep,allowobfuscation,allowoptimization class info.osom.typetokenminify.MyImplementation {
  @com.google.gson.annotations.SerializedName <fields>;
}

I'm wondering if more general rule can be used in such cases.

I'm using gson's 2.10.1 release with those proguard rules. I have uploaded a sample project for demonstration.

[Marcono1234]

@alipov, I was going to suggest that you could try omitting the TypeToken.getType() call, and directly call the Gson.fromJson(..., TypeToken) overloads added in Gson 2.10 which provide more type-safety (unrelated to R8). But in my testing it looks like this still causes a ClassCastException.

As mentioned in #2420 (comment) and in the comment in gson.pro, the intention with the current R8 / ProGuard config is to not keep classes with @SerializedName which are not actually used at all. But as you correctly point out, this seems to not work properly when the class is only referenced from a TypeToken.

@sgjesse since you are a lot more familiar with this, do you know if there is a way to define in the .pro file that any types referenced by a TypeToken should be considered used, and not be removed by R8 (and ProGuard)?

[Marcono1234]

@alipov, would you mind creating a separate GitHub issue for your problem? That would probably make it easier to track and also easier to find for other users affected by it.

The points which were originally mentioned in the description of this issue here were addressed, so it would probably be confusing to reopen or repurpose it for your problem.