-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve JsonReader.skipValue()
#2062
Merged
eamonnmcmanus
merged 8 commits into
google:master
from
Marcono1234:marcono1234/JsonReader-skipValue-improvements
Oct 10, 2022
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d3c584b
Fix JsonReader.skipValue() not behaving properly at end of document
Marcono1234 08e614d
Fix JsonReader.skipValue() not behaving properly at end of array and …
Marcono1234 98f6246
Only have JsonReader.skipValue() overwrite path name when name was sk…
Marcono1234 1d43ecf
Merge branch 'master' into marcono1234/JsonReader-skipValue-improvements
eamonnmcmanus 242a1aa
Merge branch 'master' into marcono1234/JsonReader-skipValue-improvements
Marcono1234 873aa6a
Merge branch 'master' into marcono1234/JsonReader-skipValue-improvements
Marcono1234 9291d3c
Address feedback; improve test coverage
Marcono1234 b7b5c3e
Merge branch 'master' into marcono1234/JsonReader-skipValue-improvements
Marcono1234 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -777,10 +777,9 @@ private boolean isLiteral(char c) throws IOException { | |
} | ||
|
||
/** | ||
* Returns the next token, a {@link com.google.gson.stream.JsonToken#NAME property name}, and | ||
* consumes it. | ||
* Returns the next token, a {@link JsonToken#NAME property name}, and consumes it. | ||
* | ||
* @throws java.io.IOException if the next token in the stream is not a property | ||
* @throws IOException if the next token in the stream is not a property | ||
* name. | ||
*/ | ||
public String nextName() throws IOException { | ||
|
@@ -804,7 +803,7 @@ public String nextName() throws IOException { | |
} | ||
|
||
/** | ||
* Returns the {@link com.google.gson.stream.JsonToken#STRING string} value of the next token, | ||
* Returns the {@link JsonToken#STRING string} value of the next token, | ||
* consuming it. If the next token is a number, this method will return its | ||
* string form. | ||
* | ||
|
@@ -840,7 +839,7 @@ public String nextString() throws IOException { | |
} | ||
|
||
/** | ||
* Returns the {@link com.google.gson.stream.JsonToken#BOOLEAN boolean} value of the next token, | ||
* Returns the {@link JsonToken#BOOLEAN boolean} value of the next token, | ||
* consuming it. | ||
* | ||
* @throws IllegalStateException if the next token is not a boolean or if | ||
|
@@ -884,7 +883,7 @@ public void nextNull() throws IOException { | |
} | ||
|
||
/** | ||
* Returns the {@link com.google.gson.stream.JsonToken#NUMBER double} value of the next token, | ||
* Returns the {@link JsonToken#NUMBER double} value of the next token, | ||
* consuming it. If the next token is a string, this method will attempt to | ||
* parse it as a double using {@link Double#parseDouble(String)}. | ||
* | ||
|
@@ -930,7 +929,7 @@ public double nextDouble() throws IOException { | |
} | ||
|
||
/** | ||
* Returns the {@link com.google.gson.stream.JsonToken#NUMBER long} value of the next token, | ||
* Returns the {@link JsonToken#NUMBER long} value of the next token, | ||
* consuming it. If the next token is a string, this method will attempt to | ||
* parse it as a long. If the next token's numeric value cannot be exactly | ||
* represented by a Java {@code long}, this method throws. | ||
|
@@ -1163,7 +1162,7 @@ private void skipUnquotedValue() throws IOException { | |
} | ||
|
||
/** | ||
* Returns the {@link com.google.gson.stream.JsonToken#NUMBER int} value of the next token, | ||
* Returns the {@link JsonToken#NUMBER int} value of the next token, | ||
* consuming it. If the next token is a string, this method will attempt to | ||
* parse it as an int. If the next token's numeric value cannot be exactly | ||
* represented by a Java {@code int}, this method throws. | ||
|
@@ -1223,7 +1222,7 @@ public int nextInt() throws IOException { | |
} | ||
|
||
/** | ||
* Closes this JSON reader and the underlying {@link java.io.Reader}. | ||
* Closes this JSON reader and the underlying {@link Reader}. | ||
*/ | ||
@Override public void close() throws IOException { | ||
peeked = PEEKED_NONE; | ||
|
@@ -1233,9 +1232,19 @@ public int nextInt() throws IOException { | |
} | ||
|
||
/** | ||
* Skips the next value recursively. If it is an object or array, all nested | ||
* elements are skipped. This method is intended for use when the JSON token | ||
* stream contains unrecognized or unhandled values. | ||
* Skips the next value recursively. This method is intended for use when | ||
* the JSON token stream contains unrecognized or unhandled values. | ||
* | ||
* <p>The behavior depends on the type of the next JSON token: | ||
* <ul> | ||
* <li>Start of a JSON array or object: It and all of its nested values are skipped.</li> | ||
* <li>Primitive value (for example a JSON number): The primitive value is skipped.</li> | ||
* <li>Property name: Only the name but not the value of the property is skipped. | ||
* {@code skipValue()} has to be called again to skip the property value as well.</li> | ||
* <li>End of a JSON array or object: Only this end token is skipped.</li> | ||
* <li>End of JSON document: Skipping has no effect, the next token continues to be the | ||
* end of the document.</li> | ||
* </ul> | ||
*/ | ||
public void skipValue() throws IOException { | ||
int count = 0; | ||
|
@@ -1245,32 +1254,69 @@ public void skipValue() throws IOException { | |
p = doPeek(); | ||
} | ||
|
||
if (p == PEEKED_BEGIN_ARRAY) { | ||
push(JsonScope.EMPTY_ARRAY); | ||
count++; | ||
} else if (p == PEEKED_BEGIN_OBJECT) { | ||
push(JsonScope.EMPTY_OBJECT); | ||
count++; | ||
} else if (p == PEEKED_END_ARRAY) { | ||
stackSize--; | ||
count--; | ||
} else if (p == PEEKED_END_OBJECT) { | ||
stackSize--; | ||
count--; | ||
} else if (p == PEEKED_UNQUOTED_NAME || p == PEEKED_UNQUOTED) { | ||
skipUnquotedValue(); | ||
} else if (p == PEEKED_SINGLE_QUOTED || p == PEEKED_SINGLE_QUOTED_NAME) { | ||
skipQuotedValue('\''); | ||
} else if (p == PEEKED_DOUBLE_QUOTED || p == PEEKED_DOUBLE_QUOTED_NAME) { | ||
skipQuotedValue('"'); | ||
} else if (p == PEEKED_NUMBER) { | ||
pos += peekedNumberLength; | ||
switch (p) { | ||
case PEEKED_BEGIN_ARRAY: | ||
push(JsonScope.EMPTY_ARRAY); | ||
count++; | ||
break; | ||
case PEEKED_BEGIN_OBJECT: | ||
push(JsonScope.EMPTY_OBJECT); | ||
count++; | ||
break; | ||
case PEEKED_END_ARRAY: | ||
stackSize--; | ||
count--; | ||
break; | ||
case PEEKED_END_OBJECT: | ||
// Only update when object end is explicitly skipped, otherwise stack is not updated anyways | ||
if (count == 0) { | ||
pathNames[stackSize - 1] = null; // Free the last path name so that it can be garbage collected | ||
} | ||
stackSize--; | ||
count--; | ||
break; | ||
case PEEKED_UNQUOTED: | ||
skipUnquotedValue(); | ||
break; | ||
case PEEKED_SINGLE_QUOTED: | ||
skipQuotedValue('\''); | ||
break; | ||
case PEEKED_DOUBLE_QUOTED: | ||
skipQuotedValue('"'); | ||
break; | ||
case PEEKED_UNQUOTED_NAME: | ||
skipUnquotedValue(); | ||
// Only update when name is explicitly skipped, otherwise stack is not updated anyways | ||
if (count == 0) { | ||
pathNames[stackSize - 1] = "<skipped>"; | ||
} | ||
break; | ||
case PEEKED_SINGLE_QUOTED_NAME: | ||
skipQuotedValue('\''); | ||
// Only update when name is explicitly skipped, otherwise stack is not updated anyways | ||
if (count == 0) { | ||
pathNames[stackSize - 1] = "<skipped>"; | ||
} | ||
break; | ||
case PEEKED_DOUBLE_QUOTED_NAME: | ||
skipQuotedValue('"'); | ||
// Only update when name is explicitly skipped, otherwise stack is not updated anyways | ||
if (count == 0) { | ||
pathNames[stackSize - 1] = "<skipped>"; | ||
} | ||
break; | ||
case PEEKED_NUMBER: | ||
pos += peekedNumberLength; | ||
break; | ||
case PEEKED_EOF: | ||
// Do nothing | ||
return; | ||
// For all other tokens there is nothing to do; token has already been consumed from underlying reader | ||
} | ||
peeked = PEEKED_NONE; | ||
} while (count != 0); | ||
} while (count > 0); | ||
|
||
pathIndices[stackSize - 1]++; | ||
pathNames[stackSize - 1] = "null"; | ||
} | ||
|
||
private void push(int newTop) { | ||
|
@@ -1505,7 +1551,7 @@ private String getPath(boolean usePreviousPath) { | |
* <li>For JSON arrays the path points to the index of the previous element.<br> | ||
* If no element has been consumed yet it uses the index 0 (even if there are no elements).</li> | ||
* <li>For JSON objects the path points to the last property, or to the current | ||
* property if its value has not been consumed yet.</li> | ||
* property if its name has already been consumed.</li> | ||
Comment on lines
-1508
to
+1554
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous documentation (introduced in my PR #2000) might have been a bit misleading because whether or not the value has been consumed does not make a difference. (It might only affect which property you consider the "last" and the "previous".) |
||
* </ul> | ||
* | ||
* <p>This method can be useful to add additional context to exception messages | ||
|
@@ -1522,7 +1568,7 @@ public String getPreviousPath() { | |
* <li>For JSON arrays the path points to the index of the next element (even | ||
* if there are no further elements).</li> | ||
* <li>For JSON objects the path points to the last property, or to the current | ||
* property if its value has not been consumed yet.</li> | ||
* property if its name has already been consumed.</li> | ||
* </ul> | ||
* | ||
* <p>This method can be useful to add additional context to exception messages | ||
|
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.
I hope the wording here and below is fine; please let me know if you think it could be improved.