diff --git a/.travis_suppressions b/.travis_suppressions index ab61da314..111d1f8b8 100644 --- a/.travis_suppressions +++ b/.travis_suppressions @@ -5,6 +5,7 @@ nullPointer:lib/checkother.cpp nullPointer:build/checkother.cpp missingOverride noValidConfiguration +useStlAlgorithm *:gui/test/* *:test/test.cxx diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 491ab7a2c..6af7b11c2 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1746,3 +1746,343 @@ void CheckStl::readingEmptyStlContainerError(const Token *tok, const ValueFlow:: reportError(errorPath, value ? (value->errorSeverity() ? Severity::error : Severity::warning) : Severity::style, "reademptycontainer", "$symbol:" + varname +"\n" + errmsg, CWE398, !value); } + +void CheckStl::useStlAlgorithmError(const Token *tok, const std::string &algoName) +{ + reportError(tok, Severity::style, "useStlAlgorithm", + "Consider using " + algoName + " algorithm instead of a raw loop.", CWE398, false); +} + +static bool isEarlyExit(const Token *start) +{ + if (start->str() != "{") + return false; + const Token *endToken = start->link(); + const Token *tok = Token::findmatch(start, "return|throw|break", endToken); + if (!tok) + return false; + const Token *endStatement = Token::findsimplematch(tok, "; }", endToken); + if (!endStatement) + return false; + if (endStatement->next() != endToken) + return false; + return true; +} + +static const Token *singleStatement(const Token *start) +{ + if (start->str() != "{") + return nullptr; + const Token *endToken = start->link(); + const Token *endStatement = Token::findsimplematch(start->next(), ";"); + if (!Token::simpleMatch(endStatement, "; }")) + return nullptr; + if (endStatement->next() != endToken) + return nullptr; + return endStatement; +} + +static const Token *singleAssignInScope(const Token *start, unsigned int varid, bool &input) +{ + const Token *endStatement = singleStatement(start); + if (!endStatement) + return nullptr; + if (!Token::Match(start->next(), "%var% %assign%")) + return nullptr; + const Token *assignTok = start->tokAt(2); + if (isVariableChanged(assignTok->next(), endStatement, assignTok->astOperand1()->varId(), false, nullptr, true)) + return nullptr; + if (isVariableChanged(assignTok->next(), endStatement, varid, false, nullptr, true)) + return nullptr; + input = Token::findmatch(assignTok->next(), "%varid%", endStatement, varid); + return assignTok; +} + +static const Token *singleMemberCallInScope(const Token *start, unsigned int varid, bool &input) +{ + if (start->str() != "{") + return nullptr; + const Token *endToken = start->link(); + if (!Token::Match(start->next(), "%var% . %name% (")) + return nullptr; + if (!Token::simpleMatch(start->linkAt(4), ") ; }")) + return nullptr; + const Token *endStatement = start->linkAt(4)->next(); + if (endStatement->next() != endToken) + return nullptr; + + const Token *dotTok = start->tokAt(2); + if (!Token::findmatch(dotTok->tokAt(2), "%varid%", endStatement, varid)) + return nullptr; + input = Token::Match(start->next(), "%var% . %name% ( %varid% )", varid); + if (isVariableChanged(dotTok->next(), endStatement, dotTok->astOperand1()->varId(), false, nullptr, true)) + return nullptr; + return dotTok; +} + +static const Token *singleIncrementInScope(const Token *start, unsigned int varid, bool &input) +{ + if (start->str() != "{") + return nullptr; + const Token *varTok = nullptr; + if (Token::Match(start->next(), "++ %var% ; }")) + varTok = start->tokAt(2); + else if (Token::Match(start->next(), "%var% ++ ; }")) + varTok = start->tokAt(1); + if (!varTok) + return nullptr; + input = varTok->varId() == varid; + return varTok; +} + +static const Token *singleConditionalInScope(const Token *start, unsigned int varid) +{ + if (start->str() != "{") + return nullptr; + const Token *endToken = start->link(); + if (!Token::simpleMatch(start->next(), "if (")) + return nullptr; + if (!Token::simpleMatch(start->linkAt(2), ") {")) + return nullptr; + const Token *bodyTok = start->linkAt(2)->next(); + const Token *endBodyTok = bodyTok->link(); + if (!Token::simpleMatch(endBodyTok, "} }")) + return nullptr; + if (endBodyTok->next() != endToken) + return nullptr; + if (!Token::findmatch(start, "%varid%", bodyTok, varid)) + return nullptr; + if (isVariableChanged(start, bodyTok, varid, false, nullptr, true)) + return nullptr; + return bodyTok; +} + +static bool addByOne(const Token *tok, unsigned int varid) +{ + if (Token::Match(tok, "+= %any% ;") && + tok->tokAt(1)->hasKnownIntValue() && + tok->tokAt(1)->getValue(1)) { + return true; + } + if (Token::Match(tok, "= %varid% + %any% ;", varid) && + tok->tokAt(3)->hasKnownIntValue() && + tok->tokAt(3)->getValue(1)) { + return true; + } + return false; +} + +static bool accumulateBoolLiteral(const Token *tok, unsigned int varid) +{ + // TODO: Missing %oreq% + if (Token::Match(tok, "=|&= %bool% ;") && + tok->tokAt(1)->hasKnownIntValue()) { + return true; + } + if (Token::Match(tok, "= %varid% %oror%|%or%|&&|& %bool% ;", varid) && + tok->tokAt(3)->hasKnownIntValue()) { + return true; + } + return false; +} + +static bool accumulateBool(const Token *tok, unsigned int varid) +{ + // TODO: Missing %oreq% + if (Token::simpleMatch(tok, "&=")) { + return true; + } + if (Token::Match(tok, "= %varid% %oror%|%or%|&&|&", varid)) { + return true; + } + return false; +} + +static bool hasVarIds(const Token *tok, unsigned int var1, unsigned int var2) +{ + if (tok->astOperand1()->varId() == tok->astOperand2()->varId()) + return false; + if (tok->astOperand1()->varId() == var1 || tok->astOperand1()->varId() == var2) { + if (tok->astOperand2()->varId() == var1 || tok->astOperand2()->varId() == var2) { + return true; + } + } + return false; +} + +static std::string flipMinMax(const std::string &algo) +{ + if (algo == "std::max_element") + return "std::min_element"; + if (algo == "std::min_element") + return "std::max_element"; + return algo; +} + +static std::string minmaxCompare(const Token *condTok, unsigned int loopVar, unsigned int assignVar, bool invert = false) +{ + if (!Token::Match(condTok, "<|<=|>=|>")) + return "std::accumulate"; + if (!hasVarIds(condTok, loopVar, assignVar)) + return "std::accumulate"; + std::string algo = "std::max_element"; + if (Token::Match(condTok, "<|<=")) + algo = "std::min_element"; + if (condTok->astOperand1()->varId() == assignVar) + algo = flipMinMax(algo); + if (invert) + algo = flipMinMax(algo); + return algo; +} + +void CheckStl::useStlAlgorithm() +{ + if (!mSettings->isEnabled(Settings::STYLE)) + return; + for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) { + for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) { + // Parse range-based for loop + if (!Token::simpleMatch(tok, "for (")) + continue; + if (!Token::simpleMatch(tok->next()->link(), ") {")) + continue; + const Token *bodyTok = tok->next()->link()->next(); + const Token *splitTok = tok->next()->astOperand2(); + if (!Token::simpleMatch(splitTok, ":")) + continue; + const Token *loopVar = splitTok->previous(); + if (!Token::Match(loopVar, "%var%")) + continue; + + // Check for single assignment + bool useLoopVarInAssign; + const Token *assignTok = singleAssignInScope(bodyTok, loopVar->varId(), useLoopVarInAssign); + if (assignTok) { + unsigned int assignVarId = assignTok->astOperand1()->varId(); + std::string algo; + if (assignVarId == loopVar->varId()) { + if (useLoopVarInAssign) + algo = "std::transform"; + else if (Token::Match(assignTok->next(), "%var%|%bool%|%num%|%char% ;")) + algo = "std::fill"; + else if (Token::Match(assignTok->next(), "%name% ( )")) + algo = "std::generate"; + else + algo = "std::fill or std::generate"; + } else { + if (addByOne(assignTok, assignVarId)) + algo = "std::distance"; + else if (accumulateBool(assignTok, assignVarId)) + algo = "std::any_of, std::all_of, std::none_of, or std::accumulate"; + else if (Token::Match(assignTok, "= %var% <|<=|>=|> %var% ? %var% : %var%") && hasVarIds(assignTok->tokAt(6), loopVar->varId(), assignVarId)) + algo = minmaxCompare(assignTok->tokAt(2), loopVar->varId(), assignVarId, assignTok->tokAt(5)->varId() == assignVarId); + else + algo = "std::accumulate"; + } + useStlAlgorithmError(assignTok, algo); + continue; + } + // Check for container calls + bool useLoopVarInMemCall; + const Token *memberAccessTok = singleMemberCallInScope(bodyTok, loopVar->varId(), useLoopVarInMemCall); + if (memberAccessTok) { + const Token *memberCallTok = memberAccessTok->astOperand2(); + unsigned int contVarId = memberAccessTok->astOperand1()->varId(); + if (contVarId == loopVar->varId()) + continue; + if (memberCallTok->str() == "push_back" || + memberCallTok->str() == "push_front" || + memberCallTok->str() == "emplace_back") { + std::string algo; + if (useLoopVarInMemCall) + algo = "std::copy"; + else + algo = "std::transform"; + useStlAlgorithmError(assignTok, algo); + } + continue; + } + + // Check for increment in loop + bool useLoopVarInIncrement; + const Token *incrementTok = singleIncrementInScope(bodyTok, loopVar->varId(), useLoopVarInIncrement); + if (incrementTok) { + std::string algo; + if (useLoopVarInIncrement) + algo = "std::transform"; + else + algo = "std::distance"; + useStlAlgorithmError(incrementTok, algo); + continue; + } + + // Check for conditionals + const Token *condBodyTok = singleConditionalInScope(bodyTok, loopVar->varId()); + if (condBodyTok) { + const Token *beginCondTok = condBodyTok->previous()->link(); + // Check for single assign + assignTok = singleAssignInScope(condBodyTok, loopVar->varId(), useLoopVarInAssign); + if (assignTok) { + unsigned int assignVarId = assignTok->astOperand1()->varId(); + std::string algo; + if (assignVarId == loopVar->varId()) { + if (useLoopVarInAssign) + algo = "std::transform"; + else + algo = "std::replace_if"; + } else { + if (addByOne(assignTok, assignVarId)) + algo = "std::count_if"; + else if (accumulateBoolLiteral(assignTok, assignVarId)) + algo = "std::any_of, std::all_of, std::none_of, or std::accumulate"; + else + algo = "std::accumulate"; + } + useStlAlgorithmError(assignTok, algo); + continue; + } + + // Check for container call + memberAccessTok = singleMemberCallInScope(condBodyTok, loopVar->varId(), useLoopVarInMemCall); + if (memberAccessTok) { + const Token *memberCallTok = memberAccessTok->astOperand2(); + unsigned int contVarId = memberAccessTok->astOperand1()->varId(); + if (contVarId == loopVar->varId()) + continue; + if (memberCallTok->str() == "push_back" || + memberCallTok->str() == "push_front" || + memberCallTok->str() == "emplace_back") { + if (useLoopVarInMemCall) + useStlAlgorithmError(memberAccessTok, "std::copy_if"); + // There is no transform_if to suggest + } + continue; + } + + // Check for increment in loop + incrementTok = singleIncrementInScope(condBodyTok, loopVar->varId(), useLoopVarInIncrement); + if (incrementTok) { + std::string algo; + if (useLoopVarInIncrement) + algo = "std::transform"; + else + algo = "std::count_if"; + useStlAlgorithmError(incrementTok, algo); + continue; + } + + // Check early return + if (isEarlyExit(condBodyTok)) { + const Token *loopVar2 = Token::findmatch(condBodyTok, "%varid%", condBodyTok->link(), loopVar->varId()); + std::string algo; + if (loopVar2) + algo = "std::find_if"; + else + algo = "std::any_of"; + useStlAlgorithmError(condBodyTok, algo); + continue; + } + } + } + } +} diff --git a/lib/checkstl.h b/lib/checkstl.h index ac33bca7f..b01146937 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -88,6 +88,7 @@ public: checkStl.size(); checkStl.redundantCondition(); checkStl.missingComparison(); + checkStl.useStlAlgorithm(); } /** Accessing container out of bounds using ValueFlow */ @@ -179,7 +180,11 @@ public: /** @brief Reading from empty stl container (using valueflow) */ void readingEmptyStlContainer2(); -private: + + /** @brief Look for loops that can replaced with std algorithms */ + void useStlAlgorithm(); + + private: void missingComparisonError(const Token* incrementToken1, const Token* incrementToken2); void string_c_strThrowError(const Token* tok); void string_c_strError(const Token* tok); @@ -216,6 +221,8 @@ private: void readingEmptyStlContainerError(const Token* tok, const ValueFlow::Value *value=nullptr); + void useStlAlgorithmError(const Token *tok, const std::string &algoName); + void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const override { CheckStl c(nullptr, settings, errorLogger); c.outOfBoundsError(nullptr, nullptr, nullptr); @@ -250,6 +257,7 @@ private: c.uselessCallsRemoveError(nullptr, "remove"); c.dereferenceInvalidIteratorError(nullptr, "i"); c.readingEmptyStlContainerError(nullptr); + c.useStlAlgorithmError(nullptr, ""); } static std::string myName() { @@ -271,7 +279,8 @@ private: "- using auto pointer (auto_ptr)\n" "- useless calls of string and STL functions\n" "- dereferencing an invalid iterator\n" - "- reading from empty STL container\n"; + "- reading from empty STL container\n" + "- consider using an STL algorithm instead of raw loop\n"; } }; /// @} diff --git a/test/teststl.cpp b/test/teststl.cpp index c10435b69..9ef1c2cc2 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -144,6 +144,13 @@ private: TEST_CASE(dereferenceInvalidIterator); TEST_CASE(dereferenceInvalidIterator2); // #6572 TEST_CASE(dereference_auto); + + TEST_CASE(loopAlgoElementAssign); + TEST_CASE(loopAlgoAccumulateAssign); + TEST_CASE(loopAlgoContainerInsert); + TEST_CASE(loopAlgoIncrement); + TEST_CASE(loopAlgoConditional); + TEST_CASE(loopAlgoMinMax); } void check(const char code[], const bool inconclusive=false, const Standards::cppstd_t cppstandard=Standards::CPP11) { @@ -3156,6 +3163,514 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:18]: (error, inconclusive) Invalid iterator 'it' used.\n", errout.str()); } + + void loopAlgoElementAssign() + { + check("void foo() {\n" + " for(int& x:v)\n" + " x = 1;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::fill algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " for(int& x:v)\n" + " x = x + 1;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo(int a, int b) {\n" + " for(int& x:v)\n" + " x = a + b;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::fill or std::generate algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " for(int& x:v)\n" + " x += 1;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " for(int& x:v)\n" + " x = f();\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::generate algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " for(int& x:v) {\n" + " f();\n" + " x = 1;\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " for(int& x:v) {\n" + " x = 1;\n" + " f();\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + // There should probably be a message for unconditional break + check("void foo() {\n" + " for(int& x:v) {\n" + " x = 1;\n" + " break;\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " for(int& x:v)\n" + " x = ++x;\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + } + + void loopAlgoAccumulateAssign() + { + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n += x;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = n + x;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n += 1;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::distance algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = n + 1;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::distance algorithm instead of a raw loop.\n", errout.str()); + + check("bool f(int);\n" + "void foo() {\n" + " bool b = false;\n" + " for(int x:v)\n" + " b &= f(x);\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool f(int);\n" + "void foo() {\n" + " bool b = false;\n" + " for(int x:v)\n" + " b |= f(x);\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool f(int);\n" + "void foo() {\n" + " bool b = false;\n" + " for(int x:v)\n" + " b = b && f(x);\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool f(int);\n" + "void foo() {\n" + " bool b = false;\n" + " for(int x:v)\n" + " b = b || f(x);\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int& x:v)\n" + " n = ++x;\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + } + + void loopAlgoContainerInsert() + { + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_back(x);\n" + "}\n", + true); + ASSERT_EQUALS("(style) Consider using std::copy algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_back(f(x));\n" + "}\n", + true); + ASSERT_EQUALS("(style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_back(x + 1);\n" + "}\n", + true); + ASSERT_EQUALS("(style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_front(x);\n" + "}\n", + true); + ASSERT_EQUALS("(style) Consider using std::copy algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_front(f(x));\n" + "}\n", + true); + ASSERT_EQUALS("(style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_front(x + 1);\n" + "}\n", + true); + ASSERT_EQUALS("(style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_back(v);\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " std::vector c;\n" + " for(int x:v)\n" + " c.push_back(0);\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + } + + void loopAlgoIncrement() + { + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n++;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::distance algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " ++n;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::distance algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " for(int& x:v)\n" + " x++;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " for(int& x:v)\n" + " ++x;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:3]: (style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + } + + void loopAlgoConditional() + { + check("bool pred(int x);\n" + "void foo() {\n" + " for(int& x:v) {\n" + " if (pred(x)) {\n" + " x = 1; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::replace_if algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void foo() {\n" + " int n = 0;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " n += x; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void foo() {\n" + " int n = 0;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " n += 1; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::count_if algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void foo() {\n" + " int n = 0;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " n++; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::count_if algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void foo() {\n" + " for(int& x:v) {\n" + " if (pred(x)) {\n" + " x = x + 1; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::transform algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void foo() {\n" + " std::vector c;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " c.push_back(x); \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::copy_if algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "bool foo() {\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " return false; \n" + " }\n" + " }\n" + " return true;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "bool foo() {\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " break; \n" + " }\n" + " }\n" + " return true;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void f();\n" + "void foo() {\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " f(); \n" + " break; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::any_of algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "void f(int x);\n" + "void foo() {\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " f(x); \n" + " break; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::find_if algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "bool foo() {\n" + " bool b = false;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " b = true;\n" + " }\n" + " }\n" + " if(b) {}\n" + " return true;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "bool foo() {\n" + " bool b = false;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " b |= true;\n" + " }\n" + " }\n" + " return true;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "bool foo() {\n" + " bool b = false;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " b &= true;\n" + " }\n" + " }\n" + " return true;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:6]: (style) Consider using std::any_of, std::all_of, std::none_of, or std::accumulate algorithm instead of a raw loop.\n", errout.str()); + + check("bool pred(int x);\n" + "bool foo() {\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " return false; \n" + " }\n" + " return true;\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + // There is no transform_if + check("bool pred(int x);\n" + "void foo() {\n" + " std::vector c;\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " c.push_back(x + 1); \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool pred(int x);\n" + "void foo() {\n" + " for(int& x:v) {\n" + " x++;\n" + " if (pred(x)) {\n" + " x = 1; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + + check("bool pred(int x);\n" + "void f();\n" + "void foo() {\n" + " for(int x:v) {\n" + " if (pred(x)) {\n" + " if(x) { return; }\n" + " break; \n" + " }\n" + " }\n" + "}\n", + true); + ASSERT_EQUALS("", errout.str()); + } + + void loopAlgoMinMax() + { + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = x > n ? x : n;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::max_element algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = x < n ? x : n;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::min_element algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = x > n ? n : x;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::min_element algorithm instead of a raw loop.\n", errout.str()); + + check("void foo() {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = x < n ? n : x;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::max_element algorithm instead of a raw loop.\n", errout.str()); + + check("void foo(int m) {\n" + " int n = 0;\n" + " for(int x:v)\n" + " n = x > m ? x : n;\n" + "}\n", + true); + ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str()); + } }; REGISTER_TEST(TestStl)