-
Notifications
You must be signed in to change notification settings - Fork 50
Trailing commas not marked as syntax errors if line breaks presentΒ #59
Description
Prerequisites
- [ X ] Put an X between the brackets on this line if you have done all of the following:
- Reproduced the problem in Safe Mode: http://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
- Followed all applicable steps in the debugging guide: http://flight-manual.atom.io/hacking-atom/sections/debugging/
- Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
- Checked that your issue isn't already filed: https://github.com/issues?utf8=β&q=is%3Aissue+user%3Aatom
- Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages
Description
#45, which supposedly fixed #11, introduced a regex that, in normal regex-land, should identify trailing commas after the last key-value pair in an object. This is strictly illegal syntax per the JSON spec: there is no trailing value-separator in 2.2 in https://www.ietf.org/rfc/rfc4627.txt , regardless of ignored whitespace. Node's implementation of the spec concurs:
> JSON.parse('{"a": 1,\n}')
SyntaxError: Unexpected token } in JSON at position 9
> JSON.parse('{"a": 1\n}')
{ a: 1 }
However, because Atom apparently feeds only one line at a time to the regex, and #45 handles newlines by placing them inside a lookahead group in a regex ... if there is a newline between the trailing comma and the end of the object, it will not be matched!
For anyone handcrafting JSON Schemas or config files (read: everyone these days) that will be read by strict parsers, this is a huge pain, especially since "trivial" config file changes that seem fine in an editor may break things in production, or only after a lengthy test run. More generally, we should strive to be compliant with the spec.
We may need to introduce a new scope in the grammar for the "space between a trailing comma and a closing }", rather than just having it as a lookahead. That way it would match over multiple lines.
Steps to Reproduce
In Atom console:
g = atom.grammars.grammarForScopeName('source.json'); JSON.stringify(g.tokenizeLines('{"a":1,\n}'), null, ' ')
Expected behavior: One token should have invalid.illegal.trailing-dictionary-separator.json scope.
Actual behavior: None of them do.
Reproduces how often: All the time.
Versions
atom --version
Atom : 1.17.0
Electron: 1.3.15
Chrome : 52.0.2743.82
Node : 6.5.0
apm --version
apm 1.17.0
npm 3.10.5
node 6.9.5 x64
python 2.7.12
git 2.7.1
OS 10.11.6
Additional Information
Any additional information, configuration or data that might be necessary to reproduce the issue.

