Add check for cppcheck's internal API usage (#3263)

First checks:
- Simple pattern used inside Token::Match()
- Complex pattern used inside Token::simpleMatch()

The checks get enabled by passing "--enable=internal".
It's not included in "--enable=all".

If you see something that needs further tweaking, just go for it!

Makefile

@@ -55,6 +55,7 @@ LIBOBJ =      lib/check64bit.o \
               lib/checkbufferoverrun.o \
               lib/checkclass.o \
               lib/checkexceptionsafety.o \
+              lib/checkinternal.o \
               lib/checkmemoryleak.o \
               lib/checknonreentrantfunctions.o \
               lib/checknullpointer.o \
@@ -101,6 +102,7 @@ TESTOBJ =     test/options.o \
               test/testexceptionsafety.o \
               test/testfilelister.o \
               test/testincompletestatement.o \
+              test/testinternal.o \
               test/testmathlib.o \
               test/testmemleak.o \
               test/testnonreentrantfunctions.o \
@@ -195,6 +197,9 @@ lib/checkclass.o: lib/checkclass.cpp lib/checkclass.h lib/check.h lib/token.h li
 lib/checkexceptionsafety.o: lib/checkexceptionsafety.cpp lib/checkexceptionsafety.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkexceptionsafety.o lib/checkexceptionsafety.cpp
 
+lib/checkinternal.o: lib/checkinternal.cpp lib/checkinternal.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h lib/symboldatabase.h lib/mathlib.h
+	$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkinternal.o lib/checkinternal.cpp
+
 lib/checkmemoryleak.o: lib/checkmemoryleak.cpp lib/checkmemoryleak.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h lib/symboldatabase.h lib/mathlib.h lib/executionpath.h lib/checkuninitvar.h
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkmemoryleak.o lib/checkmemoryleak.cpp
 
@@ -327,6 +332,9 @@ test/testfilelister.o: test/testfilelister.cpp test/testsuite.h lib/errorlogger.
 test/testincompletestatement.o: test/testincompletestatement.cpp test/testsuite.h lib/errorlogger.h lib/settings.h lib/suppressions.h lib/standards.h test/redirect.h lib/tokenize.h lib/checkother.h lib/check.h lib/token.h
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testincompletestatement.o test/testincompletestatement.cpp
 
+test/testinternal.o: test/testinternal.cpp lib/tokenize.h lib/checkinternal.h lib/check.h lib/token.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h test/testsuite.h test/redirect.h
+	$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testinternal.o test/testinternal.cpp
+
 test/testmathlib.o: test/testmathlib.cpp lib/mathlib.h test/testsuite.h lib/errorlogger.h lib/settings.h lib/suppressions.h lib/standards.h test/redirect.h
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testmathlib.o test/testmathlib.cpp
 

lib/checkinternal.cpp

@@ -0,0 +1,131 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "checkinternal.h"
+#include "symboldatabase.h"
+#include <string>
+#include <set>
+
+using namespace std;
+
+// Register this check class (by creating a static instance of it)
+namespace {
+    CheckInternal instance;
+}
+
+void CheckInternal::checkTokenMatchPatterns()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (!Token::simpleMatch(tok, "Token :: Match (") && !Token::simpleMatch(tok, "Token :: findmatch ("))
+            continue;
+
+        const std::string funcname = tok->strAt(2);
+
+        // Get pattern string
+        const Token *pattern_tok = tok->tokAt(4)->nextArgument();
+        if (!pattern_tok || !Token::Match(pattern_tok, "%str%"))
+            continue;
+
+        const string pattern = pattern_tok->strValue();
+        if (pattern.empty()) {
+            simplePatternError(tok, pattern, funcname);
+            continue;
+        }
+
+        // Check for signs of complex patterns
+        if (pattern.find_first_of("[|%") != string::npos)
+            continue;
+        else if (pattern.find("!!") != string::npos)
+            continue;
+
+        simplePatternError(tok, pattern, funcname);
+    }
+}
+
+void CheckInternal::checkTokenSimpleMatchPatterns()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (!Token::simpleMatch(tok, "Token :: simpleMatch (") && !Token::simpleMatch(tok, "Token :: findsimplematch ("))
+            continue;
+
+        const std::string funcname = tok->strAt(2);
+
+        // Get pattern string
+        const Token *pattern_tok = tok->tokAt(4)->nextArgument();
+        if (!pattern_tok || !Token::Match(pattern_tok, "%str%"))
+            continue;
+
+        const string pattern = pattern_tok->strValue();
+        if (pattern.empty()) {
+            complexPatternError(tok, pattern, funcname);
+            continue;
+        }
+
+        // Check for [xyz] usage - but exclude standalone square brackets
+        unsigned int char_count = 0;
+        for (string::size_type pos = 0; pos < pattern.size(); ++pos) {
+            unsigned char c = pattern[pos];
+
+            if (c == ' ') {
+                char_count = 0;
+            } else if (c == ']') {
+                if (char_count > 0) {
+                    complexPatternError(tok, pattern, funcname);
+                    continue;
+                }
+            } else {
+                ++char_count;
+            }
+        }
+
+        // Check | usage: Count characters before the symbol
+        char_count = 0;
+        for (string::size_type pos = 0; pos < pattern.size(); ++pos) {
+            unsigned char c = pattern[pos];
+
+            if (c == ' ') {
+                char_count = 0;
+            } else if (c == '|') {
+                if (char_count > 0) {
+                    complexPatternError(tok, pattern, funcname);
+                    continue;
+                }
+            } else {
+                ++char_count;
+            }
+        }
+
+        // Check for real errors
+        if (pattern.find_first_of("%") != string::npos || pattern.find("!!") != string::npos)
+            complexPatternError(tok, pattern, funcname);
+    }
+}
+
+void CheckInternal::simplePatternError(const Token* tok, const string& pattern, const std::string &funcname)
+{
+    reportError(tok, Severity::warning, "simplePatternError",
+                "Found simple pattern inside Token::" + funcname + "() call: \"" + pattern + "\""
+               );
+}
+
+void CheckInternal::complexPatternError(const Token* tok, const string& pattern, const std::string &funcname)
+{
+    reportError(tok, Severity::error, "complexPatternError",
+                "Found complex pattern inside Token::" + funcname + "() call: \"" + pattern + "\""
+               );
+}

lib/checkinternal.h

@@ -0,0 +1,84 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+//---------------------------------------------------------------------------
+#ifndef CHECKINTERNAL_H
+#define CHECKINTERNAL_H
+//---------------------------------------------------------------------------
+
+#include "check.h"
+
+class Token;
+
+/// @addtogroup Checks
+/// @{
+
+
+/** @brief %Check Internal cppcheck API usage */
+class CheckInternal : public Check {
+public:
+    /** This constructor is used when registering the CheckClass */
+    CheckInternal() : Check(myName())
+    { }
+
+    /** This constructor is used when running checks. */
+    CheckInternal(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
+        : Check(myName(), tokenizer, settings, errorLogger)
+    { }
+
+    /** Simplified checks. The token list is simplified. */
+    void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
+        if (!settings->isEnabled("internal"))
+            return;
+
+        CheckInternal checkInternal(tokenizer, settings, errorLogger);
+
+        checkInternal.checkTokenMatchPatterns();
+        checkInternal.checkTokenSimpleMatchPatterns();
+    }
+
+    /** @brief %Check if a simple pattern is used inside Token::Match or Token::findmatch */
+    void checkTokenMatchPatterns();
+
+    /** @brief %Check if a complex pattern is used inside Token::simpleMatch or Token::findsimplematch */
+    void checkTokenSimpleMatchPatterns();
+
+private:
+    void simplePatternError(const Token *tok, const std::string &pattern, const std::string &funcname);
+    void complexPatternError(const Token *tok, const std::string &pattern, const std::string &funcname);
+
+    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
+        CheckInternal c(0, settings, errorLogger);
+        c.simplePatternError(0, "class {", "Match");
+        c.complexPatternError(0, "%type% ( )", "Match");
+    }
+
+    std::string myName() const {
+        return "cppcheck internal API usage";
+    }
+
+    std::string classInfo() const {
+        return "Check for wrong or unsuitable internal API usage:\n"
+               "* Found simple pattern inside Token::Match() call: \"class {\"\n"
+               "* Found complex pattern inside Token::simpleMatch() call: \"%type\"\n";
+    }
+};
+/// @}
+//---------------------------------------------------------------------------
+#endif

lib/lib.pri

@@ -10,6 +10,7 @@ HEADERS += $${BASEPATH}check.h \
            $${BASEPATH}checkbufferoverrun.h \
            $${BASEPATH}checkclass.h \
            $${BASEPATH}checkexceptionsafety.h \
+           $${BASEPATH}checkinternal.h \
            $${BASEPATH}checkmemoryleak.h \
            $${BASEPATH}checknonreentrantfunctions.h \
            $${BASEPATH}checknullpointer.h \
@@ -40,6 +41,7 @@ SOURCES += $${BASEPATH}check64bit.cpp \
            $${BASEPATH}checkbufferoverrun.cpp \
            $${BASEPATH}checkclass.cpp \
            $${BASEPATH}checkexceptionsafety.cpp \
+           $${BASEPATH}checkinternal.cpp \
            $${BASEPATH}checkmemoryleak.cpp \
            $${BASEPATH}checknonreentrantfunctions.cpp \
            $${BASEPATH}checknullpointer.cpp \

lib/settings.cpp

@@ -83,13 +83,18 @@ std::string Settings::addEnabled(const std::string &str)
     id.insert("performance");
     id.insert("portability");
     id.insert("information");
+    id.insert("internal");
     id.insert("missingInclude");
     id.insert("unusedFunction");
 
     if (str == "all") {
         std::set<std::string>::const_iterator it;
-        for (it = id.begin(); it != id.end(); ++it)
+        for (it = id.begin(); it != id.end(); ++it) {
+            if (*it == "internal")
+                continue;
+
             _enabled.insert(*it);
+        }
     } else if (id.find(str) != id.end()) {
         _enabled.insert(str);
     } else if (!handled) {

test/testcmdlineparser.cpp

@@ -63,6 +63,7 @@ private:
         TEST_CASE(enabledPortability);
         TEST_CASE(enabledUnusedFunction);
         TEST_CASE(enabledMissingInclude);
+        TEST_CASE(enabledInternal);
         TEST_CASE(errorExitcode);
         TEST_CASE(errorExitcodeMissing);
         TEST_CASE(errorExitcodeStr);
@@ -384,6 +385,7 @@ private:
         ASSERT(settings.isEnabled("style"));
         ASSERT(settings.isEnabled("unusedFunction"));
         ASSERT(settings.isEnabled("missingInclude"));
+        ASSERT(!settings.isEnabled("internal"));
     }
 
     void enabledStyle() {
@@ -443,6 +445,15 @@ private:
         ASSERT(settings.isEnabled("missingInclude"));
     }
 
+    void enabledInternal() {
+        REDIRECT;
+        const char *argv[] = {"cppcheck", "--enable=internal", "file.cpp"};
+        Settings settings;
+        CmdLineParser parser(&settings);
+        ASSERT(parser.ParseFromArgs(3, argv));
+        ASSERT(settings.isEnabled("internal"));
+    }
+
     void errorExitcode() {
         REDIRECT;
         const char *argv[] = {"cppcheck", "--error-exitcode=5", "file.cpp"};

test/testinternal.cpp

@@ -0,0 +1,176 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+
+#include "tokenize.h"
+#include "checkinternal.h"
+#include "testsuite.h"
+#include <sstream>
+
+extern std::ostringstream errout;
+
+class TestInternal : public TestFixture {
+public:
+    TestInternal() : TestFixture("TestInternal")
+    { }
+
+private:
+    void run() {
+        TEST_CASE(simplePatternInTokenMatch)
+        TEST_CASE(complexPatternInTokenSimpleMatch)
+        TEST_CASE(simplePatternSquareBrackets)
+        TEST_CASE(simplePatternAlternatives)
+    }
+
+    void check(const std::string &code) {
+        // Clear the error buffer..
+        errout.str("");
+
+        Settings settings;
+        settings.addEnabled("internal");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code.c_str());
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.simplifyTokenList();
+
+        // Check..
+        CheckInternal checkInternal;
+        checkInternal.runSimplifiedChecks(&tokenizer, &settings, this);
+    }
+
+    void simplePatternInTokenMatch() {
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::Match(tok, \";\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Found simple pattern inside Token::Match() call: \";\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::Match(tok, \"%type%\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::findmatch(tok, \";\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Found simple pattern inside Token::findmatch() call: \";\"\n", errout.str());
+    }
+
+    void complexPatternInTokenSimpleMatch() {
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"%type%\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::simpleMatch() call: \"%type%\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::findsimplematch(tok, \"%type%\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::findsimplematch() call: \"%type%\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::findsimplematch(tok, \"} !!else\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::findsimplematch() call: \"} !!else\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::findsimplematch(tok, \"foobar\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void simplePatternSquareBrackets() {
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"[\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"[ ]\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"[]\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::simpleMatch() call: \"[]\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"] [\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"] [ [abc]\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::simpleMatch() call: \"] [ [abc]\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"[.,;]\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::simpleMatch() call: \"[.,;]\"\n", errout.str());
+    }
+
+    void simplePatternAlternatives() {
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"||\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"|\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"a|b\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Found complex pattern inside Token::simpleMatch() call: \"a|b\"\n", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"|= 0\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    const Token *tok;\n"
+              "    Token::simpleMatch(tok, \"| 0 )\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+};
+
+REGISTER_TEST(TestInternal)