From 4522c804f342707ec2bb86a19735a4084108d96d Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Mon, 3 Sep 2018 18:17:59 +0200 Subject: [PATCH 1/3] Disallow single statement var decl in if/while/for without blocks --- Changelog.md | 1 + libsolidity/analysis/SyntaxChecker.cpp | 21 +++++++++++++++++++-- libsolidity/analysis/SyntaxChecker.h | 6 ++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 1e16df590..ec494c418 100644 --- a/Changelog.md +++ b/Changelog.md @@ -68,6 +68,7 @@ Breaking Changes: * Syntax Checker: Named return values in function types are an error. * Syntax Checker: Strictly require visibility specifier for functions. This was already the case in the experimental 0.5.0 mode. * Syntax Checker: Disallow unary ``+``. This was already the case in the experimental 0.5.0 mode. + * Syntax Checker: Disallow single statement variable declaration inside if/while/for without a block. * View Pure Checker: Strictly enfore state mutability. This was already the case in the experimental 0.5.0 mode. Language Features: diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index ac4fa72b0..1b5c00c43 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -138,9 +138,25 @@ void SyntaxChecker::endVisit(ModifierDefinition const& _modifier) m_placeholderFound = false; } -bool SyntaxChecker::visit(WhileStatement const&) +void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const* _statement) +{ + auto varDecl = dynamic_cast(_statement); + if (varDecl) + m_errorReporter.syntaxError(_statement->location(), "Invalid variable declaration. Please declare it inside a block."); +} + +bool SyntaxChecker::visit(IfStatement const& _ifStatement) +{ + checkSingleStatementVariableDeclaration(&_ifStatement.trueStatement()); + if (Statement const* _statement = _ifStatement.falseStatement()) + checkSingleStatementVariableDeclaration(_statement); + return true; +} + +bool SyntaxChecker::visit(WhileStatement const& _whileStatement) { m_inLoopDepth++; + checkSingleStatementVariableDeclaration(&_whileStatement.body()); return true; } @@ -149,9 +165,10 @@ void SyntaxChecker::endVisit(WhileStatement const&) m_inLoopDepth--; } -bool SyntaxChecker::visit(ForStatement const&) +bool SyntaxChecker::visit(ForStatement const& _forStatement) { m_inLoopDepth++; + checkSingleStatementVariableDeclaration(&_forStatement.body()); return true; } diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index 897df6767..27d98401a 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -52,6 +52,12 @@ private: virtual bool visit(ModifierDefinition const& _modifier) override; virtual void endVisit(ModifierDefinition const& _modifier) override; + // Reports an error if _statement is a VariableDeclarationStatement. + // Used by if/while/for to check for single statement variable declarations + // without a block. + void checkSingleStatementVariableDeclaration(ASTNode const* _statement); + + virtual bool visit(IfStatement const& _ifStatement) override; virtual bool visit(WhileStatement const& _whileStatement) override; virtual void endVisit(WhileStatement const& _whileStatement) override; virtual bool visit(ForStatement const& _forStatement) override; From 17176871ab4903498be0f9d62997ca1a9ace04d8 Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Tue, 4 Sep 2018 11:48:58 +0200 Subject: [PATCH 2/3] Changed error message and added tests --- libsolidity/analysis/SyntaxChecker.cpp | 2 +- .../syntaxTests/variableDeclaration/do_while.sol | 12 ++++++++++++ .../syntaxTests/variableDeclaration/else.sol | 13 +++++++++++++ .../syntaxTests/variableDeclaration/for.sol | 11 +++++++++++ .../syntaxTests/variableDeclaration/if.sol | 11 +++++++++++ .../syntaxTests/variableDeclaration/while.sol | 11 +++++++++++ 6 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/variableDeclaration/do_while.sol create mode 100644 test/libsolidity/syntaxTests/variableDeclaration/else.sol create mode 100644 test/libsolidity/syntaxTests/variableDeclaration/for.sol create mode 100644 test/libsolidity/syntaxTests/variableDeclaration/if.sol create mode 100644 test/libsolidity/syntaxTests/variableDeclaration/while.sol diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index 1b5c00c43..ab883a21c 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -142,7 +142,7 @@ void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const* _stat { auto varDecl = dynamic_cast(_statement); if (varDecl) - m_errorReporter.syntaxError(_statement->location(), "Invalid variable declaration. Please declare it inside a block."); + m_errorReporter.syntaxError(_statement->location(), "Variable declarations can only be used inside blocks."); } bool SyntaxChecker::visit(IfStatement const& _ifStatement) diff --git a/test/libsolidity/syntaxTests/variableDeclaration/do_while.sol b/test/libsolidity/syntaxTests/variableDeclaration/do_while.sol new file mode 100644 index 000000000..8fc48b334 --- /dev/null +++ b/test/libsolidity/syntaxTests/variableDeclaration/do_while.sol @@ -0,0 +1,12 @@ +pragma solidity >0.4.24; + +contract C +{ + function f(uint x) public pure { + do + uint y; + while (x > 0); + } +} +// ---- +// SyntaxError: (81-87): Variable declarations can only be used inside blocks. diff --git a/test/libsolidity/syntaxTests/variableDeclaration/else.sol b/test/libsolidity/syntaxTests/variableDeclaration/else.sol new file mode 100644 index 000000000..914e0c0c0 --- /dev/null +++ b/test/libsolidity/syntaxTests/variableDeclaration/else.sol @@ -0,0 +1,13 @@ +pragma solidity >0.4.24; + +contract C +{ + function f(uint x) public pure { + if (x > 0) + {uint y;} + else + uint z; + } +} +// ---- +// SyntaxError: (109-115): Variable declarations can only be used inside blocks. diff --git a/test/libsolidity/syntaxTests/variableDeclaration/for.sol b/test/libsolidity/syntaxTests/variableDeclaration/for.sol new file mode 100644 index 000000000..bc137f935 --- /dev/null +++ b/test/libsolidity/syntaxTests/variableDeclaration/for.sol @@ -0,0 +1,11 @@ +pragma solidity >0.4.24; + +contract C +{ + function f(uint x) public pure { + for (uint i = 0; i < x; ++i) + uint y; + } +} +// ---- +// SyntaxError: (107-113): Variable declarations can only be used inside blocks. diff --git a/test/libsolidity/syntaxTests/variableDeclaration/if.sol b/test/libsolidity/syntaxTests/variableDeclaration/if.sol new file mode 100644 index 000000000..75ab20266 --- /dev/null +++ b/test/libsolidity/syntaxTests/variableDeclaration/if.sol @@ -0,0 +1,11 @@ +pragma solidity >0.4.24; + +contract C +{ + function f(uint x) public pure { + if (x > 0) + uint y; + } +} +// ---- +// SyntaxError: (89-95): Variable declarations can only be used inside blocks. diff --git a/test/libsolidity/syntaxTests/variableDeclaration/while.sol b/test/libsolidity/syntaxTests/variableDeclaration/while.sol new file mode 100644 index 000000000..2997d80c8 --- /dev/null +++ b/test/libsolidity/syntaxTests/variableDeclaration/while.sol @@ -0,0 +1,11 @@ +pragma solidity >0.4.24; + +contract C +{ + function f(uint x) public pure { + while (x > 0) + uint y; + } +} +// ---- +// SyntaxError: (92-98): Variable declarations can only be used inside blocks. From ac8892e0e3ab4e0152ba74c5857b79aec54e7f1b Mon Sep 17 00:00:00 2001 From: Leonardo Alt Date: Tue, 4 Sep 2018 12:14:04 +0200 Subject: [PATCH 3/3] Review suggestions --- Changelog.md | 2 +- libsolidity/analysis/SyntaxChecker.cpp | 14 +++++++------- libsolidity/analysis/SyntaxChecker.h | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Changelog.md b/Changelog.md index ec494c418..c9d804324 100644 --- a/Changelog.md +++ b/Changelog.md @@ -68,7 +68,7 @@ Breaking Changes: * Syntax Checker: Named return values in function types are an error. * Syntax Checker: Strictly require visibility specifier for functions. This was already the case in the experimental 0.5.0 mode. * Syntax Checker: Disallow unary ``+``. This was already the case in the experimental 0.5.0 mode. - * Syntax Checker: Disallow single statement variable declaration inside if/while/for without a block. + * Syntax Checker: Disallow single statement variable declaration inside if/while/for bodies that are not blocks. * View Pure Checker: Strictly enfore state mutability. This was already the case in the experimental 0.5.0 mode. Language Features: diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index ab883a21c..0bc20f2e1 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -138,25 +138,25 @@ void SyntaxChecker::endVisit(ModifierDefinition const& _modifier) m_placeholderFound = false; } -void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const* _statement) +void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const& _statement) { - auto varDecl = dynamic_cast(_statement); + auto varDecl = dynamic_cast(&_statement); if (varDecl) - m_errorReporter.syntaxError(_statement->location(), "Variable declarations can only be used inside blocks."); + m_errorReporter.syntaxError(_statement.location(), "Variable declarations can only be used inside blocks."); } bool SyntaxChecker::visit(IfStatement const& _ifStatement) { - checkSingleStatementVariableDeclaration(&_ifStatement.trueStatement()); + checkSingleStatementVariableDeclaration(_ifStatement.trueStatement()); if (Statement const* _statement = _ifStatement.falseStatement()) - checkSingleStatementVariableDeclaration(_statement); + checkSingleStatementVariableDeclaration(*_statement); return true; } bool SyntaxChecker::visit(WhileStatement const& _whileStatement) { m_inLoopDepth++; - checkSingleStatementVariableDeclaration(&_whileStatement.body()); + checkSingleStatementVariableDeclaration(_whileStatement.body()); return true; } @@ -168,7 +168,7 @@ void SyntaxChecker::endVisit(WhileStatement const&) bool SyntaxChecker::visit(ForStatement const& _forStatement) { m_inLoopDepth++; - checkSingleStatementVariableDeclaration(&_forStatement.body()); + checkSingleStatementVariableDeclaration(_forStatement.body()); return true; } diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index 27d98401a..f5716bf93 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -52,10 +52,10 @@ private: virtual bool visit(ModifierDefinition const& _modifier) override; virtual void endVisit(ModifierDefinition const& _modifier) override; - // Reports an error if _statement is a VariableDeclarationStatement. - // Used by if/while/for to check for single statement variable declarations - // without a block. - void checkSingleStatementVariableDeclaration(ASTNode const* _statement); + /// Reports an error if _statement is a VariableDeclarationStatement. + /// Used by if/while/for to check for single statement variable declarations + /// without a block. + void checkSingleStatementVariableDeclaration(ASTNode const& _statement); virtual bool visit(IfStatement const& _ifStatement) override; virtual bool visit(WhileStatement const& _whileStatement) override;