diff --git a/.arclint b/.arclint --- a/.arclint +++ b/.arclint @@ -80,6 +80,13 @@ "spelling.dictionaries": [ "test/lint/dictionary/english.json" ] + }, + "lint-assert-with-side-effects": { + "type": "lint-assert-with-side-effects", + "include": "(^src/.*\\.(h|cpp)$)", + "exclude": [ + "(^src/(secp256k1|univalue|leveldb)/)" + ] } } } diff --git a/arcanist/.phutil_module_cache b/arcanist/.phutil_module_cache --- a/arcanist/.phutil_module_cache +++ b/arcanist/.phutil_module_cache @@ -1 +1 @@ -{"__symbol_cache_version__":11,"90a8b110dc475955f15bb81d37268cb5":{"have":{"class":{"AutoPEP8FormatLinter":75}},"need":{"function":{"pht":297,"execx":769,"id":1903},"class":{"ArcanistExternalLinter":104,"ArcanistLintMessage":1910,"Filesystem":1754,"ArcanistLinter":2017,"ArcanistLintSeverity":2095}},"xmap":{"AutoPEP8FormatLinter":["ArcanistExternalLinter"]}},"bf0805c02029a7226e8c0d7dee039b3c":{"have":{"class":{"CheckDocLinter":106}},"need":{"function":{"pht":323,"id":1847},"class":{"ArcanistExternalLinter":129,"ArcanistLintMessage":1854,"Filesystem":731,"ArcanistLinter":1902,"ArcanistLintSeverity":1988}},"xmap":{"CheckDocLinter":["ArcanistExternalLinter"]}},"6af7410cfea496ff1d4dcc2624b6b8ea":{"have":{"class":{"ClangFormatLinter":79}},"need":{"function":{"pht":302,"execx":781,"id":1653},"class":{"ArcanistExternalLinter":105,"ArcanistLintMessage":1660,"Filesystem":1504,"ArcanistLinter":1767,"ArcanistLintSeverity":1845}},"xmap":{"ClangFormatLinter":["ArcanistExternalLinter"]}},"9285ad9415f8ebe564f7119e5a72c559":{"have":{"class":{"FormatStringLinter":146}},"need":{"function":{"pht":377,"csprintf":1492,"id":1872},"class":{"ArcanistExternalLinter":173,"ArcanistLintMessage":1879,"Filesystem":827,"ArcanistLinter":1956,"ArcanistLintSeverity":2044}},"xmap":{"FormatStringLinter":["ArcanistExternalLinter"]}},"224d394856b17878058b4c14acb7178b":{"have":{"class":{"LocaleDependenceLinter":160}},"need":{"function":{"pht":5400},"class":{"ArcanistLinter":191,"ArcanistLintSeverity":5903,"Filesystem":6149}},"xmap":{"LocaleDependenceLinter":["ArcanistLinter"]}},"6f2f22dd0f259fb2eaa284b4fab3bc29":{"have":{"class":{"PythonFormatLinter":123}},"need":{"function":{"pht":353,"id":1838},"class":{"ArcanistExternalLinter":150,"ArcanistLintMessage":1845,"Filesystem":776,"ArcanistLinter":1970,"ArcanistLintSeverity":2053}},"xmap":{"PythonFormatLinter":["ArcanistExternalLinter"]}},"25781df78f6eebfb223296b8265e9d19":{"have":{"class":{"TestsLinter":103}},"need":{"function":{"pht":318,"id":2629},"class":{"ArcanistExternalLinter":123,"ArcanistLintMessage":2636,"Filesystem":776,"ArcanistLinter":2684,"ArcanistLintSeverity":2792}},"xmap":{"TestsLinter":["ArcanistExternalLinter"]}},"38f0c676bff5192a344464142caaa253":{"have":{"class":{"CHeaderLinter":99}},"need":{"function":{"pht":611},"class":{"ArcanistLinter":121,"ArcanistLintSeverity":1060,"Filesystem":1307}},"xmap":{"CHeaderLinter":["ArcanistLinter"]}}} \ No newline at end of file +{"__symbol_cache_version__":11,"90a8b110dc475955f15bb81d37268cb5":{"have":{"class":{"AutoPEP8FormatLinter":75}},"need":{"function":{"pht":297,"execx":769,"id":1903},"class":{"ArcanistExternalLinter":104,"ArcanistLintMessage":1910,"Filesystem":1754,"ArcanistLinter":2017,"ArcanistLintSeverity":2095}},"xmap":{"AutoPEP8FormatLinter":["ArcanistExternalLinter"]}},"38f0c676bff5192a344464142caaa253":{"have":{"class":{"CHeaderLinter":99}},"need":{"function":{"pht":611},"class":{"ArcanistLinter":121,"ArcanistLintSeverity":1060,"Filesystem":1307}},"xmap":{"CHeaderLinter":["ArcanistLinter"]}},"a30e4e25376ca05d4ae719915441be9e":{"have":{"class":{"CheckDocLinter":106}},"need":{"function":{"pht":323,"id":1848},"class":{"ArcanistExternalLinter":129,"ArcanistLintMessage":1855,"Filesystem":731,"ArcanistLinter":1903,"ArcanistLintSeverity":1989}},"xmap":{"CheckDocLinter":["ArcanistExternalLinter"]}},"6af7410cfea496ff1d4dcc2624b6b8ea":{"have":{"class":{"ClangFormatLinter":79}},"need":{"function":{"pht":302,"execx":781,"id":1653},"class":{"ArcanistExternalLinter":105,"ArcanistLintMessage":1660,"Filesystem":1504,"ArcanistLinter":1767,"ArcanistLintSeverity":1845}},"xmap":{"ClangFormatLinter":["ArcanistExternalLinter"]}},"9285ad9415f8ebe564f7119e5a72c559":{"have":{"class":{"FormatStringLinter":146}},"need":{"function":{"pht":377,"csprintf":1492,"id":1872},"class":{"ArcanistExternalLinter":173,"ArcanistLintMessage":1879,"Filesystem":827,"ArcanistLinter":1956,"ArcanistLintSeverity":2044}},"xmap":{"FormatStringLinter":["ArcanistExternalLinter"]}},"224d394856b17878058b4c14acb7178b":{"have":{"class":{"LocaleDependenceLinter":160}},"need":{"function":{"pht":5400},"class":{"ArcanistLinter":191,"ArcanistLintSeverity":5903,"Filesystem":6149}},"xmap":{"LocaleDependenceLinter":["ArcanistLinter"]}},"6f2f22dd0f259fb2eaa284b4fab3bc29":{"have":{"class":{"PythonFormatLinter":123}},"need":{"function":{"pht":353,"id":1838},"class":{"ArcanistExternalLinter":150,"ArcanistLintMessage":1845,"Filesystem":776,"ArcanistLinter":1970,"ArcanistLintSeverity":2053}},"xmap":{"PythonFormatLinter":["ArcanistExternalLinter"]}},"25781df78f6eebfb223296b8265e9d19":{"have":{"class":{"TestsLinter":103}},"need":{"function":{"pht":318,"id":2629},"class":{"ArcanistExternalLinter":123,"ArcanistLintMessage":2636,"Filesystem":776,"ArcanistLinter":2684,"ArcanistLintSeverity":2792}},"xmap":{"TestsLinter":["ArcanistExternalLinter"]}},"2809b09d2021203b43c57da33d1fe8bf":{"have":{"class":{"AssertWithSideEffectsLinter":210}},"need":{"function":{"pht":439},"class":{"ArcanistLinter":246,"ArcanistLintSeverity":926,"Filesystem":1170}},"xmap":{"AssertWithSideEffectsLinter":["ArcanistLinter"]}}} \ No newline at end of file diff --git a/arcanist/__phutil_library_map__.php b/arcanist/__phutil_library_map__.php --- a/arcanist/__phutil_library_map__.php +++ b/arcanist/__phutil_library_map__.php @@ -9,6 +9,7 @@ phutil_register_library_map(array( '__library_version__' => 2, 'class' => array( + 'AssertWithSideEffectsLinter' => 'linter/AssertWithSideEffectsLinter.php', 'AutoPEP8FormatLinter' => 'linter/AutoPEP8Linter.php', 'CHeaderLinter' => 'linter/CHeaderLinter.php', 'CheckDocLinter' => 'linter/CheckDocLinter.php', @@ -20,6 +21,7 @@ ), 'function' => array(), 'xmap' => array( + 'AssertWithSideEffectsLinter' => 'ArcanistLinter', 'AutoPEP8FormatLinter' => 'ArcanistExternalLinter', 'CHeaderLinter' => 'ArcanistLinter', 'CheckDocLinter' => 'ArcanistExternalLinter', diff --git a/arcanist/linter/AssertWithSideEffectsLinter.php b/arcanist/linter/AssertWithSideEffectsLinter.php new file mode 100644 --- /dev/null +++ b/arcanist/linter/AssertWithSideEffectsLinter.php @@ -0,0 +1,70 @@ + ArcanistLintSeverity::SEVERITY_ERROR, + ); + } + + public function getLintNameMap() { + return array( + self::ASSERT_SIDE_EFFECTS_FOUND => pht('Assertion has side-effects.'), + ); + } + + public function lintPath($path) { + $path = Filesystem::resolvePath($path, $this->getProjectRoot()); + $fileContent = Filesystem::readFile($path); + + if (!preg_match_all("/[^_]assert\(.*(\+\+|\-\-|[^=!<>]=[^=!<>]).*\);/", + $fileContent, $matches, PREG_OFFSET_CAPTURE)) { + return; + } + + foreach ($matches[1] as $match) { + list($sideEffect, $offset) = $match; + + $this->raiseLintAtOffset( + $offset, + self::ASSERT_SIDE_EFFECTS_FOUND, + pht('Having side-effects in assertions is unexpected and makes the '. + 'code harder to understand. From PRE31-C (SEI CERT C Coding '. + 'Standard): "Assertions should not contain assignments, '. + 'increment, or decrement operators."'), + $sideEffect, + null); + } + } +} diff --git a/src/test/scheduler_tests.cpp b/src/test/scheduler_tests.cpp --- a/src/test/scheduler_tests.cpp +++ b/src/test/scheduler_tests.cpp @@ -202,9 +202,15 @@ // then the callbacks should run in exactly the order in which they were // enqueued for (int i = 0; i < 100; ++i) { - queue1.AddToProcessQueue([i, &counter1]() { assert(i == counter1++); }); - - queue2.AddToProcessQueue([i, &counter2]() { assert(i == counter2++); }); + queue1.AddToProcessQueue([i, &counter1]() { + bool expectation = i == counter1++; + assert(expectation); + }); + + queue2.AddToProcessQueue([i, &counter2]() { + bool expectation = i == counter2++; + assert(expectation); + }); } // finish up