Skip to content

Commit

Permalink
Implements code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
MaicolAntali committed Feb 24, 2023
1 parent bd2916e commit 13ab165
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,6 @@ public static class MyParameterizedTypeInstanceCreator<T>
* This means that the fields of the same objects will be overwritten by Gson.
* This is usually fine in tests since there we deserialize just once, but quite
* dangerous in practice.
*
* @param instanceOfT Instance of Type
*/
public MyParameterizedTypeInstanceCreator(T instanceOfT) {
this.instanceOfT = instanceOfT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import static com.google.common.truth.Truth.assertThat;

import com.google.errorprone.annotations.Immutable;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonDeserializationContext;
Expand Down
6 changes: 4 additions & 2 deletions gson/src/test/java/com/google/gson/functional/EnumTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,9 @@ public String getExpectedJson() {
* Test for issue 226.
*/
@Test
@SuppressWarnings("GetClassOnEnum")
public void testEnumSubclass() {
assertThat(Roshambo.ROCK.getDeclaringClass()).isAssignableTo(Roshambo.class);
assertThat(Roshambo.ROCK.getClass()).isNotEqualTo(Roshambo.class);
assertThat(gson.toJson(Roshambo.ROCK)).isEqualTo("\"ROCK\"");
assertThat(gson.toJson(EnumSet.allOf(Roshambo.class))).isEqualTo("[\"ROCK\",\"PAPER\",\"SCISSORS\"]");
assertThat(gson.fromJson("\"ROCK\"", Roshambo.class)).isEqualTo(Roshambo.ROCK);
Expand All @@ -131,11 +132,12 @@ public void testEnumSubclass() {
}

@Test
@SuppressWarnings("GetClassOnEnum")
public void testEnumSubclassWithRegisteredTypeAdapter() {
gson = new GsonBuilder()
.registerTypeHierarchyAdapter(Roshambo.class, new MyEnumTypeAdapter())
.create();
assertThat(Roshambo.ROCK.getDeclaringClass()).isAssignableTo(Roshambo.class);
assertThat(Roshambo.ROCK.getClass()).isNotEqualTo(Roshambo.class);
assertThat(gson.toJson(Roshambo.ROCK)).isEqualTo("\"123ROCK\"");
assertThat(gson.toJson(EnumSet.allOf(Roshambo.class))).isEqualTo("[\"123ROCK\",\"123PAPER\",\"123SCISSORS\"]");
assertThat(gson.fromJson("\"123ROCK\"", Roshambo.class)).isEqualTo(Roshambo.ROCK);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@
package com.google.gson.functional;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.junit.Assert.assertThrows;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.TypeAdapter;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import java.io.IOException;
import java.util.regex.Pattern;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -46,7 +45,7 @@ public void setUp() {
@Override public void write(JsonWriter out, TestType value) {
throw new AssertionError("Expected during serialization");
}
@Override public TestType read(JsonReader in) throws IOException {
@Override public TestType read(JsonReader in) {
throw new AssertionError("Expected during deserialization");
}
}).create();
Expand All @@ -59,25 +58,17 @@ public void testVersionPattern() {
}

@Test
@SuppressWarnings("AssertionFailureIgnored")
public void testAssertionErrorInSerializationPrintsVersion() {
try {
gson.toJson(new TestType());
fail();
} catch (AssertionError expected) {
ensureAssertionErrorPrintsGsonVersion(expected);
}
AssertionError e = assertThrows(AssertionError.class, () -> gson.toJson(new TestType()));
ensureAssertionErrorPrintsGsonVersion(e);
}

@Test
@SuppressWarnings("AssertionFailureIgnored")
public void testAssertionErrorInDeserializationPrintsVersion() {
try {
gson.fromJson("{'a':'abc'}", TestType.class);
fail();
} catch (AssertionError expected) {
ensureAssertionErrorPrintsGsonVersion(expected);
}
AssertionError e = assertThrows(AssertionError.class,
() -> gson.fromJson("{'a':'abc'}", TestType.class));

ensureAssertionErrorPrintsGsonVersion(e);
}

private void ensureAssertionErrorPrintsGsonVersion(AssertionError expected) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ public Object read(JsonReader in) throws IOException {
* <p>See https://github.com/google/gson/issues/1875
*/
@Test
@SuppressWarnings("CatchFail")
public void testSerializeInternalImplementationObject() {
Gson gson = new Gson();
String json = gson.toJson(Collections.emptyList());
Expand All @@ -131,7 +130,7 @@ public void testSerializeInternalImplementationObject() {
gson.fromJson("[]", internalClass);
fail("Missing exception; test has to be run with `--illegal-access=deny`");
} catch (JsonSyntaxException e) {
fail("Unexpected exception; test has to be run with `--illegal-access=deny`");
throw new AssertionError("Unexpected exception; test has to be run with `--illegal-access=deny`", e);
} catch (JsonIOException expected) {
assertThat(expected).hasMessageThat().startsWith("Failed making constructor 'java.util.Collections$EmptyList()' accessible;"
+ " either increase its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type: ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,12 @@ public void testAssignmentCharSerialization() {
* Created in response to http://groups.google.com/group/google-gson/browse_thread/thread/2431d4a3d0d6cb23
*/
@Test
@SuppressWarnings("UnicodeEscape")
public void testAssignmentCharDeserialization() {
String json = "\"abc=\"";
String value = gson.fromJson(json, String.class);
assertThat(value).isEqualTo("abc=");

json = "'abc\u003d'";
json = "'abc\\u003d'";
value = gson.fromJson(json, String.class);
assertThat(value).isEqualTo("abc=");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
import com.google.gson.stream.JsonReader;
import java.lang.reflect.Type;
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedList;
import java.util.List;
import org.junit.Test;

Expand Down Expand Up @@ -104,16 +102,12 @@ public void testAsListOfLongsOrDoubles() {
.setObjectToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE)
.setNumberToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE)
.create();
List<Object> expected = new ArrayList<>();
expected.add(null);
expected.add(10L);
expected.add(10.0);
Type objectCollectionType = new TypeToken<Collection<Object>>() { }.getType();
Collection<Object> objects = gson.fromJson("[null,10,10.0]", objectCollectionType);
assertThat(objects).containsExactlyElementsIn(expected);
assertThat(objects).containsExactly(null, 10L, 10.0).inOrder();
Type numberCollectionType = new TypeToken<Collection<Number>>() { }.getType();
Collection<Object> numbers = gson.fromJson("[null,10,10.0]", numberCollectionType);
assertThat(numbers).containsExactlyElementsIn(expected);
assertThat(numbers).containsExactly(null, 10L, 10.0).inOrder();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;

import org.junit.Test;

Expand All @@ -41,29 +40,23 @@ public static class ConcreteClass {
* to instantiate an interface
*/
@Test
@SuppressWarnings("AssertionFailureIgnored")
public void testInterfaceInstantiation() throws Exception {
try {
UnsafeAllocator.INSTANCE.newInstance(Interface.class);
fail();
} catch (AssertionError e) {
assertThat(e).hasMessageThat().startsWith("UnsafeAllocator is used for non-instantiable type");
}
public void testInterfaceInstantiation() {
AssertionError e = assertThrows(AssertionError.class,
() -> UnsafeAllocator.INSTANCE.newInstance(Interface.class));

assertThat(e).hasMessageThat().startsWith("UnsafeAllocator is used for non-instantiable type");
}

/**
* Ensure that an {@link AssertionError} is thrown when trying
* to instantiate an abstract class
*/
@Test
@SuppressWarnings("AssertionFailureIgnored")
public void testAbstractClassInstantiation() throws Exception {
try {
UnsafeAllocator.INSTANCE.newInstance(AbstractClass.class);
fail();
} catch (AssertionError e) {
assertThat(e).hasMessageThat().startsWith("UnsafeAllocator is used for non-instantiable type");
}
public void testAbstractClassInstantiation() {
AssertionError e = assertThrows(AssertionError.class,
() -> UnsafeAllocator.INSTANCE.newInstance(AbstractClass.class));

assertThat(e).hasMessageThat().startsWith("UnsafeAllocator is used for non-instantiable type");
}

/**
Expand Down
5 changes: 2 additions & 3 deletions gson/src/test/java/com/google/gson/stream/JsonReaderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ private void assertNotANumber(String s) throws IOException {
strictReader.nextDouble();
fail("Should have failed reading " + s + " as double");
} catch (MalformedJsonException e) {
// OK: Should fail because `s` can't be read as double
assertThat(e).hasMessageThat().startsWith("Use JsonReader.setLenient(true) to accept malformed JSON");
}
}

Expand Down Expand Up @@ -786,9 +786,8 @@ public void testPeekMuchLargerThanLongMinValue() throws IOException {
}

@Test
@SuppressWarnings("UnicodeEscape")
public void testQuotedNumberWithEscape() throws IOException {
JsonReader reader = new JsonReader(reader("[\"12\u00334\"]"));
JsonReader reader = new JsonReader(reader("[\"12\\u00334\"]"));
reader.setLenient(true);
reader.beginArray();
assertThat(reader.peek()).isEqualTo(STRING);
Expand Down

0 comments on commit 13ab165

Please sign in to comment.