Merge pull request #13210 from timweri/display_human_readable_type_name_in_conversion

Display human readable type name in error messages

Author: Marenz

Changelog.md

@@ -9,6 +9,7 @@ Compiler Features:
  * Yul Optimizer: Add rule to convert `mod(mul(X, Y), A)` into `mulmod(X, Y, A)`, if `A` is a power of two.
  * Yul Optimizer: Add rule to convert `mod(add(X, Y), A)` into `addmod(X, Y, A)`, if `A` is a power of two.
  * Code Generator: More efficient code for checked addition and subtraction.
+ * Error Reporter: More readable and informative error/warning messages.
 
 Bugfixes:
  * Commandline Interface: Disallow the following options outside of the compiler mode: ``--via-ir``,``--metadata-literal``, ``--metadata-hash``, ``--model-checker-show-unproved``, ``--model-checker-div-mod-no-slacks``, ``--model-checker-engine``, ``--model-checker-invariants``, ``--model-checker-solvers``, ``--model-checker-timeout``, ``--model-checker-contracts``, ``--model-checker-targets``.

libsolidity/analysis/TypeChecker.cpp

@@ -143,10 +143,10 @@ string ASTJsonExporter::namePathToString(std::vector<ASTString> const& _namePath
 	return boost::algorithm::join(_namePath, ".");
 }
 
-Json::Value ASTJsonExporter::typePointerToJson(Type const* _tp, bool _short)
+Json::Value ASTJsonExporter::typePointerToJson(Type const* _tp, bool _withoutDataLocation)
 {
 	Json::Value typeDescriptions(Json::objectValue);
-	typeDescriptions["typeString"] = _tp ? Json::Value(_tp->toString(_short)) : Json::nullValue;
+	typeDescriptions["typeString"] = _tp ? Json::Value(_tp->toString(_withoutDataLocation)) : Json::nullValue;
 	typeDescriptions["typeIdentifier"] = _tp ? Json::Value(_tp->identifier()) : Json::nullValue;
 	return typeDescriptions;
 

libsolidity/ast/ASTJsonExporter.cpp

@@ -184,7 +184,7 @@ class ASTJsonExporter: public ASTConstVisitor
 
 		return json;
 	}
-	static Json::Value typePointerToJson(Type const* _tp, bool _short = false);
+	static Json::Value typePointerToJson(Type const* _tp, bool _withoutDataLocation = false);
 	static Json::Value typePointerToJson(std::optional<FuncCallArguments> const& _tps);
 	void appendExpressionAttributes(
 		std::vector<std::pair<std::string, Json::Value>> &_attributes,

libsolidity/ast/ASTJsonExporter.h

@@ -110,10 +110,10 @@ util::Result<TypePointers> transformParametersToExternal(TypePointers const& _pa
 	return transformed;
 }
 
-string toStringInParentheses(TypePointers const& _types, bool _short)
+string toStringInParentheses(TypePointers const& _types, bool _withoutDataLocation)
 {
 	return '(' + util::joinHumanReadable(
-		_types | ranges::views::transform([&](auto const* _type) { return _type->toString(_short); }),
+		_types | ranges::views::transform([&](auto const* _type) { return _type->toString(_withoutDataLocation); }),
 		","
 	) + ')';
 }
@@ -1789,7 +1789,7 @@ vector<tuple<string, Type const*>> ArrayType::makeStackItems() const
 	solAssert(false, "");
 }
 
-string ArrayType::toString(bool _short) const
+string ArrayType::toString(bool _withoutDataLocation) const
 {
 	string ret;
 	if (isString())
@@ -1798,16 +1798,34 @@ string ArrayType::toString(bool _short) const
 		ret = "bytes";
 	else
 	{
-		ret = baseType()->toString(_short) + "[";
+		ret = baseType()->toString(_withoutDataLocation) + "[";
 		if (!isDynamicallySized())
 			ret += length().str();
 		ret += "]";
 	}
-	if (!_short)
+	if (!_withoutDataLocation)
 		ret += " " + stringForReferencePart();
 	return ret;
 }
 
+string ArrayType::humanReadableName() const
+{
+	string ret;
+	if (isString())
+		ret = "string";
+	else if (isByteArrayOrString())
+		ret = "bytes";
+	else
+	{
+		ret = baseType()->toString(true) + "[";
+		if (!isDynamicallySized())
+			ret += length().str();
+		ret += "]";
+	}
+	ret += " " + stringForReferencePart();
+	return ret;
+}
+
 string ArrayType::canonicalName() const
 {
 	string ret;
@@ -1990,9 +2008,14 @@ bool ArraySliceType::operator==(Type const& _other) const
 	return false;
 }
 
-string ArraySliceType::toString(bool _short) const
+string ArraySliceType::toString(bool _withoutDataLocation) const
+{
+	return m_arrayType.toString(_withoutDataLocation) + " slice";
+}
+
+string ArraySliceType::humanReadableName() const
 {
-	return m_arrayType.toString(_short) + " slice";
+	return m_arrayType.humanReadableName() + " slice";
 }
 
 Type const* ArraySliceType::mobileType() const
@@ -2256,10 +2279,10 @@ bool StructType::containsNestedMapping() const
 	return m_struct.annotation().containsNestedMapping.value();
 }
 
-string StructType::toString(bool _short) const
+string StructType::toString(bool _withoutDataLocation) const
 {
 	string ret = "struct " + *m_struct.annotation().canonicalName;
-	if (!_short)
+	if (!_withoutDataLocation)
 		ret += " " + stringForReferencePart();
 	return ret;
 }
@@ -2611,7 +2634,7 @@ bool UserDefinedValueType::operator==(Type const& _other) const
 	return other.definition() == definition();
 }
 
-string UserDefinedValueType::toString(bool /* _short */) const
+string UserDefinedValueType::toString(bool /* _withoutDataLocation */) const
 {
 	return *definition().annotation().canonicalName;
 }
@@ -2659,13 +2682,24 @@ bool TupleType::operator==(Type const& _other) const
 		return false;
 }
 
-string TupleType::toString(bool _short) const
+string TupleType::toString(bool _withoutDataLocation) const
+{
+	if (components().empty())
+		return "tuple()";
+	string str = "tuple(";
+	for (auto const& t: components())
+		str += (t ? t->toString(_withoutDataLocation) : "") + ",";
+	str.pop_back();
+	return str + ")";
+}
+
+string TupleType::humanReadableName() const
 {
 	if (components().empty())
 		return "tuple()";
 	string str = "tuple(";
 	for (auto const& t: components())
-		str += (t ? t->toString(_short) : "") + ",";
+		str += (t ? t->humanReadableName() : "") + ",";
 	str.pop_back();
 	return str + ")";
 }
@@ -3103,15 +3137,15 @@ string FunctionType::humanReadableName() const
 	switch (m_kind)
 	{
 	case Kind::Error:
-		return "error " + m_declaration->name() + toStringInParentheses(m_parameterTypes, /* _short */ true);
+		return "error " + m_declaration->name() + toStringInParentheses(m_parameterTypes, /* _withoutDataLocation */ true);
 	case Kind::Event:
-		return "event " + m_declaration->name() + toStringInParentheses(m_parameterTypes, /* _short */ true);
+		return "event " + m_declaration->name() + toStringInParentheses(m_parameterTypes, /* _withoutDataLocation */ true);
 	default:
-		return toString(/* _short */ false);
+		return toString(/* _withoutDataLocation */ false);
 	}
 }
 
-string FunctionType::toString(bool _short) const
+string FunctionType::toString(bool _withoutDataLocation) const
 {
 	string name = "function ";
 	if (m_kind == Kind::Declaration)
@@ -3122,15 +3156,15 @@ string FunctionType::toString(bool _short) const
 			name += *contract->annotation().canonicalName + ".";
 		name += functionDefinition->name();
 	}
-	name += toStringInParentheses(m_parameterTypes, _short);
+	name += toStringInParentheses(m_parameterTypes, _withoutDataLocation);
 	if (m_stateMutability != StateMutability::NonPayable)
 		name += " " + stateMutabilityToString(m_stateMutability);
 	if (m_kind == Kind::External)
 		name += " external";
 	if (!m_returnParameterTypes.empty())
 	{
 		name += " returns ";
-		name += toStringInParentheses(m_returnParameterTypes, _short);
+		name += toStringInParentheses(m_returnParameterTypes, _withoutDataLocation);
 	}
 	return name;
 }
@@ -3722,9 +3756,9 @@ bool MappingType::operator==(Type const& _other) const
 	return *other.m_keyType == *m_keyType && *other.m_valueType == *m_valueType;
 }
 
-string MappingType::toString(bool _short) const
+string MappingType::toString(bool _withoutDataLocation) const
 {
-	return "mapping(" + keyType()->toString(_short) + " => " + valueType()->toString(_short) + ")";
+	return "mapping(" + keyType()->toString(_withoutDataLocation) + " => " + valueType()->toString(_withoutDataLocation) + ")";
 }
 
 string MappingType::canonicalName() const
@@ -3949,11 +3983,11 @@ bool ModifierType::operator==(Type const& _other) const
 	return true;
 }
 
-string ModifierType::toString(bool _short) const
+string ModifierType::toString(bool _withoutDataLocation) const
 {
 	string name = "modifier (";
 	for (auto it = m_parameterTypes.begin(); it != m_parameterTypes.end(); ++it)
-		name += (*it)->toString(_short) + (it + 1 == m_parameterTypes.end() ? "" : ",");
+		name += (*it)->toString(_withoutDataLocation) + (it + 1 == m_parameterTypes.end() ? "" : ",");
 	return name + ")";
 }
 
@@ -4149,7 +4183,7 @@ MemberList::MemberMap MagicType::nativeMembers(ASTNode const*) const
 	return {};
 }
 
-string MagicType::toString(bool _short) const
+string MagicType::toString(bool _withoutDataLocation) const
 {
 	switch (m_kind)
 	{
@@ -4163,7 +4197,7 @@ string MagicType::toString(bool _short) const
 		return "abi";
 	case Kind::MetaType:
 		solAssert(m_typeArgument, "");
-		return "type(" + m_typeArgument->toString(_short) + ")";
+		return "type(" + m_typeArgument->toString(_withoutDataLocation) + ")";
 	}
 	solAssert(false, "Unknown kind of magic.");
 	return {};