CertainLach / jrsonnet

Rust implementation of Jsonnet language
MIT License
290 stars 33 forks source link

Fix: field in super should not be interpreted with standalone-super concept. #154

Closed CertainLach closed 3 months ago

CertainLach commented 3 months ago

Jsonnet spec defines special behavior for super.field and field in super expressions, while in jrsonnet it is implemented by standalone-super concept, with optimizations in case of super.field.

However, standalone-super is conflicting with field in super behavior, so special handling is needed for this expression.

Fixes: #153

CertainLach commented 3 months ago

Note that jrsonnet-specific value-path is not affected, and

local upper = {
  'bar': super?.baz ?? 'nope',
};

local obj = {
  foo+: upper,
};

obj

Already works as intended

davidreuss commented 3 months ago

Oh, awesome -- i can confirm that this works, and removes the problem in our project ... 🎉

Now ... i does look like we may be hitting other problems further along in evaluation now, but ... this particular problem is solved 🥇

I'm currently staring at a stack overflow, but ... i'll open another issue for that, when i have a good reproduction case, if possible to figure that out.

Thanks so much!


edit: evaluation succeeds if i run it with --max-stack 1000 ... looks like there's maybe some code we need to take a deeper look at 😂

CertainLach commented 3 months ago

edit: evaluation succeeds if i run it with --max-stack 1000 ... looks like there's maybe some code we need to take a deeper look at 😂

All implementations, sjsonnet, jrsonnet and go-jsonnet/jsonnet-cpp (Those two match in that regard) have different stack frame assignment methods, so this is normal if jrsonnet fails with lower max-stack value.

This is not necessary you have some things to fix at your side, just use larger max-stack value. I might increase default max-stack value, since jrsonnet implements infinite recursion detection, and it is less useful here now.

CertainLach commented 3 months ago

but it has to be said that we're also removing a bunch of validation which we have natively available in our own build of go-jsonnet in this calculation

Jrsonnet has great (If I say so) api for embedding, and Rust makes it very easy to write bug-free code, so you might consider writing your checking utilities in Rust

Examples of jrsonnet embedding: https://github.com/uniquenetwork/chainql https://github.com/uniquenetwork/baedeker https://github.com/CertainLach/jrsonnet/tree/master/crates/jrsonnet-stdlib/src (stdlib has no magic, you can write your own stdlib with no problems)

I also plan to open-source my own jrsonnet wrapper with useful utilities (Jsonschema validation, by-url imports with integrated jsonnet-bundler, etc) based on my kubernetes deployment tool (Which is also closed-source right now, only very old prototype is available right now: https://github.com/CertainLach/hayasaka), so if you have any wanted features - you may suggest them here, and maybe I'll add jrsonnetx binary/jrsonnet-stdlibx library with those features included.

davidreuss commented 3 months ago

json schema validation, and kubernetes manifest validation (we currently embed kubeconform) are the two big things we need right now … so — that would be very interesting indeed 😄

integrated jsonnet-bundler sounds intriguing ad well, our current vendoring solution is also quite problematic in a multitude of ways .. 🤣