Properly escape user strings and lists.

3 files changed, 98 insertions, 56 deletions

diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 9e722474..6e9e9d7e 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -21,15 +21,21 @@
  */
 
 #include <libsolidity/ast/Types.h>
-#include <limits>
-#include <boost/range/adaptor/reversed.hpp>
-#include <boost/range/adaptor/sliced.hpp>
+
+#include <libsolidity/interface/Utils.h>
+#include <libsolidity/ast/AST.h>
+
 #include <libdevcore/CommonIO.h>
 #include <libdevcore/CommonData.h>
 #include <libdevcore/SHA3.h>
 #include <libdevcore/UTF8.h>
-#include <libsolidity/interface/Utils.h>
-#include <libsolidity/ast/AST.h>
+
+#include <boost/algorithm/string/join.hpp>
+#include <boost/range/adaptor/reversed.hpp>
+#include <boost/range/adaptor/sliced.hpp>
+#include <boost/range/adaptor/transformed.hpp>
+
+#include <limits>
 
 using namespace std;
 using namespace dev;
@@ -117,6 +123,51 @@ u256 const& MemberList::storageSize() const
 	return m_storageOffsets->storageSize();
 }
 
+/// Helper functions for type identifier
+namespace
+{
+
+string parenthesizeIdentifier(string const& _internal)
+{
+	return "$_" + _internal + "_$";
+}
+
+template <class Range>
+string identifierList(Range const&& _list)
+{
+	return parenthesizeIdentifier(boost::algorithm::join(_list, "_$_"));
+}
+
+string identifier(TypePointer const& _type)
+{
+	return _type ? _type->identifier() : "";
+}
+
+string identifierList(vector<TypePointer> const& _list)
+{
+	return identifierList(_list | boost::adaptors::transformed(identifier));
+}
+
+string identifierList(TypePointer const& _type)
+{
+	return parenthesizeIdentifier(identifier(_type));
+}
+
+string identifierList(TypePointer const& _type1, TypePointer const& _type2)
+{
+	TypePointers list;
+	list.push_back(_type1);
+	list.push_back(_type2);
+	return identifierList(list);
+}
+
+string parenthesizeUserIdentifier(string const& _internal)
+{
+	return parenthesizeIdentifier(boost::replace_all_copy(_internal, "$", "$$$"));
+}
+
+}
+
 TypePointer Type::fromElementaryTypeName(ElementaryTypeNameToken const& _type)
 {
 	solAssert(Token::isElementaryTypeName(_type.token()),
@@ -1218,16 +1269,17 @@ string ArrayType::identifier() const
 {
 	string id;
 	if (isString())
-		id += "t_string";
+		id = "t_string";
 	else if (isByteArray())
-		id += "t_bytes";
+		id = "t_bytes";
 	else
 	{
-		id = baseType()->identifier();
+		id = "t_array";
+		id += identifierList(baseType());
 		if (isDynamicallySized())
-			id += "_arraydyn";
+			id += "dyn";
 		else
-			id += string("_array") + length().str();
+			id += length().str();
 	}
 	id += identifierLocationSuffix();
 
@@ -1422,7 +1474,7 @@ TypePointer ArrayType::copyForLocation(DataLocation _location, bool _isPointer)
 
 string ContractType::identifier() const
 {
-	return (m_super ? "t_super_" : "t_contract_") + m_contract.name() + "_" + std::to_string(m_contract.id());
+	return (m_super ? "t_super" : "t_contract") + parenthesizeUserIdentifier(m_contract.name()) + std::to_string(m_contract.id());
 }
 
 bool ContractType::operator==(Type const& _other) const
@@ -1536,7 +1588,7 @@ bool StructType::isImplicitlyConvertibleTo(const Type& _convertTo) const
 
 string StructType::identifier() const
 {
-	return "t_struct_" + m_struct.name() + "_" + std::to_string(m_struct.id()) + identifierLocationSuffix();
+	return "t_struct" + parenthesizeUserIdentifier(m_struct.name()) + std::to_string(m_struct.id()) + identifierLocationSuffix();
 }
 
 bool StructType::operator==(Type const& _other) const
@@ -1681,7 +1733,7 @@ TypePointer EnumType::unaryOperatorResult(Token::Value _operator) const
 
 string EnumType::identifier() const
 {
-	return "t_enum_" + m_enum.name() + "_" + std::to_string(m_enum.id());
+	return "t_enum" + parenthesizeUserIdentifier(m_enum.name()) + std::to_string(m_enum.id());
 }
 
 bool EnumType::operator==(Type const& _other) const
@@ -1767,14 +1819,7 @@ bool TupleType::isImplicitlyConvertibleTo(Type const& _other) const
 
 string TupleType::identifier() const
 {
-	string id = "t_tuple" + std::to_string(components().size()) + "_";
-	for (auto const& c: components())
-		if (c)
-			id += c->identifier() + "_";
-		else
-			id += "t_empty_";
-	id += "tuple_end";
-	return id;
+	return "t_tuple" + identifierList(components());
 }
 
 bool TupleType::operator==(Type const& _other) const
@@ -2062,19 +2107,13 @@ string FunctionType::identifier() const
 	}
 	if (isConstant())
 		id += "_constant";
-	id += "_param" + std::to_string(m_parameterTypes.size()) + "_";
-	for (auto const& p: m_parameterTypes)
-		id += p->identifier() + "_";
-	id += "return" + std::to_string(m_returnParameterTypes.size()) + "_";
-	for (auto const& r: m_returnParameterTypes)
-		id += r->identifier() + "_";
+	id += identifierList(m_parameterTypes) + "returns" + identifierList(m_returnParameterTypes);
 	if (m_gasSet)
 		id += "gas_set_";
 	if (m_valueSet)
 		id += "value_set_";
 	if (bound())
-		id += "bound_to" + selfType()->identifier() + "_";
-	id += "function_end";
+		id += "bound_to" + identifierList(selfType());
 	return id;
 }
 
@@ -2482,7 +2521,7 @@ vector<string> const FunctionType::returnParameterTypeNames(bool _addDataLocatio
 	return names;
 }
 
-TypePointer FunctionType::selfType() const
+TypePointer const& FunctionType::selfType() const
 {
 	solAssert(bound(), "Function is not bound.");
 	solAssert(m_parameterTypes.size() > 0, "Function has no self type.");
@@ -2500,7 +2539,7 @@ ASTPointer<ASTString> FunctionType::documentation() const
 
 string MappingType::identifier() const
 {
-	return "t_mapping_" + m_keyType->identifier() + "_to_" + m_valueType->identifier() + "_mapping_end";
+	return "t_mapping" + identifierList(m_keyType, m_valueType);
 }
 
 bool MappingType::operator==(Type const& _other) const
@@ -2523,7 +2562,7 @@ string MappingType::canonicalName(bool) const
 
 string TypeType::identifier() const
 {
-	return "t_type_" + actualType()->identifier();
+	return "t_type" + identifierList(actualType());
 }
 
 bool TypeType::operator==(Type const& _other) const
@@ -2612,10 +2651,7 @@ u256 ModifierType::storageSize() const
 
 string ModifierType::identifier() const
 {
-	string id = "t_modifier_param" + std::to_string(m_parameterTypes.size()) + "_";
-	for (auto const& p: m_parameterTypes)
-		id += p->identifier() + "_";
-	return id + "end_modifier";
+	return "t_modifier" + identifierList(m_parameterTypes);
 }
 
 bool ModifierType::operator==(Type const& _other) const
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index c1f6a476..1e94631e 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -22,18 +22,21 @@
 
 #pragma once
 
-#include <memory>
-#include <string>
-#include <map>
-#include <boost/noncopyable.hpp>
-#include <boost/rational.hpp>
-#include <libdevcore/Common.h>
-#include <libdevcore/CommonIO.h>
 #include <libsolidity/interface/Exceptions.h>
 #include <libsolidity/ast/ASTForward.h>
 #include <libsolidity/parsing/Token.h>
+
+#include <libdevcore/Common.h>
+#include <libdevcore/CommonIO.h>
 #include <libdevcore/UndefMacros.h>
 
+#include <boost/noncopyable.hpp>
+#include <boost/rational.hpp>
+
+#include <memory>
+#include <string>
+#include <map>
+
 namespace dev
 {
 namespace solidity
@@ -158,6 +161,9 @@ public:
 	/// @returns a valid solidity identifier such that two types should compare equal if and
 	/// only if they have the same identifier.
 	/// The identifier should start with "t_".
+	/// More complex identifier strings use "parentheses", where $_ is interpreted as as
+	/// "opening parenthesis", _$ as "closing parenthesis", _$_ as "comma" and any $ that
+	/// appears as part of a user-supplied identifier is escaped as _$$$_.
 	virtual std::string identifier() const = 0;
 	virtual bool isImplicitlyConvertibleTo(Type const& _other) const { return *this == _other; }
 	virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const
@@ -912,7 +918,7 @@ public:
 	std::vector<std::string> const& returnParameterNames() const { return m_returnParameterNames; }
 	std::vector<std::string> const returnParameterTypeNames(bool _addDataLocation) const;
 	/// @returns the "self" parameter type for a bound function
-	TypePointer selfType() const;
+	TypePointer const& selfType() const;
 
 	virtual std::string identifier() const override;
 	virtual bool operator==(Type const& _other) const override;
diff --git a/test/libsolidity/SolidityTypes.cpp b/test/libsolidity/SolidityTypes.cpp
index 1e7bcc7b..b4c06873 100644
--- a/test/libsolidity/SolidityTypes.cpp
+++ b/test/libsolidity/SolidityTypes.cpp
@@ -109,39 +109,39 @@ BOOST_AUTO_TEST_CASE(type_identifiers)
 	BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("bytes")->identifier(), "t_bytes_storage_ptr");
 	BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("string")->identifier(), "t_string_storage_ptr");
 	ArrayType largeintArray(DataLocation::Memory, Type::fromElementaryTypeName("int128"), u256("2535301200456458802993406410752"));
-	BOOST_CHECK_EQUAL(largeintArray.identifier(), "t_int128_array2535301200456458802993406410752_memory_ptr");
+	BOOST_CHECK_EQUAL(largeintArray.identifier(), "t_array$_t_int128_$2535301200456458802993406410752_memory_ptr");
 	TypePointer stringArray = make_shared<ArrayType>(DataLocation::Storage, Type::fromElementaryTypeName("string"), u256("20"));
 	TypePointer multiArray = make_shared<ArrayType>(DataLocation::Storage, stringArray);
-	BOOST_CHECK_EQUAL(multiArray->identifier(), "t_string_storage_array20_storage_arraydyn_storage_ptr");
+	BOOST_CHECK_EQUAL(multiArray->identifier(), "t_array$_t_array$_t_string_storage_$20_storage_$dyn_storage_ptr");
 
-	ContractDefinition c(SourceLocation{}, make_shared<string>("MyContract"), {}, {}, {}, false);
-	BOOST_CHECK_EQUAL(c.type()->identifier(), "t_type_t_contract_MyContract_2");
-	BOOST_CHECK_EQUAL(ContractType(c, true).identifier(), "t_super_MyContract_2");
+	ContractDefinition c(SourceLocation{}, make_shared<string>("MyContract$"), {}, {}, {}, false);
+	BOOST_CHECK_EQUAL(c.type()->identifier(), "t_type$_t_contract$_MyContract$$$_$2_$");
+	BOOST_CHECK_EQUAL(ContractType(c, true).identifier(), "t_super$_MyContract$$$_$2");
 
 	StructDefinition s({}, make_shared<string>("Struct"), {});
-	BOOST_CHECK_EQUAL(s.type()->identifier(), "t_type_t_struct_Struct_3_storage_ptr");
+	BOOST_CHECK_EQUAL(s.type()->identifier(), "t_type$_t_struct$_Struct_$3_storage_ptr_$");
 
 	EnumDefinition e({}, make_shared<string>("Enum"), {});
-	BOOST_CHECK_EQUAL(e.type()->identifier(), "t_type_t_enum_Enum_4");
+	BOOST_CHECK_EQUAL(e.type()->identifier(), "t_type$_t_enum$_Enum_$4_$");
 
 	TupleType t({e.type(), s.type(), stringArray, nullptr});
-	BOOST_CHECK_EQUAL(t.identifier(), "t_tuple4_t_type_t_enum_Enum_4_t_type_t_struct_Struct_3_storage_ptr_t_string_storage_array20_storage_ptr_t_empty_tuple_end");
+	BOOST_CHECK_EQUAL(t.identifier(), "t_tuple$_t_type$_t_enum$_Enum_$4_$_$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$_t_array$_t_string_storage_$20_storage_ptr_$__$");
 
 	TypePointer sha3fun = make_shared<FunctionType>(strings{}, strings{}, FunctionType::Location::SHA3);
-	BOOST_CHECK_EQUAL(sha3fun->identifier(), "t_function_sha3_param0_return0_function_end");
+	BOOST_CHECK_EQUAL(sha3fun->identifier(), "t_function_sha3$__$returns$__$");
 
 	FunctionType metaFun(TypePointers{sha3fun}, TypePointers{s.type()});
-	BOOST_CHECK_EQUAL(metaFun.identifier(), "t_function_internal_param1_t_function_sha3_param0_return0_function_end_return1_t_type_t_struct_Struct_3_storage_ptr_function_end");
+	BOOST_CHECK_EQUAL(metaFun.identifier(), "t_function_internal$_t_function_sha3$__$returns$__$_$returns$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$");
 
 	TypePointer m = make_shared<MappingType>(Type::fromElementaryTypeName("bytes32"), s.type());
 	MappingType m2(Type::fromElementaryTypeName("uint64"), m);
-	BOOST_CHECK_EQUAL(m2.identifier(), "t_mapping_t_uint64_to_t_mapping_t_bytes32_to_t_type_t_struct_Struct_3_storage_ptr_mapping_end_mapping_end");
+	BOOST_CHECK_EQUAL(m2.identifier(), "t_mapping$_t_uint64_$_t_mapping$_t_bytes32_$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$_$");
 
 	// TypeType is tested with contract
 
 	auto emptyParams = make_shared<ParameterList>(SourceLocation(), std::vector<ASTPointer<VariableDeclaration>>());
 	ModifierDefinition mod(SourceLocation{}, make_shared<string>("modif"), {}, emptyParams, {});
-	BOOST_CHECK_EQUAL(ModifierType(mod).identifier(), "t_modifier_param0_end_modifier");
+	BOOST_CHECK_EQUAL(ModifierType(mod).identifier(), "t_modifier$__$");
 
 	SourceUnit su({}, {});
 	BOOST_CHECK_EQUAL(ModuleType(su).identifier(), "t_module_7");