Fix #9091 std::accumulate suggested instead of std::max_element (danmar#7316)

Author: chrchr-github

lib/checkstl.cpp

@@ -2899,10 +2899,21 @@ void CheckStl::useStlAlgorithm()
         return !astIsContainer(tok); // don't warn for containers, where overloaded operators can be costly
     };
 
-    auto isConditionWithoutSideEffects = [this](const Token* tok) -> bool {
+    enum class ConditionOpType : std::uint8_t { OTHER, MIN, MAX };
+    auto isConditionWithoutSideEffects = [this](const Token* tok, ConditionOpType& type) -> bool {
         if (!Token::simpleMatch(tok, "{") || !Token::simpleMatch(tok->previous(), ")"))
             return false;
-        return isConstExpression(tok->linkAt(-1)->astOperand2(), mSettings->library);
+        const Token* condTok = tok->linkAt(-1)->astOperand2();
+        if (isConstExpression(condTok, mSettings->library)) {
+            if (condTok->str() == "<")
+                type = ConditionOpType::MIN;
+            else if (condTok->str() == ">")
+                type = ConditionOpType::MAX;
+            else
+                type = ConditionOpType::OTHER;
+            return true;
+        }
+        return false;
     };
 
     auto isAccumulation = [](const Token* tok, int varId) {
@@ -3035,14 +3046,27 @@ void CheckStl::useStlAlgorithm()
                         else
                             algo = "std::replace_if";
                     } else {
+                        ConditionOpType type{};
                         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 if (assignTok->str() != "=")
                             algo = "std::accumulate";
-                        else if (hasBreak && isConditionWithoutSideEffects(condBodyTok))
-                            algo = "std::any_of, std::all_of, std::none_of";
+                        else if (isConditionWithoutSideEffects(condBodyTok, type)) {
+                            if (hasBreak)
+                                algo = "std::any_of, std::all_of, std::none_of";
+                            else if (assignTok->astOperand2()->varId() == loopVar->varId()) {
+                                if (type == ConditionOpType::MIN)
+                                    algo = "std::min_element";
+                                else if (type == ConditionOpType::MAX)
+                                    algo = "std::max_element";
+                                else
+                                    continue;
+                            }
+                            else
+                                continue;
+                        }
                         else
                             continue;
                     }

test/teststl.cpp

@@ -5859,17 +5859,26 @@ class TestStl : public TestFixture {
               dinit(CheckOptions, $.inconclusive = true));
         ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout_str());
 
-        check("void f(const std::vector<int>& v) {\n"
+        check("void f(const std::vector<int>& v) {\n" // #9091
               "    int maxY = 0;\n"
               "    for (int y : v) {\n"
               "        if (y > maxY)\n"
               "            maxY = y;\n"
               "    }\n"
               "}\n",
               dinit(CheckOptions, $.inconclusive = true));
-        TODO_ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::max_element algorithm instead of a raw loop.\n",
-                           "",
-                           errout_str());
+        ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::max_element algorithm instead of a raw loop.\n", errout_str());
+
+        check("int f(const std::vector<int>& v) {\n"
+              "    int minY = 0;\n"
+              "    for (int y : v) {\n"
+              "        if (y < minY)\n"
+              "            minY = y;\n"
+              "    }\n"
+              "    return minY;\n"
+              "}\n",
+              dinit(CheckOptions, $.inconclusive = true));
+        ASSERT_EQUALS("[test.cpp:5]: (style) Consider using std::min_element algorithm instead of a raw loop.\n", errout_str());
     }
 
     void loopAlgoMultipleReturn()