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

Add parseJsonKeys() cheatcode #5252

Merged
merged 6 commits into from
Aug 9, 2023

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Jun 29, 2023

Motivation

Currently there is no way (or at least I didn't find one) to get keys of a JSON object

Solution

Create new cheatcode parseJsonKeys(string calldata json, string calldata key)

Examples:

parseJsonKeys('{"key1": "value1", "key2": "value2"}', '$') // returns ["key1", "key2"]
parseJsonKeys('{"key1": {"inner_key1": "inner_value1", "inner_key2": "inner_value2"}, "value1", "key2": "value2"}', '$.key1') // returns ["inner_key1", "inner_key2"]

@klkvr
Copy link
Member Author

klkvr commented Jun 29, 2023

Actually, this could have been possible just with usage of something like parseJson(jsonString, ".*~"), but jsonpath lib for Rust does not support such operators (freestrings/jsonpath#69)

@klkvr klkvr marked this pull request as draft June 29, 2023 18:59
@klkvr klkvr marked this pull request as ready for review June 29, 2023 19:05
@klkvr klkvr changed the title add parseJsonKeys() cheatcode Add parseJsonKeys() cheatcode Jul 1, 2023
@Evalir Evalir requested review from Evalir and mattsse July 3, 2023 02:44
@klkvr klkvr force-pushed the feature/parse-json-keys-cheatcode branch 2 times, most recently from 7792fbb to c208810 Compare July 11, 2023 22:17
@@ -180,6 +180,7 @@ writeJson(string, string)
writeJson(string, string, string)
parseJson(string)(bytes)
parseJson(string, string)(bytes)
parseJsonKeys(string, string)(string[])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a parseJsonKeys(string)(string[]) to parse the root keys? might be better UX than passing $ as the key, but I don't do tons of jsonpath stuff so maybe $ is common/standardized and we should stick with just that?

@klkvr klkvr force-pushed the feature/parse-json-keys-cheatcode branch from 0d9664b to d650008 Compare July 12, 2023 00:12
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey! sorry for the delay on this, looks great. just a nit! also need to resolve conflicts


let res = value
.as_object()
.unwrap()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use ? instead of unwrapping here so we don't panic

@klkvr klkvr force-pushed the feature/parse-json-keys-cheatcode branch from d650008 to d4a10cd Compare August 3, 2023 22:33
@klkvr
Copy link
Member Author

klkvr commented Aug 3, 2023

hey @Evalir I have pushed resolved conflicts

regarding unwrap(), there is an is_object() check above which guarantees, that if it returns True, than as_object will not return None. Considering this, should I add ? anyways?

@Evalir
Copy link
Member

Evalir commented Aug 9, 2023

hey hey @klkvr coming back to this—yes ideally! even if it's a guarantee it's best to not introduce any panicking. feel free to use wrap_err to add a message to it indicating that it should not fail

Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm as soon as we remove the unwraps!

@klkvr klkvr force-pushed the feature/parse-json-keys-cheatcode branch from d4a10cd to d5606b2 Compare August 9, 2023 16:00
@klkvr
Copy link
Member Author

klkvr commented Aug 9, 2023

hey @Evalir rebased from master and removed unwrap

@Evalir Evalir merged commit 16208aa into foundry-rs:master Aug 9, 2023
20 checks passed
@Evalir
Copy link
Member

Evalir commented Aug 9, 2023

Thank you! Let's add docs for this on the book & forge-std pr @klkvr

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.

3 participants