Merge pull request #2997 from federicobond/duplicate-events

Emit error when declaring event with same name and arguments twice

Changelog.md

@@ -9,6 +9,7 @@ Bugfixes:
  * Type Checker: Properly check array length and don't rely on an assertion in code generation.
  * Type Checker: Properly support overwriting members inherited from ``address`` in a contract
    (such as ``balance``, ``transfer``, etc.)
+ * Type Checker: Prevent duplicate event declarations.
 
 ### 0.4.17 (2017-09-21)
 

libsolidity/analysis/TypeChecker.cpp

@@ -75,6 +75,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 	ASTNode::listAccept(_contract.baseContracts(), *this);
 
 	checkContractDuplicateFunctions(_contract);
+	checkContractDuplicateEvents(_contract);
 	checkContractIllegalOverrides(_contract);
 	checkContractAbstractFunctions(_contract);
 	checkContractAbstractConstructors(_contract);
@@ -183,9 +184,27 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con
 			msg
 		);
 	}
-	for (auto const& it: functions)
+
+	findDuplicateDefinitions(functions, "Function with same name and arguments defined twice.");
+}
+
+void TypeChecker::checkContractDuplicateEvents(ContractDefinition const& _contract)
 {
-		vector<FunctionDefinition const*> const& overloads = it.second;
+	/// Checks that two events with the same name defined in this contract have different
+	/// argument types
+	map<string, vector<EventDefinition const*>> events;
+	for (EventDefinition const* event: _contract.events())
+		events[event->name()].push_back(event);
+
+	findDuplicateDefinitions(events, "Event with same name and arguments defined twice.");
+}
+
+template <class T>
+void TypeChecker::findDuplicateDefinitions(map<string, vector<T>> const& _definitions, string _message)
+{
+	for (auto const& it: _definitions)
+	{
+		vector<T> const& overloads = it.second;
 		set<size_t> reported;
 		for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i)
 		{
@@ -200,18 +219,17 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con
 
 			if (ssl.infos.size() > 0)
 			{
-				string msg = "Function with same name and arguments defined twice.";
 				size_t occurrences = ssl.infos.size();
 				if (occurrences > 32)
 				{
 					ssl.infos.resize(32);
-					msg += " Truncated from " + boost::lexical_cast<string>(occurrences) + " to the first 32 occurrences.";
+					_message += " Truncated from " + boost::lexical_cast<string>(occurrences) + " to the first 32 occurrences.";
 				}
 
 				m_errorReporter.declarationError(
 					overloads[i]->location(),
 					ssl,
-					msg
+					_message
 				);
 			}
 		}

libsolidity/analysis/TypeChecker.h

@@ -61,6 +61,7 @@ private:
 	/// Checks that two functions defined in this contract with the same name have different
 	/// arguments and that there is at most one constructor.
 	void checkContractDuplicateFunctions(ContractDefinition const& _contract);
+	void checkContractDuplicateEvents(ContractDefinition const& _contract);
 	void checkContractIllegalOverrides(ContractDefinition const& _contract);
 	/// Reports a type error with an appropiate message if overriden function signature differs.
 	/// Also stores the direct super function in the AST annotations.
@@ -108,6 +109,9 @@ private:
 	virtual void endVisit(ElementaryTypeNameExpression const& _expr) override;
 	virtual void endVisit(Literal const& _literal) override;
 
+	template <class T>
+	void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message);
+
 	bool contractDependenciesAreCyclic(
 		ContractDefinition const& _contract,
 		std::set<ContractDefinition const*> const& _seenContracts = std::set<ContractDefinition const*>()

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -1395,6 +1395,61 @@ BOOST_AUTO_TEST_CASE(events_with_same_name)
 	BOOST_CHECK(success(text));
 }
 
+BOOST_AUTO_TEST_CASE(events_with_same_name_unnamed_arguments)
+{
+	char const* text = R"(
+		contract test {
+			event A(uint);
+			event A(uint, uint);
+		}
+	)";
+	CHECK_SUCCESS(text);
+}
+
+BOOST_AUTO_TEST_CASE(events_with_same_name_different_types)
+{
+	char const* text = R"(
+		contract test {
+			event A(uint);
+			event A(bytes);
+		}
+	)";
+	CHECK_SUCCESS(text);
+}
+
+BOOST_AUTO_TEST_CASE(double_event_declaration)
+{
+	char const* text = R"(
+		contract test {
+			event A(uint i);
+			event A(uint i);
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Event with same name and arguments defined twice.");
+}
+
+BOOST_AUTO_TEST_CASE(double_event_declaration_ignores_anonymous)
+{
+	char const* text = R"(
+		contract test {
+			event A(uint i);
+			event A(uint i) anonymous;
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Event with same name and arguments defined twice.");
+}
+
+BOOST_AUTO_TEST_CASE(double_event_declaration_ignores_indexed)
+{
+	char const* text = R"(
+		contract test {
+			event A(uint i);
+			event A(uint indexed i);
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Event with same name and arguments defined twice.");
+}
+
 BOOST_AUTO_TEST_CASE(event_call)
 {
 	char const* text = R"(