Merge pull request #11976 from ethereum/json-cleanup

Some cleanup and testing for JSON
2023-10-03 13:03:40 +00:00 · 2021-09-24 19:03:45 +01:00 · 2021-09-24 19:03:45 +01:00 · 3c8846e669
commit 3c8846e669
parent 55467c1cca b5e68df3cd
8 changed files with 128 additions and 46 deletions
--- a/libevmasm/Assembly.cpp
+++ b/libevmasm/Assembly.cpp
@ -203,7 +203,7 @@ string Assembly::assemblyString(StringMap const& _sourceCodes) const

 Json::Value Assembly::createJsonValue(string _name, int _source, int _begin, int _end, string _value, string _jumpType)
 {
-	Json::Value value;
+	Json::Value value{Json::objectValue};
 	value["name"] = _name;
 	value["source"] = _source;
 	value["begin"] = _begin;
@ -225,8 +225,9 @@ string Assembly::toStringInHex(u256 _value)
 Json::Value Assembly::assemblyJSON(map<string, unsigned> const& _sourceIndices) const
 {
 	Json::Value root;
+	root[".code"] = Json::arrayValue;

-	Json::Value& collection = root[".code"] = Json::arrayValue;
+	Json::Value& collection = root[".code"];
 	for (AssemblyItem const& i: m_items)
 	{
 		int sourceIndex = -1;
@ -320,7 +321,8 @@ Json::Value Assembly::assemblyJSON(map<string, unsigned> const& _sourceIndices)

 	if (!m_data.empty() || !m_subs.empty())
 	{
-		Json::Value& data = root[".data"] = Json::objectValue;
+		root[".data"] = Json::objectValue;
+		Json::Value& data = root[".data"];
 		for (auto const& i: m_data)
 			if (u256(i.first) >= m_subs.size())
 				data[toStringInHex((u256)i.first)] = util::toHex(i.second);
--- a/libsolidity/interface/ABI.cpp
+++ b/libsolidity/interface/ABI.cpp
@ -56,7 +56,7 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef)

 		FunctionType const* externalFunctionType = it.second->interfaceFunctionType();
 		solAssert(!!externalFunctionType, "");
-		Json::Value method;
+		Json::Value method{Json::objectValue};
 		method["type"] = "function";
 		method["name"] = it.second->declaration().name();
 		method["stateMutability"] = stateMutabilityToString(externalFunctionType->stateMutability());
@ -80,7 +80,7 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef)
 		FunctionType constrType(*constructor);
 		FunctionType const* externalFunctionType = constrType.interfaceFunctionType();
 		solAssert(!!externalFunctionType, "");
-		Json::Value method;
+		Json::Value method{Json::objectValue};
 		method["type"] = "constructor";
 		method["stateMutability"] = stateMutabilityToString(externalFunctionType->stateMutability());
 		method["inputs"] = formatTypeList(
@ -96,23 +96,22 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef)
 		{
 			auto const* externalFunctionType = FunctionType(*fallbackOrReceive).interfaceFunctionType();
 			solAssert(!!externalFunctionType, "");
-			Json::Value method;
+			Json::Value method{Json::objectValue};
 			method["type"] = TokenTraits::toString(fallbackOrReceive->kind());
 			method["stateMutability"] = stateMutabilityToString(externalFunctionType->stateMutability());
 			abi.emplace(std::move(method));
 		}
 	for (auto const& it: _contractDef.interfaceEvents())
 	{
-		Json::Value event;
+		Json::Value event{Json::objectValue};
 		event["type"] = "event";
 		event["name"] = it->name();
 		event["anonymous"] = it->isAnonymous();
-		Json::Value params(Json::arrayValue);
+		Json::Value params{Json::arrayValue};
 		for (auto const& p: it->parameters())
 		{
 			Type const* type = p->annotation().type->interfaceType(false);
 			solAssert(type, "");
-			Json::Value input;
 			auto param = formatType(p->name(), *type, *p->annotation().type, false);
 			param["indexed"] = p->isIndexed();
 			params.append(std::move(param));
@ -123,7 +122,7 @@ Json::Value ABI::generate(ContractDefinition const& _contractDef)

 	for (ErrorDefinition const* error: _contractDef.interfaceErrors())
 	{
-		Json::Value errorJson;
+		Json::Value errorJson{Json::objectValue};
 		errorJson["type"] = "error";
 		errorJson["name"] = error->name();
 		errorJson["inputs"] = Json::arrayValue;
@ -151,7 +150,7 @@ Json::Value ABI::formatTypeList(
 	bool _forLibrary
 )
 {
-	Json::Value params(Json::arrayValue);
+	Json::Value params{Json::arrayValue};
 	solAssert(_names.size() == _encodingTypes.size(), "Names and types vector size does not match");
 	solAssert(_names.size() == _solidityTypes.size(), "");
 	for (unsigned i = 0; i < _names.size(); ++i)
@ -169,7 +168,7 @@ Json::Value ABI::formatType(
 	bool _forLibrary
 )
 {
-	Json::Value ret;
+	Json::Value ret{Json::objectValue};
 	ret["name"] = _name;
 	ret["internalType"] = _solidityType.toString(true);
 	string suffix = (_forLibrary && _encodingType.dataStoredIn(DataLocation::Storage)) ? " storage" : "";
--- a/libsolidity/interface/CompilerStack.cpp
+++ b/libsolidity/interface/CompilerStack.cpp
@ -1426,7 +1426,7 @@ CompilerStack::Source const& CompilerStack::source(string const& _sourceName) co

 string CompilerStack::createMetadata(Contract const& _contract, bool _forIR) const
 {
-	Json::Value meta;
+	Json::Value meta{Json::objectValue};
 	meta["version"] = 1;
 	meta["language"] = m_importedSources ? "SolidityAST" : "Solidity";
 	meta["compiler"]["version"] = VersionStringStrict;
--- a/libsolidity/interface/Natspec.cpp
+++ b/libsolidity/interface/Natspec.cpp
@ -37,7 +37,7 @@ using namespace solidity::frontend;

 Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef)
 {
-	Json::Value doc;
+	Json::Value doc{Json::objectValue};

 	doc["version"] = Json::Value(c_natspecVersion);
 	doc["kind"]    = Json::Value("user");
@ -50,7 +50,7 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef)
 		if (!value.empty())
 		{
 			// add the constructor, only if we have any documentation to add
-			Json::Value user;
+			Json::Value user{Json::objectValue};
 			user["notice"] = Json::Value(value);
 			doc["methods"]["constructor"] = user;
 		}
@ -90,7 +90,7 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef)
 		string value = extractDoc(error->annotation().docTags, "notice");
 		if (!value.empty())
 		{
-			Json::Value errorDoc;
+			Json::Value errorDoc{Json::objectValue};
 			errorDoc["notice"] = value;
 			doc["errors"][error->functionType(true)->externalSignature()].append(move(errorDoc));
 		}
@ -225,7 +225,10 @@ Json::Value Natspec::extractCustomDoc(multimap<string, DocTag> const& _tags)
 	for (auto const& [tag, value]: _tags)
 		if (boost::starts_with(tag, "custom"))
 			concatenated[tag] += value.content;
-	Json::Value result;
+	// We do not want to create an object if there are no custom tags found.
+	if (concatenated.empty())
+		return Json::nullValue;
+	Json::Value result{Json::objectValue};
 	for (auto& [tag, value]: concatenated)
 		result[tag] = move(value);
 	return result;
--- a/libsolidity/interface/StandardCompiler.cpp
+++ b/libsolidity/interface/StandardCompiler.cpp
@ -59,7 +59,7 @@ Json::Value formatError(
 	Json::Value const& _secondarySourceLocation = Json::Value()
 )
 {
-	Json::Value error = Json::objectValue;
+	Json::Value error{Json::objectValue};
 	error["type"] = _type;
 	error["component"] = _component;
 	error["severity"] = Error::formatErrorSeverityLowercase(_severity);
@ -74,7 +74,7 @@ Json::Value formatError(

 Json::Value formatFatalError(string const& _type, string const& _message)
 {
-	Json::Value output = Json::objectValue;
+	Json::Value output{Json::objectValue};
 	output["errors"] = Json::arrayValue;
 	output["errors"].append(formatError(Error::Severity::Error, _type, "general", _message));
 	return output;
@ -82,23 +82,22 @@ Json::Value formatFatalError(string const& _type, string const& _message)

 Json::Value formatSourceLocation(SourceLocation const* location)
 {
-	Json::Value sourceLocation;
-	if (location && location->sourceName)
-	{
-		sourceLocation["file"] = *location->sourceName;
-		sourceLocation["start"] = location->start;
-		sourceLocation["end"] = location->end;
-	}
+	if (!location || !location->sourceName)
+		return Json::nullValue;

+	Json::Value sourceLocation{Json::objectValue};
+	sourceLocation["file"] = *location->sourceName;
+	sourceLocation["start"] = location->start;
+	sourceLocation["end"] = location->end;
 	return sourceLocation;
 }

 Json::Value formatSecondarySourceLocation(SecondarySourceLocation const* _secondaryLocation)
 {
 	if (!_secondaryLocation)
-		return {};
+		return Json::nullValue;

-	Json::Value secondarySourceLocation = Json::arrayValue;
+	Json::Value secondarySourceLocation{Json::arrayValue};
 	for (auto const& location: _secondaryLocation->infos)
 	{
 		Json::Value msg = formatSourceLocation(&location.second);
@ -330,7 +329,7 @@ bool isIRRequested(Json::Value const& _outputSelection)

 Json::Value formatLinkReferences(std::map<size_t, std::string> const& linkReferences)
 {
-	Json::Value ret(Json::objectValue);
+	Json::Value ret{Json::objectValue};

 	for (auto const& ref: linkReferences)
 	{
@ -345,7 +344,7 @@ Json::Value formatLinkReferences(std::map<size_t, std::string> const& linkRefere
 		Json::Value fileObject = ret.get(file, Json::objectValue);
 		Json::Value libraryArray = fileObject.get(name, Json::arrayValue);

-		Json::Value entry = Json::objectValue;
+		Json::Value entry{Json::objectValue};
 		entry["start"] = Json::UInt(ref.first);
 		entry["length"] = 20;

@ -359,7 +358,7 @@ Json::Value formatLinkReferences(std::map<size_t, std::string> const& linkRefere

 Json::Value formatImmutableReferences(map<u256, pair<string, vector<size_t>>> const& _immutableReferences)
 {
-	Json::Value ret(Json::objectValue);
+	Json::Value ret{Json::objectValue};

 	for (auto const& immutableReference: _immutableReferences)
 	{
@ -367,7 +366,7 @@ Json::Value formatImmutableReferences(map<u256, pair<string, vector<size_t>>> co
 		Json::Value array(Json::arrayValue);
 		for (size_t byteOffset: byteOffsets)
 		{
-			Json::Value byteRange(Json::objectValue);
+			Json::Value byteRange{Json::objectValue};
 			byteRange["start"] = Json::UInt(byteOffset);
 			byteRange["length"] = Json::UInt(32); // immutable references are currently always 32 bytes wide
 			array.append(byteRange);
@ -386,7 +385,7 @@ Json::Value collectEVMObject(
 	function<bool(string)> const& _artifactRequested
 )
 {
-	Json::Value output = Json::objectValue;
+	Json::Value output{Json::objectValue};
 	if (_artifactRequested("object"))
 		output["object"] = _object.toHex();
 	if (_artifactRequested("opcodes"))
--- a/libsolutil/JSON.h
+++ b/libsolutil/JSON.h
@ -38,8 +38,8 @@ struct JsonFormat
 {
 	enum Format
 	{
-		Compact,
-		Pretty
+		Compact, // No unnecessary whitespace (including new lines and indentation)
+		Pretty,  // Nicely indented, with new lines
 	};

 	static constexpr uint32_t defaultIndent = 2;
--- a/test/libsolidity/SolidityNatspecJSON.cpp
+++ b/test/libsolidity/SolidityNatspecJSON.cpp
@ -63,8 +63,8 @@ public:

 		BOOST_CHECK_MESSAGE(
 			expectedDocumentation == generatedDocumentation,
-			"Expected:\n" << expectedDocumentation.toStyledString() <<
-			"\n but got:\n" << generatedDocumentation.toStyledString()
+			"Expected:\n" << util::jsonPrettyPrint(expectedDocumentation) <<
+			"\n but got:\n" << util::jsonPrettyPrint(generatedDocumentation)
 		);
 	}

@ -2089,8 +2089,8 @@ BOOST_AUTO_TEST_CASE(dev_explicit_inehrit_complex)

 	BOOST_CHECK_MESSAGE(
 		expectedDocumentation == generatedDocumentation,
-		"Expected:\n" << expectedDocumentation.toStyledString() <<
-		"\n but got:\n" << generatedDocumentation.toStyledString()
+		"Expected:\n" << util::jsonPrettyPrint(expectedDocumentation) <<
+		"\n but got:\n" << util::jsonPrettyPrint(generatedDocumentation)
 	);
 }

--- a/test/libsolutil/JSON.cpp
+++ b/test/libsolutil/JSON.cpp
@ -33,6 +33,48 @@ namespace solidity::util::test

 BOOST_AUTO_TEST_SUITE(JsonTest, *boost::unit_test::label("nooptions"))

+BOOST_AUTO_TEST_CASE(json_types)
+{
+	auto check = [](Json::Value value, string const& expectation) {
+		BOOST_CHECK(jsonCompactPrint(value) == expectation);
+	};
+
+	Json::Value value;
+	BOOST_CHECK(value.empty());
+	value = {};
+	BOOST_CHECK(value.empty());
+	value = Json::Value();
+	BOOST_CHECK(value.empty());
+	value = Json::nullValue;
+	BOOST_CHECK(value.empty());
+
+	check(value, "null");
+	check({}, "null");
+	check(Json::Value(), "null");
+	check(Json::nullValue, "null");
+	check(Json::objectValue, "{}");
+	check(Json::arrayValue, "[]");
+	check(Json::UInt(1), "1");
+	check(Json::UInt(-1), "4294967295");
+	check(Json::UInt64(1), "1");
+	check(Json::UInt64(-1), "18446744073709551615");
+	check(Json::LargestUInt(1), "1");
+	check(Json::LargestUInt(-1), "18446744073709551615");
+	check(Json::LargestUInt(0xffffffff), "4294967295");
+	check(Json::Value("test"), "\"test\"");
+	check("test", "\"test\"");
+	check(true, "true");
+
+	value = Json::objectValue;
+	value["key"] = "value";
+	check(value, "{\"key\":\"value\"}");
+
+	value = Json::arrayValue;
+	value.append(1);
+	value.append(2);
+	check(value, "[1,2]");
+}
+
 BOOST_AUTO_TEST_CASE(json_pretty_print)
 {
 	Json::Value json;
@ -43,6 +85,8 @@ BOOST_AUTO_TEST_CASE(json_pretty_print)
 	json["1"] = 1;
 	json["2"] = "2";
 	json["3"] = jsonChild;
+	json["4"] = "ऑ ऒ ओ औ क ख";
+	json["5"] = "\xff";

 	BOOST_CHECK(
 	"{\n"
@ -52,7 +96,9 @@ BOOST_AUTO_TEST_CASE(json_pretty_print)
 	"  {\n"
 	"    \"3.1\": \"3.1\",\n"
 	"    \"3.2\": 2\n"
-	"  }\n"
+	"  },\n"
+	"  \"4\": \"\\u0911 \\u0912 \\u0913 \\u0914 \\u0915 \\u0916\",\n"
+	"  \"5\": \"\\ufffd\"\n"
 	"}" == jsonPrettyPrint(json));
 }

@ -66,26 +112,32 @@ BOOST_AUTO_TEST_CASE(json_compact_print)
 	json["1"] = 1;
 	json["2"] = "2";
 	json["3"] = jsonChild;
+	json["4"] = "ऑ ऒ ओ औ क ख";
+	json["5"] = "\xff";

-	BOOST_CHECK("{\"1\":1,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":2}}" == jsonCompactPrint(json));
+	BOOST_CHECK("{\"1\":1,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":2},\"4\":\"\\u0911 \\u0912 \\u0913 \\u0914 \\u0915 \\u0916\",\"5\":\"\\ufffd\"}" == jsonCompactPrint(json));
 }

 BOOST_AUTO_TEST_CASE(parse_json_strict)
 {
+	// In this test we check conformance against JSON.parse (https://tc39.es/ecma262/multipage/structured-data.html#sec-json.parse)
+	// and ECMA-404 (https://www.ecma-international.org/publications-and-standards/standards/ecma-404/)
+
 	Json::Value json;
 	std::string errors;

-	// just parse a valid json input
+	// Just parse a valid json input
 	BOOST_CHECK(jsonParseStrict("{\"1\":1,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":2}}", json, &errors));
 	BOOST_CHECK(json["1"] == 1);
 	BOOST_CHECK(json["2"] == "2");
 	BOOST_CHECK(json["3"]["3.1"] == "3.1");
 	BOOST_CHECK(json["3"]["3.2"] == 2);

-	// trailing garbage is not allowed in strict-mode
+	// Trailing garbage is not allowed in ECMA-262
 	BOOST_CHECK(!jsonParseStrict("{\"1\":2,\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":3}}}}}}}}}}", json, &errors));

-	// comments are allowed in strict-mode? - that's strange...
+	// Comments are not allowed in ECMA-262
+	// ... but JSONCPP allows them
 	BOOST_CHECK(jsonParseStrict(
 		"{\"1\":3, // awesome comment\n\"2\":\"2\",\"3\":{\"3.1\":\"3.1\",\"3.2\":5}}", json, &errors
 	));
@ -94,15 +146,42 @@ BOOST_AUTO_TEST_CASE(parse_json_strict)
 	BOOST_CHECK(json["3"]["3.1"] == "3.1");
 	BOOST_CHECK(json["3"]["3.2"] == 5);

-	// root element can only be object or array
+	// According to ECMA-404 object, array, number, string, true, false, null are allowed
+	// ... but JSONCPP disallows value types
 	BOOST_CHECK(jsonParseStrict("[]", json, &errors));
+	BOOST_CHECK(json.isArray());
 	BOOST_CHECK(jsonParseStrict("{}", json, &errors));
+	BOOST_CHECK(json.isObject());
 	BOOST_CHECK(!jsonParseStrict("1", json, &errors));
+	// BOOST_CHECK(json.isNumeric());
 	BOOST_CHECK(!jsonParseStrict("\"hello\"", json, &errors));
+	// BOOST_CHECK(json.isString());
+	BOOST_CHECK(!jsonParseStrict("true", json, &errors));
+	// BOOST_CHECK(json.isBool());
+	BOOST_CHECK(!jsonParseStrict("null", json, &errors));
+	// BOOST_CHECK(json.isNull());

-	// non-UTF-8 escapes allowed??
+	// Single quotes are also disallowed by ECMA-404
+	BOOST_CHECK(!jsonParseStrict("'hello'", json, &errors));
+	// BOOST_CHECK(json.isString());
+
+	// Only string keys in objects are allowed in ECMA-404
+	BOOST_CHECK(!jsonParseStrict("{ 42: \"hello\" }", json, &errors));
+
+	// According to ECMA-404 hex escape sequences are not allowed, only unicode (\uNNNN) and
+	// a few control characters (\b, \f, \n, \r, \t)
+	//
+	// More lenient parsers allow hex escapes as long as they translate to a valid UTF-8 encoding.
+	//
+	// ... but JSONCPP allows any hex escapes
 	BOOST_CHECK(jsonParseStrict("[ \"\x80\xec\x80\" ]", json, &errors));
+	BOOST_CHECK(json.isArray());
 	BOOST_CHECK(json[0] == "\x80\xec\x80");
+
+	// This would be valid more lenient parsers.
+	BOOST_CHECK(jsonParseStrict("[ \"\xF0\x9F\x98\x8A\" ]", json, &errors));
+	BOOST_CHECK(json.isArray());
+	BOOST_CHECK(json[0] == "😊");
 }

 BOOST_AUTO_TEST_SUITE_END()