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

Fix error prone warnings #2316

Merged
merged 26 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b10218e
Fix `OperatorPrecedence` warn in `JsonWriter#close`
MaicolAntali Feb 14, 2023
02c27fb
Fix `ReferenceEquality` warn in `LinkedTreeMap#replaceInParent`
MaicolAntali Feb 14, 2023
e4c468a
Fix `UnnecessaryParentheses` warn in `LinkedTreeMap#replaceInParent`
MaicolAntali Feb 14, 2023
476eb84
Fix `ReferenceEquality` warn in `LinkedTreeMap#hasNext`
MaicolAntali Feb 14, 2023
387746c
Fix `ReferenceEquality` warn in `LinkedTreeMap#nextNode`
MaicolAntali Feb 14, 2023
14af14d
Adds `error_prone_annotations` to the `pom.xml` of `gson`
MaicolAntali Feb 14, 2023
095bfd5
Fix `InlineMeSuggester` warns in `JsonParser`
MaicolAntali Feb 14, 2023
f17f676
Fix `UnnecessaryParentheses` warns in `ConstructorConstructor#newDefa…
MaicolAntali Feb 14, 2023
8552f56
Fix `ThreadLocalUsage` warn in `Gson`
MaicolAntali Feb 14, 2023
9f99b27
Fix `JdkObsolete` warn in `GsonBuilder`
MaicolAntali Feb 14, 2023
2f426a0
Fix `ReferenceEquality` warn in `LazilyParsedNumber#equals`
MaicolAntali Feb 14, 2023
b55570e
Fix `OperatorPrecedence` warn in `TreeTypeAdapter#create`
MaicolAntali Feb 14, 2023
2061287
Fix `OperatorPrecedence` warn in `ArrayTypeAdapter`
MaicolAntali Feb 14, 2023
44315b3
Fix `UnnecessaryParentheses` warn in `TypeAdapters`
MaicolAntali Feb 14, 2023
14876ca
Adds `-XepExcludedPaths` flag to ErrorProne plugin to exclude tests a…
MaicolAntali Feb 14, 2023
14e1ab4
Fix `ClassNewInstance` warn in `InterceptorAdapter`
MaicolAntali Feb 14, 2023
0c9cac4
Fix `ThreadLocalUsage` warn in `GraphAdapterBuilder`
MaicolAntali Feb 14, 2023
ae095b1
Fix `JdkObsolete` warn in `GraphAdapterBuilder`
MaicolAntali Feb 14, 2023
5c84b63
Revert "Adds `error_prone_annotations` to the `pom.xml` of `gson`"
MaicolAntali Feb 14, 2023
8b87c36
Revert "Fix `InlineMeSuggester` warns in `JsonParser`"
MaicolAntali Feb 14, 2023
e0f38d3
Adds `@SuppressWarnings("ThreadLocalUsage")`
MaicolAntali Feb 15, 2023
4823160
Fix `OperatorPrecedence` in `JsonWriter`
MaicolAntali Feb 15, 2023
da33810
Revert "Fix `ReferenceEquality` warn in `LinkedTreeMap#nextNode`"
MaicolAntali Feb 15, 2023
ada8b0e
Adds `@SuppressWarnings("ReferenceEquality")`
MaicolAntali Feb 15, 2023
fc6183d
Adds `guava-testlib` to the gson `pom.xml`
MaicolAntali Feb 15, 2023
70a834e
`@SuppressWarnings("TruthSelfEquals")` removed to use `EqualsTester()`
MaicolAntali Feb 15, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
import com.google.gson.stream.JsonWriter;
import java.io.IOException;
import java.lang.reflect.Type;
import java.util.ArrayDeque;
eamonnmcmanus marked this conversation as resolved.
Show resolved Hide resolved
import java.util.Collections;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.LinkedList;
import java.util.Map;
import java.util.Queue;

Expand Down Expand Up @@ -79,6 +79,7 @@ public void registerOn(GsonBuilder gsonBuilder) {

static class Factory implements TypeAdapterFactory, InstanceCreator<Object> {
private final Map<Type, InstanceCreator<?>> instanceCreators;
@SuppressWarnings("ThreadLocalUsage")
private final ThreadLocal<Graph> graphThreadLocal = new ThreadLocal<>();

Factory(Map<Type, InstanceCreator<?>> instanceCreators) {
Expand Down Expand Up @@ -240,7 +241,7 @@ static class Graph {
* The queue of elements to write during serialization. Unused during
* deserialization.
*/
private final Queue<Element<?>> queue = new LinkedList<>();
private final Queue<Element<?>> queue = new ArrayDeque<>();

/**
* The instance currently being deserialized. Used as a backdoor between
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static class InterceptorAdapter<T> extends TypeAdapter<T> {
public InterceptorAdapter(TypeAdapter<T> delegate, Intercept intercept) {
try {
this.delegate = delegate;
this.postDeserializer = intercept.postDeserialize().newInstance();
this.postDeserializer = intercept.postDeserialize().getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand Down
6 changes: 6 additions & 0 deletions gson/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@
<version>1.1.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava-testlib</artifactId>
<version>31.1-jre</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
1 change: 1 addition & 0 deletions gson/src/main/java/com/google/gson/Gson.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public final class Gson {
* with the type token provided to {@code getAdapter} as key and either
* {@code FutureTypeAdapter} or a regular {@code TypeAdapter} as value.
*/
@SuppressWarnings("ThreadLocalUsage")
private final ThreadLocal<Map<TypeToken<?>, TypeAdapter<?>>> threadLocalAdapterResults = new ThreadLocal<>();

private final ConcurrentMap<TypeToken<?>, TypeAdapter<?>> typeTokenCache = new ConcurrentHashMap<>();
Expand Down
6 changes: 3 additions & 3 deletions gson/src/main/java/com/google/gson/GsonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
import static com.google.gson.Gson.DEFAULT_COMPLEX_MAP_KEYS;
import static com.google.gson.Gson.DEFAULT_DATE_PATTERN;
import static com.google.gson.Gson.DEFAULT_ESCAPE_HTML;
import static com.google.gson.Gson.DEFAULT_FORMATTING_STYLE;
import static com.google.gson.Gson.DEFAULT_JSON_NON_EXECUTABLE;
import static com.google.gson.Gson.DEFAULT_LENIENT;
import static com.google.gson.Gson.DEFAULT_NUMBER_TO_NUMBER_STRATEGY;
import static com.google.gson.Gson.DEFAULT_OBJECT_TO_NUMBER_STRATEGY;
import static com.google.gson.Gson.DEFAULT_FORMATTING_STYLE;
import static com.google.gson.Gson.DEFAULT_SERIALIZE_NULLS;
import static com.google.gson.Gson.DEFAULT_SPECIALIZE_FLOAT_VALUES;
import static com.google.gson.Gson.DEFAULT_USE_JDK_UNSAFE;
Expand All @@ -41,11 +41,11 @@
import com.google.gson.stream.JsonWriter;
import java.lang.reflect.Type;
import java.text.DateFormat;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -104,7 +104,7 @@ public final class GsonBuilder {
private boolean useJdkUnsafe = DEFAULT_USE_JDK_UNSAFE;
private ToNumberStrategy objectToNumberStrategy = DEFAULT_OBJECT_TO_NUMBER_STRATEGY;
private ToNumberStrategy numberToNumberStrategy = DEFAULT_NUMBER_TO_NUMBER_STRATEGY;
private final LinkedList<ReflectionAccessFilter> reflectionFilters = new LinkedList<>();
private final ArrayDeque<ReflectionAccessFilter> reflectionFilters = new ArrayDeque<>();

/**
* Creates a GsonBuilder instance that can be used to build Gson with various configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ private static <T> ObjectConstructor<T> newDefaultImplementationConstructor(
return (T) new TreeMap<>();
}
};
} else if (type instanceof ParameterizedType && !(String.class.isAssignableFrom(
TypeToken.get(((ParameterizedType) type).getActualTypeArguments()[0]).getRawType()))) {
} else if (type instanceof ParameterizedType && !String.class.isAssignableFrom(
TypeToken.get(((ParameterizedType) type).getActualTypeArguments()[0]).getRawType())) {
return new ObjectConstructor<T>() {
@Override public T construct() {
return (T) new LinkedHashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public boolean equals(Object obj) {
}
if (obj instanceof LazilyParsedNumber) {
LazilyParsedNumber other = (LazilyParsedNumber) obj;
return value == other.value || value.equals(other.value);
return value.equals(other.value);
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ Node<K, V> removeInternalByKey(Object key) {
return node;
}

@SuppressWarnings("ReferenceEquality")
private void replaceInParent(Node<K, V> node, Node<K, V> replacement) {
Node<K, V> parent = node.parent;
node.parent = null;
Expand All @@ -315,7 +316,7 @@ private void replaceInParent(Node<K, V> node, Node<K, V> replacement) {
if (parent.left == node) {
parent.left = replacement;
} else {
assert (parent.right == node);
assert parent.right == node;
parent.right = replacement;
}
} else {
Expand Down Expand Up @@ -559,10 +560,13 @@ private abstract class LinkedTreeMapIterator<T> implements Iterator<T> {
LinkedTreeMapIterator() {
}

@Override public final boolean hasNext() {
@Override
@SuppressWarnings("ReferenceEquality")
public final boolean hasNext() {
return next != header;
}

@SuppressWarnings("ReferenceEquality")
final Node<K, V> nextNode() {
Node<K, V> e = next;
if (e == header) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public final class ArrayTypeAdapter<E> extends TypeAdapter<Object> {
public static final TypeAdapterFactory FACTORY = new TypeAdapterFactory() {
@Override public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
Type type = typeToken.getType();
if (!(type instanceof GenericArrayType || type instanceof Class && ((Class<?>) type).isArray())) {
if (!(type instanceof GenericArrayType || (type instanceof Class && ((Class<?>) type).isArray()))) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private static final class SingleTypeFactory implements TypeAdapterFactory {
@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
boolean matches = exactType != null
? exactType.equals(type) || matchRawType && exactType.getType() == type.getRawType()
? exactType.equals(type) || (matchRawType && exactType.getType() == type.getRawType())
: hierarchyType.isAssignableFrom(type.getRawType());
return matches
? new TreeTypeAdapter<>((JsonSerializer<T>) serializer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public Class read(JsonReader in) throws IOException {
@Override public void write(JsonWriter out, BitSet src) throws IOException {
out.beginArray();
for (int i = 0, length = src.length(); i < length; i++) {
int value = (src.get(i)) ? 1 : 0;
int value = src.get(i) ? 1 : 0;
out.value(value);
}
out.endArray();
Expand Down Expand Up @@ -883,7 +883,7 @@ public EnumTypeAdapter(final Class<T> classOfT) {
});
for (Field constantField : constantFields) {
@SuppressWarnings("unchecked")
T constant = (T)(constantField.get(null));
T constant = (T) constantField.get(null);
String name = constant.name();
String toStringVal = constant.toString();

Expand Down
2 changes: 1 addition & 1 deletion gson/src/main/java/com/google/gson/stream/JsonWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ public JsonWriter value(Number value) throws IOException {
out.close();

int size = stackSize;
if (size > 1 || size == 1 && stack[size - 1] != NONEMPTY_DOCUMENT) {
if (size > 1 || (size == 1 && stack[size - 1] != NONEMPTY_DOCUMENT)) {
throw new IOException("Incomplete document");
}
stackSize = 0;
Expand Down
4 changes: 2 additions & 2 deletions gson/src/test/java/com/google/gson/JsonArrayTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.fail;

import com.google.common.testing.EqualsTester;
import com.google.gson.common.MoreAsserts;
import java.math.BigInteger;
import org.junit.Test;
Expand All @@ -35,12 +36,11 @@ public void testEqualsOnEmptyArray() {
}

@Test
@SuppressWarnings("TruthSelfEquals")
public void testEqualsNonEmptyArray() {
JsonArray a = new JsonArray();
JsonArray b = new JsonArray();

assertThat(a).isEqualTo(a);
new EqualsTester().addEqualityGroup(a).testEquals();

a.add(new JsonObject());
assertThat(a.equals(b)).isFalse();
Expand Down
4 changes: 2 additions & 2 deletions gson/src/test/java/com/google/gson/JsonObjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;

import com.google.common.testing.EqualsTester;
import com.google.gson.common.MoreAsserts;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayDeque;
Expand Down Expand Up @@ -163,12 +164,11 @@ public void testEqualsOnEmptyObject() {
}

@Test
@SuppressWarnings("TruthSelfEquals")
public void testEqualsNonEmptyObject() {
JsonObject a = new JsonObject();
JsonObject b = new JsonObject();

assertThat(a).isEqualTo(a);
new EqualsTester().addEqualityGroup(a).testEquals();

a.add("foo", new JsonObject());
assertThat(a.equals(b)).isFalse();
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
<compilerArgs>
<!-- Args related to Error Prone, see: https://errorprone.info/docs/installation#maven -->
<arg>-XDcompilePolicy=simple</arg>
<arg>-Xplugin:ErrorProne</arg>
<arg>-Xplugin:ErrorProne -XepExcludedPaths:.*gson/src/test.*|.*extras/src/test.*|.*proto.*</arg>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we might want to have the tests pass Error Prone too, but I agree that is a much lower priority.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended that you disable warnings for the complete proto module? Maybe it would be better to only disable warnings for the generated source code, as suggested in #2308 (comment).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to go ahead and merge this anyway, but we could indeed consider refining the pattern here.

<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
Expand Down