diff --git a/Troubleshooting.md b/Troubleshooting.md index 57e781cbb9..184f19166e 100644 --- a/Troubleshooting.md +++ b/Troubleshooting.md @@ -313,6 +313,8 @@ Note: For newer Gson versions these rules might be applied automatically; make s **Symptom:** A `JsonIOException` with the message 'Abstract classes can't be instantiated!' is thrown; the class mentioned in the exception message is not actually `abstract` in your source code, and you are using the code shrinking tool R8 (Android app builds normally have this configured by default). +Note: If the class which you are trying to deserialize is actually abstract, then this exception is probably unrelated to R8 and you will have to implement a custom [`InstanceCreator`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/InstanceCreator.html) or [`TypeAdapter`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html) which creates an instance of a non-abstract subclass of the class. + **Reason:** The code shrinking tool R8 performs optimizations where it removes the no-args constructor from a class and makes the class `abstract`. Due to this Gson cannot create an instance of the class. **Solution:** Make sure the class has a no-args constructor, then adjust your R8 configuration file to keep the constructor of the class. For example: @@ -324,6 +326,10 @@ Note: For newer Gson versions these rules might be applied automatically; make s } ``` +You can also use `(...);` to keep all constructors of that class, but then you might actually rely on `sun.misc.Unsafe` on both JDK and Android to create classes without no-args constructor, see [`GsonBuilder.disableJdkUnsafe()`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/GsonBuilder.html#disableJdkUnsafe()) for more information. + For Android you can add this rule to the `proguard-rules.pro` file, see also the [Android documentation](https://developer.android.com/build/shrink-code#keep-code). In case the class name in the exception message is obfuscated, see the Android documentation about [retracing](https://developer.android.com/build/shrink-code#retracing). -Note: If the class which you are trying to deserialize is actually abstract, then this exception is probably unrelated to R8 and you will have to implement a custom [`InstanceCreator`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/InstanceCreator.html) or [`TypeAdapter`](https://javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/TypeAdapter.html) which creates an instance of a non-abstract subclass of the class. +For Android you can alternatively use the [`@Keep` annotation](https://developer.android.com/studio/write/annotations#keep) on the class or constructor you want to keep. That might be easier than having to maintain a custom R8 configuration. + +Note that the latest Gson versions (> 2.10.1) specify a default R8 configuration. If your class is a top-level class or is `static`, has a no-args constructor and its fields are annotated with Gson's [`@SerializedName`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/annotations/SerializedName.html), you might not have to perform any additional R8 configuration. diff --git a/examples/android-proguard-example/README.md b/examples/android-proguard-example/README.md index 942477d71a..902960fdfa 100644 --- a/examples/android-proguard-example/README.md +++ b/examples/android-proguard-example/README.md @@ -12,6 +12,9 @@ details on how ProGuard can be configured. The R8 code shrinker uses the same rule format as ProGuard, but there are differences between these two tools. Have a look at R8's Compatibility FAQ, and especially at the [Gson section](https://r8.googlesource.com/r8/+/refs/heads/main/compatibility-faq.md#gson). -Note that newer Gson versions apply some of the rules shown in `proguard.cfg` automatically by default, +Note that the latest Gson versions (> 2.10.1) apply some of the rules shown in `proguard.cfg` automatically by default, see the file [`gson/META-INF/proguard/gson.pro`](/gson/src/main/resources/META-INF/proguard/gson.pro) for -the Gson version you are using. +the Gson version you are using. In general if your classes are top-level classes or are `static`, have a no-args constructor and their fields are annotated with Gson's [`@SerializedName`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/annotations/SerializedName.html), you might not have to perform any additional ProGuard or R8 configuration. + +An alternative to writing custom keep rules for your classes in the ProGuard configuration can be to use +Android's [`@Keep` annotation](https://developer.android.com/studio/write/annotations#keep). diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java index 7d2dc9b622..0f488a9bb9 100644 --- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java +++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java @@ -76,14 +76,15 @@ static String checkInstantiable(Class c) { if (Modifier.isAbstract(modifiers)) { // R8 performs aggressive optimizations where it removes the default constructor of a class // and makes the class `abstract`; check for that here explicitly - if (c.getDeclaredConstructors().length == 0) { - return "Abstract classes can't be instantiated! Adjust the R8 configuration or register" - + " an InstanceCreator or a TypeAdapter for this type. Class name: " + c.getName() - + "\nSee " + TroubleshootingGuide.createUrl("r8-abstract-class"); - } - - return "Abstract classes can't be instantiated! Register an InstanceCreator" - + " or a TypeAdapter for this type. Class name: " + c.getName(); + /* + * Note: Ideally should only show this R8-specific message when it is clear that R8 was + * used (e.g. when `c.getDeclaredConstructors().length == 0`), but on Android where this + * issue with R8 occurs most, R8 seems to keep some constructors for some reason while + * still making the class abstract + */ + return "Abstract classes can't be instantiated! Adjust the R8 configuration or register" + + " an InstanceCreator or a TypeAdapter for this type. Class name: " + c.getName() + + "\nSee " + TroubleshootingGuide.createUrl("r8-abstract-class"); } return null; } diff --git a/gson/src/main/resources/META-INF/proguard/gson.pro b/gson/src/main/resources/META-INF/proguard/gson.pro index c9f235e95b..59d3bb441d 100644 --- a/gson/src/main/resources/META-INF/proguard/gson.pro +++ b/gson/src/main/resources/META-INF/proguard/gson.pro @@ -13,7 +13,7 @@ # Keep Gson annotations # Note: Cannot perform finer selection here to only cover Gson annotations, see also https://stackoverflow.com/q/47515093 --keepattributes *Annotation* +-keepattributes RuntimeVisibleAnnotations,AnnotationDefault ### The following rules are needed for R8 in "full mode" which only adheres to `-keepattribtues` if @@ -24,10 +24,10 @@ -keep class com.google.gson.reflect.TypeToken { *; } # Keep any (anonymous) classes extending TypeToken --keep class * extends com.google.gson.reflect.TypeToken +-keep,allowobfuscation class * extends com.google.gson.reflect.TypeToken # Keep classes with @JsonAdapter annotation --keep @com.google.gson.annotations.JsonAdapter class * +-keep,allowobfuscation,allowoptimization @com.google.gson.annotations.JsonAdapter class * # Keep fields with @SerializedName annotation, but allow obfuscation of their names -keepclassmembers,allowobfuscation class * { @@ -35,7 +35,9 @@ } # Keep fields with any other Gson annotation --keepclassmembers class * { +# Also allow obfuscation, assuming that users will additionally use @SerializedName or +# other means to preserve the field names +-keepclassmembers,allowobfuscation class * { @com.google.gson.annotations.Expose ; @com.google.gson.annotations.JsonAdapter ; @com.google.gson.annotations.Since ; @@ -44,15 +46,25 @@ # Keep no-args constructor of classes which can be used with @JsonAdapter # By default their no-args constructor is invoked to create an adapter instance --keep class * extends com.google.gson.TypeAdapter { +-keepclassmembers class * extends com.google.gson.TypeAdapter { (); } --keep class * implements com.google.gson.TypeAdapterFactory { +-keepclassmembers class * implements com.google.gson.TypeAdapterFactory { (); } --keep class * implements com.google.gson.JsonSerializer { +-keepclassmembers class * implements com.google.gson.JsonSerializer { (); } --keep class * implements com.google.gson.JsonDeserializer { +-keepclassmembers class * implements com.google.gson.JsonDeserializer { (); } + +# If a class is used in some way by the application, and has fields annotated with @SerializedName +# and a no-args constructor, keep those fields and the constructor +# Based on https://issuetracker.google.com/issues/150189783#comment11 +# See also https://github.com/google/gson/pull/2420#discussion_r1241813541 for a more detailed explanation +-if class * +-keepclasseswithmembers,allowobfuscation,allowoptimization class <1> { + (); + @com.google.gson.annotations.SerializedName ; +} diff --git a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java index 0b9a6c078c..bc9e8f06b6 100644 --- a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java +++ b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java @@ -89,7 +89,7 @@ public int i() { var exception = assertThrows(JsonIOException.class, () -> gson.getAdapter(LocalRecord.class)); assertThat(exception).hasMessageThat() - .isEqualTo("@SerializedName on method '" + LocalRecord.class.getName() + "#i()' is not supported"); + .isEqualTo("@SerializedName on method '" + LocalRecord.class.getName() + "#i()' is not supported"); } @Test @@ -154,7 +154,7 @@ record LocalRecord(String s) { // TODO: Adjust this once Gson throws more specific exception type catch (RuntimeException e) { assertThat(e).hasMessageThat() - .isEqualTo("Failed to invoke constructor '" + LocalRecord.class.getName() + "(String)' with args [value]"); + .isEqualTo("Failed to invoke constructor '" + LocalRecord.class.getName() + "(String)' with args [value]"); assertThat(e).hasCauseThat().isSameInstanceAs(LocalRecord.thrownException); } } @@ -227,7 +227,7 @@ public void testPrimitiveJsonNullValue() { String s = "{'aString': 's', 'aByte': null, 'aShort': 0}"; var e = assertThrows(JsonParseException.class, () -> gson.fromJson(s, RecordWithPrimitives.class)); assertThat(e).hasMessageThat() - .isEqualTo("null is not allowed as value for record component 'aByte' of primitive type; at path $.aByte"); + .isEqualTo("null is not allowed as value for record component 'aByte' of primitive type; at path $.aByte"); } /** @@ -384,8 +384,8 @@ record Blocked(int b) {} var exception = assertThrows(JsonIOException.class, () -> gson.toJson(new Blocked(1))); assertThat(exception).hasMessageThat() - .isEqualTo("ReflectionAccessFilter does not permit using reflection for class " + Blocked.class.getName() + - ". Register a TypeAdapter for this type or adjust the access filter."); + .isEqualTo("ReflectionAccessFilter does not permit using reflection for class " + Blocked.class.getName() + + ". Register a TypeAdapter for this type or adjust the access filter."); } @Test @@ -396,15 +396,15 @@ public void testReflectionFilterBlockInaccessible() { var exception = assertThrows(JsonIOException.class, () -> gson.toJson(new PrivateRecord(1))); assertThat(exception).hasMessageThat() - .isEqualTo("Constructor 'com.google.gson.functional.Java17RecordTest$PrivateRecord(int)' is not accessible and" - + " ReflectionAccessFilter does not permit making it accessible. Register a TypeAdapter for the declaring" - + " type, adjust the access filter or increase the visibility of the element and its declaring type."); + .isEqualTo("Constructor 'com.google.gson.functional.Java17RecordTest$PrivateRecord(int)' is not accessible and" + + " ReflectionAccessFilter does not permit making it accessible. Register a TypeAdapter for the declaring" + + " type, adjust the access filter or increase the visibility of the element and its declaring type."); exception = assertThrows(JsonIOException.class, () -> gson.fromJson("{}", PrivateRecord.class)); assertThat(exception).hasMessageThat() - .isEqualTo("Constructor 'com.google.gson.functional.Java17RecordTest$PrivateRecord(int)' is not accessible and" - + " ReflectionAccessFilter does not permit making it accessible. Register a TypeAdapter for the declaring" - + " type, adjust the access filter or increase the visibility of the element and its declaring type."); + .isEqualTo("Constructor 'com.google.gson.functional.Java17RecordTest$PrivateRecord(int)' is not accessible and" + + " ReflectionAccessFilter does not permit making it accessible. Register a TypeAdapter for the declaring" + + " type, adjust the access filter or increase the visibility of the element and its declaring type."); assertThat(gson.toJson(new PublicRecord(1))).isEqualTo("{\"i\":1}"); assertThat(gson.fromJson("{\"i\":2}", PublicRecord.class)).isEqualTo(new PublicRecord(2)); @@ -427,7 +427,8 @@ record LocalRecord(int i) {} var exception = assertThrows(JsonIOException.class, () -> gson.fromJson("{}", Record.class)); assertThat(exception).hasMessageThat() - .isEqualTo("Abstract classes can't be instantiated! Register an InstanceCreator or a TypeAdapter for" - + " this type. Class name: java.lang.Record"); + .isEqualTo("Abstract classes can't be instantiated! Adjust the R8 configuration or register an InstanceCreator" + + " or a TypeAdapter for this type. Class name: java.lang.Record" + + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class"); } } diff --git a/gson/src/test/java/com/google/gson/internal/ConstructorConstructorTest.java b/gson/src/test/java/com/google/gson/internal/ConstructorConstructorTest.java index 602ba074e9..e582ad08ac 100644 --- a/gson/src/test/java/com/google/gson/internal/ConstructorConstructorTest.java +++ b/gson/src/test/java/com/google/gson/internal/ConstructorConstructorTest.java @@ -19,17 +19,14 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; -import com.google.gson.InstanceCreator; -import com.google.gson.ReflectionAccessFilter; import com.google.gson.reflect.TypeToken; -import java.lang.reflect.Type; import java.util.Collections; import org.junit.Test; public class ConstructorConstructorTest { private ConstructorConstructor constructorConstructor = new ConstructorConstructor( - Collections.>emptyMap(), true, - Collections.emptyList() + Collections.emptyMap(), true, + Collections.emptyList() ); private abstract static class AbstractClass { @@ -39,7 +36,7 @@ public AbstractClass() { } private interface Interface { } /** - * Verify that ConstructorConstructor does not try to invoke no-arg constructor + * Verify that ConstructorConstructor does not try to invoke no-args constructor * of abstract class. */ @Test @@ -49,9 +46,10 @@ public void testGet_AbstractClassNoArgConstructor() { constructor.construct(); fail("Expected exception"); } catch (RuntimeException exception) { - assertThat(exception).hasMessageThat().isEqualTo("Abstract classes can't be instantiated! " - + "Register an InstanceCreator or a TypeAdapter for this type. " - + "Class name: com.google.gson.internal.ConstructorConstructorTest$AbstractClass"); + assertThat(exception).hasMessageThat().isEqualTo("Abstract classes can't be instantiated!" + + " Adjust the R8 configuration or register an InstanceCreator or a TypeAdapter for this type." + + " Class name: com.google.gson.internal.ConstructorConstructorTest$AbstractClass" + + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class"); } } @@ -62,9 +60,9 @@ public void testGet_Interface() { constructor.construct(); fail("Expected exception"); } catch (RuntimeException exception) { - assertThat(exception).hasMessageThat().isEqualTo("Interfaces can't be instantiated! " - + "Register an InstanceCreator or a TypeAdapter for this type. " - + "Interface name: com.google.gson.internal.ConstructorConstructorTest$Interface"); + assertThat(exception).hasMessageThat().isEqualTo("Interfaces can't be instantiated!" + + " Register an InstanceCreator or a TypeAdapter for this type." + + " Interface name: com.google.gson.internal.ConstructorConstructorTest$Interface"); } } } diff --git a/shrinker-test/pom.xml b/shrinker-test/pom.xml index 7c6a4bacc0..b46b978d4c 100644 --- a/shrinker-test/pom.xml +++ b/shrinker-test/pom.xml @@ -191,6 +191,8 @@ + com.android.tools r8 8.0.40 diff --git a/shrinker-test/proguard.pro b/shrinker-test/proguard.pro index 0cb35048c9..3e8a812041 100644 --- a/shrinker-test/proguard.pro +++ b/shrinker-test/proguard.pro @@ -18,6 +18,7 @@ -keep class com.example.DefaultConstructorMain { public static java.lang.String runTest(); public static java.lang.String runTestNoJdkUnsafe(); + public static java.lang.String runTestNoDefaultConstructor(); } @@ -27,3 +28,21 @@ -keepclassmembers class com.example.ClassWithNamedFields { !transient ; } + +-keepclassmembernames class com.example.ClassWithExposeAnnotation { + ; +} +-keepclassmembernames class com.example.ClassWithJsonAdapterAnnotation { + ** f; +} +-keepclassmembernames class com.example.ClassWithVersionAnnotations { + ; +} + + +-keepclassmembernames class com.example.DefaultConstructorMain$TestClass { + ; +} +-keepclassmembernames class com.example.DefaultConstructorMain$TestClassNotAbstract { + ; +} diff --git a/shrinker-test/r8.pro b/shrinker-test/r8.pro index 690fe339b5..392f0f0d9c 100644 --- a/shrinker-test/r8.pro +++ b/shrinker-test/r8.pro @@ -4,20 +4,6 @@ ### The following rules are needed for R8 in "full mode", which performs more aggressive optimizations than ProGuard ### See https://r8.googlesource.com/r8/+/refs/heads/main/compatibility-faq.md#r8-full-mode -# Keep the no-args constructor of deserialized classes --keepclassmembers class com.example.ClassWithDefaultConstructor { - (); -} --keepclassmembers class com.example.GenericClasses$GenericClass { - (); -} --keepclassmembers class com.example.GenericClasses$UsingGenericClass { - (); -} --keepclassmembers class com.example.GenericClasses$GenericUsingGenericClass { - (); -} - # For classes with generic type parameter R8 in "full mode" requires to have a keep rule to # preserve the generic signature -keep,allowshrinking,allowoptimization,allowobfuscation,allowaccessmodification class com.example.GenericClasses$GenericClass @@ -25,12 +11,14 @@ # Don't obfuscate class name, to check it in exception message -keep,allowshrinking,allowoptimization class com.example.DefaultConstructorMain$TestClass +-keep,allowshrinking,allowoptimization class com.example.DefaultConstructorMain$TestClassWithoutDefaultConstructor + # This rule has the side-effect that R8 still removes the no-args constructor, but does not make the class abstract -keep class com.example.DefaultConstructorMain$TestClassNotAbstract { @com.google.gson.annotations.SerializedName ; } # Keep enum constants which are not explicitly used in code --keep class com.example.EnumClass { +-keepclassmembers class com.example.EnumClass { ** SECOND; } diff --git a/shrinker-test/src/main/java/com/example/ClassWithJsonAdapterAnnotation.java b/shrinker-test/src/main/java/com/example/ClassWithJsonAdapterAnnotation.java index 238ee1818e..0cb05a25a6 100644 --- a/shrinker-test/src/main/java/com/example/ClassWithJsonAdapterAnnotation.java +++ b/shrinker-test/src/main/java/com/example/ClassWithJsonAdapterAnnotation.java @@ -23,6 +23,7 @@ */ public class ClassWithJsonAdapterAnnotation { // For this field don't use @SerializedName and ignore it for deserialization + // Has custom ProGuard rule to keep the field name @JsonAdapter(value = Adapter.class, nullSafe = false) DummyClass f; diff --git a/shrinker-test/src/main/java/com/example/DefaultConstructorMain.java b/shrinker-test/src/main/java/com/example/DefaultConstructorMain.java index e570866bec..4f0152f7d1 100644 --- a/shrinker-test/src/main/java/com/example/DefaultConstructorMain.java +++ b/shrinker-test/src/main/java/com/example/DefaultConstructorMain.java @@ -8,14 +8,24 @@ public class DefaultConstructorMain { static class TestClass { - @SerializedName("s") public String s; } // R8 rule for this class still removes no-args constructor, but doesn't make class abstract static class TestClassNotAbstract { + public String s; + } + + // Current Gson ProGuard rules only keep default constructor (and only then prevent R8 from + // making class abstract); other constructors are ignored to suggest to user adding default + // constructor instead of implicitly relying on JDK Unsafe + static class TestClassWithoutDefaultConstructor { @SerializedName("s") public String s; + + public TestClassWithoutDefaultConstructor(String s) { + this.s = s; + } } /** @@ -34,4 +44,12 @@ public static String runTestNoJdkUnsafe() { TestClassNotAbstract deserialized = gson.fromJson("{\"s\": \"value\"}", same(TestClassNotAbstract.class)); return deserialized.s; } + + /** + * Main entrypoint, called by {@code ShrinkingIT.testNoDefaultConstructor()}. + */ + public static String runTestNoDefaultConstructor() { + TestClassWithoutDefaultConstructor deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClassWithoutDefaultConstructor.class)); + return deserialized.s; + } } diff --git a/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java b/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java index 9c32f1aa57..3304570197 100644 --- a/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java +++ b/shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java @@ -220,4 +220,24 @@ public void testDefaultConstructorNoJdkUnsafe() throws Exception { } }); } + + @Test + public void testNoDefaultConstructor() throws Exception { + runTest("com.example.DefaultConstructorMain", c -> { + Method m = c.getMethod("runTestNoDefaultConstructor"); + + if (jarToTest.equals(PROGUARD_RESULT_PATH)) { + Object result = m.invoke(null); + assertThat(result).isEqualTo("value"); + } else { + // R8 performs more aggressive optimizations + Exception e = assertThrows(InvocationTargetException.class, () -> m.invoke(null)); + assertThat(e).hasCauseThat().hasMessageThat().isEqualTo( + "Abstract classes can't be instantiated! Adjust the R8 configuration or register an InstanceCreator" + + " or a TypeAdapter for this type. Class name: com.example.DefaultConstructorMain$TestClassWithoutDefaultConstructor" + + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" + ); + } + }); + } }