Throw exception when calling `JsonWriter.name` outside JSON object (#2476)

Previous fix in 392cc65ff3 only covered writing
name as top level value, but not when trying to write name inside JSON array.

Removed `stackSize == 0` check from `JsonWriter.name` because that is done
already by `peek()` call.

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

@@ -139,14 +139,14 @@ public final class JsonTreeWriter extends JsonWriter {
   @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 class JsonWriter implements Closeable, Flushable {
     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 final class JsonTreeWriterTest {
     }
   }
 
+  @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.io.StringWriter;
 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 final class JsonWriterTest {
   }
 
   @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 final class JsonWriterTest {
         + "}";
     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;
-  }
 }