Compute canonical names of types for function signatures.

12 files changed, 145 insertions, 39 deletions

diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp
index 00b51c42..d55bc13c 100644
--- a/libsolidity/AST.cpp
+++ b/libsolidity/AST.cpp
@@ -135,7 +135,7 @@ vector<pair<FixedHash<4>, FunctionTypePointer>> const& ContractDefinition::inter
 					FunctionType ftype(*v);
 					solAssert(!!v->annotation().type.get(), "");
 					functionsSeen.insert(v->name());
-					FixedHash<4> hash(dev::sha3(ftype.externalSignature(v->name())));
+					FixedHash<4> hash(dev::sha3(ftype.externalSignature()));
 					m_interfaceFunctionList->push_back(make_pair(hash, make_shared<FunctionType>(*v)));
 				}
 		}
@@ -215,6 +215,13 @@ TypePointer StructDefinition::type(ContractDefinition const*) const
 	return make_shared<TypeType>(make_shared<StructType>(*this));
 }
 
+TypeDeclarationAnnotation& StructDefinition::annotation() const
+{
+	if (!m_annotation)
+		m_annotation = new TypeDeclarationAnnotation();
+	return static_cast<TypeDeclarationAnnotation&>(*m_annotation);
+}
+
 TypePointer EnumValue::type(ContractDefinition const*) const
 {
 	auto parentDef = dynamic_cast<EnumDefinition const*>(scope());
@@ -227,6 +234,13 @@ TypePointer EnumDefinition::type(ContractDefinition const*) const
 	return make_shared<TypeType>(make_shared<EnumType>(*this));
 }
 
+TypeDeclarationAnnotation& EnumDefinition::annotation() const
+{
+	if (!m_annotation)
+		m_annotation = new TypeDeclarationAnnotation();
+	return static_cast<TypeDeclarationAnnotation&>(*m_annotation);
+}
+
 TypePointer FunctionDefinition::type(ContractDefinition const*) const
 {
 	return make_shared<FunctionType>(*this);
@@ -234,7 +248,7 @@ TypePointer FunctionDefinition::type(ContractDefinition const*) const
 
 string FunctionDefinition::externalSignature() const
 {
-	return FunctionType(*this).externalSignature(name());
+	return FunctionType(*this).externalSignature();
 }
 
 TypePointer ModifierDefinition::type(ContractDefinition const*) const
diff --git a/libsolidity/AST.h b/libsolidity/AST.h
index c7eaa41d..1d2babbe 100644
--- a/libsolidity/AST.h
+++ b/libsolidity/AST.h
@@ -352,6 +352,8 @@ public:
 
 	virtual TypePointer type(ContractDefinition const* m_currentContract) const override;
 
+	virtual TypeDeclarationAnnotation& annotation() const override;
+
 private:
 	std::vector<ASTPointer<VariableDeclaration>> m_members;
 };
@@ -372,6 +374,8 @@ public:
 
 	virtual TypePointer type(ContractDefinition const* m_currentContract) const override;
 
+	virtual TypeDeclarationAnnotation& annotation() const override;
+
 private:
 	std::vector<ASTPointer<EnumValue>> m_members;
 };
diff --git a/libsolidity/ASTAnnotations.h b/libsolidity/ASTAnnotations.h
index 195f11c8..dad7b205 100644
--- a/libsolidity/ASTAnnotations.h
+++ b/libsolidity/ASTAnnotations.h
@@ -40,7 +40,13 @@ struct ASTAnnotation
 	virtual ~ASTAnnotation() {}
 };
 
-struct ContractDefinitionAnnotation: ASTAnnotation
+struct TypeDeclarationAnnotation: ASTAnnotation
+{
+	/// The name of this type, prefixed by proper namespaces if globally accessible.
+	std::string canonicalName;
+};
+
+struct ContractDefinitionAnnotation: TypeDeclarationAnnotation
 {
 	/// Whether all functions are implemented.
 	bool isFullyImplemented = true;
diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp
index 6ee19d58..7d9ca32c 100644
--- a/libsolidity/CompilerStack.cpp
+++ b/libsolidity/CompilerStack.cpp
@@ -130,12 +130,15 @@ bool CompilerStack::parse()
 				m_globalContext->setCurrentContract(*contract);
 				resolver.updateDeclaration(*m_globalContext->currentThis());
 				TypeChecker typeChecker;
-				if (!typeChecker.checkTypeRequirements(*contract))
+				if (typeChecker.checkTypeRequirements(*contract))
+				{
+					contract->setDevDocumentation(interfaceHandler.devDocumentation(*contract));
+					contract->setUserDocumentation(interfaceHandler.userDocumentation(*contract));
+				}
+				else
 					typesFine = false;
-				m_errors += typeChecker.errors();
-				contract->setDevDocumentation(interfaceHandler.devDocumentation(*contract));
-				contract->setUserDocumentation(interfaceHandler.userDocumentation(*contract));
 				m_contracts[contract->name()].contract = contract;
+				m_errors += typeChecker.errors();
 			}
 	m_parseSuccessful = typesFine;
 	return m_parseSuccessful;
diff --git a/libsolidity/DeclarationContainer.h b/libsolidity/DeclarationContainer.h
index ffbd1fcb..3d6ed2cc 100644
--- a/libsolidity/DeclarationContainer.h
+++ b/libsolidity/DeclarationContainer.h
@@ -40,8 +40,10 @@ namespace solidity
 class DeclarationContainer
 {
 public:
-	explicit DeclarationContainer(Declaration const* _enclosingDeclaration = nullptr,
-								  DeclarationContainer const* _enclosingContainer = nullptr):
+	explicit DeclarationContainer(
+		Declaration const* _enclosingDeclaration = nullptr,
+		DeclarationContainer const* _enclosingContainer = nullptr
+	):
 		m_enclosingDeclaration(_enclosingDeclaration), m_enclosingContainer(_enclosingContainer) {}
 	/// Registers the declaration in the scope unless its name is already declared or the name is empty.
 	/// @param _invisible if true, registers the declaration, reports name clashes but does not return it in @a resolveName
diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp
index a4b63696..c11ef29e 100644
--- a/libsolidity/ExpressionCompiler.cpp
+++ b/libsolidity/ExpressionCompiler.cpp
@@ -585,7 +585,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				}
 			if (!event.isAnonymous())
 			{
-				m_context << u256(h256::Arith(dev::sha3(function.externalSignature(event.name()))));
+				m_context << u256(h256::Arith(dev::sha3(function.externalSignature())));
 				++numIndexed;
 			}
 			solAssert(numIndexed <= 4, "Too many indexed arguments.");
diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp
index 4837fcfe..533634c2 100644
--- a/libsolidity/InterfaceHandler.cpp
+++ b/libsolidity/InterfaceHandler.cpp
@@ -64,11 +64,11 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef)
 		method["constant"] = it.second->isConstant();
 		method["inputs"] = populateParameters(
 			externalFunctionType->parameterNames(),
-			externalFunctionType->parameterTypeNames()
+			externalFunctionType->parameterTypeNames(_contractDef.isLibrary())
 		);
 		method["outputs"] = populateParameters(
 			externalFunctionType->returnParameterNames(),
-			externalFunctionType->returnParameterTypeNames()
+			externalFunctionType->returnParameterTypeNames(_contractDef.isLibrary())
 		);
 		abi.append(method);
 	}
@@ -80,7 +80,7 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef)
 		solAssert(!!externalFunction, "");
 		method["inputs"] = populateParameters(
 			externalFunction->parameterNames(),
-			externalFunction->parameterTypeNames()
+			externalFunction->parameterTypeNames(_contractDef.isLibrary())
 		);
 		abi.append(method);
 	}
@@ -96,7 +96,7 @@ string InterfaceHandler::abiInterface(ContractDefinition const& _contractDef)
 		{
 			Json::Value input;
 			input["name"] = p->name();
-			input["type"] = p->annotation().type->toString(true);
+			input["type"] = p->annotation().type->canonicalName(false);
 			input["indexed"] = p->isIndexed();
 			params.append(input);
 		}
@@ -125,16 +125,24 @@ string InterfaceHandler::ABISolidityInterface(ContractDefinition const& _contrac
 		ret +=
 			"function " +
 			_contractDef.name() +
-			populateParameters(externalFunction->parameterNames(), externalFunction->parameterTypeNames()) +
+			populateParameters(
+				externalFunction->parameterNames(),
+				externalFunction->parameterTypeNames(_contractDef.isLibrary())
+			) +
 			";";
 	}
 	for (auto const& it: _contractDef.interfaceFunctions())
 	{
 		ret += "function " + it.second->declaration().name() +
-			populateParameters(it.second->parameterNames(), it.second->parameterTypeNames()) +
-			(it.second->isConstant() ? "constant " : "");
+			populateParameters(
+				it.second->parameterNames(),
+				it.second->parameterTypeNames(_contractDef.isLibrary())
+			) + (it.second->isConstant() ? "constant " : "");
 		if (it.second->returnParameterTypes().size())
-			ret += "returns" + populateParameters(it.second->returnParameterNames(), it.second->returnParameterTypeNames());
+			ret += "returns" + populateParameters(
+				it.second->returnParameterNames(),
+				it.second->returnParameterTypeNames(_contractDef.isLibrary())
+			);
 		else if (ret.back() == ' ')
 			ret.pop_back();
 		ret += ";";
diff --git a/libsolidity/NameAndTypeResolver.cpp b/libsolidity/NameAndTypeResolver.cpp
index c3e31728..6b9b9584 100644
--- a/libsolidity/NameAndTypeResolver.cpp
+++ b/libsolidity/NameAndTypeResolver.cpp
@@ -263,6 +263,7 @@ DeclarationRegistrationHelper::DeclarationRegistrationHelper(map<ASTNode const*,
 bool DeclarationRegistrationHelper::visit(ContractDefinition& _contract)
 {
 	registerDeclaration(_contract, true);
+	_contract.annotation().canonicalName = currentCanonicalName();
 	return true;
 }
 
@@ -274,6 +275,7 @@ void DeclarationRegistrationHelper::endVisit(ContractDefinition&)
 bool DeclarationRegistrationHelper::visit(StructDefinition& _struct)
 {
 	registerDeclaration(_struct, true);
+	_struct.annotation().canonicalName = currentCanonicalName();
 	return true;
 }
 
@@ -285,6 +287,7 @@ void DeclarationRegistrationHelper::endVisit(StructDefinition&)
 bool DeclarationRegistrationHelper::visit(EnumDefinition& _enum)
 {
 	registerDeclaration(_enum, true);
+	_enum.annotation().canonicalName = currentCanonicalName();
 	return true;
 }
 
@@ -400,5 +403,21 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio
 		enterNewSubScope(_declaration);
 }
 
+string DeclarationRegistrationHelper::currentCanonicalName() const
+{
+	string ret;
+	for (
+		Declaration const* scope = m_currentScope;
+		scope != nullptr;
+		scope = m_scopes[scope].enclosingDeclaration()
+	)
+	{
+		if (!ret.empty())
+			ret = "." + ret;
+		ret = scope->name() + ret;
+	}
+	return ret;
+}
+
 }
 }
diff --git a/libsolidity/NameAndTypeResolver.h b/libsolidity/NameAndTypeResolver.h
index d28671ed..528930f7 100644
--- a/libsolidity/NameAndTypeResolver.h
+++ b/libsolidity/NameAndTypeResolver.h
@@ -119,6 +119,9 @@ private:
 	void closeCurrentScope();
 	void registerDeclaration(Declaration& _declaration, bool _opensScope);
 
+	/// @returns the canonical name of the current scope.
+	std::string currentCanonicalName() const;
+
 	std::map<ASTNode const*, DeclarationContainer>& m_scopes;
 	Declaration const* m_currentScope;
 	VariableScope* m_currentFunction;
diff --git a/libsolidity/TypeChecker.cpp b/libsolidity/TypeChecker.cpp
index 74347e1f..fe6fb970 100644
--- a/libsolidity/TypeChecker.cpp
+++ b/libsolidity/TypeChecker.cpp
@@ -299,7 +299,7 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co
 			if (f->isPartOfExternalInterface())
 			{
 				auto functionType = make_shared<FunctionType>(*f);
-				externalDeclarations[functionType->externalSignature(f->name())].push_back(
+				externalDeclarations[functionType->externalSignature()].push_back(
 					make_pair(f.get(), functionType)
 				);
 			}
@@ -307,7 +307,7 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co
 			if (v->isPartOfExternalInterface())
 			{
 				auto functionType = make_shared<FunctionType>(*v);
-				externalDeclarations[functionType->externalSignature(v->name())].push_back(
+				externalDeclarations[functionType->externalSignature()].push_back(
 					make_pair(v.get(), functionType)
 				);
 			}
@@ -403,7 +403,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 		if (!type(*var)->canLiveOutsideStorage())
 			typeError(*var, "Type is required to live outside storage.");
 		if (_function.visibility() >= FunctionDefinition::Visibility::Public && !(type(*var)->interfaceType(isLibraryFunction)))
-			typeError(*var, "Internal type is not allowed for public and external functions.");
+			fatalTypeError(*var, "Internal type is not allowed for public or external functions.");
 	}
 	for (ASTPointer<ModifierInvocation> const& modifier: _function.modifiers())
 		visitManually(
diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp
index faac2447..f0c67bba 100644
--- a/libsolidity/Types.cpp
+++ b/libsolidity/Types.cpp
@@ -839,6 +839,25 @@ string ArrayType::toString(bool _short) const
 	return ret;
 }
 
+string ArrayType::canonicalName(bool _addDataLocation) const
+{
+	string ret;
+	if (isString())
+		ret = "string";
+	else if (isByteArray())
+		ret = "bytes";
+	else
+	{
+		ret = baseType()->canonicalName(false) + "[";
+		if (!isDynamicallySized())
+			ret += length().str();
+		ret += "]";
+	}
+	if (_addDataLocation && location() == DataLocation::Storage)
+		ret += " storage";
+	return ret;
+}
+
 TypePointer ArrayType::encodingType() const
 {
 	if (location() == DataLocation::Storage)
@@ -912,6 +931,11 @@ string ContractType::toString(bool) const
 		m_contract.name();
 }
 
+string ContractType::canonicalName(bool) const
+{
+	return m_contract.annotation().canonicalName;
+}
+
 MemberList const& ContractType::members() const
 {
 	// We need to lazy-initialize it because of recursive references.
@@ -1093,6 +1117,14 @@ TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer)
 	return copy;
 }
 
+string StructType::canonicalName(bool _addDataLocation) const
+{
+	string ret = m_struct.annotation().canonicalName;
+	if (_addDataLocation && location() == DataLocation::Storage)
+		ret += " storage";
+	return ret;
+}
+
 FunctionTypePointer StructType::constructorType() const
 {
 	TypePointers paramTypes;
@@ -1168,6 +1200,11 @@ string EnumType::toString(bool) const
 	return string("enum ") + m_enum.name();
 }
 
+string EnumType::canonicalName(bool) const
+{
+	return m_enum.annotation().canonicalName;
+}
+
 bool EnumType::isExplicitlyConvertibleTo(Type const& _convertTo) const
 {
 	return _convertTo.category() == category() || _convertTo.category() == Category::Integer;
@@ -1483,15 +1520,13 @@ bool FunctionType::isBareCall() const
 	}
 }
 
-string FunctionType::externalSignature(std::string const& _name) const
+string FunctionType::externalSignature() const
 {
-	std::string funcName = _name;
-	if (_name == "")
-	{
-		solAssert(m_declaration != nullptr, "Function type without name needs a declaration");
-		funcName = m_declaration->name();
-	}
-	string ret = funcName + "(";
+	solAssert(m_declaration != nullptr, "External signature of function needs declaration");
+
+	bool _inLibrary = dynamic_cast<ContractDefinition const&>(*m_declaration->scope()).isLibrary();
+
+	string ret = m_declaration->name() + "(";
 
 	FunctionTypePointer external = interfaceFunctionType();
 	solAssert(!!external, "External function type requested.");
@@ -1499,7 +1534,7 @@ string FunctionType::externalSignature(std::string const& _name) const
 	for (auto it = externalParameterTypes.cbegin(); it != externalParameterTypes.cend(); ++it)
 	{
 		solAssert(!!(*it), "Parameter should have external type");
-		ret += (*it)->toString(true) + (it + 1 == externalParameterTypes.cend() ? "" : ",");
+		ret += (*it)->canonicalName(_inLibrary) + (it + 1 == externalParameterTypes.cend() ? "" : ",");
 	}
 
 	return ret + ")";
@@ -1567,20 +1602,20 @@ FunctionTypePointer FunctionType::asMemberFunction(bool _inLibrary) const
 	);
 }
 
-vector<string> const FunctionType::parameterTypeNames() const
+vector<string> const FunctionType::parameterTypeNames(bool _addDataLocation) const
 {
 	vector<string> names;
 	for (TypePointer const& t: m_parameterTypes)
-		names.push_back(t->toString(true));
+		names.push_back(t->canonicalName(_addDataLocation));
 
 	return names;
 }
 
-vector<string> const FunctionType::returnParameterTypeNames() const
+vector<string> const FunctionType::returnParameterTypeNames(bool _addDataLocation) const
 {
 	vector<string> names;
 	for (TypePointer const& t: m_returnParameterTypes)
-		names.push_back(t->toString(true));
+		names.push_back(t->canonicalName(_addDataLocation));
 
 	return names;
 }
@@ -1607,6 +1642,11 @@ string MappingType::toString(bool _short) const
 	return "mapping(" + keyType()->toString(_short) + " => " + valueType()->toString(_short) + ")";
 }
 
+string MappingType::canonicalName(bool) const
+{
+	return "mapping(" + keyType()->canonicalName(false) + " => " + valueType()->canonicalName(false) + ")";
+}
+
 u256 VoidType::storageSize() const
 {
 	BOOST_THROW_EXCEPTION(
diff --git a/libsolidity/Types.h b/libsolidity/Types.h
index 09654bfe..7a65ca92 100644
--- a/libsolidity/Types.h
+++ b/libsolidity/Types.h
@@ -218,6 +218,9 @@ public:
 
 	virtual std::string toString(bool _short) const = 0;
 	std::string toString() const { return toString(false); }
+	/// @returns the canonical name of this type for use in function signatures.
+	/// @param _addDataLocation if true, includes data location for reference types if it is "storage".
+	virtual std::string canonicalName(bool /*_addDataLocation*/) const { return toString(true); }
 	virtual u256 literalValue(Literal const*) const
 	{
 		BOOST_THROW_EXCEPTION(
@@ -501,6 +504,7 @@ public:
 	virtual bool canLiveOutsideStorage() const override { return m_baseType->canLiveOutsideStorage(); }
 	virtual unsigned sizeOnStack() const override;
 	virtual std::string toString(bool _short) const override;
+	virtual std::string canonicalName(bool _addDataLocation) const override;
 	virtual MemberList const& members() const override
 	{
 		return isString() ? EmptyMemberList : s_arrayTypeMemberList;
@@ -554,6 +558,7 @@ public:
 	virtual bool canLiveOutsideStorage() const override { return true; }
 	virtual bool isValueType() const override { return true; }
 	virtual std::string toString(bool _short) const override;
+	virtual std::string canonicalName(bool _addDataLocation) const override;
 
 	virtual MemberList const& members() const override;
 	virtual TypePointer encodingType() const override
@@ -617,6 +622,8 @@ public:
 
 	TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override;
 
+	virtual std::string canonicalName(bool _addDataLocation) const override;
+
 	/// @returns a function that peforms the type conversion between a list of struct members
 	/// and a memory struct of this type.
 	FunctionTypePointer constructorType() const;
@@ -652,6 +659,7 @@ public:
 	virtual unsigned storageBytes() const override;
 	virtual bool canLiveOutsideStorage() const override { return true; }
 	virtual std::string toString(bool _short) const override;
+	virtual std::string canonicalName(bool _addDataLocation) const override;
 	virtual bool isValueType() const override { return true; }
 
 	virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override;
@@ -756,10 +764,10 @@ public:
 
 	TypePointers const& parameterTypes() const { return m_parameterTypes; }
 	std::vector<std::string> const& parameterNames() const { return m_parameterNames; }
-	std::vector<std::string> const parameterTypeNames() const;
+	std::vector<std::string> const parameterTypeNames(bool _addDataLocation) const;
 	TypePointers const& returnParameterTypes() const { return m_returnParameterTypes; }
 	std::vector<std::string> const& returnParameterNames() const { return m_returnParameterNames; }
-	std::vector<std::string> const returnParameterTypeNames() const;
+	std::vector<std::string> const returnParameterTypeNames(bool _addDataLocation) const;
 
 	virtual bool operator==(Type const& _other) const override;
 	virtual std::string toString(bool _short) const override;
@@ -786,9 +794,7 @@ public:
 	bool isBareCall() const;
 	Location const& location() const { return m_location; }
 	/// @returns the external signature of this function type given the function name
-	/// If @a _name is not provided (empty string) then the @c m_declaration member of the
-	/// function type is used
-	std::string externalSignature(std::string const& _name = "") const;
+	std::string externalSignature() const;
 	/// @returns the external identifier of this function (the hash of the signature).
 	u256 externalIdentifier() const;
 	Declaration const& declaration() const
@@ -849,6 +855,7 @@ public:
 
 	virtual bool operator==(Type const& _other) const override;
 	virtual std::string toString(bool _short) const override;
+	virtual std::string canonicalName(bool _addDataLocation) const override;
 	virtual bool canLiveOutsideStorage() const override { return false; }
 	virtual TypePointer encodingType() const override
 	{