Skip to content

Commit

Permalink
Fix error prone warns (#2320)
Browse files Browse the repository at this point in the history
* Adds `@SuppressWarnings("NarrowingCompoundAssignment")`

* Adds `@SuppressWarnings("TypeParameterUnusedInFormals")`

* Adds `@SuppressWarnings("JavaUtilDate")`

* Adds a limit to `String.split()`

* Add `error_prone_annotations` to the `pom.xml`

* Adds `@InlineMe(...)` to deprecated methods

* Adds `@SuppressWarnings("ImmutableEnumChecker")`

* Adds `@SuppressWarnings("ModifiedButNotUsed")`

* Adds `@SuppressWarnings("MixedMutabilityReturnType")`

* Removes an unused import

* Adds `requires` to `module-info.java`

* Adds ErrorProne `link` into `pom.xml`

* Remove unused imports

Removed from:
- ParseBenchmark

* Adds `@SuppressWarnings("EqualsGetClass")`

* Excludes from `proto` just the generated code.

Replaces `.*proto.*` with `.*/generated-test-sources/protobuf/.*` in such way will be excluded just the generated code and not the whole `proto` directory

* Removes an unused variable

Removes the `descriptor` variable because is unused.

* Fixes the `BadImport` warn into `ProtosWithAnnotationsTest`

* Fixes the `BadImport` warns into `ProtosWithAnnotationsTest`

* Enables ErrorProne in `gson/src/test.*`

Removes the `gson/src/test.*` path from the `-XepExcludedPaths` parameter of the ErrorProne plugin

* Fixes `UnusedVariable` warns

This commit fix all `UnusedVariable` warns given by ErrorProne in the `gson/src/test.*` path.

Some field is annotated with `@Keep` that means is used by reflection.
In some other case the record is annotated with `@SuppressWarnings("unused")`

* Fixes `JavaUtilDate` warns

This commit fix all `JavaUtilDate` warns given by ErrorProne in the `gson/src/test.*` path.

Classes/Methods are annotated with `@SuppressWarnings("JavaUtilDate")` because it's not possible use differente Date API.

* Fixes `EqualsGetClass` warns

This commit fix all `EqualsGetClass` warns given by ErrorProne in the `gson/src/test.*` path.

I have rewrite the `equals()` methods to use `instanceof`

* Replaces pattern matching for instanceof with casts

* Fixes `JdkObsolete` warns

This commit fix all `JdkObsolete` warns given by ErrorProne in the `gson/src/test.*` path.

In some cases I have replaced the obsolete JDK classes with the newest alternatives.  In other cases, I have added the `@SuppressWarnings("JdkObsolete")`

* Fixes `ClassCanBeStatic` warns

This commit fix all `ClassCanBeStatic` warns given by ErrorProne in the `gson/src/test.*` path.

I have added the `static` keyword, or I have added `@SuppressWarnings("ClassCanBeStatic")`

* Fixes `UndefinedEquals` warns

This commit fix all `UndefinedEquals` warns given by ErrorProne in the `gson/src/test.*` path.

I have added `@SuppressWarnings("UndefinedEquals")` or fixed the asserts

Note: In this commit I have also renamed a test from `testRawCollectionDeserializationNotAlllowed` to `testRawCollectionDeserializationNotAllowed`

* Fixes `GetClassOnEnum` warns

This commit fix all `GetClassOnEnum` warns given by ErrorProne in the `gson/src/test.*` path.

I have replaced the `.getClass()` with `.getDeclaringClass()`

* Fixes `ImmutableEnumChecker` warns

This commit fix all `ImmutableEnumChecker` warns given by ErrorProne in the `gson/src/test.*` path.

I have added the `final` keyword, or I have added the `@SuppressWarnings("ImmutableEnumChecker")` annotation

* Fixes `StaticAssignmentOfThrowable` warns

This commit fix all `StaticAssignmentOfThrowable` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `AssertionFailureIgnored` warns

This commit fix all `AssertionFailureIgnored` warns given by ErrorProne in the `gson/src/test.*` path.

I have added the `@SuppressWarnings("AssertionFailureIgnored")` annotation

* Fixes `ModifiedButNotUsed` warns

This commit fix all `ModifiedButNotUsed` warns given by ErrorProne in the `gson/src/test.*` path.

I have added the `@SuppressWarnings("ModifiedButNotUsed")` annotation

* Fixes `MissingSummary` warns

This commit fix all `MissingSummary` warns given by ErrorProne in the `gson/src/test.*` path.

I have remove the Javadoc `@author`

* Fixes `FloatingPointLiteralPrecision` warns

This commit fix all `FloatingPointLiteralPrecision` warns given by ErrorProne in the `gson/src/test.*` path.

I have added the `@SuppressWarnings("FloatingPointLiteralPrecision")` annotation

* Fixes `StringSplitter` warns

This commit fix all `StringSplitter` warns given by ErrorProne in the `gson/src/test.*` path.

I have replaced the `String.split(...)` with `Splitter`

* Fixes `EmptyCatch` warns

This commit fix all `EmptyCatch` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `UnicodeEscape` warns

This commit fix all `UnicodeEscape` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `EmptyBlockTag` warns

This commit fix all `EmptyBlockTag` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `LongFloatConversion` warns

This commit fix all `LongFloatConversion` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `LongDoubleConversion` warns

This commit fix all `LongDoubleConversion` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `TruthAssertExpected` warns

This commit fix all `TruthAssertExpected` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `UnusedMethod` warns

This commit fix all `UnusedMethod` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `UnusedTypeParameter` warns

This commit fix all `UnusedTypeParameter` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `CatchFail` warns

This commit fix all `CatchFail` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `MathAbsoluteNegative` warns

This commit fix all `MathAbsoluteNegative` warns given by ErrorProne in the `gson/src/test.*` path.

* Fixes `LoopOverCharArray` warns

This commit fix all `LoopOverCharArray` warns given by ErrorProne in the `gson/src/test.*` path.

`toCharArray` allocates a new array, using `charAt` is more efficient

* Fixes `HidingField` warns

This commit fix all `HidingField` warns given by ErrorProne in the `gson/src/test.*` path.

* Implements code review feedback

* Implements code review feedback

This commit implements some other code review feedback

* Enable ErrorProne in `extra`

Thi commit removes the `.*extras/src/test.*` path from the `-XepExcludedPaths` parameter of ErrorProne.

* Fix the `JavaUtilDate` warns

This commit fix all `JavaUtilDate` warns given by ErrorProne in the `extras/src/test.*` path.

* Implements code review feedback

* Removes redundant new-line

* Implements code review feedback

* Adds `JDK11` to run test with `--release 11`

* Revert "Adds `JDK11` to run test with `--release 11`"

This reverts commit a7cca38.
  • Loading branch information
MaicolAntali committed Mar 1, 2023
1 parent bfbbd0d commit dc20b75
Show file tree
Hide file tree
Showing 60 changed files with 225 additions and 193 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.TimeZone;
import org.junit.Test;

@SuppressWarnings("JavaUtilDate")
public final class UtcDateTypeAdapterTest {
private final Gson gson = new GsonBuilder()
.registerTypeAdapter(Date.class, new UtcDateTypeAdapter())
Expand Down
6 changes: 6 additions & 0 deletions gson/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@
<version>31.1-jre</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.errorprone</groupId>
<artifactId>error_prone_annotations</artifactId>
<version>2.18.0</version>
<optional>true</optional>
</dependency>
</dependencies>

<build>
Expand Down
8 changes: 4 additions & 4 deletions gson/src/main/java/com/google/gson/Gson.java
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ public <T> T fromJson(String json, Class<T> classOfT) throws JsonSyntaxException
* @see #fromJson(String, Class)
* @see #fromJson(String, TypeToken)
*/
@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
public <T> T fromJson(String json, Type typeOfT) throws JsonSyntaxException {
return (T) fromJson(json, TypeToken.get(typeOfT));
}
Expand Down Expand Up @@ -1104,7 +1104,7 @@ public <T> T fromJson(Reader json, Class<T> classOfT) throws JsonSyntaxException
* @see #fromJson(Reader, Class)
* @see #fromJson(Reader, TypeToken)
*/
@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
public <T> T fromJson(Reader json, Type typeOfT) throws JsonIOException, JsonSyntaxException {
return (T) fromJson(json, TypeToken.get(typeOfT));
}
Expand Down Expand Up @@ -1183,7 +1183,7 @@ private static void assertFullConsumption(Object obj, JsonReader reader) {
* @see #fromJson(Reader, Type)
* @see #fromJson(JsonReader, TypeToken)
*/
@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
public <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, JsonSyntaxException {
return (T) fromJson(reader, TypeToken.get(typeOfT));
}
Expand Down Expand Up @@ -1297,7 +1297,7 @@ public <T> T fromJson(JsonElement json, Class<T> classOfT) throws JsonSyntaxExce
* @see #fromJson(JsonElement, Class)
* @see #fromJson(JsonElement, TypeToken)
*/
@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
public <T> T fromJson(JsonElement json, Type typeOfT) throws JsonSyntaxException {
return (T) fromJson(json, TypeToken.get(typeOfT));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ public interface JsonDeserializationContext {
* @return An object of type typeOfT.
* @throws JsonParseException if the parse tree does not contain expected data.
*/
@SuppressWarnings("TypeParameterUnusedInFormals")
public <T> T deserialize(JsonElement json, Type typeOfT) throws JsonParseException;
}
4 changes: 4 additions & 0 deletions gson/src/main/java/com/google/gson/JsonParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.google.gson;

import com.google.errorprone.annotations.InlineMe;
import com.google.gson.internal.Streams;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
Expand Down Expand Up @@ -111,18 +112,21 @@ public static JsonElement parseReader(JsonReader reader)

/** @deprecated Use {@link JsonParser#parseString} */
@Deprecated
@InlineMe(replacement = "JsonParser.parseString(json)", imports = "com.google.gson.JsonParser")
public JsonElement parse(String json) throws JsonSyntaxException {
return parseString(json);
}

/** @deprecated Use {@link JsonParser#parseReader(Reader)} */
@Deprecated
@InlineMe(replacement = "JsonParser.parseReader(json)", imports = "com.google.gson.JsonParser")
public JsonElement parse(Reader json) throws JsonIOException, JsonSyntaxException {
return parseReader(json);
}

/** @deprecated Use {@link JsonParser#parseReader(JsonReader)} */
@Deprecated
@InlineMe(replacement = "JsonParser.parseReader(json)", imports = "com.google.gson.JsonParser")
public JsonElement parse(JsonReader json) throws JsonIOException, JsonSyntaxException {
return parseReader(json);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static int getMajorJavaVersion(String javaVersion) {
// Parses both legacy 1.8 style and newer 9.0.4 style
private static int parseDotted(String javaVersion) {
try {
String[] parts = javaVersion.split("[._]");
String[] parts = javaVersion.split("[._]", 3);
int firstVer = Integer.parseInt(parts[0]);
if (firstVer == 1 && parts.length > 1) {
return Integer.parseInt(parts[1]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ private boolean includeField(Field f, boolean serialize) {
}

/** first element holds the default name */
@SuppressWarnings("MixedMutabilityReturnType")
private List<String> getFieldNames(Field f) {
SerializedName annotation = f.getAnnotation(SerializedName.class);
if (annotation == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,9 @@ private final class GsonContextImpl implements JsonSerializationContext, JsonDes
@Override public JsonElement serialize(Object src, Type typeOfSrc) {
return gson.toJsonTree(src, typeOfSrc);
}
@SuppressWarnings("unchecked")
@Override public <R> R deserialize(JsonElement json, Type typeOfT) throws JsonParseException {
@Override
@SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
public <R> R deserialize(JsonElement json, Type typeOfT) throws JsonParseException {
return gson.fromJson(json, typeOfT);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
* this class state. DateFormat isn't thread safe either, so this class has
* to synchronize its read and write methods.
*/
@SuppressWarnings("JavaUtilDate")
final class SqlDateTypeAdapter extends TypeAdapter<java.sql.Date> {
static final TypeAdapterFactory FACTORY = new TypeAdapterFactory() {
@SuppressWarnings("unchecked") // we use a runtime check to make sure the 'T's equal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
* this class state. DateFormat isn't thread safe either, so this class has
* to synchronize its read and write methods.
*/
@SuppressWarnings("JavaUtilDate")
final class SqlTimeTypeAdapter extends TypeAdapter<Time> {
static final TypeAdapterFactory FACTORY = new TypeAdapterFactory() {
@SuppressWarnings("unchecked") // we use a runtime check to make sure the 'T's equal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.sql.Timestamp;
import java.util.Date;

@SuppressWarnings("JavaUtilDate")
class SqlTimestampTypeAdapter extends TypeAdapter<Timestamp> {
static final TypeAdapterFactory FACTORY = new TypeAdapterFactory() {
@SuppressWarnings("unchecked") // we use a runtime check to make sure the 'T's equal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* it is {@code false} all other constants will be {@code null} and
* there will be no support for {@code java.sql} types.
*/
@SuppressWarnings("JavaUtilDate")
public final class SqlTypesSupport {
/**
* {@code true} if {@code java.sql} types are supported,
Expand Down
4 changes: 2 additions & 2 deletions gson/src/main/java/com/google/gson/stream/JsonReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -1603,7 +1603,7 @@ private char readEscapeCharacter() throws IOException {
throw syntaxError("Unterminated escape sequence");
}
// Equivalent to Integer.parseInt(stringPool.get(buffer, pos, 4), 16);
char result = 0;
int result = 0;
for (int i = pos, end = i + 4; i < end; i++) {
char c = buffer[i];
result <<= 4;
Expand All @@ -1618,7 +1618,7 @@ private char readEscapeCharacter() throws IOException {
}
}
pos += 4;
return result;
return (char) result;

case 't':
return '\t';
Expand Down
3 changes: 3 additions & 0 deletions gson/src/main/java/module-info.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
exports com.google.gson.reflect;
exports com.google.gson.stream;

// Dependency on Error Prone Annotations
requires static com.google.errorprone.annotations;

// Optional dependency on java.sql
requires static java.sql;

Expand Down
2 changes: 2 additions & 0 deletions gson/src/test/java/com/google/gson/CommentsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.junit.Test;

/**
* Tests that by default Gson accepts several forms of comments.
*
* @author Jesse Wilson
*/
public final class CommentsTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public void testIncludeStaticNestedClassField() throws Exception {
assertThat(excluder.excludeField(f, true)).isFalse();
}

@SuppressWarnings("ClassCanBeStatic")
class InnerClass {
}

Expand Down
2 changes: 2 additions & 0 deletions gson/src/test/java/com/google/gson/JsonArrayTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.junit.Test;

/**
* Tests handling of JSON arrays.
*
* @author Jesse Wilson
*/
public final class JsonArrayTest {
Expand Down
2 changes: 2 additions & 0 deletions gson/src/test/java/com/google/gson/JsonNullTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.junit.Test;

/**
* Tests handling of JSON nulls.
*
* @author Jesse Wilson
*/
public final class JsonNullTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void testDeserializeDeeplyNestedObjects() throws IOException {
assertThat(actualTimes).isEqualTo(times);
}

@SuppressWarnings("unused")
@SuppressWarnings({"unused", "ClassCanBeStatic"})
private class RuntimeType {
Object a = 5;
Object b = Arrays.asList(1, 2, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.junit.Test;

/**
* Tests handling of Core Type Adapters
*
* @author Jesse Wilson
*/
public class OverrideCoreTypeAdaptersTest {
Expand Down
22 changes: 5 additions & 17 deletions gson/src/test/java/com/google/gson/ParameterizedTypeFixtures.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.gson;

import com.google.common.base.Objects;
import com.google.gson.internal.$Gson$Types;

import com.google.gson.internal.Primitives;
Expand All @@ -35,7 +36,7 @@
*/
public class ParameterizedTypeFixtures {

public static class MyParameterizedType<T> {
public static final class MyParameterizedType<T> {
public final T value;
public MyParameterizedType(T value) {
this.value = value;
Expand Down Expand Up @@ -80,27 +81,16 @@ public int hashCode() {
return value == null ? 0 : value.hashCode();
}

@SuppressWarnings("unchecked")
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
MyParameterizedType<T> other = (MyParameterizedType<T>) obj;
if (value == null) {
if (other.value != null) {
return false;
}
} else if (!value.equals(other.value)) {
if (!(obj instanceof MyParameterizedType<?>)) {
return false;
}
return true;
MyParameterizedType<?> that = (MyParameterizedType<?>) obj;
return Objects.equal(getValue(), that.getValue());
}
}

Expand All @@ -112,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
*/
public MyParameterizedTypeInstanceCreator(T instanceOfT) {
this.instanceOfT = instanceOfT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

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

import com.google.errorprone.annotations.Keep;
import com.google.gson.annotations.Since;
import com.google.gson.annotations.Until;
import com.google.gson.internal.Excluder;
Expand Down Expand Up @@ -75,13 +76,15 @@ public void testOlderVersion() throws Exception {
private static class MockClassSince {

@Since(VERSION)
@Keep
public final int someField = 0;
}

@Until(VERSION)
private static class MockClassUntil {

@Until(VERSION)
@Keep
public final int someField = 0;
}

Expand All @@ -91,6 +94,7 @@ private static class MockClassBoth {

@Since(VERSION)
@Until(VERSION + 2)
@Keep
public final int someField = 0;
}
}
31 changes: 12 additions & 19 deletions gson/src/test/java/com/google/gson/common/TestTypes.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.gson.common;

import com.google.common.base.Objects;
import com.google.gson.JsonDeserializationContext;
import com.google.gson.JsonDeserializer;
import com.google.gson.JsonElement;
Expand Down Expand Up @@ -146,26 +147,18 @@ public int hashCode() {
}

@Override
public boolean equals(Object obj) {
if (this == obj)
public boolean equals(Object o) {
if (this == o) {
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
BagOfPrimitives other = (BagOfPrimitives) obj;
if (booleanValue != other.booleanValue)
return false;
if (intValue != other.intValue)
return false;
if (longValue != other.longValue)
return false;
if (stringValue == null) {
if (other.stringValue != null)
return false;
} else if (!stringValue.equals(other.stringValue))
}
if (!(o instanceof BagOfPrimitives)) {
return false;
return true;
}
BagOfPrimitives that = (BagOfPrimitives) o;
return longValue == that.longValue
&& getIntValue() == that.getIntValue()
&& booleanValue == that.booleanValue
&& Objects.equal(stringValue, that.stringValue);
}

@Override
Expand Down Expand Up @@ -233,7 +226,7 @@ public static class ClassWithNoFields {
// Nothing here..
@Override
public boolean equals(Object other) {
return other.getClass() == ClassWithNoFields.class;
return other instanceof ClassWithNoFields;
}
}

Expand Down
Loading

0 comments on commit dc20b75

Please sign in to comment.