Fix errors in the JSON parser.

Fix errors in the JSON parser.


BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=7122

[Anton Muhin, 2012-04-28 09:09:24]

LGTM

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart
File lib/json/json.dart (right):

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:99: _JsonParser._internal(String json)
nit: ._internal(this.json) ?

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:282: if (_isChar(MINUS)) position++;
just an idea: maybe fetch char once with _char() and then do a dispatch?

int char = _char();
if (char == MINUS) { /* Rare case */ position++; char = _char9)) }
if (char == CHAR_0)
...

?

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:300: _error("Expected digit following comma");
nit: single quotes

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:300: _error("Expected digit following comma");
is it a correct thing, may we double check with JS if indeed 1. shouldn't be
parsable.  I just asking.  And if not, please, add a test (if absent)

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:304: if (_isChar(CHAR_E) || _isChar(CHAR_CAPITAL_E)) {
ditto here: fetch char once and then do both checks.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:306: if (_isChar(MINUS) || _isChar(PLUS)) position++;
ditto

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:312: _error("Expected digit following 'e' or 'E'");
ditto nit for ''

[Anders Johnsen, 2012-04-28 09:28:26]

Updated.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart
File lib/json/json.dart (right):

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:99: _JsonParser._internal(String json)
On 2012/04/28 09:09:24, antonmuhin wrote:
> nit: ._internal(this.json) ?

Not possible when extracting length in initializer list. :(

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:282: if (_isChar(MINUS)) position++;
On 2012/04/28 09:09:24, antonmuhin wrote:
> just an idea: maybe fetch char once with _char() and then do a dispatch?
> 
> int char = _char();
> if (char == MINUS) { /* Rare case */ position++; char = _char9)) }
> if (char == CHAR_0)
> ...
> 
> ?

Done.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:300: _error("Expected digit following comma");
On 2012/04/28 09:09:24, antonmuhin wrote:
> nit: single quotes

Done.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:300: _error("Expected digit following comma");
On 2012/04/28 09:09:24, antonmuhin wrote:
> is it a correct thing, may we double check with JS if indeed 1. shouldn't be
> parsable.  I just asking.  And if not, please, add a test (if absent)

There is not token in JSON that can follow a number and start with a '.', so I
think saying expected digit after '.' is better then saying invalid token to
'.'.

I test for the case of '3.a'.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:304: if (_isChar(CHAR_E) || _isChar(CHAR_CAPITAL_E)) {
On 2012/04/28 09:09:24, antonmuhin wrote:
> ditto here: fetch char once and then do both checks.

Done.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:306: if (_isChar(MINUS) || _isChar(PLUS)) position++;
On 2012/04/28 09:09:24, antonmuhin wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/10254029/diff/3/lib/json/json.dart#new...
lib/json/json.dart:312: _error("Expected digit following 'e' or 'E'");
On 2012/04/28 09:09:24, antonmuhin wrote:
> ditto nit for ''

Done.

[Anton Muhin, 2012-04-28 09:30:35]

Thanks a lot for explanations.

https://chromiumcodereview.appspot.com/10254029/diff/7001/lib/json/json.dart
File lib/json/json.dart (right):

https://chromiumcodereview.appspot.com/10254029/diff/7001/lib/json/json.dart#...
lib/json/json.dart:309: char = _nextChar();
char might be either the code or false, is this intended?  And both _isDigit and
_nextChar fetch the char, do we want to do that?

[Anders Johnsen, 2012-04-28 09:35:56]

Updated and committing - thanks for the reviews! :)

https://chromiumcodereview.appspot.com/10254029/diff/7001/lib/json/json.dart
File lib/json/json.dart (right):

https://chromiumcodereview.appspot.com/10254029/diff/7001/lib/json/json.dart#...
lib/json/json.dart:309: char = _nextChar();
On 2012/04/28 09:30:35, antonmuhin wrote:
> char might be either the code or false, is this intended?  And both _isDigit
and
> _nextChar fetch the char, do we want to do that?

Nope, nice catch! :)