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

JSON linter not commenting on the correct line #95

Open
adrianmoisey opened this issue Apr 26, 2016 · 6 comments
Open

JSON linter not commenting on the correct line #95

adrianmoisey opened this issue Apr 26, 2016 · 6 comments

Comments

@adrianmoisey
Copy link
Contributor

$ cat test.json
{
  "one": "value",
  "two": 2,
  "three": "three",
}
$ jsonlint test.json
test.json:5:0: Warning: Strict JSON does not allow a final comma in an object (dictionary) literal
   |  At line 5, column 0, offset 52
   |  Object started at line 1, column 0, offset 0 (AT-START)
test.json: ok, with warnings

If "three": "three" was added in the PR, lintreview wouldn't comment on this error, as it reports the error 1 line down.

I'm not sure what the solution so this problem is.

@zoidyzoidzoid
Copy link
Contributor

I guess this is a JSON issue, and not a jsonlint issue, but maybe we should switch to the npm-installable one:

± cat test.json
{
  "one": "value",
  "two": 2,
  "three": "three",
}
± jsonlint -c test.json
test.json: line 4, col 19, found: '}' - expected: 'STRING'.

@zoidyzoidzoid
Copy link
Contributor

I can't see public access to the source for demjson or a public issue tracker either.

@adrianmoisey
Copy link
Contributor Author

adrianmoisey commented Apr 26, 2016

Seems legit:

$ jsonlint -c has_errors.json
has_errors.json: line 1, col 0, found: 'INVALID' - expected: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['.
$ jsonlint -c has_warnings.json
has_warnings.json: line 2, col 8, found: 'INVALID' - expected: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['.
$ jsonlint -c no_errors.json
{
  "one": "value",
  "two": 2
}

@zoidyzoidzoid
Copy link
Contributor

I take that back.

The errors are definitely not as pretty, and it doesn't have have warnings. The npm one hasn't been updated in two years.

± jsonlint -c -s tests/fixtures/jsonlint/has_warnings.json
tests/fixtures/jsonlint/has_warnings.json: line 2, col 8, found: 'INVALID' - expected: 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '['.

± virtualenv/bin/jsonlint tests/fixtures/jsonlint/has_warnings.json
tests/fixtures/jsonlint/has_warnings.json:2:9: Warning: String literals must use double quotation marks in strict JSON
   |  At line 2, column 9, offset 11
tests/fixtures/jsonlint/has_warnings.json:3:12: Warning: JSON does not allow identifiers to be used as strings: u'three'
   |  At line 3, column 12, offset 32
tests/fixtures/jsonlint/has_warnings.json:3:28: Warning: Strict JSON does not allow a final comma in an object (dictionary) literal
   |  At line 3, column 28, offset 48
   |  Object started at line 1, column 0, offset 0 (AT-START)
tests/fixtures/jsonlint/has_warnings.json: ok, with warnings

@zoidyzoidzoid
Copy link
Contributor

Looking at how Yelp's pre-commit does checking if JSON is correct and compares it to pretty formatted JSON, maybe we should do this ourselves?

Though I guess as a standalone library?

@markstory
Copy link
Owner

@zoidbergwill If you build a tool that can be integrated with lintreview, we could switch tools 😄

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

No branches or pull requests

3 participants