exercism / exercism/configlet

lint: consider the JSON parsing/deserialization design

Open
#312 14 comments 0 reactions 0 assignees View on GitHub
cmd: lint kind: design
Dominant language
Nim
Stars
23
Forks
17
Avg merge
13h 57m
Merged PRs (30d)
1

Description

Main options:

1. [`std/json`](https://nim-lang.github.io/Nim/json.html)
1a. The approach so far: parse into a `JsonNode` and work only with that.
1b. Parse into a `JsonNode`, then unmarshall into some object using [`to`](https://nim-lang.github.io/Nim/json.html#to%2CJsonNode%2Ctypedesc%5BT%5D).
1c. Plus [`std/jsonutils`](https://nim-lang.github.io/Nim/jsonutils.html)
1. [`Araq/packedjson`](https://github.com/Araq/packedjson) - keeps everything as a string. Lower memory usage than `std/json`, and sometimes faster.
1. [`planetis-m/eminim`](https://github.com/planetis-m/eminim) - deserializes using `std/streams` directly to an `object`. Doesn't fully support object variants, but maybe that isn't a problem for us.
1. [`status-im/nim-json-serialization`](https://github.com/status-im/nim-json-serialization) - deserializes using [`nim-faststreams`](https://github.com/status-im/nim-faststreams) directly to an `object`. Probably the most mature third-party option. Currently has a large dependency tree, including `chronos` and `bearssl`.
1. [`treeform/jsony`](https://github.com/treeform/jsony) - deserializes from `string` directly to an `object`.

(Note that [`disruptek/jason`](https://github.com/disruptek/jason) is serialization-only).

There are also some more obscure ones that I haven't tried, and don't know anything about:

- [`gabbhack/deser`](https://github.com/gabbhack/deser) and [`gabbhack/deser_json`](https://github.com/gabbhack/deser_json)
- [`Q-Master/packets`](https://github.com/Q-Master/packets.nim)
- [`xomachine/NESM`](https://xomachine.gitlab.io/NESM/)

Some of the above are possibly too lenient or require special handling in some edge cases.

Summary:

| Library | Permits a trailing comma? | Permits comment? | Duplicate key handling |
| -------------------- | ------------------------- | ------------------ | ---------------------- |
| Ruby stdlib json | :x: | :white_check_mark: | Uses last value |
| `std/json` | :white_check_mark: | :white_check_mark: | Uses last value |
| `std/json` [patched](https://github.com/exercism/configlet/commit/15c8403) | :x: | :x: | Uses last value |
| `packedjson` | :white_check_mark: | :white_check_mark: | Uses **first** value |
| `eminim` | :white_check_mark: | :white_check_mark: | Uses last value |
| `json_serialization` | :x: | :white_check_mark: | Uses last value |
| `jsony` | :white_check_mark: | :x: | Uses last value |

For example:

- There is no correct behaviour for a duplicate key - one library may produce an error, another may silently use the value of the first key, and another may silently use the value of the last key.
- `std/json` permits a trailing comma, and comments with `//` and `/* */`. This is the main reason that it took a while to tick the boxes for "the file must be valid JSON" in https://github.com/exercism/configlet/issues/249. But we now have own patched `std/json` with stricter parsing. `configlet lint` must exit with a non-zero exit code for a trailing comma because the Ruby library that parses it later produces an error for a trailing comma.
- Some libraries may use a default value when the key is missing, which we might want to distinguish from e.g. a value that is the empty string.
- Edge cases around a literal `null`.

See also:

- https://einarwh.wordpress.com/2020/05/08/on-the-complexity-of-json-serialization/
- https://labs.bishopfox.com/tech-blog/an-exploration-of-json-interoperability-vulnerabilities

I'd suggest that `jsony` or `nim-json-serialization` might be best in the long-term. But maybe it's better to stick with the current approach until we've implemented all the linting rules, and refactor it later.

One advantage of the current approach is that it's more low-level, which might better ensure that we're "checking the JSON file itself" rather than "checking that each value is valid when parsed with library X".

Contributor guide

No contributing guide indexed for this repository

Research direction

No implementation files, tests, or entry points are named. Start by locating the current JSON parsing used by configlet lint, then compare its behavior with the listed libraries and edge cases; this issue is done only after the parsing approach and required behavior are decided and documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
nim
Domain
cli, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.