Merge pull request #347 from chriseth/libraryNameClashes

Detect library name clashes

4 files changed, 53 insertions, 2 deletions

diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index f0dab41a..040217da 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -807,7 +807,6 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 			);
 			auto contract = dynamic_cast<ContractDefinition const*>(funType->declaration().scope());
 			solAssert(contract && contract->isLibrary(), "");
-			//@TODO library name might not be unique
 			m_context.appendLibraryAddress(contract->name());
 			m_context << funType->externalIdentifier();
 			utils().moveIntoStack(funType->selfType()->sizeOnStack(), 2);
@@ -1118,7 +1117,6 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier)
 	else if (auto contract = dynamic_cast<ContractDefinition const*>(declaration))
 	{
 		if (contract->isLibrary())
-			//@todo name should be unique, change once we have module management
 			m_context.appendLibraryAddress(contract->name());
 	}
 	else if (dynamic_cast<EventDefinition const*>(declaration))
diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp
index a6f6f224..aaff09ba 100644
--- a/libsolidity/interface/CompilerStack.cpp
+++ b/libsolidity/interface/CompilerStack.cpp
@@ -148,6 +148,9 @@ bool CompilerStack::parse()
 				m_contracts[contract->name()].contract = contract;
 			}
 
+	if (!checkLibraryNameClashes())
+		noErrors = false;
+
 	for (Source const* source: m_sourceOrder)
 		for (ASTPointer<ASTNode> const& node: source->ast->nodes())
 			if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
@@ -400,6 +403,37 @@ void CompilerStack::resolveImports()
 	swap(m_sourceOrder, sourceOrder);
 }
 
+bool CompilerStack::checkLibraryNameClashes()
+{
+	bool clashFound = false;
+	map<string, SourceLocation> libraries;
+	for (Source const* source: m_sourceOrder)
+		for (ASTPointer<ASTNode> const& node: source->ast->nodes())
+			if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get()))
+				if (contract->isLibrary())
+				{
+					if (libraries.count(contract->name()))
+					{
+						auto err = make_shared<Error>(Error::Type::DeclarationError);
+						*err <<
+							errinfo_sourceLocation(contract->location()) <<
+							errinfo_comment(
+								"Library \"" + contract->name() + "\" declared twice "
+								"(will create ambiguities during linking)."
+							) <<
+							errinfo_secondarySourceLocation(SecondarySourceLocation().append(
+									"The other declaration is here:", libraries[contract->name()]
+							));
+
+						m_errors.push_back(err);
+						clashFound = true;
+					}
+					else
+						libraries[contract->name()] = contract->location();
+				}
+	return !clashFound;
+}
+
 string CompilerStack::absolutePath(string const& _path, string const& _reference) const
 {
 	// Anything that does not start with `.` is an absolute path.
diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h
index 3e6dc456..6cf19db4 100644
--- a/libsolidity/interface/CompilerStack.h
+++ b/libsolidity/interface/CompilerStack.h
@@ -199,6 +199,9 @@ private:
 	};
 
 	void resolveImports();
+	/// Checks whether there are libraries with the same name, reports that as an error and
+	/// @returns false in this case.
+	bool checkLibraryNameClashes();
 	/// @returns the absolute path corresponding to @a _path relative to @a _reference.
 	std::string absolutePath(std::string const& _path, std::string const& _reference) const;
 	/// Compile a single contract and put the result in @a _compiledContracts.
diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp
index c074b332..94d3e423 100644
--- a/test/libsolidity/Imports.cpp
+++ b/test/libsolidity/Imports.cpp
@@ -101,6 +101,22 @@ BOOST_AUTO_TEST_CASE(simple_alias)
 	BOOST_CHECK(c.compile());
 }
 
+BOOST_AUTO_TEST_CASE(library_name_clash)
+{
+	CompilerStack c;
+	c.addSource("a", "library A {}");
+	c.addSource("b", "library A {}");
+	BOOST_CHECK(!c.compile());
+}
+
+BOOST_AUTO_TEST_CASE(library_name_clash_with_contract)
+{
+	CompilerStack c;
+	c.addSource("a", "contract A {}");
+	c.addSource("b", "library A {}");
+	BOOST_CHECK(c.compile());
+}
+
 BOOST_AUTO_TEST_CASE(complex_import)
 {
 	CompilerStack c;