diff --git a/shrinker-test/common.pro b/shrinker-test/common.pro index b30faa13..9995925e 100644 --- a/shrinker-test/common.pro +++ b/shrinker-test/common.pro @@ -16,9 +16,9 @@ public static void runTests(...); } -keep class com.example.NoSerializedNameMain { - public static java.lang.String runTest(); + public static java.lang.String runTestNoArgsConstructor(); public static java.lang.String runTestNoJdkUnsafe(); - public static java.lang.String runTestNoDefaultConstructor(); + public static java.lang.String runTestHasArgsConstructor(); } diff --git a/shrinker-test/proguard.pro b/shrinker-test/proguard.pro index ee960733..7922bc09 100644 --- a/shrinker-test/proguard.pro +++ b/shrinker-test/proguard.pro @@ -6,12 +6,12 @@ # Unlike R8, ProGuard does not perform aggressive optimization which makes classes abstract, # therefore for ProGuard can successfully perform deserialization, and for that need to # preserve the field names --keepclassmembernames class com.example.NoSerializedNameMain$TestClass { +-keepclassmembernames class com.example.NoSerializedNameMain$TestClassNoArgsConstructor { ; } -keepclassmembernames class com.example.NoSerializedNameMain$TestClassNotAbstract { ; } --keepclassmembernames class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor { +-keepclassmembernames class com.example.NoSerializedNameMain$TestClassHasArgsConstructor { ; } diff --git a/shrinker-test/r8.pro b/shrinker-test/r8.pro index 01b8c84e..642334d5 100644 --- a/shrinker-test/r8.pro +++ b/shrinker-test/r8.pro @@ -10,8 +10,8 @@ -keep,allowshrinking,allowoptimization,allowobfuscation,allowaccessmodification class com.example.GenericClasses$GenericUsingGenericClass # Don't obfuscate class name, to check it in exception message --keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClass --keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor +-keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassNoArgsConstructor +-keep,allowshrinking,allowoptimization class com.example.NoSerializedNameMain$TestClassHasArgsConstructor # 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.NoSerializedNameMain$TestClassNotAbstract { diff --git a/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java similarity index 61% rename from shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java rename to shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java index 2af57367..c65281e0 100644 --- a/shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java +++ b/shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java @@ -3,15 +3,15 @@ package com.example; import com.google.gson.annotations.SerializedName; /** - * Class without no-args default constructor, but with field annotated with + * Class without no-args constructor, but with field annotated with * {@link SerializedName}. */ -public class ClassWithoutDefaultConstructor { +public class ClassWithHasArgsConstructor { @SerializedName("myField") public int i; // Specify explicit constructor with args to remove implicit no-args default constructor - public ClassWithoutDefaultConstructor(int i) { + public ClassWithHasArgsConstructor(int i) { this.i = i; } } diff --git a/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java similarity index 52% rename from shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java rename to shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java index 6cff1190..f211daf9 100644 --- a/shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java +++ b/shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java @@ -3,14 +3,14 @@ package com.example; import com.google.gson.annotations.SerializedName; /** - * Class with no-args default constructor and with field annotated with + * Class with no-args constructor and with field annotated with * {@link SerializedName}. */ -public class ClassWithDefaultConstructor { +public class ClassWithNoArgsConstructor { @SerializedName("myField") public int i; - public ClassWithDefaultConstructor() { + public ClassWithNoArgsConstructor() { i = -3; } } diff --git a/shrinker-test/src/main/java/com/example/ClassWithUnreferencedHasArgsConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedHasArgsConstructor.java new file mode 100644 index 00000000..b0178bf6 --- /dev/null +++ b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedHasArgsConstructor.java @@ -0,0 +1,19 @@ +package com.example; + +import com.google.gson.annotations.SerializedName; + +/** + * Class without no-args constructor, but with field annotated with + * {@link SerializedName}. The constructor should not actually be used in the + * code, but this shouldn't lead to R8 concluding that values of the type are + * not constructible and therefore must be null. + */ +public class ClassWithUnreferencedHasArgsConstructor { + @SerializedName("myField") + public int i; + + // Specify explicit constructor with args to remove implicit no-args default constructor + public ClassWithUnreferencedHasArgsConstructor(int i) { + this.i = i; + } +} diff --git a/shrinker-test/src/main/java/com/example/ClassWithUnreferencedNoArgsConstructor.java b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedNoArgsConstructor.java new file mode 100644 index 00000000..2d430321 --- /dev/null +++ b/shrinker-test/src/main/java/com/example/ClassWithUnreferencedNoArgsConstructor.java @@ -0,0 +1,18 @@ +package com.example; + +import com.google.gson.annotations.SerializedName; + +/** + * Class with no-args constructor and with field annotated with + * {@link SerializedName}. The constructor should not actually be used in the + * code, but this shouldn't lead to R8 concluding that values of the type are + * not constructible and therefore must be null. + */ +public class ClassWithUnreferencedNoArgsConstructor { + @SerializedName("myField") + public int i; + + public ClassWithUnreferencedNoArgsConstructor() { + i = -3; + } +} diff --git a/shrinker-test/src/main/java/com/example/Main.java b/shrinker-test/src/main/java/com/example/Main.java index 488bc03b..d2d88021 100644 --- a/shrinker-test/src/main/java/com/example/Main.java +++ b/shrinker-test/src/main/java/com/example/Main.java @@ -32,7 +32,11 @@ public class Main { testNamedFields(outputConsumer); testSerializedName(outputConsumer); - testNoDefaultConstructor(outputConsumer); + testConstructorNoArgs(outputConsumer); + testConstructorHasArgs(outputConsumer); + testUnreferencedConstructorNoArgs(outputConsumer); + testUnreferencedConstructorHasArgs(outputConsumer); + testNoJdkUnsafe(outputConsumer); testEnum(outputConsumer); @@ -90,12 +94,57 @@ public class Main { }); } - private static void testNoDefaultConstructor(BiConsumer outputConsumer) { + private static void testConstructorNoArgs(BiConsumer outputConsumer) { Gson gson = new GsonBuilder().setPrettyPrinting().create(); - TestExecutor.run(outputConsumer, "Write: No default constructor", () -> toJson(gson, new ClassWithoutDefaultConstructor(2))); + TestExecutor.run(outputConsumer, "Write: No args constructor", () -> toJson( + gson, new ClassWithNoArgsConstructor())); + TestExecutor.run(outputConsumer, "Read: No args constructor; initial constructor value", () -> { + ClassWithNoArgsConstructor deserialized = fromJson(gson, "{}", ClassWithNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + TestExecutor.run(outputConsumer, "Read: No args constructor; custom value", () -> { + ClassWithNoArgsConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + } + + private static void testConstructorHasArgs(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + TestExecutor.run(outputConsumer, "Write: Constructor with args", () -> toJson( + gson, new ClassWithHasArgsConstructor(2))); // This most likely relies on JDK Unsafe (unless the shrinker rewrites the constructor in some way) - TestExecutor.run(outputConsumer, "Read: No default constructor", () -> { - ClassWithoutDefaultConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithoutDefaultConstructor.class); + TestExecutor.run(outputConsumer, "Read: Constructor with args", () -> { + ClassWithHasArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithHasArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + } + + private static void testUnreferencedConstructorNoArgs(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + // No write because we're not referencing this class's constructor. + + // This runs the no-args constructor. + TestExecutor.run(outputConsumer, "Read: Unreferenced no args constructor; initial constructor value", () -> { + ClassWithUnreferencedNoArgsConstructor deserialized = fromJson( + gson, "{}", ClassWithUnreferencedNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + TestExecutor.run(outputConsumer, "Read: Unreferenced no args constructor; custom value", () -> { + ClassWithUnreferencedNoArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithUnreferencedNoArgsConstructor.class); + return Integer.toString(deserialized.i); + }); + } + + private static void testUnreferencedConstructorHasArgs(BiConsumer outputConsumer) { + Gson gson = new GsonBuilder().setPrettyPrinting().create(); + // No write because we're not referencing this class's constructor. + + // This most likely relies on JDK Unsafe (unless the shrinker rewrites the constructor in some way) + TestExecutor.run(outputConsumer, "Read: Unreferenced constructor with args", () -> { + ClassWithUnreferencedHasArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithUnreferencedHasArgsConstructor.class); return Integer.toString(deserialized.i); }); } @@ -103,11 +152,13 @@ public class Main { private static void testNoJdkUnsafe(BiConsumer outputConsumer) { Gson gson = new GsonBuilder().disableJdkUnsafe().create(); TestExecutor.run(outputConsumer, "Read: No JDK Unsafe; initial constructor value", () -> { - ClassWithDefaultConstructor deserialized = fromJson(gson, "{}", ClassWithDefaultConstructor.class); + ClassWithNoArgsConstructor deserialized = fromJson( + gson, "{}", ClassWithNoArgsConstructor.class); return Integer.toString(deserialized.i); }); TestExecutor.run(outputConsumer, "Read: No JDK Unsafe; custom value", () -> { - ClassWithDefaultConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithDefaultConstructor.class); + ClassWithNoArgsConstructor deserialized = fromJson( + gson, "{\"myField\": 3}", ClassWithNoArgsConstructor.class); return Integer.toString(deserialized.i); }); } diff --git a/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java b/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java index cd70af34..a43fc0bd 100644 --- a/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java +++ b/shrinker-test/src/main/java/com/example/NoSerializedNameMain.java @@ -10,7 +10,8 @@ import com.google.gson.GsonBuilder; * therefore not matched by the default {@code gson.pro} rules. */ public class NoSerializedNameMain { - static class TestClass { + static class TestClassNoArgsConstructor { + // Has a no-args default constructor. public String s; } @@ -19,37 +20,40 @@ public class NoSerializedNameMain { public String s; } - static class TestClassWithoutDefaultConstructor { + static class TestClassHasArgsConstructor { public String s; // Specify explicit constructor with args to remove implicit no-args default constructor - public TestClassWithoutDefaultConstructor(String s) { + public TestClassHasArgsConstructor(String s) { this.s = s; } } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructor()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoArgsConstructor()}. */ - public static String runTest() { - TestClass deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClass.class)); + public static String runTestNoArgsConstructor() { + TestClassNoArgsConstructor deserialized = new Gson().fromJson( + "{\"s\":\"value\"}", same(TestClassNoArgsConstructor.class)); return deserialized.s; } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructorNoJdkUnsafe()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoArgsConstructorNoJdkUnsafe()}. */ public static String runTestNoJdkUnsafe() { Gson gson = new GsonBuilder().disableJdkUnsafe().create(); - TestClassNotAbstract deserialized = gson.fromJson("{\"s\": \"value\"}", same(TestClassNotAbstract.class)); + TestClassNotAbstract deserialized = gson.fromJson( + "{\"s\": \"value\"}", same(TestClassNotAbstract.class)); return deserialized.s; } /** - * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoDefaultConstructor()}. + * Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_HasArgsConstructor()}. */ - public static String runTestNoDefaultConstructor() { - TestClassWithoutDefaultConstructor deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClassWithoutDefaultConstructor.class)); + public static String runTestHasArgsConstructor() { + TestClassHasArgsConstructor deserialized = new Gson().fromJson( + "{\"s\":\"value\"}", same(TestClassHasArgsConstructor.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 be5a6213..a017c2c3 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 @@ -129,12 +129,32 @@ public class ShrinkingIT { "Read: SerializedName", "3", "===", - "Write: No default constructor", + "Write: No args constructor", + "{", + " \"myField\": -3", + "}", + "===", + "Read: No args constructor; initial constructor value", + "-3", + "===", + "Read: No args constructor; custom value", + "3", + "===", + "Write: Constructor with args", "{", " \"myField\": 2", "}", "===", - "Read: No default constructor", + "Read: Constructor with args", + "3", + "===", + "Read: Unreferenced no args constructor; initial constructor value", + "-3", + "===", + "Read: Unreferenced no args constructor; custom value", + "3", + "===", + "Read: Unreferenced constructor with args", "3", "===", "Read: No JDK Unsafe; initial constructor value", @@ -191,9 +211,9 @@ public class ShrinkingIT { } @Test - public void testNoSerializedName_DefaultConstructor() throws Exception { + public void testNoSerializedName_NoArgsConstructor() throws Exception { runTest("com.example.NoSerializedNameMain", c -> { - Method m = c.getMethod("runTest"); + Method m = c.getMethod("runTestNoArgsConstructor"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { Object result = m.invoke(null); @@ -203,7 +223,7 @@ public class ShrinkingIT { 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.NoSerializedNameMain$TestClass" + + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClassNoArgsConstructor" + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" ); } @@ -211,7 +231,7 @@ public class ShrinkingIT { } @Test - public void testNoSerializedName_DefaultConstructorNoJdkUnsafe() throws Exception { + public void testNoSerializedName_NoArgsConstructorNoJdkUnsafe() throws Exception { runTest("com.example.NoSerializedNameMain", c -> { Method m = c.getMethod("runTestNoJdkUnsafe"); @@ -232,9 +252,9 @@ public class ShrinkingIT { } @Test - public void testNoSerializedName_NoDefaultConstructor() throws Exception { + public void testNoSerializedName_HasArgsConstructor() throws Exception { runTest("com.example.NoSerializedNameMain", c -> { - Method m = c.getMethod("runTestNoDefaultConstructor"); + Method m = c.getMethod("runTestHasArgsConstructor"); if (jarToTest.equals(PROGUARD_RESULT_PATH)) { Object result = m.invoke(null); @@ -244,7 +264,7 @@ public class ShrinkingIT { 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.NoSerializedNameMain$TestClassWithoutDefaultConstructor" + + " or a TypeAdapter for this type. Class name: com.example.NoSerializedNameMain$TestClassHasArgsConstructor" + "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class" ); }