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

Custom JSON parsing #115

Merged
merged 22 commits into from
May 24, 2022
Merged

Custom JSON parsing #115

merged 22 commits into from
May 24, 2022

Conversation

samhh
Copy link
Member

@samhh samhh commented May 2, 2022

Depends #114.

Whilst implementing our own JSON parser seems a bit wild, we don't really need a full JSON parser on account of our input following a fairly rigid schema. This is mirrored in the relative simplicity of the added parser. And it gives us a great deal of flexibility, enabling as of this PR:

  • Useful file and line information on reported errors
  • Reporting of duplicate keys
  • Reporting of unexpected keys

As with #114, both of the key error types are recoverable, and this recovery interoperates with other recoverable errors such as in message parsing. Note that failing to parse JSON is generally not recoverable.

Thus, given via intlc compile -l foo ./x-all.json:

{
  "dupeKey": {
    "message": ""
  },
  "badMsg": {
    "message": "{x, bad}"
  },
  "dupeKey": {
    "message": ""
  },
  "ok": {
    "message": ""
  },
  "badInnerKey": {
    "message": "",
    "beckand": ""
  }
}

Before:

badMsg:1:5:
  |
1 | {x, bad}
  |     ^^^^
unexpected "bad}"
expecting "boolean", "date", "number", "plural", "select", "selectordinal", "time", or white space

After:

./x-all.json:6:21:
  |
6 |     "message": "{x, bad}"
  |                     ^^^^^^
unexpected "bad}"<newline>  },<newline>  "
expecting "boolean", "date", "number", "plural", "select", "selectordinal", "time", or white space

./x-all.json:8:3:
  |
8 |   "dupeKey": {
  |   ^
Duplicate key: "dupeKey"

./x-all.json:16:5:
   |
16 |     "beckand": ""
   |     ^
unexpected '"'
expecting ""backend"", ""description"", '}', or white space

Detaching ourselves from Aeson decoding suggests we could and should implement our own encoder, which is really just going to be another "compiler" in the spirit of those we already have. This would solve our inability to pretty print flattened JSON output.

I'd additionally like to slightly modify the error output to include an additional line or two of context so that the offending key is visible (#116). Further refinements could also be made to various (sub)parsers to improve error output legibility.

@samhh samhh marked this pull request as ready for review May 3, 2022 12:59
@samhh samhh requested a review from a team as a code owner May 3, 2022 12:59
@samhh samhh requested review from nim442 and removed request for a team May 3, 2022 12:59
@samhh
Copy link
Member Author

samhh commented May 3, 2022

For additional confidence given this is a big diff, I've run this against everything in Unsplash web and there are no errors. Inserted errors are caught.

@samhh samhh mentioned this pull request May 3, 2022
@samhh
Copy link
Member Author

samhh commented May 5, 2022

Note to self: Add some end-to-end tests for multiple error reporting across different parser error types (keys, ICU, etc). Context: #114 (comment)

Base automatically changed from multi-error to master May 5, 2022 11:46
@samhh
Copy link
Member Author

samhh commented May 11, 2022

@OliverJAsh Your suggestion for further tests caught two bugs with error reporting! One new and one preexisting. 😎

@samhh samhh merged commit 866e9b1 into master May 24, 2022
@samhh samhh deleted the inhouse-json branch May 24, 2022 17:06
@samhh samhh mentioned this pull request May 24, 2022
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

Successfully merging this pull request may close these issues.

1 participant