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

Index annotations #2

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Index annotations #2

wants to merge 12 commits into from

Conversation

PRITI1999
Copy link

@Maxi17 Index annotations are added. Please review.

Copy link
Collaborator

@Maxi17 Maxi17 left a comment

Choose a reason for hiding this comment

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

Another good case study. Please address my comments and tag me again.

@@ -171,7 +172,7 @@ public boolean contains(JsonElement element) {
public int size() {
return elements.size();
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

This diff file is too cluttered with empty spaces.

@@ -233,9 +233,11 @@ public byte getAsByte() {
return isNumber() ? getAsNumber().byteValue() : Byte.parseByte(getAsString());
}

//getAsString returns a String of minimum length 1, charAt(0) is safe
@SuppressWarnings("index:argument.type.incompatible")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why using a @MinLen annotation to express what you said here doesn't work?

@@ -274,10 +286,12 @@ static Type getGenericSupertype(Type context, Class<?> rawType, Class<?> toResol
*
* @param supertype a superclass of, or interface implemented by, this.
*/
/*Missing annotation in JDK in @MinLen(1) getUpperBounds() of WildcardType #5*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remember to delete these comments once missing JDK annotations are added.

@@ -130,7 +130,8 @@ public ConstructorConstructor(Map<Type, InstanceCreator<?>> instanceCreators) {
* Constructors for common interface types like Map and List and their
* subtypes.
*/
@SuppressWarnings("unchecked") // use runtime checks to guarantee that 'T' is what it is
//getActualTypeArguments may return an empty array #1 #2
Copy link
Collaborator

Choose a reason for hiding this comment

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

So is this a possible bug in Gson?

@@ -78,13 +80,15 @@ protected TypeToken() {
* Returns the type from super class's type parameter in {@link $Gson$Types#canonicalize
* canonical form}.
*/
//getActualTypeArguments() may return an empty array as per documentation
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, can you write a test case in which unexpected behavior happens?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants