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

Formalize null handling of type tokens #2030

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions gson/src/main/java/com/google/gson/Gson.java
Expand Up @@ -111,7 +111,6 @@ public final class Gson {
static final boolean DEFAULT_COMPLEX_MAP_KEYS = false;
static final boolean DEFAULT_SPECIALIZE_FLOAT_VALUES = false;

private static final TypeToken<?> NULL_KEY_SURROGATE = TypeToken.get(Object.class);
private static final String JSON_NON_EXECUTABLE_PREFIX = ")]}'\n";

/**
Expand Down Expand Up @@ -440,7 +439,11 @@ private static TypeAdapter<AtomicLongArray> atomicLongArrayAdapter(final TypeAda
*/
@SuppressWarnings("unchecked")
public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
TypeAdapter<?> cached = typeTokenCache.get(type == null ? NULL_KEY_SURROGATE : type);
if (type == null) {
throw new NullPointerException("No type specified");
}

TypeAdapter<?> cached = typeTokenCache.get(type);
if (cached != null) {
return (TypeAdapter<T>) cached;
}
Expand Down
16 changes: 16 additions & 0 deletions gson/src/test/java/com/google/gson/GsonTest.java
Expand Up @@ -17,6 +17,7 @@
package com.google.gson;

import com.google.gson.internal.Excluder;
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import java.io.IOException;
Expand All @@ -26,6 +27,9 @@
import java.util.ArrayList;
import java.util.HashMap;
import junit.framework.TestCase;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
Comment on lines +30 to +32
Copy link
Collaborator

Choose a reason for hiding this comment

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

These imports seem to be unused.


/**
* Unit tests for {@link Gson}.
Expand Down Expand Up @@ -82,4 +86,16 @@ private static final class TestTypeAdapter extends TypeAdapter<Object> {
}
@Override public Object read(JsonReader in) throws IOException { return null; }
}

public void testNullAdapterLookup() {
// Formalize the handling of null type tokens (previously it was partially handled, but still throwing an error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this comment line here is useful in the test code. To me this message only seems to be relevant regarding why this pull request or commit was created, but not why this test exists.


Note that unlike eamonnmcmanus, I am not a member of this project. Feel free to consider my comments only as suggestion.

// Note: ExpectedException cannot be used due to test suite extending TestCase
try {
new Gson().getAdapter((TypeToken<Object>) null);
assertTrue("Unreachable", false);
Copy link
Member

Choose a reason for hiding this comment

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

Just fail() would be fine here.

}
catch (NullPointerException e) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you join this to the previous line?

} catch (NullPointerException e) {

assertEquals("No type specified", e.getMessage());
}
}
}