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

parseJson with invalid object name reports error in "key" when key parameter is fine #5423

Closed
2 tasks done
Tracked by #3801
DavidOrchard opened this issue Jul 17, 2023 · 4 comments · Fixed by #5439
Closed
2 tasks done
Tracked by #3801
Labels
T-bug Type: bug

Comments

@DavidOrchard
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 (08a629a 2023-06-03T00:14:26.563741000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

The parseJson docs at https://book.getfoundry.sh/cheatcodes/parse-json#signature
contain

// Return the value(s) that correspond to 'key'
vm.parseJson(string memory json, string memory key)

as well as

[JSON Encoding Rules](https://book.getfoundry.sh/cheatcodes/parse-json#json-encoding-rules)
We use the terms number, string, object, array, boolean as they are defined in the [JSON spec](https://www.w3schools.com/js/js_json_datatypes.asp).

A call to parseJson with json that has an object with an error in the object's name field, such as

        string memory json = '{\'timestamp": 1655140035}';

demonstrated in foundry-rs/forge-std#421
erroneously reports this as

FAIL. Reason: key must be a string at line 1 column 2

The 2nd parameter to parseJson is called key (but probably should be called something like JSONPath or jq).

The JSON spec reference erroneously links to W3Schools.com (which should never be the linked to authoritatively as it's poaching the official w3c) instead of a true JSON spec, such as the official ECMA spec at https://www.ecma-international.org/publications-and-standards/standards/ecma-404/.

An official JSON spec starts the object definition with

An object structure is represented as a pair of curly bracket tokens surrounding zero or more name/value pairs.
A name is a string. A single colon token follows each name, separating the name from the value.```

The error message returned should the term `name` or even better `json object name` instead of the overloaded and term `key`.  Ideally, the documentation link to JSON spec should be corrected & the parseJson function 2nd parameter name would be improved.

I've forked and made a PR that adds the failing test. https://github.com/foundry-rs/forge-std/pull/421.

I did the fork & branch against forge-std as I'm new to solidity/forge and couldn't get the foundry/forge solidity tests to run.


@Evalir
Copy link
Member

Evalir commented Jul 18, 2023

Hey hey! Could you expand a bit more on what the error is? It's not really clear to me what the issue is.

@DavidOrchard
Copy link
Author

the error is this string Reason: key must be a string at line 1 column 2. key is incorrect and confusing.

@DaniPopes
Copy link
Member

The error occurs while parsing a key of an object, expecting a string but getting an invalid value. The message is coming from an external library (serde_json), but I guess it could be preceded by "Failed parsing JSON: "

@DavidOrchard
Copy link
Author

I observe that the docs mention the json spec, but said spec defines objects as having name/value pairs, and does not mention keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants