From 6d9544795543498e9bc92943c41f07c2daf3bb78 Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Fri, 29 Sep 2017 18:45:17 -0300 Subject: [PATCH 1/2] Emit error when declaring event with same name and arguments twice --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 44 +++++++++++++++ libsolidity/analysis/TypeChecker.h | 1 + .../SolidityNameAndTypeResolution.cpp | 55 +++++++++++++++++++ 4 files changed, 101 insertions(+) diff --git a/Changelog.md b/Changelog.md index 008b3c868..81f0b3a6f 100644 --- a/Changelog.md +++ b/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) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 439301252..601547581 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/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); @@ -218,6 +219,49 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con } } +void TypeChecker::checkContractDuplicateEvents(ContractDefinition const& _contract) +{ + /// Checks that two events with the same name defined in this contract have different + /// argument types + map> events; + for (EventDefinition const* event: _contract.events()) + events[event->name()].push_back(event); + + for (auto const& it: events) + { + vector const& overloads = it.second; + set reported; + for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i) + { + SecondarySourceLocation ssl; + + for (size_t j = i + 1; j < overloads.size(); ++j) + if (FunctionType(*overloads[i]).hasEqualArgumentTypes(FunctionType(*overloads[j]))) + { + ssl.append("Other declaration is here:", overloads[j]->location()); + reported.insert(j); + } + + if (ssl.infos.size() > 0) + { + string msg = "Event 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(occurrences) + " to the first 32 occurrences."; + } + + m_errorReporter.declarationError( + overloads[i]->location(), + ssl, + msg + ); + } + } + } +} + void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _contract) { // Mapping from name to function definition (exactly one per argument type equality class) and diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 0c6f54d3f..929be6e6e 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/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. diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index c3a1ce087..8b1122579 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/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"( From 1e7b6875b7c5ddc9d257bac65e17d25b8f26fdd2 Mon Sep 17 00:00:00 2001 From: Federico Bond Date: Fri, 29 Sep 2017 19:45:56 -0300 Subject: [PATCH 2/2] Extract duplicate function or event finding logic --- libsolidity/analysis/TypeChecker.cpp | 48 +++++++--------------------- libsolidity/analysis/TypeChecker.h | 3 ++ 2 files changed, 14 insertions(+), 37 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 601547581..d45e9e899 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -184,39 +184,8 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con msg ); } - for (auto const& it: functions) - { - vector const& overloads = it.second; - set reported; - for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i) - { - SecondarySourceLocation ssl; - for (size_t j = i + 1; j < overloads.size(); ++j) - if (FunctionType(*overloads[i]).hasEqualArgumentTypes(FunctionType(*overloads[j]))) - { - ssl.append("Other declaration is here:", overloads[j]->location()); - reported.insert(j); - } - - 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(occurrences) + " to the first 32 occurrences."; - } - - m_errorReporter.declarationError( - overloads[i]->location(), - ssl, - msg - ); - } - } - } + findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); } void TypeChecker::checkContractDuplicateEvents(ContractDefinition const& _contract) @@ -227,9 +196,15 @@ void TypeChecker::checkContractDuplicateEvents(ContractDefinition const& _contra for (EventDefinition const* event: _contract.events()) events[event->name()].push_back(event); - for (auto const& it: events) + findDuplicateDefinitions(events, "Event with same name and arguments defined twice."); +} + +template +void TypeChecker::findDuplicateDefinitions(map> const& _definitions, string _message) +{ + for (auto const& it: _definitions) { - vector const& overloads = it.second; + vector const& overloads = it.second; set reported; for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i) { @@ -244,18 +219,17 @@ void TypeChecker::checkContractDuplicateEvents(ContractDefinition const& _contra if (ssl.infos.size() > 0) { - string msg = "Event 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(occurrences) + " to the first 32 occurrences."; + _message += " Truncated from " + boost::lexical_cast(occurrences) + " to the first 32 occurrences."; } m_errorReporter.declarationError( overloads[i]->location(), ssl, - msg + _message ); } } diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 929be6e6e..abe6dac1e 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -109,6 +109,9 @@ private: virtual void endVisit(ElementaryTypeNameExpression const& _expr) override; virtual void endVisit(Literal const& _literal) override; + template + void findDuplicateDefinitions(std::map> const& _definitions, std::string _message); + bool contractDependenciesAreCyclic( ContractDefinition const& _contract, std::set const& _seenContracts = std::set()