Fixed ticket #3447 (Improve void CheckStl::if_find())

lib/checkstl.cpp

@@ -741,35 +741,99 @@ void CheckStl::if_find()
 
         const Token* tok = i->classDef;
 
-        if (Token::Match(tok, "if ( !| %var% . find ( %any% ) )")) {
+        if (Token::Match(tok, "if ( !| %var% . find (")) {
             // goto %var%
             tok = tok->tokAt(2);
-            if (!tok->isName())
+            if (tok->str() == "!")
+                tok = tok->next();
+            if (!Token::simpleMatch(tok->linkAt(3), ") )"))
+                continue;
+
+            const unsigned int varid = tok->varId();
+            const Variable *var = symbolDatabase->getVariableFromVarId(varid);
+            if (var) {
+                // Is the variable a std::string or STL container?
+                const Token * decl = var->typeStartToken();
+
+                if (decl->str() == "const")
+                    decl = decl->next();
+                // stl container
+                if (Token::Match(decl, "std :: %var% < %type% > &| %varid%", varid))
+                    if_findError(tok, false);
+                else if (Token::Match(decl, "std :: string &| %varid%", varid))
+                    if_findError(tok, true);
+            }
+        }
+
+        //check also for vector-like or pointer containers
+        else if (Token::Match(tok, "if ( !| * %var%") || Token::Match(tok, "if ( !| %var% [")) {
+            tok = tok->tokAt(2);
+
+            const Token *tok2 = tok;
+            if (tok2->str() == "!")
+                tok2 = tok2->next();
+            tok2 = tok2->next();
+            if (tok2->str() == "[")
+                tok2 = tok2->link();
+            tok2 = tok2->next();
+
+            if (!Token::simpleMatch(tok2, ". find ("))
+                continue;
+            if (!Token::simpleMatch(tok2->linkAt(2), ") )"))
+                continue;
+
+            // goto %var%
+            if (tok->str() == "!")
+                tok = tok->next();
+            if (tok->str() == "*")
                 tok = tok->next();
 
             const unsigned int varid = tok->varId();
             const Variable *var = symbolDatabase->getVariableFromVarId(varid);
             if (var) {
                 // Is the variable a std::string or STL container?
-                const Token * decl = var->nameToken();
+                const Token * decl = var->typeStartToken();
-                while (decl && !Token::Match(decl, "[;{}(,]"))
-                    decl = decl->previous();
 
-                if (decl)
+                //jump next to 'const'
+                if (decl->str() == "const")
                     decl = decl->next();
+                //pretty bad limitation.. but it is there in order to avoid
+                //own implementations of 'find' or any container
+                if (!Token::simpleMatch(decl, "std ::"))
+                    continue;
 
-                // stl container
+                decl = decl->tokAt(2);
-                if (Token::Match(decl, "const| std :: %var% < %type% > &|*| %varid%", varid))
+
+                if (Token::Match(decl, "%var% <")) {
+                    decl = decl->tokAt(2);
+                    //stl-like
+                    if (Token::Match(decl, "std :: %var% < %type% >| >>|> &| %varid%", varid))
                         if_findError(tok, false);
-                else if (Token::Match(decl, "const| std :: string &|*| %varid%", varid))
+                    //not stl-like, then let's hope it's a pointer or an array
+                    else if (Token::Match(decl, "%type% >")) {
+                        decl = decl->tokAt(2);
+                        if (Token::Match(decl, "* &| %varid%", varid) ||
+                            Token::Match(decl, "&| %varid% [ ]| %any% ]| ", varid))
+                            if_findError(tok, false);
+                    }
+
+                    else if (Token::Match(decl, "std :: string > &| %varid%", varid))
                         if_findError(tok, true);
                 }
+
+                else if (decl && decl->str() == "string") {
+                    decl = decl->next();
+                    if (Token::Match(decl, "* &| %varid%", varid) ||
+                        Token::Match(decl, "&| %varid% [ ]| %any% ]| ", varid))
+                        if_findError(tok, true);
+                }
+            }
         }
 
         else if (Token::Match(tok, "if ( !| std :: find|find_if (")) {
             // goto '(' for the find
             tok = tok->tokAt(4);
-            if (tok->isName())
+            if (tok->str() != "(")
                 tok = tok->next();
 
             // check that result is checked properly

test/teststl.cpp

@@ -1107,20 +1107,90 @@ private:
         // set::find
         // ---------------------------
 
-        // error
+        // error (simple)
         check("void f(std::set<int> s)\n"
               "{\n"
               "    if (s.find(12)) { }\n"
               "}\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());
 
-        // ok
+        // error (pointer)
+        check("void f(std::set<int> *s)\n"
+              "{\n"
+              "    if (*s.find(12)) { }\n"
+              "}\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 (array-like pointer)
+        check("void f(std::set<int> *s)\n"
+              "{\n"
+              "    if (s[0].find(12)) { }\n"
+              "}\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 (array)
+        check("void f(std::set<int> s [10])\n"
+              "{\n"
+              "    if (s[0].find(12)) { }\n"
+              "}\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 (undefined length array)
+        check("void f(std::set<int> s [])\n"
+              "{\n"
+              "    if (s[0].find(12)) { }\n"
+              "}\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 (vector)
+        check("void f(std::vector<std::set<int> > s)\n"
+              "{\n"
+              "    if (s[0].find(12)) { }\n"
+              "}\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());
+
+        // ok (simple)
         check("void f(std::set<int> s)\n"
               "{\n"
               "    if (s.find(123) != s.end()) { }\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        // ok (pointer)
+        check("void f(std::set<int> *s)\n"
+              "{\n"
+              "    if (*s.find(12) != s.end()) { }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // ok (array-like pointer)
+        check("void f(std::set<int> *s)\n"
+              "{\n"
+              "    if (s[0].find(12) != s.end()) { }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // ok (array)
+        check("void f(std::set<int> s [10])\n"
+              "{\n"
+              "    if (s[0].find(123) != s.end()) { }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // ok (undefined length array)
+        check("void f(std::set<int> s [])\n"
+              "{\n"
+              "    if (s[0].find(123) != s.end()) { }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        // ok (vector)
+        check("void f(std::vector<std::set<int> > s)\n"
+              "{\n"
+              "    if (s[0].find(123) != s.end()) { }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
 
         // ---------------------------
         // std::find
@@ -1142,12 +1212,26 @@ private:
     }
 
     void if_str_find() {
-        // error
+        // error (simple)
         check("void f(const std::string &s)\n"
               "{\n"
               "    if (s.find(\"abc\")) { }\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious checking of string::find() return value.\n", errout.str());
+
+        // error (pointer)
+        check("void f(const std::string *s)\n"
+              "{\n"
+              "    if (*s.find(\"abc\")) { }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious checking of string::find() return value.\n", errout.str());
+
+        // error (vector)
+        check("void f(const std::vector<std::string> &s)\n"
+              "{\n"
+              "    if (s[0].find(\"abc\")) { }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious checking of string::find() return value.\n", errout.str());
     }