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

feat(cheatcodes): read length of arbitrary array #8467

Open
2 tasks done
Tracked by #3801
ivanzhelyazkov opened this issue Jul 18, 2024 · 3 comments
Open
2 tasks done
Tracked by #3801

feat(cheatcodes): read length of arbitrary array #8467

ivanzhelyazkov opened this issue Jul 18, 2024 · 3 comments
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge P-low Priority: low T-bug Type: bug T-feature Type: feature

Comments

@ivanzhelyazkov
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (af97b2c 2024-07-18T00:22:15.301937000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

sometime between the latest nightly version (18-07-2024) and this commit: 7bef9ca (abt 3 weeks ago) a change was pushed which breaks the current json parsing:
string memory path = "./test.json";
string memory json = vm.readFile(path);
testCase = parseTestCase(json, "testCase");
string[] memory testCaseString = vm.parseJsonStringArray(json, initialParseString);

now fails with:
[FAIL. Reason: expected string, found JSON object;]

note above code previously worked for a long time (maybe longer than 10 months)

To reproduce, please find attached test which breaks in this repo (along with 22 more after upgrading to latest foundry version):
https://github.com/bancorprotocol/carbon-contracts/blob/317e0e9f785b62b1e73a017c9fe6c38bf2e06d27/test/forge/VortexTestCaseParser.t.sol#L51

@ivanzhelyazkov ivanzhelyazkov added the T-bug Type: bug label Jul 18, 2024
@mattsse
Copy link
Member

mattsse commented Jul 18, 2024

@klkvr likely related to the recent parser features?

@klkvr
Copy link
Member

klkvr commented Jul 18, 2024

So this code relies on parseJsonStringArray successfully parsing arrays of objects. However, the output in this case doesn't make much sense because we are receiving objects without quotes which is basically broken JSON: vm.parseJsonStringArray('[{"a": 1}, {"b": 2}]', "$"); would return ["{a:1}", "{b:2}"]

In linked codebase, this cheatcodes is only used for getting length of a given array by doing something like parseJsonStringArray(...).length. The contents of returned array are never used

I'd prefer to not allow such parsing but it seems that there's basically no other way to get a length of arbitrary array. I think we should either add a cheatcode for this or migrate to smarter jsonpath lib which allows doing e.g. $.array.length().

wdyt @mattsse

@mattsse
Copy link
Member

mattsse commented Jul 19, 2024

I think we should either add a cheatcode for this

this seems reasonable, although not ideal

@zerosnacks zerosnacks added the C-forge Command: forge label Jul 22, 2024
@zerosnacks zerosnacks changed the title New nightly release breaks json parsing feat(cheatcodes): add cheatcode to read length of arbitrary array Jul 22, 2024
@zerosnacks zerosnacks added T-feature Type: feature A-cheatcodes Area: cheatcodes labels Jul 22, 2024
@zerosnacks zerosnacks changed the title feat(cheatcodes): add cheatcode to read length of arbitrary array feat(cheatcodes): read length of arbitrary array Jul 22, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@jenpaff jenpaff removed this from the v1.0.0 milestone Sep 26, 2024
@jenpaff jenpaff added the P-low Priority: low label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge P-low Priority: low T-bug Type: bug T-feature Type: feature
Projects
None yet
Development

No branches or pull requests

5 participants