Merge pull request #1397 from roadriverrail/contract_collision

Error out when contracts collide on name

Changelog.md

@@ -16,6 +16,9 @@ Bugfixes:
  * Imports: ``import ".dir/a"`` is not a relative path.  Relative paths begin with directory ``.`` or ``..``.
  * Type checker, disallow inheritances of different kinds (e.g. a function and a modifier) of members of the same name
 
+Features:
+ * Contracts and libraries are now unique to their source files, rather than globally.
+
 ### 0.4.7 (2016-12-15)
 
 Features:

libsolidity/ast/AST.cpp

@@ -191,7 +191,6 @@ void ContractDefinition::setUserDocumentation(Json::Value const& _userDocumentat
 	m_userDocumentation = _userDocumentation;
 }
 
-
 vector<Declaration const*> const& ContractDefinition::inheritableMembers() const
 {
 	if (!m_inheritableMembers)

libsolidity/ast/AST.h

@@ -165,6 +165,7 @@ public:
 	/// @returns the source name this declaration is present in.
 	/// Can be combined with annotation().canonicalName to form a globally unique name.
 	std::string sourceUnitName() const;
+	std::string fullyQualifiedName() const { return sourceUnitName() + ":" + name(); }
 
 	virtual bool isLValue() const { return false; }
 	virtual bool isPartOfExternalInterface() const { return false; }

libsolidity/codegen/ContractCompiler.cpp

@@ -575,7 +575,7 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly)
 				else if (auto contract = dynamic_cast<ContractDefinition const*>(decl))
 				{
 					solAssert(contract->isLibrary(), "");
-					_assembly.appendLibraryAddress(contract->name());
+					_assembly.appendLibraryAddress(contract->fullyQualifiedName());
 				}
 				else
 					solAssert(false, "Invalid declaration type.");

libsolidity/codegen/ExpressionCompiler.cpp

@@ -892,7 +892,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 				solAssert(funType->location() == FunctionType::Location::DelegateCall, "");
 				auto contract = dynamic_cast<ContractDefinition const*>(funType->declaration().scope());
 				solAssert(contract && contract->isLibrary(), "");
-				m_context.appendLibraryAddress(contract->name());
+				m_context.appendLibraryAddress(contract->fullyQualifiedName());
 				m_context << funType->externalIdentifier();
 				utils().moveIntoStack(funType->selfType()->sizeOnStack(), 2);
 			}
@@ -1270,7 +1270,7 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier)
 	else if (auto contract = dynamic_cast<ContractDefinition const*>(declaration))
 	{
 		if (contract->isLibrary())
-			m_context.appendLibraryAddress(contract->name());
+			m_context.appendLibraryAddress(contract->fullyQualifiedName());
 	}
 	else if (dynamic_cast<EventDefinition const*>(declaration))
 	{

libsolidity/interface/CompilerStack.cpp

@@ -21,6 +21,7 @@
  * Full-stack compiler that converts a source code string to bytecode.
  */
 
+
 #include <libsolidity/interface/CompilerStack.h>
 
 #include <libsolidity/interface/Version.h>
@@ -180,11 +181,15 @@ bool CompilerStack::parse()
 				if (!resolver.updateDeclaration(*m_globalContext->currentThis())) return false;
 				if (!resolver.updateDeclaration(*m_globalContext->currentSuper())) return false;
 				if (!resolver.resolveNamesAndTypes(*contract)) return false;
-				m_contracts[contract->name()].contract = contract;
-			}
 
-	if (!checkLibraryNameClashes())
+				// Note that we now reference contracts by their fully qualified names, and
-		noErrors = false;
+				// thus contracts can only conflict if declared in the same source file.  This
+				// already causes a double-declaration error elsewhere, so we do not report
+				// an error here and instead silently drop any additional contracts we find.
+
+				if (m_contracts.find(contract->fullyQualifiedName()) == m_contracts.end())
+					m_contracts[contract->fullyQualifiedName()].contract = contract;
+			}
 
 	for (Source const* source: m_sourceOrder)
 		for (ASTPointer<ASTNode> const& node: source->ast->nodes())
@@ -201,7 +206,13 @@ bool CompilerStack::parse()
 				else
 					noErrors = false;
 
-				m_contracts[contract->name()].contract = contract;
+				// Note that we now reference contracts by their fully qualified names, and
+				// thus contracts can only conflict if declared in the same source file.  This
+				// already causes a double-declaration error elsewhere, so we do not report
+				// an error here and instead silently drop any additional contracts we find.
+
+				if (m_contracts.find(contract->fullyQualifiedName()) == m_contracts.end())
+					m_contracts[contract->fullyQualifiedName()].contract = contract;
 			}
 
 	if (noErrors)
@@ -315,6 +326,27 @@ string const* CompilerStack::runtimeSourceMapping(string const& _contractName) c
 	return c.runtimeSourceMapping.get();
 }
 
+std::string const CompilerStack::filesystemFriendlyName(string const& _contractName) const
+{
+	// Look up the contract (by its fully-qualified name)
+	Contract const& matchContract = m_contracts.at(_contractName);
+	// Check to see if it could collide on name
+	for (auto const& contract: m_contracts)
+	{
+		if (contract.second.contract->name() == matchContract.contract->name() &&
+				contract.second.contract != matchContract.contract)
+		{
+			// If it does, then return its fully-qualified name, made fs-friendly
+			std::string friendlyName = boost::algorithm::replace_all_copy(_contractName, "/", "_");
+			boost::algorithm::replace_all(friendlyName, ":", "_");
+			boost::algorithm::replace_all(friendlyName, ".", "_");
+			return friendlyName;
+		}
+	}
+	// If no collision, return the contract's name
+	return matchContract.contract->name();
+}
+
 eth::LinkerObject const& CompilerStack::object(string const& _contractName) const
 {
 	return contract(_contractName).object;
@@ -569,37 +601,6 @@ 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
 {
 	using path = boost::filesystem::path;
@@ -628,7 +629,7 @@ void CompilerStack::compileContract(
 		compileContract(*dependency, _compiledContracts);
 
 	shared_ptr<Compiler> compiler = make_shared<Compiler>(m_optimize, m_optimizeRuns);
-	Contract& compiledContract = m_contracts.at(_contract.name());
+	Contract& compiledContract = m_contracts.at(_contract.fullyQualifiedName());
 	string onChainMetadata = createOnChainMetadata(compiledContract);
 	bytes cborEncodedMetadata =
 		// CBOR-encoding of {"bzzr0": dev::swarmHash(onChainMetadata)}
@@ -674,10 +675,27 @@ CompilerStack::Contract const& CompilerStack::contract(string const& _contractNa
 		for (auto const& it: m_sources)
 			for (ASTPointer<ASTNode> const& node: it.second.ast->nodes())
 				if (auto contract = dynamic_cast<ContractDefinition const*>(node.get()))
-					contractName = contract->name();
+					contractName = contract->fullyQualifiedName();
 	auto it = m_contracts.find(contractName);
-	if (it == m_contracts.end())
+	// To provide a measure of backward-compatibility, if a contract is not located by its
+	// fully-qualified name, a lookup will be attempted purely on the contract's name to see
+	// if anything will satisfy.
+	if (it == m_contracts.end() && contractName.find(":") == string::npos)
+	{
+		for (auto const& contractEntry: m_contracts)
+		{
+			stringstream ss;
+			ss.str(contractEntry.first);
+			// All entries are <source>:<contract>
+			string source;
+			string foundName;
+			getline(ss, source, ':');
+			getline(ss, foundName, ':');
+			if (foundName == contractName) return contractEntry.second;
+		}
+		// If we get here, both lookup methods failed.
 		BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Contract " + _contractName + " not found."));
+	}
 	return it->second;
 }
 

libsolidity/interface/CompilerStack.h

@@ -148,6 +148,10 @@ public:
 	/// @returns the string that provides a mapping between runtime bytecode and sourcecode.
 	/// if the contract does not (yet) have bytecode.
 	std::string const* runtimeSourceMapping(std::string const& _contractName = "") const;
+
+	/// @returns either the contract's name or a mixture of its name and source file, sanitized for filesystem use
+	std::string const filesystemFriendlyName(std::string const& _contractName) const;
+
 	/// @returns hash of the runtime bytecode for the contract, i.e. the code that is
 	/// returned by the constructor or the zero-h256 if the contract still needs to be linked or
 	/// does not have runtime code.
@@ -230,9 +234,6 @@ private:
 	StringMap loadMissingSources(SourceUnit const& _ast, std::string const& _path);
 	std::string applyRemapping(std::string const& _path, std::string const& _context);
 	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;
 	/// Helper function to return path converted strings.

solc/CommandLineInterface.cpp

@@ -186,7 +186,7 @@ void CommandLineInterface::handleBinary(string const& _contract)
 	if (m_args.count(g_argBinary))
 	{
 		if (m_args.count(g_argOutputDir))
-			createFile(_contract + ".bin", m_compiler->object(_contract).toHex());
+			createFile(m_compiler->filesystemFriendlyName(_contract) + ".bin", m_compiler->object(_contract).toHex());
 		else
 		{
 			cout << "Binary: " << endl;

test/libsolidity/Imports.cpp

@@ -106,6 +106,7 @@ BOOST_AUTO_TEST_CASE(library_name_clash)
 	CompilerStack c;
 	c.addSource("a", "library A {} pragma solidity >=0.0;");
 	c.addSource("b", "library A {} pragma solidity >=0.0;");
+	c.addSource("c", "import {A} from \"./a\"; import {A} from \"./b\";");
 	BOOST_CHECK(!c.compile());
 }
 

test/libsolidity/SolidityEndToEndTest.cpp

@@ -3192,7 +3192,7 @@ BOOST_AUTO_TEST_CASE(library_call_in_homestead)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "Lib");
-	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{":Lib", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs());
 	BOOST_CHECK(callContractFunction("sender()") == encodeArgs(u160(m_sender)));
 }
@@ -6191,7 +6191,7 @@ BOOST_AUTO_TEST_CASE(library_call)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "Lib");
-	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{":Lib", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(33)) == encodeArgs(u256(33) * 9));
 }
 
@@ -6208,7 +6208,7 @@ BOOST_AUTO_TEST_CASE(library_stray_values)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "Lib");
-	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{":Lib", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(33)) == encodeArgs(u256(42)));
 }
 
@@ -6341,7 +6341,7 @@ BOOST_AUTO_TEST_CASE(internal_types_in_library)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "Lib");
-	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{":Lib", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(4), u256(17)));
 }
 
@@ -6368,7 +6368,7 @@ BOOST_AUTO_TEST_CASE(using_library_structs)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "Lib");
-	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{":Lib", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(7), u256(8)));
 }
 
@@ -6902,7 +6902,7 @@ BOOST_AUTO_TEST_CASE(using_for_function_on_int)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "D");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"D", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":D", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(9)) == encodeArgs(u256(2 * 9)));
 }
 
@@ -6920,7 +6920,7 @@ BOOST_AUTO_TEST_CASE(using_for_function_on_struct)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "D");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"D", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":D", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(3 * 7)));
 	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(3 * 7)));
 }
@@ -6943,7 +6943,7 @@ BOOST_AUTO_TEST_CASE(using_for_overload)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "D");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"D", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":D", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(6 * 7)));
 	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(6 * 7)));
 }
@@ -6962,7 +6962,7 @@ BOOST_AUTO_TEST_CASE(using_for_by_name)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "D");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"D", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":D", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(6 * 7)));
 	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(6 * 7)));
 }
@@ -6982,7 +6982,7 @@ BOOST_AUTO_TEST_CASE(bound_function_in_var)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "D");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"D", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":D", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f(uint256)", u256(7)) == encodeArgs(u256(6 * 7)));
 	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(6 * 7)));
 }
@@ -7005,7 +7005,7 @@ BOOST_AUTO_TEST_CASE(bound_function_to_string)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "D");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"D", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":D", m_contractAddress}});
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(3)));
 	BOOST_CHECK(callContractFunction("g()") == encodeArgs(u256(3)));
 }
@@ -7751,7 +7751,7 @@ BOOST_AUTO_TEST_CASE(payable_function_calls_library)
 		}
 	)";
 	compileAndRun(sourceCode, 0, "L");
-	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{"L", m_contractAddress}});
+	compileAndRun(sourceCode, 0, "C", bytes(), map<string, Address>{{":L", m_contractAddress}});
 	BOOST_CHECK(callContractFunctionWithValue("f()", 27) == encodeArgs(u256(7)));
 }