Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shrinker-test examples with never-referenced constructors #2478

Merged
2 changes: 1 addition & 1 deletion shrinker-test/common.pro
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
-keep class com.example.NoSerializedNameMain {
public static java.lang.String runTest();
public static java.lang.String runTestNoJdkUnsafe();
public static java.lang.String runTestNoDefaultConstructor();
public static java.lang.String runTestConstructorHasArgs();
}


Expand Down
2 changes: 1 addition & 1 deletion shrinker-test/proguard.pro
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
-keepclassmembernames class com.example.NoSerializedNameMain$TestClassNotAbstract {
<fields>;
}
-keepclassmembernames class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor {
-keepclassmembernames class com.example.NoSerializedNameMain$TestClassConstructorHasArgs {
<fields>;
}
2 changes: 1 addition & 1 deletion shrinker-test/r8.pro
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

# 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$TestClassConstructorHasArgs

# 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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
* Class with no-args default constructor and with field annotated with
sfreilich marked this conversation as resolved.
Show resolved Hide resolved
* {@link SerializedName}.
*/
public class ClassWithDefaultConstructor {
public class ClassWithNoArgsConstructor {
@SerializedName("myField")
public int i;

public ClassWithDefaultConstructor() {
public ClassWithNoArgsConstructor() {
i = -3;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
65 changes: 58 additions & 7 deletions shrinker-test/src/main/java/com/example/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ public static void runTests(BiConsumer<String, String> outputConsumer) {
testNamedFields(outputConsumer);
testSerializedName(outputConsumer);

testNoDefaultConstructor(outputConsumer);
testConstructorNoArgs(outputConsumer);
testConstructorHasArgs(outputConsumer);
testUnreferencedConstructorNoArgs(outputConsumer);
testUnreferencedConstructorHasArgs(outputConsumer);

testNoJdkUnsafe(outputConsumer);

testEnum(outputConsumer);
Expand Down Expand Up @@ -90,24 +94,71 @@ private static void testSerializedName(BiConsumer<String, String> outputConsumer
});
}

private static void testNoDefaultConstructor(BiConsumer<String, String> outputConsumer) {
private static void testConstructorNoArgs(BiConsumer<String, String> outputConsumer) {
Gson gson = new GsonBuilder().setPrettyPrinting().create();
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<String, String> outputConsumer) {
Gson gson = new GsonBuilder().setPrettyPrinting().create();
TestExecutor.run(outputConsumer, "Write: No default constructor", () -> toJson(gson, new ClassWithoutDefaultConstructor(2)));
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: Constructor with args", () -> {
ClassWithHasArgsConstructor deserialized = fromJson(
gson, "{\"myField\": 3}", ClassWithHasArgsConstructor.class);
return Integer.toString(deserialized.i);
});
}

private static void testUnreferencedConstructorNoArgs(BiConsumer<String, String> 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<String, String> 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: No default constructor", () -> {
ClassWithoutDefaultConstructor deserialized = fromJson(gson, "{\"myField\": 3}", ClassWithoutDefaultConstructor.class);
TestExecutor.run(outputConsumer, "Read: Unreferenced constructor with args", () -> {
ClassWithUnreferencedHasArgsConstructor deserialized = fromJson(
gson, "{\"myField\": 3}", ClassWithUnreferencedHasArgsConstructor.class);
return Integer.toString(deserialized.i);
});
}

private static void testNoJdkUnsafe(BiConsumer<String, String> 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);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,25 @@ static class TestClassNotAbstract {
public String s;
}

static class TestClassWithoutDefaultConstructor {
static class TestClassConstructorHasArgs {
public String s;

// Specify explicit constructor with args to remove implicit no-args default constructor
public TestClassWithoutDefaultConstructor(String s) {
public TestClassConstructorHasArgs(String s) {
this.s = s;
}
}

/**
* Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_DefaultConstructor()}.
* Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_ConstructorHasArgs()}.
sfreilich marked this conversation as resolved.
Show resolved Hide resolved
sfreilich marked this conversation as resolved.
Show resolved Hide resolved
*/
public static String runTest() {
TestClass deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClass.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();
Expand All @@ -46,10 +46,10 @@ public static String runTestNoJdkUnsafe() {
}

/**
* Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_NoDefaultConstructor()}.
* Main entrypoint, called by {@code ShrinkingIT.testNoSerializedName_ConstructorHasArgs()}.
sfreilich marked this conversation as resolved.
Show resolved Hide resolved
*/
public static String runTestNoDefaultConstructor() {
TestClassWithoutDefaultConstructor deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClassWithoutDefaultConstructor.class));
public static String runTestConstructorHasArgs() {
TestClassConstructorHasArgs deserialized = new Gson().fromJson("{\"s\":\"value\"}", same(TestClassConstructorHasArgs.class));
return deserialized.s;
}
}
34 changes: 27 additions & 7 deletions shrinker-test/src/test/java/com/google/gson/it/ShrinkingIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,32 @@ public void test() throws Exception {
"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",
Expand Down Expand Up @@ -191,7 +211,7 @@ public void test() throws Exception {
}

@Test
public void testNoSerializedName_DefaultConstructor() throws Exception {
public void testNoSerializedName_NoArgsConstructor() throws Exception {
runTest("com.example.NoSerializedNameMain", c -> {
Method m = c.getMethod("runTest");

Expand All @@ -211,7 +231,7 @@ public void testNoSerializedName_DefaultConstructor() throws Exception {
}

@Test
public void testNoSerializedName_DefaultConstructorNoJdkUnsafe() throws Exception {
public void testNoSerializedName_NoArgsConstructorNoJdkUnsafe() throws Exception {
runTest("com.example.NoSerializedNameMain", c -> {
Method m = c.getMethod("runTestNoJdkUnsafe");

Expand All @@ -232,9 +252,9 @@ public void testNoSerializedName_DefaultConstructorNoJdkUnsafe() throws Exceptio
}

@Test
public void testNoSerializedName_NoDefaultConstructor() throws Exception {
public void testNoSerializedName_ConstructorHasArgs() throws Exception {
runTest("com.example.NoSerializedNameMain", c -> {
Method m = c.getMethod("runTestNoDefaultConstructor");
Method m = c.getMethod("runTestConstructorHasArgs");

if (jarToTest.equals(PROGUARD_RESULT_PATH)) {
Object result = m.invoke(null);
Expand All @@ -244,7 +264,7 @@ public void testNoSerializedName_NoDefaultConstructor() throws Exception {
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$TestClassConstructorHasArgs"
+ "\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#r8-abstract-class"
);
}
Expand Down