Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Pretty formatting puts commas inside empty arrays or objects #26

Closed
jkeljo opened this issue Jul 1, 2019 · 3 comments
Closed

Pretty formatting puts commas inside empty arrays or objects #26

jkeljo opened this issue Jul 1, 2019 · 3 comments

Comments

@jkeljo
Copy link

jkeljo commented Jul 1, 2019

The stock json module doesn't do this. Repro:

% python3.7 
Python 3.7.0 (default, Oct  2 2018, 09:20:07) 
[Clang 10.0.0 (clang-1000.11.45.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import json
>>> import json5
>>> json.dumps([], indent=2)
'[]'
>>> json5.dumps([], indent=2)
'[\n  ,\n]'
>>> json.dumps({}, indent=2)
'{}'
>>> json5.dumps({}, indent=2)
'{\n  ,\n}'
@jkeljo
Copy link
Author

jkeljo commented Jul 3, 2019

Also looks like these strings won't round-trip through the parser:

>>> json5.loads(json5.dumps({}, indent=2))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.7/site-packages/json5/lib.py", line 80, in loads
    raise ValueError(err)
ValueError: <string>:2 Unexpected "
" at column 4

@dpranke
Copy link
Owner

dpranke commented Jul 3, 2019

Thanks for the report.

The '[\n ,\n]' is definitely a bug.

The fact that the strings won't round-trip is not a bug, because the input string isn't legal :)

That fact that the error message is reporting the wrong unexpected character, however, is a bug :).

These should be easy fixes, I'll try to get to them today.

dpranke added a commit that referenced this issue Jul 5, 2019
This addresses #26:
`json5.dumps([], indent=2)` incorrectly returned '[\n  ,\n]',
when it should've returned '[]' (and the did the same thing for
objects).
dpranke added a commit that referenced this issue Jul 5, 2019
This patch addresses an error reported in
#26, where calling
`json.loads('[ ,]') would report an error on column 4, not column 3.

The reason for this had to do with the way the optional whitespace (sp*)
rule was coded in the grammar, and a bug in the underlying parser generator
(glop), where if you had a rule like (a ?())* and you matched a and not b,
the errposition wouldn't be properly rolled back before the conditional
expression. That bug was fixed in glop v0.6.2; this CL regenerates the
JSON5 parser with that version of the parser generator, and then adds
json5-specific tests to check for the correct position reporting.
@dpranke
Copy link
Owner

dpranke commented Jul 5, 2019

Fixed in 6c4038e (v0.8.5). This actually required a fix to the underlying parser generator (glop, v0.6.2) to fix the error reporting, so double-bonus bug report :).

@dpranke dpranke closed this as completed Jul 5, 2019
dpranke added a commit to dpranke/glop that referenced this issue May 9, 2022
This modifies the fix in f259ff, which wasn't quite
right. We were incorrectly rolling back the `errpos` field
when rules failed, which would lead to errors being
reported as having happened at the beginning of a rule,
rather than at the spot where something actually
happened.

In debugging this, I realized that part of the problem is
that error reporting for semantic predicate failures doesn't
quite work right, so I've filed a new bug for that and
updated the tests to be clearer.

See dpranke/pyjson5#26 and
#3 for more on
this.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants