From 9bc9fe6af7241479fe3099eae235452b054a6f11 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 21 Apr 2017 11:13:10 +0200 Subject: Warn about side-effect free statements. --- Changelog.md | 1 + libsolidity/analysis/StaticAnalyzer.cpp | 7 +++ libsolidity/analysis/StaticAnalyzer.h | 2 + libsolidity/analysis/TypeChecker.cpp | 4 +- test/libsolidity/SolidityNameAndTypeResolution.cpp | 58 ++++++++++++++++------ 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/Changelog.md b/Changelog.md index 352e8374..3603d315 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ Features: * Commandline interface: Add the ``--standard-json`` parameter to process a Standard JSON I/O. * Commandline interface: Support ``--allow-paths`` to define trusted import paths. Note: the path(s) of the supplied source file(s) is always trusted. + * Static analyzer: Warn about statements without effects. Bugfixes: * Assembly output: Implement missing AssemblyItem types. diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index c39f874e..df7f6e88 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -57,6 +57,13 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&) m_nonPayablePublic = false; } +bool StaticAnalyzer::visit(ExpressionStatement const& _statement) +{ + if (_statement.expression().annotation().isPure) + warning(_statement.location(), "Statement has no effects."); + return true; +} + bool StaticAnalyzer::visit(MemberAccess const& _memberAccess) { if (m_nonPayablePublic && !m_library) diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index 0cb961bd..84342322 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -60,6 +60,8 @@ private: virtual bool visit(FunctionDefinition const& _function) override; virtual void endVisit(FunctionDefinition const& _function) override; + virtual bool visit(ExpressionStatement const& _statement) override; + virtual bool visit(MemberAccess const& _memberAccess) override; ErrorList& m_errors; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index dc04404d..b37db7b7 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1674,8 +1674,8 @@ bool TypeChecker::visit(Identifier const& _identifier) if (auto variableDeclaration = dynamic_cast(annotation.referencedDeclaration)) annotation.isPure = annotation.isConstant = variableDeclaration->isConstant(); else if (dynamic_cast(annotation.referencedDeclaration)) - if (auto functionType = dynamic_cast(annotation.type.get())) - annotation.isPure = functionType->isPure(); + if (dynamic_cast(annotation.type.get())) + annotation.isPure = true; return false; } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 1388f01b..b3aa899d 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -3653,54 +3653,56 @@ BOOST_AUTO_TEST_CASE(conditional_with_all_types) // integers uint x; uint y; - true ? x : y; + uint g = true ? x : y; // integer constants - true ? 1 : 3; + uint h = true ? 1 : 3; // string literal - true ? "hello" : "world"; - + var i = true ? "hello" : "world"; + } + function f2() { // bool - true ? true : false; + bool j = true ? true : false; // real is not there yet. // array byte[2] memory a; byte[2] memory b; - true ? a : b; + var k = true ? a : b; bytes memory e; bytes memory f; - true ? e : f; + var l = true ? e : f; // fixed bytes bytes2 c; bytes2 d; - true ? c : d; - + var m = true ? c : d; + } + function f3() { // contract doesn't fit in here // struct - true ? struct_x : struct_y; + struct_x = true ? struct_x : struct_y; // function - true ? fun_x : fun_y; + var r = true ? fun_x : fun_y; // enum small enum_x; small enum_y; - true ? enum_x : enum_y; + enum_x = true ? enum_x : enum_y; // tuple - true ? (1, 2) : (3, 4); + var (n, o) = true ? (1, 2) : (3, 4); // mapping - true ? table1 : table2; + var p = true ? table1 : table2; // typetype - true ? uint32(1) : uint32(2); + var q = true ? uint32(1) : uint32(2); // modifier doesn't fit in here @@ -5477,6 +5479,32 @@ BOOST_AUTO_TEST_CASE(using_interface_complex) success(text); } +BOOST_AUTO_TEST_CASE(bare_revert) +{ + char const* text = R"( + contract C { + function f(uint x) { + if (x > 7) + revert; + } + } + )"; + CHECK_WARNING(text, "Statement has no effects."); +} + +BOOST_AUTO_TEST_CASE(pure_statement_in_for_loop) +{ + char const* text = R"( + contract C { + function f() { + for (uint x = 0; x < 10; true) + x++; + } + } + )"; + CHECK_WARNING(text, "Statement has no effects."); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit From 937695bfdc76a7c0c8b828c26121e7dd0c641acf Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 21 Apr 2017 15:04:03 +0200 Subject: Change error message. --- libsolidity/analysis/StaticAnalyzer.cpp | 2 +- test/libsolidity/SolidityNameAndTypeResolution.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index df7f6e88..190d2420 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -60,7 +60,7 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&) bool StaticAnalyzer::visit(ExpressionStatement const& _statement) { if (_statement.expression().annotation().isPure) - warning(_statement.location(), "Statement has no effects."); + warning(_statement.location(), "Statement has no effect."); return true; } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index b3aa899d..9126806c 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5489,7 +5489,7 @@ BOOST_AUTO_TEST_CASE(bare_revert) } } )"; - CHECK_WARNING(text, "Statement has no effects."); + CHECK_WARNING(text, "Statement has no effect."); } BOOST_AUTO_TEST_CASE(pure_statement_in_for_loop) @@ -5502,7 +5502,7 @@ BOOST_AUTO_TEST_CASE(pure_statement_in_for_loop) } } )"; - CHECK_WARNING(text, "Statement has no effects."); + CHECK_WARNING(text, "Statement has no effect."); } BOOST_AUTO_TEST_SUITE_END() -- cgit From aad64d818a9ad25c19cecf1e4b4a026aeb968051 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 21 Apr 2017 20:09:37 +0200 Subject: Test for side-effect free condition. --- test/libsolidity/SolidityNameAndTypeResolution.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 9126806c..0de89aa1 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5505,6 +5505,19 @@ BOOST_AUTO_TEST_CASE(pure_statement_in_for_loop) CHECK_WARNING(text, "Statement has no effect."); } +BOOST_AUTO_TEST_CASE(pure_statement_check_for_regular_for_loop) +{ + char const* text = R"( + contract C { + function f() { + for (uint x = 0; true; x++) + {} + } + } + )"; + success(text); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit From 9577f87dfcd2d8b532f698c0aa1e8f9c01bb0ba5 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 24 Apr 2017 18:08:21 +0200 Subject: More pure tests. --- test/libsolidity/SolidityNameAndTypeResolution.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 0de89aa1..b98c3706 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5492,6 +5492,14 @@ BOOST_AUTO_TEST_CASE(bare_revert) CHECK_WARNING(text, "Statement has no effect."); } +BOOST_AUTO_TEST_CASE(bare_others) +{ + CHECK_WARNING("contract C { function f() { selfdestruct; } }", "Statement has no effect."); + CHECK_WARNING("contract C { function f() { assert; } }", "Statement has no effect."); + CHECK_WARNING("contract C { function f() { require; } }", "Statement has no effect."); + CHECK_WARNING("contract C { function f() { suicide; } }", "Statement has no effect."); +} + BOOST_AUTO_TEST_CASE(pure_statement_in_for_loop) { char const* text = R"( -- cgit