Merge pull request #3611 from ethereum/warn-using-address-overload

Warn if using address overloads on contracts
This commit is contained in:
Alex Beregszaszi 2018-03-06 19:13:55 +01:00 committed by GitHub
commit ba8819542f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 185 additions and 11 deletions

View File

@ -14,6 +14,7 @@ Features:
* Syntax Analyser: Do not warn about ``pragma experimental "v0.5.0"`` since it does not affect 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.
* Improved messaging when error spans multiple lines of a sourcefile

View File

@ -1858,6 +1858,20 @@ 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<FunctionType const*>(type(_memberAccess).get()))
{
auto kind = callType->kind();

View File

@ -7250,6 +7250,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 {}
@ -7263,12 +7265,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<std::pair<Error::Type, std::string>>{
{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 {
@ -7279,12 +7286,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<std::pair<Error::Type, std::string>>{
{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 {}
}
@ -7297,11 +7309,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<std::pair<Error::Type, std::string>>{
{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 {}
@ -7315,7 +7332,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)
@ -8277,6 +8294,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<pair<Error::Type, std::string>>{
{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<pair<Error::Type, std::string>>{
{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<pair<Error::Type, std::string>>{
{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<pair<Error::Type, std::string>>{
{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<pair<Error::Type, std::string>>{
{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<std::string>{
"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"(
@ -8335,6 +8480,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"(

View File

@ -280,11 +280,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");