Nullness annotations to gson [review 1] #1
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,7 @@ | |
| import com.google.gson.stream.JsonToken; | ||
| import com.google.gson.stream.JsonWriter; | ||
| import com.google.gson.stream.MalformedJsonException; | ||
| import org.checkerframework.checker.nullness.qual.Nullable; | ||
|
|
||
| /** | ||
| * This is the main class for using Gson. Gson is typically used by first constructing a | ||
|
|
@@ -121,8 +122,8 @@ public final class Gson { | |
| * lookup would stack overflow. We cheat by returning a proxy type adapter. | ||
| * The proxy is wired up once the initial adapter has been created. | ||
| */ | ||
| private final ThreadLocal<@Nullable Map<TypeToken<?>, FutureTypeAdapter<?>>> calls | ||
| = new ThreadLocal<@Nullable Map<TypeToken<?>, FutureTypeAdapter<?>>>(); | ||
|
|
||
| private final Map<TypeToken<?>, TypeAdapter<?>> typeTokenCache = new ConcurrentHashMap<TypeToken<?>, TypeAdapter<?>>(); | ||
|
|
||
|
|
@@ -141,7 +142,7 @@ public final class Gson { | |
| final boolean prettyPrinting; | ||
| final boolean lenient; | ||
| final boolean serializeSpecialFloatingPointValues; | ||
| final @Nullable String datePattern; | ||
| final int dateStyle; | ||
| final int timeStyle; | ||
| final LongSerializationPolicy longSerializationPolicy; | ||
|
|
@@ -192,11 +193,13 @@ public Gson() { | |
| Collections.<TypeAdapterFactory>emptyList()); | ||
| } | ||
|
|
||
| /*dangerous to call an instance method inside a constructor, framework could identify #1 and #2 out of 4 such errors*/ | ||
| @SuppressWarnings("method.invocation.invalid") | ||
|
There was a problem hiding this comment. The errors you suppressed are InitializationChecker errors. You can use this when doing nullness case studies. Can you make sure adding such annotations still produce There was a problem hiding this comment. @Maxi17 I could not completely understand this comment. Adding which annotations still produce this error? There was a problem hiding this comment. I am saying that the errors you are suppressing are because of There was a problem hiding this comment. Annotation needs to be added to the current object or |
||
| Gson(Excluder excluder, FieldNamingStrategy fieldNamingStrategy, | ||
| Map<Type, InstanceCreator<?>> instanceCreators, boolean serializeNulls, | ||
| boolean complexMapKeySerialization, boolean generateNonExecutableGson, boolean htmlSafe, | ||
| boolean prettyPrinting, boolean lenient, boolean serializeSpecialFloatingPointValues, | ||
| LongSerializationPolicy longSerializationPolicy, @Nullable String datePattern, int dateStyle, | ||
| int timeStyle, List<TypeAdapterFactory> builderFactories, | ||
| List<TypeAdapterFactory> builderHierarchyFactories, | ||
| List<TypeAdapterFactory> factoriesToBeAdded) { | ||
|
|
@@ -239,9 +242,9 @@ public Gson() { | |
| TypeAdapter<Number> longAdapter = longAdapter(longSerializationPolicy); | ||
| factories.add(TypeAdapters.newFactory(long.class, Long.class, longAdapter)); | ||
| factories.add(TypeAdapters.newFactory(double.class, Double.class, | ||
| doubleAdapter(serializeSpecialFloatingPointValues))); //#1 | ||
| factories.add(TypeAdapters.newFactory(float.class, Float.class, | ||
| floatAdapter(serializeSpecialFloatingPointValues))); //#2 | ||
| factories.add(TypeAdapters.NUMBER_FACTORY); | ||
| factories.add(TypeAdapters.ATOMIC_INTEGER_FACTORY); | ||
| factories.add(TypeAdapters.ATOMIC_BOOLEAN_FACTORY); | ||
|
|
@@ -311,7 +314,7 @@ private TypeAdapter<Number> doubleAdapter(boolean serializeSpecialFloatingPointV | |
| return TypeAdapters.DOUBLE; | ||
| } | ||
| return new TypeAdapter<Number>() { | ||
| @Override public @Nullable Double read(JsonReader in) throws IOException { | ||
| if (in.peek() == JsonToken.NULL) { | ||
| in.nextNull(); | ||
| return null; | ||
|
|
@@ -335,7 +338,7 @@ private TypeAdapter<Number> floatAdapter(boolean serializeSpecialFloatingPointVa | |
| return TypeAdapters.FLOAT; | ||
| } | ||
| return new TypeAdapter<Number>() { | ||
| @Override public @Nullable Float read(JsonReader in) throws IOException { | ||
| if (in.peek() == JsonToken.NULL) { | ||
| in.nextNull(); | ||
| return null; | ||
|
|
@@ -367,7 +370,7 @@ private static TypeAdapter<Number> longAdapter(LongSerializationPolicy longSeria | |
| return TypeAdapters.LONG; | ||
| } | ||
| return new TypeAdapter<Number>() { | ||
| @Override public @Nullable Number read(JsonReader in) throws IOException { | ||
| if (in.peek() == JsonToken.NULL) { | ||
| in.nextNull(); | ||
| return null; | ||
|
|
@@ -389,8 +392,12 @@ private static TypeAdapter<AtomicLong> atomicLongAdapter(final TypeAdapter<Numbe | |
| @Override public void write(JsonWriter out, AtomicLong value) throws IOException { | ||
| longAdapter.write(out, value.get()); | ||
| } | ||
| /*The method is private and the developers ensure to call the below | ||
| * function with an `in` that returns non null value #3, therefore code | ||
| * is safe*/ | ||
| @SuppressWarnings("dereference.of.nullable") | ||
|
There was a problem hiding this comment. The method is private. Still, can you verify that it is not possible to throw a There was a problem hiding this comment. No NullPointerException is thrown internally but as the method is private developers ensure the concerned read function does not return null so the code is safe |
||
| @Override public AtomicLong read(JsonReader in) throws IOException { | ||
| Number value = longAdapter.read(in); //#3 | ||
| return new AtomicLong(value.longValue()); | ||
| } | ||
| }.nullSafe(); | ||
|
|
@@ -405,12 +412,14 @@ private static TypeAdapter<AtomicLongArray> atomicLongArrayAdapter(final TypeAda | |
| } | ||
| out.endArray(); | ||
| } | ||
| /*in.hasNext() ensures read(in) doesnt return null in #4*/ | ||
| @SuppressWarnings({"dereference.of.nullable"}) | ||
| @Override public AtomicLongArray read(JsonReader in) throws IOException { | ||
| List<Long> list = new ArrayList<Long>(); | ||
| in.beginArray(); | ||
| while (in.hasNext()) { //#4 | ||
| long value = longAdapter.read(in).longValue(); | ||
| list.add(value); //#5 | ||
| } | ||
| in.endArray(); | ||
| int length = list.size(); | ||
|
|
@@ -522,7 +531,9 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) { | |
| * | ||
| * @since 2.2 | ||
| */ | ||
| // Possible missing annotation in class List method contains(@Nullable Object) | ||
| @SuppressWarnings("nullness:argument.type.incompatible") | ||
| public <T> TypeAdapter<T> getDelegateAdapter(@Nullable TypeAdapterFactory skipPast, TypeToken<T> type) { | ||
| // Hack. If the skipPast factory isn't registered, assume the factory is being requested via | ||
| // our @JsonAdapter annotation. | ||
| if (!factories.contains(skipPast)) { | ||
|
|
@@ -813,7 +824,7 @@ public void toJson(JsonElement jsonElement, JsonWriter writer) throws JsonIOExce | |
| * @throws JsonSyntaxException if json is not a valid representation for an object of type | ||
| * classOfT | ||
| */ | ||
| public @Nullable <T> T fromJson(String json, Class<T> classOfT) throws JsonSyntaxException { | ||
| Object object = fromJson(json, (Type) classOfT); | ||
| return Primitives.wrap(classOfT).cast(object); | ||
| } | ||
|
|
@@ -838,7 +849,7 @@ public <T> T fromJson(String json, Class<T> classOfT) throws JsonSyntaxException | |
| * @throws JsonSyntaxException if json is not a valid representation for an object of type | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public @Nullable <T> T fromJson(@Nullable String json, Type typeOfT) throws JsonSyntaxException { | ||
| if (json == null) { | ||
| return null; | ||
| } | ||
|
|
@@ -865,7 +876,7 @@ public <T> T fromJson(String json, Type typeOfT) throws JsonSyntaxException { | |
| * @throws JsonSyntaxException if json is not a valid representation for an object of type | ||
| * @since 1.2 | ||
| */ | ||
| public @Nullable <T> T fromJson(Reader json, Class<T> classOfT) throws JsonSyntaxException, JsonIOException { | ||
| JsonReader jsonReader = newJsonReader(json); | ||
| Object object = fromJson(jsonReader, classOfT); | ||
| assertFullConsumption(object, jsonReader); | ||
|
|
@@ -892,14 +903,14 @@ public <T> T fromJson(Reader json, Class<T> classOfT) throws JsonSyntaxException | |
| * @since 1.2 | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public @Nullable <T> T fromJson(Reader json, Type typeOfT) throws JsonIOException, JsonSyntaxException { | ||
| JsonReader jsonReader = newJsonReader(json); | ||
| T object = (T) fromJson(jsonReader, typeOfT); | ||
| assertFullConsumption(object, jsonReader); | ||
| return object; | ||
| } | ||
PRITI1999 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| private static void assertFullConsumption(@Nullable Object obj, JsonReader reader) { | ||
| try { | ||
| if (obj != null && reader.peek() != JsonToken.END_DOCUMENT) { | ||
| throw new JsonIOException("JSON document was not fully consumed."); | ||
|
|
@@ -920,7 +931,7 @@ private static void assertFullConsumption(Object obj, JsonReader reader) { | |
| * @throws JsonSyntaxException if json is not a valid representation for an object of type | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public @Nullable <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, JsonSyntaxException { | ||
| boolean isEmpty = true; | ||
| boolean oldLenient = reader.isLenient(); | ||
| reader.setLenient(true); | ||
|
|
@@ -971,7 +982,7 @@ public <T> T fromJson(JsonReader reader, Type typeOfT) throws JsonIOException, J | |
| * @throws JsonSyntaxException if json is not a valid representation for an object of type typeOfT | ||
| * @since 1.3 | ||
| */ | ||
| public @Nullable <T> T fromJson(JsonElement json, Class<T> classOfT) throws JsonSyntaxException { | ||
| Object object = fromJson(json, (Type) classOfT); | ||
| return Primitives.wrap(classOfT).cast(object); | ||
| } | ||
|
|
@@ -996,15 +1007,15 @@ public <T> T fromJson(JsonElement json, Class<T> classOfT) throws JsonSyntaxExce | |
| * @since 1.3 | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public @Nullable <T> T fromJson(JsonElement json, Type typeOfT) throws JsonSyntaxException { | ||
| if (json == null) { | ||
| return null; | ||
| } | ||
| return (T) fromJson(new JsonTreeReader(json), typeOfT); | ||
| } | ||
|
|
||
| static class FutureTypeAdapter<T> extends TypeAdapter<T> { | ||
| private @Nullable TypeAdapter<T> delegate; | ||
|
|
||
| public void setDelegate(TypeAdapter<T> typeAdapter) { | ||
| if (delegate != null) { | ||
|
|
@@ -1013,7 +1024,7 @@ public void setDelegate(TypeAdapter<T> typeAdapter) { | |
| delegate = typeAdapter; | ||
| } | ||
|
|
||
| @Override public @Nullable T read(JsonReader in) throws IOException { | ||
| if (delegate == null) { | ||
| throw new IllegalStateException(); | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the documentation it is not specified that the return result can be null. This annotation is forced here because the method
get(Object)injava.lang.reflecthas its return type annotated with@Nullable. I am not sure if this annotation is correct, sincegetfromjdk.internal.reflect.FieldAccessor.javais not annotated as@Nullable.