Skip to content

Commit

Permalink
lint, sync: make JSON parsing stricter (#346)
Browse files Browse the repository at this point in the history
The Nim stdlib JSON modules parse JSON in a lenient way: they silently
ignore:
- a comma after the last item in an array or object
- a line comment with `//` 
- a potentially-multi-line comment with `/* */`

This is useful for parsing real-world JSON, but is bad for our use case
of validating JSON. JSON that contains a trailing comma or such comments
is technically invalid [1], and less portable. Most JSON parsers produce
an error for a trailing comma, including the Ruby library that Exercism
uses. Therefore `configlet lint` should produce an error for a trailing
comma, so that a track doesn't merge a PR that contains one.

This commit adds our own copies of the Nim JSON modules to our repo, and
patches them to raise an exception for these cases. Each module was
copied from the latest version on Nim's `version-1-4` branch. In the
future, we will pull in upstream changes to these files when we update
the Nim version.

This approach is better than calling something like `jq` because it:
- doesn't require/prefer the user to have an extra program installed
- doesn't special-case running in CI
- doesn't require refactoring our codebase
- is much faster

We use the somewhat-obscure `nimscript.patchFile` [2] mechanism to
override the location of the `json` and `parsejson` modules, so that we
can keep `std/json` in our import statements.

Instead of the `patchFile` approach, it would also be possible to apply
some transformation to the AST of the relevant procs at compile-time,
but that's less readable and harder to maintain.

Our approach might be obsoleted in the future - for example, if we ever
move to some other JSON library, or if the upstream `parseJson` gains
e.g. an `allowTrailingCommas = true` argument.

[1] See e.g. https://www.json.org/json-en.html
[2] https://nim-lang.org/docs/nimscript.html#patchFile%2Cstring%2Cstring%2Cstring

Closes: #345
See also: #312
  • Loading branch information
ee7 committed Jun 10, 2021
1 parent 4b0d76f commit 15c8403
Show file tree
Hide file tree
Showing 5 changed files with 1,993 additions and 0 deletions.
4 changes: 4 additions & 0 deletions config.nims
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ switch("styleCheck", "hint")
hint("Name", on)
switch("experimental", "strictFuncs")

# Replace the stdlib JSON modules with our own stricter versions.
patchFile("stdlib", "json", "src/patched_stdlib/json")
patchFile("stdlib", "parsejson", "src/patched_stdlib/parsejson")

if defined(release):
switch("opt", "size")
switch("passC", "-flto")
Expand Down
Loading

0 comments on commit 15c8403

Please sign in to comment.