From fd60c1cf86e69931fda47bdd187f444550ca785f Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 20 Sep 2017 14:54:41 +0100 Subject: [PATCH 1/2] Warn if using address overloads on contracts --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 10 + .../SolidityNameAndTypeResolution.cpp | 171 +++++++++++++++++- test/libsolidity/ViewPureChecker.cpp | 10 +- 4 files changed, 181 insertions(+), 11 deletions(-) diff --git a/Changelog.md b/Changelog.md index 3c875a8fe..67634ea04 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,7 @@ Features: * Syntax Analyser: Do not warn about experimental features if they do not concern code generation. * Syntax Checker: Mark ``throw`` as an error as experimental 0.5.0 feature. * Syntax Checker: Issue error if no visibility is specified on contract functions as experimental 0.5.0 feature. + * Syntax Checker: Issue warning when using overloads of ``address`` on contract instances. * Type Checker: disallow combining hex numbers and unit denominations as experimental 0.5.0 feature. Bugfixes: diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 9846a0d09..ac8b2b63f 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1831,6 +1831,16 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) if (exprType->category() == Type::Category::Contract) { + // Warn about using address members on contracts + for (auto const& addressMember: IntegerType(160, IntegerType::Modifier::Address).nativeMembers(nullptr)) + if (addressMember.name == memberName && *annotation.type == *addressMember.type) + m_errorReporter.warning( + _memberAccess.location(), + "Using contract member \"" + memberName +"\" inherited from the address type is deprecated." + + " Convert the contract to \"address\" type to access the member." + ); + + // Warn about using send or transfer with a non-payable fallback function. if (auto callType = dynamic_cast(type(_memberAccess).get())) { auto kind = callType->kind(); diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 4fb628215..d4962b6c5 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -7011,6 +7011,8 @@ BOOST_AUTO_TEST_CASE(callable_crash) BOOST_AUTO_TEST_CASE(error_transfer_non_payable_fallback) { + // This used to be a test for a.transfer to generate a warning + // because A's fallback function is not payable. char const* text = R"( contract A { function() public {} @@ -7024,12 +7026,17 @@ BOOST_AUTO_TEST_CASE(error_transfer_non_payable_fallback) } } )"; - CHECK_ERROR(text, TypeError, "Value transfer to a contract without a payable fallback function."); + CHECK_ALLOW_MULTI(text, (std::vector>{ + {Error::Type::Warning, "Using contract member \"transfer\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); } BOOST_AUTO_TEST_CASE(error_transfer_no_fallback) { - char const* text = R"( + // This used to be a test for a.transfer to generate a warning + // because A does not have a payable fallback function. + std::string text = R"( contract A {} contract B { @@ -7040,12 +7047,17 @@ BOOST_AUTO_TEST_CASE(error_transfer_no_fallback) } } )"; - CHECK_ERROR(text, TypeError, "Value transfer to a contract without a payable fallback function."); + CHECK_ALLOW_MULTI(text, (std::vector>{ + {Error::Type::Warning, "Using contract member \"transfer\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); } BOOST_AUTO_TEST_CASE(error_send_non_payable_fallback) { - char const* text = R"( + // This used to be a test for a.send to generate a warning + // because A does not have a payable fallback function. + std::string text = R"( contract A { function() public {} } @@ -7058,11 +7070,16 @@ BOOST_AUTO_TEST_CASE(error_send_non_payable_fallback) } } )"; - CHECK_ERROR(text, TypeError, "Value transfer to a contract without a payable fallback function."); + CHECK_ALLOW_MULTI(text, (std::vector>{ + {Error::Type::Warning, "Using contract member \"send\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); } BOOST_AUTO_TEST_CASE(does_not_error_transfer_payable_fallback) { + // This used to be a test for a.transfer to generate a warning + // because A does not have a payable fallback function. char const* text = R"( contract A { function() payable public {} @@ -7076,7 +7093,7 @@ BOOST_AUTO_TEST_CASE(does_not_error_transfer_payable_fallback) } } )"; - CHECK_SUCCESS_NO_WARNINGS(text); + CHECK_WARNING(text, "Using contract member \"transfer\" inherited from the address type is deprecated."); } BOOST_AUTO_TEST_CASE(does_not_error_transfer_regular_function) @@ -7992,6 +8009,134 @@ BOOST_AUTO_TEST_CASE(array_length_invalid_expression) CHECK_ERROR(text, TypeError, "Operator / not compatible with types int_const 3 and int_const 0"); } +BOOST_AUTO_TEST_CASE(warn_about_address_members_on_contract) +{ + std::string text = R"( + contract C { + function f() view public { + this.balance; + } + } + )"; + CHECK_WARNING(text, "Using contract member \"balance\" inherited from the address type is deprecated."); + text = R"( + contract C { + function f() view public { + this.transfer; + } + } + )"; + CHECK_ALLOW_MULTI(text, (vector>{ + {Error::Type::Warning, "Using contract member \"transfer\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); + text = R"( + contract C { + function f() view public { + this.send; + } + } + )"; + CHECK_ALLOW_MULTI(text, (vector>{ + {Error::Type::Warning, "Using contract member \"send\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); + text = R"( + contract C { + function f() view public { + this.call; + } + } + )"; + CHECK_WARNING(text, "Using contract member \"call\" inherited from the address type is deprecated."); + text = R"( + contract C { + function f() view public { + this.callcode; + } + } + )"; + CHECK_ALLOW_MULTI(text, (vector>{ + {Error::Type::Warning, "Using contract member \"callcode\" inherited from the address type is deprecated"}, + {Error::Type::Warning, "\"callcode\" has been deprecated in favour of \"delegatecall\""} + })); + text = R"( + contract C { + function f() view public { + this.delegatecall; + } + } + )"; + CHECK_WARNING(text, "Using contract member \"delegatecall\" inherited from the address type is deprecated."); +} + +BOOST_AUTO_TEST_CASE(warn_about_address_members_on_non_this_contract) +{ + std::string text = R"( + contract C { + function f() view public { + C c; + c.balance; + } + } + )"; + CHECK_WARNING(text, "Using contract member \"balance\" inherited from the address type is deprecated"); + text = R"( + contract C { + function f() view public { + C c; + c.transfer; + } + } + )"; + CHECK_ALLOW_MULTI(text, (vector>{ + {Error::Type::Warning, "Using contract member \"transfer\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); + text = R"( + contract C { + function f() view public { + C c; + c.send; + } + } + )"; + CHECK_ALLOW_MULTI(text, (vector>{ + {Error::Type::Warning, "Using contract member \"send\" inherited from the address type is deprecated"}, + {Error::Type::TypeError, "Value transfer to a contract without a payable fallback function"} + })); + text = R"( + contract C { + function f() pure public { + C c; + c.call; + } + } + )"; + CHECK_WARNING(text, "Using contract member \"call\" inherited from the address type is deprecated"); + text = R"( + contract C { + function f() pure public { + C c; + c.callcode; + } + } + )"; + CHECK_WARNING_ALLOW_MULTI(text, (std::vector{ + "Using contract member \"callcode\" inherited from the address type is deprecated", + "\"callcode\" has been deprecated in favour of \"delegatecall\"" + })); + text = R"( + contract C { + function f() pure public { + C c; + c.delegatecall; + } + } + )"; + CHECK_WARNING(text, "Using contract member \"delegatecall\" inherited from the address type is deprecated"); +} + BOOST_AUTO_TEST_CASE(no_address_members_on_contract) { char const* text = R"( @@ -8050,6 +8195,20 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract) CHECK_ERROR(text, TypeError, "Member \"delegatecall\" not found or not visible after argument-dependent lookup in contract"); } +BOOST_AUTO_TEST_CASE(no_warning_for_using_members_that_look_like_address_members) +{ + char const* text = R"( + pragma experimental "v0.5.0"; + contract C { + function transfer(uint) public; + function f() public { + this.transfer(10); + } + } + )"; + CHECK_WARNING(text, "Experimental features"); +} + BOOST_AUTO_TEST_CASE(emit_events) { char const* text = R"( diff --git a/test/libsolidity/ViewPureChecker.cpp b/test/libsolidity/ViewPureChecker.cpp index e6a5cfd06..6e8620f9e 100644 --- a/test/libsolidity/ViewPureChecker.cpp +++ b/test/libsolidity/ViewPureChecker.cpp @@ -279,11 +279,11 @@ BOOST_AUTO_TEST_CASE(builtin_functions) string text = R"( contract C { function f() public { - this.transfer(1); - require(this.send(2)); - selfdestruct(this); - require(this.delegatecall()); - require(this.call()); + address(this).transfer(1); + require(address(this).send(2)); + selfdestruct(address(this)); + require(address(this).delegatecall()); + require(address(this).call()); } function g() pure public { bytes32 x = keccak256("abc"); From 1ceb0b04c1ce117366cde88eb8c3bebb1a5e9598 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 5 Mar 2018 15:55:02 +0100 Subject: [PATCH 2/2] Assert that address members are not present on contract types in 0.5.0. --- libsolidity/analysis/TypeChecker.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index ac8b2b63f..106a2c8e8 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1832,13 +1832,17 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) if (exprType->category() == Type::Category::Contract) { // Warn about using address members on contracts + bool v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); for (auto const& addressMember: IntegerType(160, IntegerType::Modifier::Address).nativeMembers(nullptr)) if (addressMember.name == memberName && *annotation.type == *addressMember.type) + { + solAssert(!v050, "Address member still present on contract in v0.5.0."); m_errorReporter.warning( _memberAccess.location(), "Using contract member \"" + memberName +"\" inherited from the address type is deprecated." + " Convert the contract to \"address\" type to access the member." ); + } // Warn about using send or transfer with a non-payable fallback function. if (auto callType = dynamic_cast(type(_memberAccess).get()))