From 182a0a95516e4f218524b929035e6a1bd5d2742c Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Wed, 27 Jun 2018 12:29:03 +0200 Subject: [PATCH 1/3] Disallows old constructor syntax. --- Changelog.md | 2 +- libsolidity/analysis/NameAndTypeResolver.cpp | 14 +++++---- libsolidity/analysis/SyntaxChecker.cpp | 28 ++++++------------ libsolidity/ast/AST.h | 1 - libsolidity/parsing/Parser.cpp | 30 +++++++++----------- libsolidity/parsing/Parser.h | 8 ++---- libsolidity/parsing/Token.h | 1 + 7 files changed, 35 insertions(+), 49 deletions(-) diff --git a/Changelog.md b/Changelog.md index 1064d8f67..aabe11edc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -24,7 +24,7 @@ Breaking Changes: * General: Disallow the ``throw`` statement. This was already the case in the experimental 0.5.0 mode. * General: Disallow the ``years`` unit denomination (was already deprecated in 0.4.24) * General: Introduce ``emit`` as a keyword instead of parsing it as identifier. - * General: New keywords: ``calldata`` + * General: New keywords: ``calldata`` and ``constructor`` * General: New reserved keywords: ``alias``, ``apply``, ``auto``, ``copyof``, ``define``, ``immutable``, ``implements``, ``macro``, ``mutable``, ``override``, ``partial``, ``promise``, ``reference``, ``sealed``, ``sizeof``, ``supports``, ``typedef`` and ``unchecked``. diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 7c23c992d..e0880ec9e 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -482,7 +482,15 @@ bool DeclarationRegistrationHelper::registerDeclaration( Declaration const* shadowedDeclaration = nullptr; if (_warnOnShadow && !name.empty() && _container.enclosingContainer()) for (auto const* decl: _container.enclosingContainer()->resolveName(name, true, true)) - shadowedDeclaration = decl; + // Do not warn about functions shadowing a contract. + if ( + !( + dynamic_cast(decl) && + dynamic_cast(&_declaration) && + name == decl->name() + ) + ) + shadowedDeclaration = decl; // We use "invisible" for both inactive variables in blocks and for members invisible in contracts. // They cannot both be true at the same time. @@ -710,10 +718,6 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio dynamic_cast(m_currentScope) ) warnAboutShadowing = false; - // Do not warn about the constructor shadowing the contract. - if (auto fun = dynamic_cast(&_declaration)) - if (fun->isConstructor()) - warnAboutShadowing = false; // Register declaration as inactive if we are in block scope. bool inactive = diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index 4d09e36d3..5d16a33f0 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -200,6 +200,14 @@ bool SyntaxChecker::visit(PlaceholderStatement const&) bool SyntaxChecker::visit(ContractDefinition const& _contract) { m_isInterface = _contract.contractKind() == ContractDefinition::ContractKind::Interface; + + ASTString const& contractName = _contract.name(); + for (FunctionDefinition const* function: _contract.definedFunctions()) + if (function->name() == contractName) + m_errorReporter.syntaxError(function->location(), + "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." + ); return true; } @@ -216,21 +224,6 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) ); } - if (_function.isOldStyleConstructor()) - { - if (v050) - m_errorReporter.syntaxError( - _function.location(), - "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." - ); - else - m_errorReporter.warning( - _function.location(), - "Defining constructors as functions with the same name as the contract is deprecated. " - "Use \"constructor(...) { ... }\" instead." - ); - } if (!_function.isImplemented() && !_function.modifiers().empty()) { if (v050) @@ -238,11 +231,6 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) else m_errorReporter.warning(_function.location(), "Modifiers of functions without implementation are ignored." ); } - if (_function.name() == "constructor") - m_errorReporter.warning(_function.location(), - "This function is named \"constructor\" but is not the constructor of the contract. " - "If you intend this to be a constructor, use \"constructor(...) { ... }\" without the \"function\" keyword to define it." - ); return true; } diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index acd90ad89..9ed3b9aad 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -614,7 +614,6 @@ public: StateMutability stateMutability() const { return m_stateMutability; } bool isConstructor() const { return m_isConstructor; } - bool isOldStyleConstructor() const { return m_isConstructor && !name().empty(); } bool isFallback() const { return !m_isConstructor && name().empty(); } bool isPayable() const { return m_stateMutability == StateMutability::Payable; } std::vector> const& modifiers() const { return m_functionModifiers; } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index e2bd6fb42..0bee2a913 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -239,13 +239,10 @@ ASTPointer Parser::parseContractDefinition(Token::Value _exp Token::Value currentTokenValue = m_scanner->currentToken(); if (currentTokenValue == Token::RBrace) break; - else if ( - currentTokenValue == Token::Function || - (currentTokenValue == Token::Identifier && m_scanner->currentLiteral() == "constructor") - ) + else if (currentTokenValue == Token::Function || currentTokenValue == Token::Constructor) // This can be a function or a state variable of function type (especially // complicated to distinguish fallback function from function type state variable) - subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable(name.get())); + subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable()); else if (currentTokenValue == Token::Struct) subNodes.push_back(parseStructDefinition()); else if (currentTokenValue == Token::Enum) @@ -340,30 +337,31 @@ StateMutability Parser::parseStateMutability(Token::Value _token) return stateMutability; } -Parser::FunctionHeaderParserResult Parser::parseFunctionHeader( - bool _forceEmptyName, - bool _allowModifiers, - ASTString const* _contractName -) +Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyName, bool _allowModifiers) { RecursionGuard recursionGuard(*this); FunctionHeaderParserResult result; result.isConstructor = false; - if (m_scanner->currentToken() == Token::Identifier && m_scanner->currentLiteral() == "constructor") + if (m_scanner->currentToken() == Token::Constructor) result.isConstructor = true; else if (m_scanner->currentToken() != Token::Function) solAssert(false, "Function or constructor expected."); m_scanner->next(); - if (result.isConstructor || _forceEmptyName || m_scanner->currentToken() == Token::LParen) + if (result.isConstructor) result.name = make_shared(); + else if (_forceEmptyName || m_scanner->currentToken() == Token::LParen) + result.name = make_shared(); + else if (m_scanner->currentToken() == Token::Constructor) + fatalParserError(string( + "This function is named \"constructor\" but is not the constructor of the contract. " + "If you intend this to be a constructor, use \"constructor(...) { ... }\" without the \"function\" keyword to define it." + )); else result.name = expectIdentifierToken(); - if (!result.name->empty() && _contractName && *result.name == *_contractName) - result.isConstructor = true; VarDeclParserOptions options; options.allowLocationSpecifier = true; @@ -435,7 +433,7 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader( return result; } -ASTPointer Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(ASTString const* _contractName) +ASTPointer Parser::parseFunctionDefinitionOrFunctionTypeStateVariable() { RecursionGuard recursionGuard(*this); ASTNodeFactory nodeFactory(*this); @@ -443,7 +441,7 @@ ASTPointer Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(A if (m_scanner->currentCommentLiteral() != "") docstring = make_shared(m_scanner->currentCommentLiteral()); - FunctionHeaderParserResult header = parseFunctionHeader(false, true, _contractName); + FunctionHeaderParserResult header = parseFunctionHeader(false, true); if ( header.isConstructor || diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index 086533645..c906771a6 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -74,12 +74,8 @@ private: ASTPointer parseInheritanceSpecifier(); Declaration::Visibility parseVisibilitySpecifier(Token::Value _token); StateMutability parseStateMutability(Token::Value _token); - FunctionHeaderParserResult parseFunctionHeader( - bool _forceEmptyName, - bool _allowModifiers, - ASTString const* _contractName = nullptr - ); - ASTPointer parseFunctionDefinitionOrFunctionTypeStateVariable(ASTString const* _contractName); + FunctionHeaderParserResult parseFunctionHeader(bool _forceEmptyName, bool _allowModifiers); + ASTPointer parseFunctionDefinitionOrFunctionTypeStateVariable(); ASTPointer parseFunctionDefinition(ASTString const* _contractName); ASTPointer parseStructDefinition(); ASTPointer parseEnumDefinition(); diff --git a/libsolidity/parsing/Token.h b/libsolidity/parsing/Token.h index cb855cbe1..7ce24e690 100644 --- a/libsolidity/parsing/Token.h +++ b/libsolidity/parsing/Token.h @@ -144,6 +144,7 @@ namespace solidity K(Assembly, "assembly", 0) \ K(Break, "break", 0) \ K(Constant, "constant", 0) \ + K(Constructor, "constructor", 0) \ K(Continue, "continue", 0) \ K(Contract, "contract", 0) \ K(Do, "do", 0) \ From de6cd2425b6ea9cdf2a7b7f49cd88e4f18fa20e3 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Fri, 29 Jun 2018 17:05:23 +0200 Subject: [PATCH 2/3] Adjusts syntax tests to new constructor syntax. --- ...sol => constructible_internal_constructor.sol} | 0 .../constructible_internal_constructor_old.sol | 9 --------- .../{constructor_new.sol => constructor.sol} | 0 .../syntaxTests/constructor/constructor_old.sol | 2 +- .../constructor/constructor_old_050.sol | 4 ---- ...y_new.sol => constructor_state_mutability.sol} | 0 .../constructor_state_mutability_old.sol | 11 ----------- ...ibility_new.sol => constructor_visibility.sol} | 0 .../constructor/constructor_visibility_old.sol | 13 ------------- ...sol => constructor_without_implementation.sol} | 0 .../constructor_without_implementation_old.sol | 6 ------ ...nstructor_new.sol => external_constructor.sol} | 0 .../constructor/external_constructor_old.sol | 6 ------ .../constructor/function_named_constructor.sol | 2 +- ...l => inconstructible_internal_constructor.sol} | 0 ...nstructible_internal_constructor_inverted.sol} | 0 ...ructible_internal_constructor_inverted_old.sol | 15 --------------- .../inconstructible_internal_constructor_old.sol | 9 --------- ...structor_new.sol => interface_constructor.sol} | 0 .../constructor/interface_constructor_old.sol | 8 -------- ...onstructor_new.sol => library_constructor.sol} | 0 .../constructor/library_constructor_old.sol | 7 ------- .../constructor/overriding_constructor.sol | 4 ---- ...tructor_new.sol => returns_in_constructor.sol} | 0 .../constructor/returns_in_constructor_old.sol | 6 ------ ..._constructors_new.sol => two_constructors.sol} | 0 .../constructor/two_constructors_mixed.sol | 7 ------- .../constructor/two_constructors_old.sol | 8 -------- .../function_type_constructor_local.sol | 2 +- 29 files changed, 3 insertions(+), 116 deletions(-) rename test/libsolidity/syntaxTests/constructor/{constructible_internal_constructor_new.sol => constructible_internal_constructor.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol rename test/libsolidity/syntaxTests/constructor/{constructor_new.sol => constructor.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/constructor_old_050.sol rename test/libsolidity/syntaxTests/constructor/{constructor_state_mutability_new.sol => constructor_state_mutability.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol rename test/libsolidity/syntaxTests/constructor/{constructor_visibility_new.sol => constructor_visibility.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol rename test/libsolidity/syntaxTests/constructor/{constructor_without_implementation_new.sol => constructor_without_implementation.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol rename test/libsolidity/syntaxTests/constructor/{external_constructor_new.sol => external_constructor.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/external_constructor_old.sol rename test/libsolidity/syntaxTests/constructor/{inconstructible_internal_constructor_new.sol => inconstructible_internal_constructor.sol} (100%) rename test/libsolidity/syntaxTests/constructor/{inconstructible_internal_constructor_inverted_new.sol => inconstructible_internal_constructor_inverted.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol delete mode 100644 test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol rename test/libsolidity/syntaxTests/constructor/{interface_constructor_new.sol => interface_constructor.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol rename test/libsolidity/syntaxTests/constructor/{library_constructor_new.sol => library_constructor.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/library_constructor_old.sol rename test/libsolidity/syntaxTests/constructor/{returns_in_constructor_new.sol => returns_in_constructor.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol rename test/libsolidity/syntaxTests/constructor/{two_constructors_new.sol => two_constructors.sol} (100%) delete mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol delete mode 100644 test/libsolidity/syntaxTests/constructor/two_constructors_old.sol diff --git a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/constructible_internal_constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol deleted file mode 100644 index 144743e38..000000000 --- a/test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol +++ /dev/null @@ -1,9 +0,0 @@ -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.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_old.sol index 9ec6257dd..127f9176f 100644 --- a/test/libsolidity/syntaxTests/constructor/constructor_old.sol +++ b/test/libsolidity/syntaxTests/constructor/constructor_old.sol @@ -1,3 +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. +// SyntaxError: (13-35): 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_old_050.sol b/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol deleted file mode 100644 index 19e46e793..000000000 --- a/test/libsolidity/syntaxTests/constructor/constructor_old_050.sol +++ /dev/null @@ -1,4 +0,0 @@ -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.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol rename to test/libsolidity/syntaxTests/constructor/constructor_state_mutability.sol diff --git a/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol deleted file mode 100644 index b9f2a4bbb..000000000 --- a/test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol +++ /dev/null @@ -1,11 +0,0 @@ -contract test1 { - function test1() public view {} -} -contract test2 { - function test2() public pure {} -} -// ---- -// Warning: (21-52): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// Warning: (76-107): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// TypeError: (21-52): Constructor must be payable or non-payable, but is "view". -// TypeError: (76-107): 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.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol rename to test/libsolidity/syntaxTests/constructor/constructor_visibility.sol diff --git a/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol deleted file mode 100644 index 65f989b02..000000000 --- a/test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol +++ /dev/null @@ -1,13 +0,0 @@ -// The constructor of a base class should not be visible in the derived class -contract A { function A(string memory 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-129): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// TypeError: (251-255): 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.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol rename to test/libsolidity/syntaxTests/constructor/constructor_without_implementation.sol diff --git a/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol deleted file mode 100644 index 12bf63158..000000000 --- a/test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol +++ /dev/null @@ -1,6 +0,0 @@ -contract C { - function C() public; -} -// ---- -// Warning: (14-34): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// TypeError: (14-34): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/external_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/external_constructor.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/external_constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/external_constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol deleted file mode 100644 index 278693615..000000000 --- a/test/libsolidity/syntaxTests/constructor/external_constructor_old.sol +++ /dev/null @@ -1,6 +0,0 @@ -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/function_named_constructor.sol b/test/libsolidity/syntaxTests/constructor/function_named_constructor.sol index 29784033e..68273c0a0 100644 --- a/test/libsolidity/syntaxTests/constructor/function_named_constructor.sol +++ b/test/libsolidity/syntaxTests/constructor/function_named_constructor.sol @@ -2,4 +2,4 @@ contract C { function constructor() public; } // ---- -// Warning: (17-47): This function is named "constructor" but is not the constructor of the contract. If you intend this to be a constructor, use "constructor(...) { ... }" without the "function" keyword to define it. +// ParserError: (26-37): This function is named "constructor" but is not the constructor of the contract. If you intend this to be a constructor, use "constructor(...) { ... }" without the "function" keyword to define it. diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol rename to test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted.sol diff --git a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol deleted file mode 100644 index 0a27e9f85..000000000 --- a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol +++ /dev/null @@ -1,15 +0,0 @@ -// 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_old.sol b/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol deleted file mode 100644 index 2897e6f35..000000000 --- a/test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol +++ /dev/null @@ -1,9 +0,0 @@ -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.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/interface_constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol deleted file mode 100644 index 2c029f4d0..000000000 --- a/test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol +++ /dev/null @@ -1,8 +0,0 @@ -interface I { - function I() public; -} -// ---- -// Warning: (15-35): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// TypeError: (15-35): Functions in interfaces must be declared external. -// TypeError: (15-35): Constructor cannot be defined in interfaces. -// TypeError: (15-35): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/library_constructor_new.sol b/test/libsolidity/syntaxTests/constructor/library_constructor.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/library_constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/library_constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol deleted file mode 100644 index 271cc790e..000000000 --- a/test/libsolidity/syntaxTests/constructor/library_constructor_old.sol +++ /dev/null @@ -1,7 +0,0 @@ -library Lib { - function Lib() public; -} -// ---- -// Warning: (15-37): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// TypeError: (15-37): Constructor cannot be defined in libraries. -// TypeError: (15-37): Constructor must be implemented if declared. diff --git a/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol index 5fb3a189c..a38d901d5 100644 --- a/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol +++ b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol @@ -4,7 +4,3 @@ contract C is A { function A() public pure returns (uint8) {} } contract D is B { function B() public pure returns (uint8) {} } contract E is D { function B() public pure returns (uint8) {} } // ---- -// Warning: (57-100): This declaration shadows an existing declaration. -// Warning: (121-164): This declaration shadows an existing declaration. -// Warning: (185-228): This declaration shadows an existing declaration. -// Warning: (249-292): 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.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol rename to test/libsolidity/syntaxTests/constructor/returns_in_constructor.sol diff --git a/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol b/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol deleted file mode 100644 index 00b3974c6..000000000 --- a/test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol +++ /dev/null @@ -1,6 +0,0 @@ -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_new.sol b/test/libsolidity/syntaxTests/constructor/two_constructors.sol similarity index 100% rename from test/libsolidity/syntaxTests/constructor/two_constructors_new.sol rename to test/libsolidity/syntaxTests/constructor/two_constructors.sol diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol deleted file mode 100644 index c757354e8..000000000 --- a/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol +++ /dev/null @@ -1,7 +0,0 @@ -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_old.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol deleted file mode 100644 index db632ced1..000000000 --- a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol +++ /dev/null @@ -1,8 +0,0 @@ -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: (51-76): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/functionTypes/function_type_constructor_local.sol b/test/libsolidity/syntaxTests/functionTypes/function_type_constructor_local.sol index b89a3bb49..42697b739 100644 --- a/test/libsolidity/syntaxTests/functionTypes/function_type_constructor_local.sol +++ b/test/libsolidity/syntaxTests/functionTypes/function_type_constructor_local.sol @@ -5,4 +5,4 @@ contract C { } } // ---- -// ParserError: (118-119): Expected ';' but got identifier +// ParserError: (104-115): Expected primary expression. From b0b35e1e6b762d3849710054fdb2e1909f6780d9 Mon Sep 17 00:00:00 2001 From: Erik Kundt Date: Tue, 17 Jul 2018 22:46:17 +0200 Subject: [PATCH 3/3] Adds warning if function is shadowing a contract. --- libsolidity/analysis/NameAndTypeResolver.cpp | 10 +--------- .../syntaxTests/constructor/constructor_old.sol | 1 + .../constructor/overriding_constructor.sol | 14 +++++++++----- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index e0880ec9e..823378c7e 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -482,15 +482,7 @@ bool DeclarationRegistrationHelper::registerDeclaration( Declaration const* shadowedDeclaration = nullptr; if (_warnOnShadow && !name.empty() && _container.enclosingContainer()) for (auto const* decl: _container.enclosingContainer()->resolveName(name, true, true)) - // Do not warn about functions shadowing a contract. - if ( - !( - dynamic_cast(decl) && - dynamic_cast(&_declaration) && - name == decl->name() - ) - ) - shadowedDeclaration = decl; + shadowedDeclaration = decl; // We use "invisible" for both inactive variables in blocks and for members invisible in contracts. // They cannot both be true at the same time. diff --git a/test/libsolidity/syntaxTests/constructor/constructor_old.sol b/test/libsolidity/syntaxTests/constructor/constructor_old.sol index 127f9176f..9ead6858a 100644 --- a/test/libsolidity/syntaxTests/constructor/constructor_old.sol +++ b/test/libsolidity/syntaxTests/constructor/constructor_old.sol @@ -1,3 +1,4 @@ contract A { function A() public {} } // ---- // SyntaxError: (13-35): 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. +// Warning: (13-35): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol index a38d901d5..30cf3bce9 100644 --- a/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol +++ b/test/libsolidity/syntaxTests/constructor/overriding_constructor.sol @@ -1,6 +1,10 @@ -contract A { constructor() public {} } -contract B is A { function A() public pure returns (uint8) {} } -contract C is A { function A() public pure returns (uint8) {} } -contract D is B { function B() public pure returns (uint8) {} } -contract E is D { function B() public pure returns (uint8) {} } +contract A { function f() public {} } +contract B is A { + function A() public pure returns (uint8) {} + function g() public { + A.f(); + } +} // ---- +// Warning: (58-101): This declaration shadows an existing declaration. +// TypeError: (130-133): Member "f" not found or not visible after argument-dependent lookup in function () pure returns (uint8).