Skip to content

Commit

Permalink
Enable additional Error Prone checks & fix violations (#2561)
Browse files Browse the repository at this point in the history
* Enable additional Error Prone checks & fix violations

Some of them also enforce additional Google Java Format requirements which
are not handled by google-java-format, such as disallowing wildcard imports.

Not all experimental checks have been listed because some are not applicable,
such as Dependency Injection framework checks, or checks related to Guava's
immutable collections (since Gson's main code does not have a dependency on
Guava).

Other checks have been omitted because they are probably not relevant
(this was a subjective choice), or would require larger refactoring or
would flag issues with the public API, which cannot be changed easily.

* Address review feedback

---------

Co-authored-by: Éamonn McManus <[email protected]>
  • Loading branch information
Marcono1234 and eamonnmcmanus authored Jan 9, 2024
1 parent 2bcaaed commit 5187808
Show file tree
Hide file tree
Showing 66 changed files with 580 additions and 433 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Collection;

@SuppressWarnings({"PrivateConstructorForUtilityClass", "SystemOut"})
public class RawCollectionsExample {
static class Event {
private String name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ public T read(JsonReader in) throws IOException {
try {
method.invoke(result);
} catch (IllegalAccessException e) {
throw new AssertionError();
throw new AssertionError(e);
} catch (InvocationTargetException e) {
if (e.getCause() instanceof RuntimeException) throw (RuntimeException) e.getCause();
if (e.getCause() instanceof RuntimeException) {
throw (RuntimeException) e.getCause();
}
throw new RuntimeException(e.getCause());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import java.util.TimeZone;

public final class UtcDateTypeAdapter extends TypeAdapter<Date> {
private final TimeZone UTC_TIME_ZONE = TimeZone.getTimeZone("UTC");
private static final TimeZone UTC_TIME_ZONE = TimeZone.getTimeZone("UTC");

@Override
public void write(JsonWriter out, Date date) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ public void postDeserialize(User user) {
if (user.name == null || user.password == null) {
throw new JsonSyntaxException("name and password are required fields.");
}
if (user.email == null) user.email = User.DEFAULT_EMAIL;
if (user.email == null) {
user.email = User.DEFAULT_EMAIL;
}
}
}

Expand All @@ -192,7 +194,9 @@ public void postDeserialize(Address address) {
if (address.city == null || address.state == null || address.zip == null) {
throw new JsonSyntaxException("Address city, state and zip are required fields.");
}
if (address.firstLine == null) address.firstLine = Address.DEFAULT_FIRST_LINE;
if (address.firstLine == null) {
address.firstLine = Address.DEFAULT_FIRST_LINE;
}
}
}
}
138 changes: 69 additions & 69 deletions gson/src/main/java/com/google/gson/Gson.java
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,16 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
return candidate;
}

/**
* Returns the type adapter for {@code type}.
*
* @throws IllegalArgumentException if this Gson instance cannot serialize and deserialize {@code
* type}.
*/
public <T> TypeAdapter<T> getAdapter(Class<T> type) {
return getAdapter(TypeToken.get(type));
}

/**
* This method is used to get an alternate type adapter for the specified type. This is used to
* access a type adapter that is overridden by a {@link TypeAdapterFactory} that you may have
Expand Down Expand Up @@ -748,16 +758,6 @@ public <T> TypeAdapter<T> getDelegateAdapter(TypeAdapterFactory skipPast, TypeTo
}
}

/**
* Returns the type adapter for {@code type}.
*
* @throws IllegalArgumentException if this Gson instance cannot serialize and deserialize {@code
* type}.
*/
public <T> TypeAdapter<T> getAdapter(Class<T> type) {
return getAdapter(TypeToken.get(type));
}

/**
* This method serializes the specified object into its equivalent representation as a tree of
* {@link JsonElement}s. This method should be used when the specified object is not a generic
Expand Down Expand Up @@ -983,53 +983,6 @@ public void toJson(JsonElement jsonElement, Appendable writer) throws JsonIOExce
}
}

/**
* Returns a new JSON writer configured for the settings on this Gson instance.
*
* <p>The following settings are considered:
*
* <ul>
* <li>{@link GsonBuilder#disableHtmlEscaping()}
* <li>{@link GsonBuilder#generateNonExecutableJson()}
* <li>{@link GsonBuilder#serializeNulls()}
* <li>{@link GsonBuilder#setStrictness(Strictness)}. If no {@linkplain
* GsonBuilder#setStrictness(Strictness) explicit strictness has been set} the created
* writer will have a strictness of {@link Strictness#LEGACY_STRICT}. Otherwise, the
* strictness of the {@code Gson} instance will be used for the created writer.
* <li>{@link GsonBuilder#setPrettyPrinting()}
* <li>{@link GsonBuilder#setFormattingStyle(FormattingStyle)}
* </ul>
*/
public JsonWriter newJsonWriter(Writer writer) throws IOException {
if (generateNonExecutableJson) {
writer.write(JSON_NON_EXECUTABLE_PREFIX);
}
JsonWriter jsonWriter = new JsonWriter(writer);
jsonWriter.setFormattingStyle(formattingStyle);
jsonWriter.setHtmlSafe(htmlSafe);
jsonWriter.setStrictness(strictness == null ? Strictness.LEGACY_STRICT : strictness);
jsonWriter.setSerializeNulls(serializeNulls);
return jsonWriter;
}

/**
* Returns a new JSON reader configured for the settings on this Gson instance.
*
* <p>The following settings are considered:
*
* <ul>
* <li>{@link GsonBuilder#setStrictness(Strictness)}. If no {@linkplain
* GsonBuilder#setStrictness(Strictness) explicit strictness has been set} the created
* reader will have a strictness of {@link Strictness#LEGACY_STRICT}. Otherwise, the
* strictness of the {@code Gson} instance will be used for the created reader.
* </ul>
*/
public JsonReader newJsonReader(Reader reader) {
JsonReader jsonReader = new JsonReader(reader);
jsonReader.setStrictness(strictness == null ? Strictness.LEGACY_STRICT : strictness);
return jsonReader;
}

/**
* Writes the JSON for {@code jsonElement} to {@code writer}.
*
Expand Down Expand Up @@ -1078,6 +1031,53 @@ public void toJson(JsonElement jsonElement, JsonWriter writer) throws JsonIOExce
}
}

/**
* Returns a new JSON writer configured for the settings on this Gson instance.
*
* <p>The following settings are considered:
*
* <ul>
* <li>{@link GsonBuilder#disableHtmlEscaping()}
* <li>{@link GsonBuilder#generateNonExecutableJson()}
* <li>{@link GsonBuilder#serializeNulls()}
* <li>{@link GsonBuilder#setStrictness(Strictness)}. If no {@linkplain
* GsonBuilder#setStrictness(Strictness) explicit strictness has been set} the created
* writer will have a strictness of {@link Strictness#LEGACY_STRICT}. Otherwise, the
* strictness of the {@code Gson} instance will be used for the created writer.
* <li>{@link GsonBuilder#setPrettyPrinting()}
* <li>{@link GsonBuilder#setFormattingStyle(FormattingStyle)}
* </ul>
*/
public JsonWriter newJsonWriter(Writer writer) throws IOException {
if (generateNonExecutableJson) {
writer.write(JSON_NON_EXECUTABLE_PREFIX);
}
JsonWriter jsonWriter = new JsonWriter(writer);
jsonWriter.setFormattingStyle(formattingStyle);
jsonWriter.setHtmlSafe(htmlSafe);
jsonWriter.setStrictness(strictness == null ? Strictness.LEGACY_STRICT : strictness);
jsonWriter.setSerializeNulls(serializeNulls);
return jsonWriter;
}

/**
* Returns a new JSON reader configured for the settings on this Gson instance.
*
* <p>The following settings are considered:
*
* <ul>
* <li>{@link GsonBuilder#setStrictness(Strictness)}. If no {@linkplain
* GsonBuilder#setStrictness(Strictness) explicit strictness has been set} the created
* reader will have a strictness of {@link Strictness#LEGACY_STRICT}. Otherwise, the
* strictness of the {@code Gson} instance will be used for the created reader.
* </ul>
*/
public JsonReader newJsonReader(Reader reader) {
JsonReader jsonReader = new JsonReader(reader);
jsonReader.setStrictness(strictness == null ? Strictness.LEGACY_STRICT : strictness);
return jsonReader;
}

/**
* This method deserializes the specified JSON into an object of the specified class. It is not
* suitable to use if the specified class is a generic type since it will not have the generic
Expand Down Expand Up @@ -1262,18 +1262,6 @@ public <T> T fromJson(Reader json, TypeToken<T> typeOfT)
return object;
}

private static void assertFullConsumption(Object obj, JsonReader reader) {
try {
if (obj != null && reader.peek() != JsonToken.END_DOCUMENT) {
throw new JsonSyntaxException("JSON document was not fully consumed.");
}
} catch (MalformedJsonException e) {
throw new JsonSyntaxException(e);
} catch (IOException e) {
throw new JsonIOException(e);
}
}

// fromJson(JsonReader, Class) is unfortunately missing and cannot be added now without breaking
// source compatibility in certain cases, see
// https://github.com/google/gson/pull/1700#discussion_r973764414
Expand Down Expand Up @@ -1472,6 +1460,18 @@ public <T> T fromJson(JsonElement json, TypeToken<T> typeOfT) throws JsonSyntaxE
return fromJson(new JsonTreeReader(json), typeOfT);
}

private static void assertFullConsumption(Object obj, JsonReader reader) {
try {
if (obj != null && reader.peek() != JsonToken.END_DOCUMENT) {
throw new JsonSyntaxException("JSON document was not fully consumed.");
}
} catch (MalformedJsonException e) {
throw new JsonSyntaxException(e);
} catch (IOException e) {
throw new JsonIOException(e);
}
}

/**
* Proxy type adapter for cyclic type graphs.
*
Expand Down
20 changes: 10 additions & 10 deletions gson/src/main/java/com/google/gson/GsonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -614,14 +614,6 @@ public GsonBuilder setDateFormat(String pattern) {
return this;
}

private static int checkDateFormatStyle(int style) {
// Valid DateFormat styles are: 0, 1, 2, 3 (FULL, LONG, MEDIUM, SHORT)
if (style < 0 || style > 3) {
throw new IllegalArgumentException("Invalid style: " + style);
}
return style;
}

/**
* Configures Gson to serialize {@code Date} objects according to the date style value provided.
* You can call this method or {@link #setDateFormat(String)} multiple times, but only the last
Expand Down Expand Up @@ -669,6 +661,14 @@ public GsonBuilder setDateFormat(int dateStyle, int timeStyle) {
return this;
}

private static int checkDateFormatStyle(int style) {
// Valid DateFormat styles are: 0, 1, 2, 3 (FULL, LONG, MEDIUM, SHORT)
if (style < 0 || style > 3) {
throw new IllegalArgumentException("Invalid style: " + style);
}
return style;
}

/**
* Configures Gson for custom serialization or deserialization. This method combines the
* registration of an {@link TypeAdapter}, {@link InstanceCreator}, {@link JsonSerializer}, and a
Expand Down Expand Up @@ -722,7 +722,7 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
return this;
}

private boolean isTypeObjectOrJsonElement(Type type) {
private static boolean isTypeObjectOrJsonElement(Type type) {
return type instanceof Class
&& (type == Object.class || JsonElement.class.isAssignableFrom((Class<?>) type));
}
Expand Down Expand Up @@ -901,7 +901,7 @@ public Gson create() {
new ArrayList<>(reflectionFilters));
}

private void addTypeAdaptersForDate(
private static void addTypeAdaptersForDate(
String datePattern, int dateStyle, int timeStyle, List<TypeAdapterFactory> factories) {
TypeAdapterFactory dateAdapterFactory;
boolean sqlTypesSupported = SqlTypesSupport.SUPPORTS_SQL_TYPES;
Expand Down
10 changes: 8 additions & 2 deletions gson/src/main/java/com/google/gson/JsonPrimitive.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ public final class JsonPrimitive extends JsonElement {
*
* @param bool the value to create the primitive with.
*/
@SuppressWarnings("deprecation") // superclass constructor
// "deprecation" suppression for superclass constructor
// "UnnecessaryBoxedVariable" Error Prone warning is correct since method does not accept
// null, but cannot be changed anymore since this is public API
@SuppressWarnings({"deprecation", "UnnecessaryBoxedVariable"})
public JsonPrimitive(Boolean bool) {
value = Objects.requireNonNull(bool);
}
Expand Down Expand Up @@ -69,7 +72,10 @@ public JsonPrimitive(String string) {
*
* @param c the value to create the primitive with.
*/
@SuppressWarnings("deprecation") // superclass constructor
// "deprecation" suppression for superclass constructor
// "UnnecessaryBoxedVariable" Error Prone warning is correct since method does not accept
// null, but cannot be changed anymore since this is public API
@SuppressWarnings({"deprecation", "UnnecessaryBoxedVariable"})
public JsonPrimitive(Character c) {
// convert characters to strings since in JSON, characters are represented as a single
// character string
Expand Down
Loading

0 comments on commit 5187808

Please sign in to comment.