feat: serializeJson cheatcode

[vdrg]

Adds a new serializeJson cheatcode which receives an id and a json object string, and internally adds the object to the serialized_jsons map or overwrites an id if it already exists.

Closes #5745

Motivation

Currently there is no way to start from an existing json string and manipulate it with the serializeX cheatcodes (unless you do something hacky like using a nested object and introducing some abstractions to operate over it...).

To be honest, I think it would be better to have "pure" setter cheatcodes that operate over a json string and return the modified version without changing the internal state. But it would need a whole bunch of new functions and could become confusing. Anyways, this is a different discussion (let me know if something like this would actually make sense, I can try to implement it too). Update: I created this issue to discuss: #5764

Solution

The value_key argument of the serialize_json function is now an Option<&str>. When value_key is Some, the behavior stays the same as before. When it is None, the provided string is parsed into a BTree<String, Value> and inserted for the provided object_key.

Also, this is my first ever Rust PR 😱 so I wasn't sure about the best way of doing those nested if lets... maybe match would be better here? All feedback is welcome.

Update: I just saw that this has conflicts with the breaking changes in #4602 . Any ideas on a different name for this function? Maybe serializeJsonFromString?

[Evalir]

actually looks pretty well implemented to me— @mds1 wdyt?

[mds1]

The PR lgtm, thanks!

Update: I just saw that this has conflicts with the breaking changes in #4602 . Any ideas on a different name for this function? Maybe serializeJsonFromString?

But @Evalir let's make sure to get @mattsse / @odyslam to check this out before merging to make sure there's no issues/conflicts between here and #4602—that one has been open a while but does seem like we should get it merged.

Maybe the cheat in this method should be named something like serializeWith or serializeFrom

[vdrg]

Maybe the cheat in this method should be named something like serializeWith or serializeFrom

I've been thinking about this... maybe having both serializeJson(objectKey, json) and serializeJson(objectKey, key, json) is fine and actually makes sense? Because in both cases we are doing the same thing, it's just that in this one the key being "null" means that we are serializing the root object.

[mds1]

Ah #4602 isn't merged yet since it's a breaking change: #4602 (comment)

---

I've been thinking about this... maybe having both serializeJson(objectKey, json) and serializeJson(objectKey, key, json) is fine and actually makes sense? Because in both cases we are doing the same thing, it's just that in this one the key being "null" means that we are serializing the root object.

Hm that seems reasonable to me, @Evalir @mattsse wdyt?

[odyslam]

It looks good to me and is a much needed feature. JSON parsing has many weird edge cases, so use many different JSON files to test it. Document the limitations of JSON parsing a string.

by the way, editing an existing JSON file existed in some twisted form via writeJson. That cheatcode enables the user to write some json object (or value) to replace some key in a json file.

[mattsse]

Ah #4602 isn't merged yet since it's a breaking change: #4602 (comment)

I've been thinking about this... maybe having both serializeJson(objectKey, json) and serializeJson(objectKey, key, json) is fine and actually makes sense? Because in both cases we are doing the same thing, it's just that in this one the key being "null" means that we are serializing the root object.

Hm that seems reasonable to me, @Evalir @mattsse wdyt?

yeah sounds reasonable to me

so no blockers?, supportive of this change

[mds1]

yep seems there's no blockers to me! once merged we'll just need book + forge-std Vm.sol updates too if you don't mind @vdrg :)

[vdrg]

Of course 🫡

[Evalir]

sweet! just catching up—happy to merge this as-is, the suggested reasons make sense to me!

[Evalir]

@vdrg merged! let's make sure we update the book and forge std :)

[vdrg]

Sure! Will prob do over the weekend