From 2ad1acaf721072d27783d586048d6377be6c3f99 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 16 Mar 2018 12:39:30 +0100 Subject: Warn if modifiers are applied to functions without implementation. --- Changelog.md | 1 + libsolidity/analysis/SyntaxChecker.cpp | 7 +++++++ libsolidity/parsing/Parser.cpp | 8 ++++---- .../modifiers/modifiers_on_abstract_functions_050.sol | 10 ++++++++++ .../modifiers_on_abstract_functions_no_parser_error.sol | 13 +++++++++++++ 5 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol create mode 100644 test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol diff --git a/Changelog.md b/Changelog.md index db080b05..e2174cfd 100644 --- a/Changelog.md +++ b/Changelog.md @@ -14,6 +14,7 @@ Features: * Optimizer: Optimize across ``mload`` if ``msize()`` is not used. * Static Analyzer: Error on duplicated super constructor calls as experimental 0.5.0 feature. * Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature). + * Syntax Checker: Warn about modifiers on functions without implementation (this will turn into an error with version 0.5.0). * Syntax Tests: Add source locations to syntax test expectations. * General: Introduce new constructor syntax using the ``constructor`` keyword as experimental 0.5.0 feature. * Inheritance: Error when using empty parentheses for base class constructors that require arguments as experimental 0.5.0 feature. diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index 343b4ba8..f648e5b4 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -232,6 +232,13 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) "Use \"constructor(...) { ... }\" instead." ); } + if (!_function.isImplemented() && !_function.modifiers().empty()) + { + if (v050) + m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers."); + else + m_errorReporter.warning( _function.location(), "Modifiers of functions without implementation are ignored." ); + } return true; } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index bdb46ad6..ea092a74 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -365,12 +365,12 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader( Token::Value token = m_scanner->currentToken(); if (_allowModifiers && token == Token::Identifier) { - // This can either be a modifier (function declaration) or the name of the - // variable (function type name plus variable). - if ( + // If the name is empty, then this can either be a modifier (fallback function declaration) + // or the name of the state variable (function type name plus variable). + if (result.name->empty() && ( m_scanner->peekNextToken() == Token::Semicolon || m_scanner->peekNextToken() == Token::Assign - ) + )) // Variable declaration, break here. break; else diff --git a/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol new file mode 100644 index 00000000..af1babbc --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol @@ -0,0 +1,10 @@ +pragma experimental "v0.5.0"; +contract C +{ + modifier only_owner() { _; } + function foo() only_owner public; + function bar() public only_owner; +} +// ---- +// SyntaxError: (80-113): Functions without implementation cannot have modifiers. +// SyntaxError: (118-151): Functions without implementation cannot have modifiers. diff --git a/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol new file mode 100644 index 00000000..e18c5cf9 --- /dev/null +++ b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol @@ -0,0 +1,13 @@ +// Previous versions of Solidity turned this +// into a parser error (they wrongly recognized +// these functions as state variables of +// function type). +contract C +{ + modifier only_owner() { _; } + function foo() only_owner public; + function bar() public only_owner; +} +// ---- +// Warning: (203-236): Modifiers of functions without implementation are ignored. +// Warning: (241-274): Modifiers of functions without implementation are ignored. -- cgit v1.2.3