Skip to content

Improve default ProGuard / R8 rules and Troubleshooting Guide #2401

@Marcono1234

Description

@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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementproguard-r8Issues relating to the use of ProGuard and/or R8, such as problems due to obfuscation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions