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 warns #2320

Merged
merged 57 commits into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from 52 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
18336d0
Adds `@SuppressWarnings("NarrowingCompoundAssignment")`
MaicolAntali Feb 18, 2023
0c2f8c3
Adds `@SuppressWarnings("TypeParameterUnusedInFormals")`
MaicolAntali Feb 18, 2023
7f64bf3
Adds `@SuppressWarnings("JavaUtilDate")`
MaicolAntali Feb 18, 2023
f34c35d
Adds a limit to `String.split()`
MaicolAntali Feb 18, 2023
5e6a279
Add `error_prone_annotations` to the `pom.xml`
MaicolAntali Feb 18, 2023
682b369
Adds `@InlineMe(...)` to deprecated methods
MaicolAntali Feb 18, 2023
41e0c28
Adds `@SuppressWarnings("ImmutableEnumChecker")`
MaicolAntali Feb 18, 2023
b6965d1
Adds `@SuppressWarnings("ModifiedButNotUsed")`
MaicolAntali Feb 18, 2023
04feff6
Adds `@SuppressWarnings("MixedMutabilityReturnType")`
MaicolAntali Feb 18, 2023
5bd180d
Removes an unused import
MaicolAntali Feb 18, 2023
373525d
Adds `requires` to `module-info.java`
MaicolAntali Feb 19, 2023
bd60abf
Adds ErrorProne `link` into `pom.xml`
MaicolAntali Feb 19, 2023
5e1e970
Remove unused imports
MaicolAntali Feb 19, 2023
f3c4763
Adds `@SuppressWarnings("EqualsGetClass")`
MaicolAntali Feb 19, 2023
71a75b8
Excludes from `proto` just the generated code.
MaicolAntali Feb 19, 2023
a1beaf2
Removes an unused variable
MaicolAntali Feb 19, 2023
9c5073f
Fixes the `BadImport` warn into `ProtosWithAnnotationsTest`
MaicolAntali Feb 19, 2023
2aad0bc
Fixes the `BadImport` warns into `ProtosWithAnnotationsTest`
MaicolAntali Feb 19, 2023
ba27715
Enables ErrorProne in `gson/src/test.*`
MaicolAntali Feb 19, 2023
e3d65bc
Fixes `UnusedVariable` warns
MaicolAntali Feb 19, 2023
d1c0554
Fixes `JavaUtilDate` warns
MaicolAntali Feb 19, 2023
7769c75
Fixes `EqualsGetClass` warns
MaicolAntali Feb 19, 2023
a8cff33
Replaces pattern matching for instanceof with casts
MaicolAntali Feb 19, 2023
eb05e0d
Fixes `JdkObsolete` warns
MaicolAntali Feb 22, 2023
ef27eb4
Fixes `ClassCanBeStatic` warns
MaicolAntali Feb 22, 2023
69b34be
Fixes `UndefinedEquals` warns
MaicolAntali Feb 22, 2023
7a20385
Fixes `GetClassOnEnum` warns
MaicolAntali Feb 22, 2023
4627307
Fixes `ImmutableEnumChecker` warns
MaicolAntali Feb 22, 2023
13de98d
Fixes `StaticAssignmentOfThrowable` warns
MaicolAntali Feb 22, 2023
3ae4fac
Fixes `AssertionFailureIgnored` warns
MaicolAntali Feb 22, 2023
661eae8
Fixes `ModifiedButNotUsed` warns
MaicolAntali Feb 22, 2023
2742881
Fixes `MissingSummary` warns
MaicolAntali Feb 22, 2023
7d688ed
Fixes `FloatingPointLiteralPrecision` warns
MaicolAntali Feb 22, 2023
67a8edf
Fixes `StringSplitter` warns
MaicolAntali Feb 22, 2023
b92cb96
Fixes `EmptyCatch` warns
MaicolAntali Feb 22, 2023
dcb277e
Fixes `UnicodeEscape` warns
MaicolAntali Feb 22, 2023
6dee398
Fixes `EmptyBlockTag` warns
MaicolAntali Feb 22, 2023
682d5df
Fixes `LongFloatConversion` warns
MaicolAntali Feb 22, 2023
c1681bf
Fixes `LongDoubleConversion` warns
MaicolAntali Feb 22, 2023
69dcd03
Fixes `TruthAssertExpected` warns
MaicolAntali Feb 22, 2023
03e86d6
Fixes `UnusedMethod` warns
MaicolAntali Feb 22, 2023
1feefb4
Fixes `UnusedTypeParameter` warns
MaicolAntali Feb 22, 2023
7ac4465
Fixes `CatchFail` warns
MaicolAntali Feb 22, 2023
92972bf
Fixes `MathAbsoluteNegative` warns
MaicolAntali Feb 22, 2023
2167d4c
Fixes `LoopOverCharArray` warns
MaicolAntali Feb 22, 2023
126b1b5
Fixes `HidingField` warns
MaicolAntali Feb 22, 2023
4aaefea
Implements code review feedback
MaicolAntali Feb 22, 2023
f1c5591
Implements code review feedback
MaicolAntali Feb 22, 2023
7590168
Enable ErrorProne in `extra`
MaicolAntali Feb 22, 2023
1bb54c4
Fix the `JavaUtilDate` warns
MaicolAntali Feb 22, 2023
a507512
Implements code review feedback
MaicolAntali Feb 23, 2023
bd2916e
Removes redundant new-line
MaicolAntali Feb 23, 2023
13ab165
Implements code review feedback
MaicolAntali Feb 24, 2023
a7cca38
Adds `JDK11` to run test with `--release 11`
MaicolAntali Feb 24, 2023
5dbbbee
Revert "Adds `JDK11` to run test with `--release 11`"
MaicolAntali Feb 24, 2023
f89d42a
Merge branch 'master' into fix-error-prone-warns
MaicolAntali Feb 28, 2023
039bf62
Merge branch 'master' into fix-error-prone-warns
MaicolAntali Mar 1, 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 @@ -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: 6 additions & 16 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<?>)) {
MaicolAntali marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
return true;
MyParameterizedType<?> that = (MyParameterizedType<?>) obj;
return Objects.equal(getValue(), that.getValue());
}
}

Expand All @@ -113,7 +103,7 @@ public static class MyParameterizedTypeInstanceCreator<T>
* This is usually fine in tests since there we deserialize just once, but quite
* dangerous in practice.
*
* @param instanceOfT
* @param instanceOfT Instance of Type
MaicolAntali marked this conversation as resolved.
Show resolved Hide resolved
*/
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)) {
Copy link
Member

Choose a reason for hiding this comment

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

Did Error Prone suggest changing the getClass() check into this instanceof? I believe it is correct, but I had to study the code a bit to make sure. There is actually a subclass of BagOfPrimitives called SubTypeOfBagOfPrimitives in JsonTreeTest which adds an extra field. But it doesn't override equals and the test doesn't rely on equals, so I think it is OK that an instance of BagOfPrimitives can now equal an instance of SubTypeOfBagOfPrimitives where it couldn't before.

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;
Copy link
Member

Choose a reason for hiding this comment

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

Again this has subclasses (anonymous ones) in ObjectTest, but I don't think the change in equals semantics matters.

}
}

Expand Down
Loading