Merge branch 'feature/issue#2436_enhancement' of https://github.com/s…

…achinp97/gson into feature/issue#2436_enhancement

graal-native-image-test/pom.xml

@@ -150,7 +150,7 @@
           <plugin>
             <groupId>org.graalvm.buildtools</groupId>
             <artifactId>native-maven-plugin</artifactId>
-            <version>0.9.25</version>
+            <version>0.9.26</version>
             <extensions>true</extensions>
             <executions>
               <execution>

gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java

@@ -139,14 +139,14 @@ private void put(JsonElement value) {
   @Override public JsonWriter name(String name) throws IOException {
     Objects.requireNonNull(name, "name == null");
     if (stack.isEmpty() || pendingName != null) {
-      throw new IllegalStateException();
+      throw new IllegalStateException("Did not expect a name");
     }
     JsonElement element = peek();
     if (element instanceof JsonObject) {
       pendingName = name;
       return this;
     }
-    throw new IllegalStateException();
+    throw new IllegalStateException("Please begin an object before writing a name.");
   }
 
   @CanIgnoreReturnValue

gson/src/main/java/com/google/gson/stream/JsonWriter.java

@@ -495,11 +495,9 @@ public JsonWriter name(String name) throws IOException {
     if (deferredName != null) {
       throw new IllegalStateException("Already wrote a name, expecting a value.");
     }
-    if (stackSize == 0) {
-      throw new IllegalStateException("JsonWriter is closed.");
-    }
-    if (stackSize == 1 && (peek() == EMPTY_DOCUMENT || peek() == NONEMPTY_DOCUMENT)) {
-      throw new IllegalStateException("Please begin an object before this.");
+    int context = peek();
+    if (context != EMPTY_OBJECT && context != NONEMPTY_OBJECT) {
+      throw new IllegalStateException("Please begin an object before writing a name.");
     }
     deferredName = name;
     return this;

gson/src/test/java/com/google/gson/internal/bind/JsonTreeWriterTest.java

@@ -17,6 +17,7 @@
 package com.google.gson.internal.bind;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
 import com.google.gson.JsonElement;
@@ -112,6 +113,45 @@ public void testPrematureClose() throws Exception {
     }
   }
 
+  @Test
+  public void testNameAsTopLevelValue() throws IOException {
+    JsonTreeWriter writer = new JsonTreeWriter();
+    IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Did not expect a name");
+
+    writer.value(12);
+    writer.close();
+
+    e = assertThrows(IllegalStateException.class, () -> writer.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name.");
+  }
+
+  @Test
+  public void testNameInArray() throws IOException {
+    JsonTreeWriter writer = new JsonTreeWriter();
+
+    writer.beginArray();
+    IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name.");
+
+    writer.value(12);
+    e = assertThrows(IllegalStateException.class, () -> writer.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name.");
+
+    writer.endArray();
+
+    assertThat(writer.get().toString()).isEqualTo("[12]");
+  }
+
+  @Test
+  public void testTwoNames() throws IOException {
+    JsonTreeWriter writer = new JsonTreeWriter();
+    writer.beginObject();
+    writer.name("a");
+    IllegalStateException e = assertThrows(IllegalStateException.class, () -> writer.name("a"));
+    assertThat(e).hasMessageThat().isEqualTo("Did not expect a name");
+  }
+
   @Test
   public void testSerializeNullsFalse() throws IOException {
     JsonTreeWriter writer = new JsonTreeWriter();

gson/src/test/java/com/google/gson/stream/JsonWriterTest.java

@@ -28,8 +28,6 @@
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import org.junit.Test;
-import java.util.Arrays;
-import java.util.List;
 
 @SuppressWarnings("resource")
 public final class JsonWriterTest {
@@ -113,20 +111,36 @@ public void testTopLevelValueTypes() throws IOException {
   }
 
   @Test
-  public void testInvalidTopLevelTypes() throws IOException {
+  public void testNameAsTopLevelValue() throws IOException {
     StringWriter stringWriter = new StringWriter();
     JsonWriter jsonWriter = new JsonWriter(stringWriter);
-    assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello"));
+    IllegalStateException e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name.");
+
+    jsonWriter.value(12);
+    jsonWriter.close();
+
+    e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("JsonWriter is closed.");
   }
 
   @Test
-  public void closeAllObjectsAndTryToAddElements() throws IOException {
-    JsonWriter jsonWriterForNameAddition = getJsonWriterWithObjects();
-    assertThrows(IllegalStateException.class, () -> jsonWriterForNameAddition.name("this_throw_exception_as_all_objects_are_closed"));
-    jsonWriterForNameAddition.close();
-    JsonWriter jsonWriterForValueAddition = getJsonWriterWithObjects();
-    assertThrows(IllegalStateException.class, () -> jsonWriterForValueAddition.value("this_throw_exception_as_only_one_top_level_entry"));
-    jsonWriterForValueAddition.close();
+  public void testNameInArray() throws IOException {
+    StringWriter stringWriter = new StringWriter();
+    JsonWriter jsonWriter = new JsonWriter(stringWriter);
+
+    jsonWriter.beginArray();
+    IllegalStateException e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name.");
+
+    jsonWriter.value(12);
+    e = assertThrows(IllegalStateException.class, () -> jsonWriter.name("hello"));
+    assertThat(e).hasMessageThat().isEqualTo("Please begin an object before writing a name.");
+
+    jsonWriter.endArray();
+    jsonWriter.close();
+
+    assertThat(stringWriter.toString()).isEqualTo("[12]");
   }
 
   @Test
@@ -979,33 +993,4 @@ public void testIndentOverwritesFormattingStyle() throws IOException {
         + "}";
     assertThat(stringWriter.toString()).isEqualTo(expected);
   }
-
-  /**
-   * This method wites a json object and return a jsonwriter object
-   * that we can use for the testing purpose
-   * @return JsonWriter Object with nested object and an array
-   */
-  private JsonWriter getJsonWriterWithObjects() throws IOException {
-    StringWriter stringWriter = new StringWriter();
-    JsonWriter jsonWriter = new JsonWriter(stringWriter);
-    jsonWriter.beginObject();
-    jsonWriter.name("a").value(20);
-    jsonWriter.name("age").value(30);
-
-    // Start the nested "address" object
-    jsonWriter.name("address").beginObject();
-    jsonWriter.name("city").value("New York");
-    jsonWriter.name("country").value("USA");
-    jsonWriter.endObject(); // End the nested "address" object
-    jsonWriter.name("random_prop").value(78);
-    // Add an array of phone numbers (list of numbers)
-    List<Integer> phoneNumbers = Arrays.asList(1234567890, 98989, 9909);
-    jsonWriter.name("phoneNumbers").beginArray();
-    for (Integer phoneNumber : phoneNumbers) {
-      jsonWriter.value(phoneNumber);
-    }
-    jsonWriter.endArray(); // End the array
-    jsonWriter.endObject(); // End the outer object
-    return jsonWriter;
-  }
 }

pom.xml

@@ -100,7 +100,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-enforcer-plugin</artifactId>
-        <version>3.4.0</version>
+        <version>3.4.1</version>
         <executions>
           <execution>
             <id>enforce-versions</id>
@@ -299,7 +299,7 @@
         <plugin>
           <groupId>com.github.siom79.japicmp</groupId>
           <artifactId>japicmp-maven-plugin</artifactId>
-          <version>0.17.2</version>
+          <version>0.17.3</version>
           <configuration>
             <oldVersion>
               <dependency>

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();
 }
 
 

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 {
   <fields>;
 }
 -keepclassmembernames class com.example.NoSerializedNameMain$TestClassNotAbstract {
   <fields>;
 }
--keepclassmembernames class com.example.NoSerializedNameMain$TestClassWithoutDefaultConstructor {
+-keepclassmembernames class com.example.NoSerializedNameMain$TestClassHasArgsConstructor {
   <fields>;
 }

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 {

shrinker-test/src/main/java/com/example/ClassWithoutDefaultConstructor.java → shrinker-test/src/main/java/com/example/ClassWithHasArgsConstructor.java

@@ -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;
   }
 }

shrinker-test/src/main/java/com/example/ClassWithDefaultConstructor.java → shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java

@@ -3,14 +3,14 @@
 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;
   }
 }

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;
+  }
+}

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;
+  }
+}