Line numbers from JSON imports

[pjjw]

tl;dr how possible is it to add line number references for records imported from json?

use case is this- we have a number of uh "organically evolved" json inputs in our environment used to describe, generally, service deployment configurations, currently only validated at runtime. over time, they've become rather beastly.

there's now a need to open these up to editing by a much larger audience, so we need to put some guardrails/validation on these json blobs. there was a first attempt using cue, and it works, but the error messages are absolutely atrocious. i'm trying to make the argument that we should use nickel for contract validation at first, using nickel itself to express these definitions eventually. i got to putting together a contract definition, and this works great- being able to customize error messages is quite nice.

however, we'll need to keep the format-of-record json for quite a while, and i've noticed that while nickel is happy to import json into a record, all line number information gets stripped off, making contract validation error messages not super useful. below is an example of output from a service definition with an intentional error:

$ echo '(import "in.json") | (import "services.schema.ncl") ' | nickel export
error: contract broken by the value of `path`: not a valid http path
   ┌─ /Users/peter.woodman/src/chef-repo/ncl/services.schema.ncl:31:32
   │
31 │                         path | HttpPath,
   │                                -------- expected type
   │
   ┌─ <unknown> (generated by evaluation):1:1
   │
 1 │ ""
   │ -- evaluated to this value

nice to know what the problem is, but not easy to run down where the problem is without extra information and a bit of a hunt. this is a lot nicer when importing a record from a .ncl file, with line numbers and everything:

$ echo 'import "in.json"' | nickel eval >| in.ncl
$ echo '(import "in.ncl") | (import "services.schema.ncl") ' | nickel export
error: contract broken by the value of `path`: not a valid http path
   ┌─ /Users/peter.woodman/src/chef-repo/ncl/services.schema.ncl:31:32
   │
31 │                         path | HttpPath,
   │                                -------- expected type
   │
   ┌─ /Users/peter.woodman/src/chef-repo/ncl/in.ncl:17:64
   │
17 │                                   http_health_check = { path = "", },
   │                                                                ^^ applied to this expression
   │
   ┌─ <unknown> (generated by evaluation):1:1
   │
 1 │ ""
   │ -- evaluated to this value

if nickel provided line info for the imported json when reporting a contract issue, i think there'd be a pretty compelling argument to use it over the other things we've been trying. how difficult would this be?

[pjjw]

oops, i swear i searched the issue list before posting, but after poking around just a little bit more, i think i see evidence you all took a pass at this in #1382, but ran into issues making use of it - is getting position info working for external formats on anyone's radar at the moment?

[pjjw]

it's the former- the conversion does significant reordering vs. json- in the example i listed in the original post, the line with the problem in the JSON is line 33, vs line 17 in the converted nickel file. in trivial files, this is tractable, but in much larger ones, it's not. there's already some momentum around usage of cue here, and my most objective argument against it is that error reporting is a lot better in nickel (e.g. we can spit out specific natural-language explanations on contract failure, vs. cue's errors, which pretty frequently don't even address the line that's causing the problem), but that argument falls apart if nickel swallows line numbers in errors on imports of non-nickel data.

[pjjw]

i had something here a bit about what i thought was a confusing error message, but that problem was in me misreading it, apologies- i'd misread the "note" segments as call-stack descent and misidentified the causal statement.

[yannham]

Ah, it's too bad, because the conversion on paper should very simple: just turn JSON's : to =. Of course it's made a bit more annoying by strings, since : there should be left untouched, but I wonder if there's a short awk script that could do it. Otherwise, I'm going to take a look at something such as https://docs.rs/json-spanned-value, which might solve our problem for JSON (and there's one for YAML).

[pjjw]

yeah, that crate was what gave me hope that this might be not a huge lift

[yannham]

I took a look, and the crate doesn't seem to be maintained anymore, unfortunately. They work around serde not having any notion of position by wrapping the input stream with a special reader which records the current position in some shared thread local storage, and some parts look rather... hacky 😅 although the code isn't that large in total.

I wonder if, at least for JSON, it wouldn't be easier to just write a custom tokenizer - just because JSON is so simple and so close to a subset of Nickel - and feed that to the normal parser. That wouldn't scale to other formats, though, and might perform worse than the serde parser. But it's probably not very difficult.

[yannham]

Hello @pjjw, I know this may come in late if you have already comitted to CUE, but FYI position information from serialized content is coming for TOML (#1949) and there is a clear path forward for YAML, and thus JSON (just using the YAML parser, as this is a superset of JSON), see Ethiraric/yaml-rust2#27 (whether it's upstreamed or not doesn't matter - we can still replicate the change locally if this doesn't make it in the parent crate fast enough). The release after the next one (1.7.0 is coming this week, so that will be 1.8.0 in approximately 6 weeks) will thus most likely have span information for TOML, YAML and JSON.

[pjjw]

beautiful! we have pushed CUE out to some places but the rather c++ templatey error messages have not made it super popular. thats great news.