Minor improvement: Check sources#3502
Conversation
| char const* input = R"( | ||
| { | ||
| "language": "Solidity", | ||
| "sources": [] |
There was a problem hiding this comment.
This should result in an error stating it is an invalid input.
There was a problem hiding this comment.
I think its nicely reflected with No input sources specified. - not sufficient?
There was a problem hiding this comment.
Looking at the other tests it seems we do not have any more granular reporting, so it should be fine for the moment. Can revisit in the future.
There was a problem hiding this comment.
Wait a second, [ "aaa", "bbb" ] will be accepted by the empty-ness check but what will sources.getMemberNames() do?
There was a problem hiding this comment.
{
"errors" :
[
{
"component" : "general",
"formattedMessage" : "JSON logic exception: in Json::Value::getMemberNames(), value must be objectValue",
"message" : "JSON logic exception: in Json::Value::getMemberNames(), value must be objectValue",
"severity" : "error",
"type" : "InternalCompilerError"
}
]
}
I guess we need another check for that, another PR?
aarlt
force-pushed
the
minor_fix_no_input_sources_specified
branch
2 times, most recently
from
0eeed12 to
f57349d
Compare
aarlt
changed the title
| Json::Value const& sources = _input["sources"]; | ||
| if (!sources) | ||
|
|
||
| if (sources.isArray()) |
There was a problem hiding this comment.
I'd use !sources.isObject() here.
aarlt
force-pushed
the
minor_fix_no_input_sources_specified
branch
2 times, most recently
from
7d6c2b7 to
ee57997
Compare
| Json::Value const& sources = _input["sources"]; | ||
| if (!sources) | ||
|
|
||
| if (!sources.isObject() && !sources.isNull()) |
There was a problem hiding this comment.
Why do we need isNull here?
Is IsObject true when the value is null?
There was a problem hiding this comment.
@axic nope.. but null is also not an object.. so if there is no sources object at all, it is also not an object.. !sources.isObject() is also true - that's why I do the additional check here.
however, just in case you didn't noticed: I introduced a slightly different error message here: Sources input is not a JSON object. in contrast to Source input is not a JSON object. - from my point of view its nice nice to distinguish between sources is not a json object, and if items of sources - so individual members of the sources object - source is not a json object.
There was a problem hiding this comment.
so if there is no sources object at all, it is also not an object
Ah, yes that makes sense :)
| } | ||
| )"; | ||
| Json::Value result = compile(input); | ||
| BOOST_CHECK(containsError(result, "JSONError", "Sources input is not a JSON object.")); |
There was a problem hiding this comment.
How about Field \"sources\" is not a JSON object (or maybe omit field)?
There was a problem hiding this comment.
Yeah, thats better. With quotes it's much more clear. I would omit field.
|
Please mention this in the changelog, something along the lines of |
|
I think these are the last two changes and we're good to go. |
- returns error, if "sources" is an array, an empty object or not defined - Added new test-cases in test/libsolidity/StandardCompiler.cpp
aarlt
force-pushed
the
minor_fix_no_input_sources_specified
branch
from
ee57997 to
1d4547a
Compare
4 participants
Uh oh!
There was an error while loading. Please reload this page.