New internal check: checkRedundantNextPrevious().

Fixed findings by new internal check

lib/checkbufferoverrun.cpp

@@ -274,7 +274,7 @@ static const Token *for_init(const Token *tok, unsigned int &varid, std::string
         }
 
         varid = tok->tokAt(2)->varId();
-        varname = tok->tokAt(2)->str();
+        varname = tok->strAt(2);
         tok = tok->tokAt(6);
     } else
         return 0;

lib/checkinternal.cpp

@@ -214,6 +214,30 @@ void CheckInternal::checkUnknownPattern()
     }
 }
 
+void CheckInternal::checkRedundantNextPrevious()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (Token::Match(tok, ". previous ( ) . next|tokAt|strAt|linkAt (") || Token::Match(tok, ". next ( ) . previous|tokAt|strAt|linkAt (") ||
+            (Token::simpleMatch(tok, ". tokAt (") && Token::Match(tok->linkAt(2), ") . previous|next|tokAt|strAt|linkAt|str|link ("))) {
+            const std::string& func1 = tok->strAt(1);
+            const std::string& func2 = tok->linkAt(2)->strAt(2);
+
+            if ((func2 == "previous" || func2 == "next" || func2 == "str" || func2 == "link") && tok->linkAt(2)->strAt(4) != ")")
+                continue;
+
+            redundantNextPreviousError(tok, func1, func2);
+        } else if (Token::Match(tok, ". next|previous ( ) . next|previous ( ) . next|previous|linkAt|strAt|link|str (")) {
+            const std::string& func1 = tok->strAt(1);
+            const std::string& func2 = tok->strAt(9);
+
+            if ((func2 == "previous" || func2 == "next" || func2 == "str" || func2 == "link") && tok->strAt(11) != ")")
+                continue;
+
+            redundantNextPreviousError(tok, func1, func2);
+        }
+    }
+}
+
 void CheckInternal::simplePatternError(const Token* tok, const std::string& pattern, const std::string &funcname)
 {
     reportError(tok, Severity::warning, "simplePatternError",
@@ -241,4 +265,10 @@ void CheckInternal::unknownPatternError(const Token* tok, const std::string& pat
                 "Unknown pattern used: \"" + pattern + "\"");
 }
 
+void CheckInternal::redundantNextPreviousError(const Token* tok, const std::string& func1, const std::string& func2)
+{
+    reportError(tok, Severity::style, "redundantNextPrevious",
+                "Call to 'Token::" + func1 + "()' followed by 'Token::" + func2 + "()' can be simplified.");
+}
+
 #endif // #ifndef NDEBUG

lib/checkinternal.h

@@ -54,6 +54,7 @@ public:
         checkInternal.checkTokenSimpleMatchPatterns();
         checkInternal.checkMissingPercentCharacter();
         checkInternal.checkUnknownPattern();
+        checkInternal.checkRedundantNextPrevious();
     }
 
     /** @brief %Check if a simple pattern is used inside Token::Match or Token::findmatch */
@@ -65,14 +66,18 @@ public:
     /** @brief %Check for missing % end character in Token::Match pattern */
     void checkMissingPercentCharacter();
 
-    /** @brief %Check for for unknown (invalid) complex patterns like "%typ%" */
+    /** @brief %Check for unknown (invalid) complex patterns like "%typ%" */
     void checkUnknownPattern();
 
+    /** @brief %Check for inefficient usage of Token::next(), Token::previous() and Token::tokAt() */
+    void checkRedundantNextPrevious();
+
 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 missingPercentCharacterError(const Token *tok, const std::string &pattern, const std::string &funcname);
     void unknownPatternError(const Token* tok, const std::string& pattern);
+    void redundantNextPreviousError(const Token* tok, const std::string& func1, const std::string& func2);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckInternal c(0, settings, errorLogger);
@@ -80,6 +85,7 @@ private:
         c.complexPatternError(0, "%type% ( )", "Match");
         c.missingPercentCharacterError(0, "%num", "Match");
         c.unknownPatternError(0, "%typ");
+        c.redundantNextPreviousError(0, "previous", "next");
     }
 
     static std::string myName() {

lib/checknullpointer.cpp

@@ -849,8 +849,8 @@ void CheckNullPointer::nullPointerByCheckAndDeRef()
             vartok = tok->next();
         } else if (Token::Match(tok, "( ! ( %var% =")) {
             vartok = tok->tokAt(3);
-            if (Token::simpleMatch(tok->tokAt(2)->link(), ") &&"))
+            if (Token::simpleMatch(tok->linkAt(2), ") &&"))
-                checkConditionStart = tok->tokAt(2)->link();
+                checkConditionStart = tok->linkAt(2);
         } else
             continue;
 

lib/checkother.cpp

@@ -2176,7 +2176,7 @@ void CheckOther::checkCCTypeFunctions()
         if (tok->varId() == 0 &&
             Token::Match(tok, "isalnum|isalpha|iscntrl|isdigit|isgraph|islower|isprint|ispunct|isspace|isupper|isxdigit ( %num% ,|)") &&
             MathLib::isNegative(tok->strAt(2))) {
-            cctypefunctionCallError(tok, tok->str(), tok->tokAt(2)->str());
+            cctypefunctionCallError(tok, tok->str(), tok->strAt(2));
         }
     }
 }
@@ -2481,12 +2481,12 @@ void CheckOther::checkDoubleFree()
             if (var) {
                 if (Token::Match(tok, "free|g_free")) {
                     if (freedVariables.find(var) != freedVariables.end())
-                        doubleFreeError(tok, tok->tokAt(2)->str());
+                        doubleFreeError(tok, tok->strAt(2));
                     else
                         freedVariables.insert(var);
                 } else if (tok->str() == "closedir") {
                     if (closeDirVariables.find(var) != closeDirVariables.end())
-                        doubleCloseDirError(tok, tok->tokAt(2)->str());
+                        doubleCloseDirError(tok, tok->strAt(2));
                     else
                         closeDirVariables.insert(var);
                 }
@@ -2500,7 +2500,7 @@ void CheckOther::checkDoubleFree()
             unsigned int var = tok->tokAt(varIdx)->varId();
             if (var) {
                 if (freedVariables.find(var) != freedVariables.end())
-                    doubleFreeError(tok, tok->tokAt(varIdx)->str());
+                    doubleFreeError(tok, tok->strAt(varIdx));
                 else
                     freedVariables.insert(var);
             }

lib/templatesimplifier.cpp

@@ -375,7 +375,7 @@ std::list<Token *> TemplateSimplifier::simplifyTemplatesGetTemplateDeclarations(
     for (Token *tok = tokens; tok; tok = tok->next()) {
         // TODO: handle namespaces. Right now we don't instantiate templates that are defined in namespaces.
         if (Token::Match(tok, "namespace %type% {"))
-            tok = tok->tokAt(2)->link();
+            tok = tok->linkAt(2);
 
         if (Token::simpleMatch(tok, "template <")) {
             codeWithTemplates = true;

lib/tokenize.cpp

@@ -8657,7 +8657,7 @@ void Tokenizer::simplifyNamespaceStd()
         if (_settings->standards.cpp == Standards::CPP11 && Token::simpleMatch(tok, "std :: tr1 ::"))
             Token::eraseTokens(tok, tok->tokAt(3));
 
-        else if (Token::Match(tok, "using namespace std ;")) {
+        else if (Token::simpleMatch(tok, "using namespace std ;")) {
             Token::eraseTokens(tok, tok->tokAt(4));
             tok->deleteThis();
         }

test/testinternal.cpp

@@ -38,6 +38,7 @@ private:
         TEST_CASE(simplePatternAlternatives)
         TEST_CASE(missingPercentCharacter)
         TEST_CASE(unknownPattern)
+        TEST_CASE(redundantNextPrevious)
         TEST_CASE(internalError)
     }
 
@@ -244,6 +245,58 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void redundantNextPrevious() {
+        check("void f() {\n"
+              "    return tok->next()->previous();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::next()' followed by 'Token::previous()' can be simplified.\n", errout.str());
+
+        check("void f() {\n"
+              "    return tok->tokAt(5)->previous();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::tokAt()' followed by 'Token::previous()' can be simplified.\n", errout.str());
+
+        check("void f() {\n"
+              "    return tok->previous()->linkAt(5);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::previous()' followed by 'Token::linkAt()' can be simplified.\n", errout.str());
+
+        check("void f() {\n"
+              "    tok->next()->previous(foo);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    return tok->next()->next();\n" // Simplification won't make code much shorter/readable
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    return tok->previous()->previous();\n" // Simplification won't make code much shorter/readable
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    return tok->tokAt(foo+bar)->tokAt();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::tokAt()' followed by 'Token::tokAt()' can be simplified.\n", errout.str());
+
+        check("void f() {\n"
+              "    return tok->tokAt(foo+bar)->link();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::tokAt()' followed by 'Token::link()' can be simplified.\n", errout.str());
+
+        check("void f() {\n"
+              "    tok->tokAt(foo+bar)->link(foo);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    return tok->next()->next()->str();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Call to 'Token::next()' followed by 'Token::str()' can be simplified.\n", errout.str());
+    }
+
     void internalError() {
         // Make sure cppcheck does not raise an internal error of Token::Match ( Ticket #3727 )
         check("class DELPHICLASS X;\n"