Fixed #6217, refactorized CheckStl::if_find(): allow all comparison operators, use AST, fixed wrong unit tests

lib/checkstl.cpp

@@ -739,27 +739,15 @@ void CheckStl::stlBoundariesError(const Token *tok, const std::string &container
                 "One should use operator!= instead to compare iterators.");
 }
 
-static bool if_findCompare(const Token * const tokBack, bool str)
+static bool if_findCompare(const Token * const tokBack)
 {
-    const Token *tok = tokBack;
+    const Token *tok = tokBack->astParent();
-    while (tok && tok->str() == ")") {
+    if (!tok)
-        tok = tok->next();
+        return false;
-
+    if (tok->isComparisonOp())
-        if (Token::Match(tok, ") !!{") &&
-            tok->link()->previous() &&
-            (Token::Match(tok->link()->previous(),",|==|!=") ||
-             tok->link()->previous()->isName()))
-            return true;
-    }
-
-    if (Token::Match(tok,",|==|!="))
         return true;
-    if (tok) {
+    if (tok->isArithmeticalOp()) // result is used in some calculation
-        if (str && tok->isComparisonOp())
+        return true;  // TODO: check if there is a comparison of the result somewhere
-            return true;
-        if (tok->isArithmeticalOp()) // result is used in some calculation
-            return true;  // TODO: check if there is a comparison of the result somewhere
-    }
     return false;
 }
 
@@ -781,48 +769,30 @@ void CheckStl::if_find()
             tok = tok->next();
 
         for (const Token* const end = tok->link(); tok != end; tok = (tok == end) ? end : tok->next()) {
-            if (Token::Match(tok, "&&|(|%oror%"))
-                tok = tok->next();
-            else
-                continue;
-
-            while (tok->str() == "(")
-                tok = tok->next();
-
-            if (tok->str() == "!")
-                tok = tok->next();
-
             if (Token::Match(tok, "%var% . find (")) {
                 const Variable *var = tok->variable();
                 if (var) {
-                    const bool isString = var->isStlStringType() && !var->isArrayOrPointer();
+                    if (if_findCompare(tok->tokAt(3)))
-                    if (if_findCompare(tok->linkAt(3), isString))
                         continue;
 
                     // Is the variable a std::string or STL container?
                     const Token * decl = var->typeStartToken();
-                    const unsigned int varid = tok->varId();
                     // stl container
-                    if (warning && Token::Match(decl, "std :: %var% < %type% > &| %varid%", varid))
+                    if (warning && Token::Match(decl, "std :: %var% <") && Token::Match(decl->linkAt(3), "> !!::"))
                         if_findError(tok, false);
-                    else if (performance && isString)
+                    else if (performance && var->isStlStringType())
                         if_findError(tok, true);
                 }
             }
 
             //check also for vector-like or pointer containers
-            else if (Token::Match(tok, "* %var%") || Token::Match(tok, "%var% [")) {
+            else if (tok->variable() && tok->astParent() && (tok->astParent()->str() == "*" || tok->astParent()->str() == "[")) {
-                // goto %var%
+                const Token *tok2 = tok->astParent();
-                if (tok->str() == "*")
-                    tok = tok->next();
 
-                const Token *tok2 = tok->next();
+                if (!Token::simpleMatch(tok2->astParent(), ". find ("))
-                if (tok2->str() == "[")
-                    tok2 = tok2->link()->next();
-
-                if (!Token::simpleMatch(tok2, ". find ("))
                     continue;
-                if (if_findCompare(tok2->linkAt(2), false))
+
+                if (if_findCompare(tok2->astParent()->tokAt(2)))
                     continue;
 
                 const Variable *var = tok->variable();
@@ -866,7 +836,7 @@ void CheckStl::if_find()
 
             else if (warning && Token::Match(tok, "std :: find|find_if (")) {
                 // check that result is checked properly
-                if (!if_findCompare(tok->linkAt(3), false)) {
+                if (!if_findCompare(tok->tokAt(3))) {
                     if_findError(tok, false);
                 }
             }

test/teststl.cpp

@@ -1458,7 +1458,14 @@ private:
         // error (pointer)
         check("void f(std::set<int> *s)\n"
               "{\n"
-              "    if (*s.find(12)) { }\n"
+              "    if ((*s).find(12)) { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find() is an iterator, but it is not properly checked.\n", errout.str());
+
+        // error (pointer)
+        check("void f(std::set<int> *s)\n"
+              "{\n"
+              "    if (s->find(12)) { }\n"
               "}");
         ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious condition. The result of find() is an iterator, but it is not properly checked.\n", errout.str());
 
@@ -1500,7 +1507,7 @@ private:
         // ok (pointer)
         check("void f(std::set<int> *s)\n"
               "{\n"
-              "    if (*s.find(12) != s.end()) { }\n"
+              "    if ((*s).find(12) != s.end()) { }\n"
               "}");
         ASSERT_EQUALS("", errout.str());
 
@@ -1551,6 +1558,13 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        // ok (less than comparison, #6217)
+        check("void f(std::vector<int> s)\n"
+              "{\n"
+              "    if (std::find(a, b, c) < d) { }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
         // #3714 - segmentation fault for syntax error
         check("void f() {\n"
               "    if (()) { }\n"
@@ -1574,7 +1588,14 @@ private:
         // error (pointer)
         check("void f(const std::string *s)\n"
               "{\n"
-              "    if (*s.find(\"abc\")) { }\n"
+              "    if ((*s).find(\"abc\")) { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (performance) Inefficient usage of string::find() in condition; string::compare() would be faster.\n", errout.str());
+
+        // error (pointer)
+        check("void f(const std::string *s)\n"
+              "{\n"
+              "    if (s->find(\"abc\")) { }\n"
               "}");
         ASSERT_EQUALS("[test.cpp:3]: (performance) Inefficient usage of string::find() in condition; string::compare() would be faster.\n", errout.str());