From f510348ff1d9f0839a4257d6e05f59304247b4e7 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 11:39:40 +0200 Subject: [PATCH 1/2] Extract tests. --- .../SolidityNameAndTypeResolution.cpp | 182 ------------------ ...constructible_internal_constructor_new.sol | 6 + ...constructible_internal_constructor_old.sol | 9 + .../constructor/constructor_new.sol | 1 + .../constructor/constructor_old.sol | 3 + .../constructor/constructor_old_050.sol | 4 + .../constructor_state_mutability_new.sol | 13 ++ .../constructor_state_mutability_old.sol | 16 ++ .../constructor_visibility_new.sol | 12 ++ .../constructor_visibility_old.sol | 13 ++ ...constructor_without_implementation_new.sol | 5 + ...constructor_without_implementation_old.sol | 6 + .../constructor/external_constructor_new.sol | 5 + .../constructor/external_constructor_old.sol | 6 + ...ible_internal_constructor_inverted_new.sol | 13 ++ ...ible_internal_constructor_inverted_old.sol | 15 ++ ...constructible_internal_constructor_new.sol | 8 + ...constructible_internal_constructor_old.sol | 9 + .../constructor/interface_constructor_new.sol | 7 + .../constructor/interface_constructor_old.sol | 8 + .../constructor/library_constructor_new.sol | 6 + .../constructor/library_constructor_old.sol | 7 + .../constructor/overriding_constructor.sol | 6 + .../returns_in_constructor_new.sol | 5 + .../returns_in_constructor_old.sol | 6 + .../constructor/two_constructors_old.sol | 8 + 26 files changed, 197 insertions(+), 182 deletions(-) create mode 100644 test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_old_050.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/external_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/external_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/library_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/library_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/overriding_constructor.sol create mode 100644 test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol create mode 100644 test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol create mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_old.sol diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index d438a9dc1..301250f0a 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -243,16 +243,6 @@ BOOST_AUTO_TEST_CASE(assignment_to_struct) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(returns_in_constructor) -{ - char const* text = R"( - contract test { - function test() public returns (uint a) { } - } - )"; - CHECK_ERROR(text, TypeError, "Non-empty \"returns\" directive for constructor."); -} - BOOST_AUTO_TEST_CASE(forward_function_reference) { char const* text = R"( @@ -869,26 +859,6 @@ BOOST_AUTO_TEST_CASE(complex_inheritance) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(constructor_visibility) -{ - // The constructor of a base class should not be visible in the derived class - char const* text = R"( - contract A { function A() public { } } - contract B is A { function f() public { A x = A(0); } } - )"; - CHECK_SUCCESS(text); -} - -BOOST_AUTO_TEST_CASE(overriding_constructor) -{ - // It is fine to "override" constructor of a base class since it is invisible - char const* text = R"( - contract A { function A() public { } } - contract B is A { function A() public returns (uint8 r) {} } - )"; - CHECK_SUCCESS(text); -} - BOOST_AUTO_TEST_CASE(missing_base_constructor_arguments) { char const* text = R"( @@ -907,35 +877,6 @@ BOOST_AUTO_TEST_CASE(base_constructor_arguments_override) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(new_constructor_syntax) -{ - char const* text = R"( - contract A { constructor() public {} } - )"; - CHECK_SUCCESS_NO_WARNINGS(text); -} - -BOOST_AUTO_TEST_CASE(old_constructor_syntax) -{ - char const* text = R"( - contract A { function A() public {} } - )"; - CHECK_WARNING( - text, - "Defining constructors as functions with the same name as the contract is deprecated." - ); - - text = R"( - pragma experimental "v0.5.0"; - contract A { function A() public {} } - )"; - CHECK_ERROR( - text, - SyntaxError, - "Functions are not allowed to have the same name as the contract." - ); -} - BOOST_AUTO_TEST_CASE(implicit_derived_to_base_conversion) { char const* text = R"( @@ -2588,17 +2529,6 @@ BOOST_AUTO_TEST_CASE(override_changes_return_types) CHECK_ERROR(sourceCode, TypeError, "Overriding function return types differ"); } -BOOST_AUTO_TEST_CASE(multiple_constructors) -{ - char const* sourceCode = R"( - contract test { - function test(uint a) public { } - function test() public {} - } - )"; - CHECK_ERROR(sourceCode, DeclarationError, "More than one constructor defined"); -} - BOOST_AUTO_TEST_CASE(equal_overload) { char const* sourceCode = R"( @@ -3122,19 +3052,6 @@ BOOST_AUTO_TEST_CASE(library_having_variables) CHECK_ERROR(text, TypeError, "Library cannot have non-constant state variables"); } -BOOST_AUTO_TEST_CASE(library_constructor) -{ - char const* text = R"( - library Lib { - function Lib(); - } - )"; - CHECK_ERROR_ALLOW_MULTI(text, TypeError, (vector{ - "Constructor cannot be defined in libraries.", - "Constructor must be implemented if declared." - })); -} - BOOST_AUTO_TEST_CASE(valid_library) { char const* text = R"( @@ -5030,38 +4947,6 @@ BOOST_AUTO_TEST_CASE(unsatisfied_version) BOOST_CHECK(searchErrorMessage(*sourceAndError.second.front(), "Source file requires different compiler version")); } -BOOST_AUTO_TEST_CASE(invalid_constructor_statemutability) -{ - char const* text = R"( - contract test { - function test() constant {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable"); - text = R"( - contract test { - function test() view {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable"); - text = R"( - contract test { - function test() pure {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable"); -} - -BOOST_AUTO_TEST_CASE(external_constructor) -{ - char const* text = R"( - contract test { - function test() external {} - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be public or internal."); -} - BOOST_AUTO_TEST_CASE(invalid_array_as_statement) { char const* text = R"( @@ -5608,50 +5493,6 @@ BOOST_AUTO_TEST_CASE(assignment_to_constant) CHECK_ERROR(text, TypeError, "Cannot assign to a constant variable."); } -BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor) -{ - char const* text = R"( - contract C { - function C() internal {} - } - contract D { - function f() public { var x = new C(); } - } - )"; - CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly."); -} - -BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor_inverted) -{ - // Previously, the type information for A was not yet available at the point of - // "new A". - char const* text = R"( - contract B { - A a; - function B() public { - a = new A(this); - } - } - contract A { - function A(address a) internal {} - } - )"; - CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly."); -} - -BOOST_AUTO_TEST_CASE(constructible_internal_constructor) -{ - char const* text = R"( - contract C { - function C() internal {} - } - contract D is C { - function D() public { } - } - )"; - CHECK_SUCCESS(text); -} - BOOST_AUTO_TEST_CASE(return_structs) { char const* text = R"( @@ -5813,19 +5654,6 @@ BOOST_AUTO_TEST_CASE(interface) CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(interface_constructor) -{ - char const* text = R"( - interface I { - function I(); - } - )"; - CHECK_ERROR_ALLOW_MULTI(text, TypeError, (std::vector{ - "Constructor cannot be defined in interfaces", - "Constructor must be implemented if declared.", - })); -} - BOOST_AUTO_TEST_CASE(interface_functions) { char const* text = R"( @@ -6907,16 +6735,6 @@ BOOST_AUTO_TEST_CASE(builtin_reject_value) CHECK_ERROR(text, TypeError, "Member \"value\" not found or not visible after argument-dependent lookup"); } -BOOST_AUTO_TEST_CASE(constructor_without_implementation) -{ - char const* text = R"( - contract C { - function C(); - } - )"; - CHECK_ERROR(text, TypeError, "Constructor must be implemented if declared."); -} - BOOST_AUTO_TEST_CASE(large_storage_array_fine) { char const* text = R"( diff --git a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol new file mode 100644 index 000000000..8dee4c712 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol @@ -0,0 +1,6 @@ +contract C { + constructor() internal {} +} +contract D is C { + constructor() public { } +} diff --git a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol new file mode 100644 index 000000000..144743e38 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol @@ -0,0 +1,9 @@ +contract C { + function C() internal {} +} +contract D is C { + function D() public {} +} +// ---- +// Warning: (14-38): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (60-82): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_new.sol new file mode 100644 index 000000000..aa3422cc2 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_new.sol @@ -0,0 +1 @@ +contract A { constructor() public {} } diff --git a/test/libsolidity/syntaxTests/constructor/constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_old.sol new file mode 100644 index 000000000..9ec6257dd --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_old.sol @@ -0,0 +1,3 @@ +contract A { function A() public {} } +// ---- +// Warning: (13-35): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol b/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol new file mode 100644 index 000000000..19e46e793 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol @@ -0,0 +1,4 @@ +pragma experimental "v0.5.0"; +contract A { function A() public {} } +// ---- +// SyntaxError: (43-65): Functions are not allowed to have the same name as the contract. If you intend this to be a constructor, use "constructor(...) { ... }" to define it. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol new file mode 100644 index 000000000..15ed0e1e7 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol @@ -0,0 +1,13 @@ +contract test1 { + constructor() constant {} +} +contract test2 { + constructor() view {} +} +contract test3 { + constructor() pure {} +} +// ---- +// TypeError: (19-44): Constructor must be payable or non-payable, but is "view". +// TypeError: (66-87): Constructor must be payable or non-payable, but is "view". +// TypeError: (109-130): Constructor must be payable or non-payable, but is "pure". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol new file mode 100644 index 000000000..6dbcbc974 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol @@ -0,0 +1,16 @@ +contract test1 { + function test1() constant {} +} +contract test2 { + function test2() view {} +} +contract test3 { + function test3() pure {} +} +// ---- +// Warning: (21-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (73-97): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (121-145): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (21-49): Constructor must be payable or non-payable, but is "view". +// TypeError: (73-97): Constructor must be payable or non-payable, but is "view". +// TypeError: (121-145): Constructor must be payable or non-payable, but is "pure". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol new file mode 100644 index 000000000..502dc0295 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol @@ -0,0 +1,12 @@ +// The constructor of a base class should not be visible in the derived class +contract A { constructor(string) public { } } +contract B is A { + function f() pure public { + A x = A(0); // convert from address + string memory y = "ab"; + A(y); // call as a function is invalid + x; + } +} +// ---- +// TypeError: (243-247): Explicit type conversion not allowed from "string memory" to "contract A". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol new file mode 100644 index 000000000..847ea27b1 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol @@ -0,0 +1,13 @@ +// The constructor of a base class should not be visible in the derived class +contract A { function A(string s) public { } } +contract B is A { + function f() pure public { + A x = A(0); // convert from address + string memory y = "ab"; + A(y); // call as a function is invalid + x; + } +} +// ---- +// Warning: (91-122): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (244-248): Explicit type conversion not allowed from "string memory" to "contract A". diff --git a/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol new file mode 100644 index 000000000..5e6191430 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol @@ -0,0 +1,5 @@ +contract C { + constructor(); +} +// ---- +// TypeError: (14-28): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol new file mode 100644 index 000000000..72458703d --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol @@ -0,0 +1,6 @@ +contract C { + function C(); +} +// ---- +// Warning: (14-27): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (14-27): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol new file mode 100644 index 000000000..30cf0668c --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol @@ -0,0 +1,5 @@ +contract test { + constructor() external {} +} +// ---- +// TypeError: (17-42): Constructor must be public or internal. diff --git a/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol new file mode 100644 index 000000000..278693615 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol @@ -0,0 +1,6 @@ +contract test { + function test() external {} +} +// ---- +// Warning: (17-44): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (17-44): Constructor must be public or internal. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol new file mode 100644 index 000000000..2a199b3a5 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol @@ -0,0 +1,13 @@ +// Previously, the type information for A was not yet available at the point of +// "new A". +contract B { + A a; + constructor() public { + a = new A(this); + } +} +contract A { + constructor(address a) internal {} +} +// ---- +// TypeError: (141-146): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol new file mode 100644 index 000000000..0a27e9f85 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol @@ -0,0 +1,15 @@ +// Previously, the type information for A was not yet available at the point of +// "new A". +contract B { + A a; + function B() public { + a = new A(this); + } +} +contract A { + function A(address a) internal {} +} +// ---- +// Warning: (112-155): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (172-205): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (140-145): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol new file mode 100644 index 000000000..2511c751c --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol @@ -0,0 +1,8 @@ +contract C { + constructor() internal {} +} +contract D { + function f() public { C c = new C(); c; } +} +// ---- +// TypeError: (84-89): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol new file mode 100644 index 000000000..2897e6f35 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol @@ -0,0 +1,9 @@ +contract C { + function C() internal {} +} +contract D { + function f() public { C x = new C(); x; } +} +// ---- +// Warning: (14-38): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (83-88): Contract with internal constructor cannot be created directly. diff --git a/test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol new file mode 100644 index 000000000..fa5d54c4f --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol @@ -0,0 +1,7 @@ +interface I { + constructor(); +} +// ---- +// Warning: (15-29): Functions in interfaces should be declared external. +// TypeError: (15-29): Constructor cannot be defined in interfaces. +// TypeError: (15-29): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol new file mode 100644 index 000000000..ddf549776 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol @@ -0,0 +1,8 @@ +interface I { + function I(); +} +// ---- +// Warning: (15-28): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (15-28): Functions in interfaces should be declared external. +// TypeError: (15-28): Constructor cannot be defined in interfaces. +// TypeError: (15-28): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol new file mode 100644 index 000000000..8db7e62a8 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol @@ -0,0 +1,6 @@ +library Lib { + constructor(); +} +// ---- +// TypeError: (15-29): Constructor cannot be defined in libraries. +// TypeError: (15-29): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol new file mode 100644 index 000000000..d44990492 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol @@ -0,0 +1,7 @@ +library Lib { + function Lib(); +} +// ---- +// Warning: (15-30): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (15-30): Constructor cannot be defined in libraries. +// TypeError: (15-30): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol new file mode 100644 index 000000000..3290a33b3 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol @@ -0,0 +1,6 @@ +// It is fine to "override" constructor of a base class since it is invisible +contract A { function A() public { } } +contract B is A { function A() public pure returns (uint8) {} } +// ---- +// Warning: (91-114): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (135-178): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol new file mode 100644 index 000000000..e6a03014e --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol @@ -0,0 +1,5 @@ +contract test { + constructor() public returns (uint a) { } +} +// ---- +// TypeError: (46-54): Non-empty "returns" directive for constructor. diff --git a/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol new file mode 100644 index 000000000..00b3974c6 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol @@ -0,0 +1,6 @@ +contract test { + function test() public returns (uint a) { } +} +// ---- +// Warning: (17-60): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// TypeError: (48-56): Non-empty "returns" directive for constructor. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol new file mode 100644 index 000000000..40341c4d5 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol @@ -0,0 +1,8 @@ +contract test { + function test(uint a) public { } + function test() public {} +} +// ---- +// Warning: (17-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// Warning: (51-76): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// DeclarationError: (17-49): More than one constructor defined. From 29a97f16411701c893b2887359524022c0fc6bd6 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 17 Apr 2018 11:40:02 +0200 Subject: [PATCH 2/2] Fix name clashes between constructor and fallback function. --- Changelog.md | 3 +- libsolidity/analysis/TypeChecker.cpp | 92 +++++++++---------- .../SolidityNameAndTypeResolution.cpp | 1 - .../constructor/two_constructors_mixed.sol | 7 ++ .../constructor/two_constructors_new.sol | 6 ++ .../constructor/two_constructors_old.sol | 2 +- 6 files changed, 61 insertions(+), 50 deletions(-) create mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol create mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_new.sol diff --git a/Changelog.md b/Changelog.md index cfd23ad0e..a0df396d6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,7 +4,8 @@ Features: * SMTChecker: Integration with CVC4 SMT solver Bugfixes: - + * Type Checker: Do not complain about new-style constructor and fallback function to have the same name. + * Type Checker: Detect multiple constructor declarations in the new syntax and old syntax. ### 0.4.22 (2018-04-16) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 47a551dc7..87d69d97f 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -109,39 +109,28 @@ bool TypeChecker::visit(ContractDefinition const& _contract) m_errorReporter.typeError(function->location(), "Constructor must be public or internal."); } - FunctionDefinition const* fallbackFunction = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) - { if (function->isFallback()) { - if (fallbackFunction) - { - m_errorReporter.declarationError(function->location(), "Only one fallback function is allowed."); - } - else - { - fallbackFunction = function; - if (_contract.isLibrary()) - m_errorReporter.typeError(fallbackFunction->location(), "Libraries cannot have fallback functions."); - if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) - m_errorReporter.typeError( - function->location(), - "Fallback function must be payable or non-payable, but is \"" + - stateMutabilityToString(function->stateMutability()) + - "\"." - ); - if (!fallbackFunction->parameters().empty()) - m_errorReporter.typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters."); - if (!fallbackFunction->returnParameters().empty()) - m_errorReporter.typeError(fallbackFunction->returnParameterList()->location(), "Fallback function cannot return values."); - if ( - _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && - fallbackFunction->visibility() != FunctionDefinition::Visibility::External - ) - m_errorReporter.typeError(fallbackFunction->location(), "Fallback function must be defined as \"external\"."); - } + if (_contract.isLibrary()) + m_errorReporter.typeError(function->location(), "Libraries cannot have fallback functions."); + if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) + m_errorReporter.typeError( + function->location(), + "Fallback function must be payable or non-payable, but is \"" + + stateMutabilityToString(function->stateMutability()) + + "\"." + ); + if (!function->parameters().empty()) + m_errorReporter.typeError(function->parameterList().location(), "Fallback function cannot take parameters."); + if (!function->returnParameters().empty()) + m_errorReporter.typeError(function->returnParameterList()->location(), "Fallback function cannot return values."); + if ( + _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && + function->visibility() != FunctionDefinition::Visibility::External + ) + m_errorReporter.typeError(function->location(), "Fallback function must be defined as \"external\"."); } - } for (auto const& n: _contract.subNodes()) if (!visited.count(n.get())) @@ -172,25 +161,34 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con /// Checks that two functions with the same name defined in this contract have different /// argument types and that there is at most one constructor. map> functions; + FunctionDefinition const* constructor = nullptr; + FunctionDefinition const* fallback = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) - functions[function->name()].push_back(function); - - // Constructor - if (functions[_contract.name()].size() > 1) - { - SecondarySourceLocation ssl; - auto it = ++functions[_contract.name()].begin(); - for (; it != functions[_contract.name()].end(); ++it) - ssl.append("Another declaration is here:", (*it)->location()); - - string msg = "More than one constructor defined."; - ssl.limitSize(msg); - m_errorReporter.declarationError( - functions[_contract.name()].front()->location(), - ssl, - msg - ); - } + if (function->isConstructor()) + { + if (constructor) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", constructor->location()), + "More than one constructor defined." + ); + constructor = function; + } + else if (function->isFallback()) + { + if (fallback) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", fallback->location()), + "Only one fallback function is allowed." + ); + fallback = function; + } + else + { + solAssert(!function->name().empty(), ""); + functions[function->name()].push_back(function); + } findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 301250f0a..7c0e86438 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1140,7 +1140,6 @@ BOOST_AUTO_TEST_CASE(fallback_function_twice) } )"; CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, (vector{ - "Function with same name and arguments defined twice.", "Only one fallback function is" })); } diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol new file mode 100644 index 000000000..c757354e8 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol @@ -0,0 +1,7 @@ +contract test { + function test(uint) public { } + constructor() public {} +} +// ---- +// Warning: (17-47): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// DeclarationError: (49-72): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol new file mode 100644 index 000000000..42c0de28f --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol @@ -0,0 +1,6 @@ +contract test { + constructor(uint) public { } + constructor() public {} +} +// ---- +// DeclarationError: (47-70): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol index 40341c4d5..db632ced1 100644 --- a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol @@ -5,4 +5,4 @@ contract test { // ---- // Warning: (17-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. // Warning: (51-76): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// DeclarationError: (17-49): More than one constructor defined. +// DeclarationError: (51-76): More than one constructor defined.