add check for same expression on both sides of an operator (part of #2700)

lib/checkother.cpp

@@ -3230,6 +3230,56 @@ void CheckOther::duplicateBranchError(const Token *tok1, const Token *tok2)
                 "carefully to determine if it is correct.");
 }
 
+//---------------------------------------------------------------------------
+// check for the same expression on both sides of an operator
+// (x == x), (x && x), (x || x)
+// (x.y == x.y), (x.y && x.y), (x.y || x.y)
+//---------------------------------------------------------------------------
+
+void CheckOther::checkDuplicateExpression()
+{
+    if (!_settings->_checkCodingStyle)
+        return;
+
+    // Parse all executing scopes..
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
+    std::list<Scope>::const_iterator scope;
+
+    for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope)
+    {
+        // only check functions
+        if (scope->type != Scope::eFunction)
+            continue;
+
+        for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next())
+        {
+            if (Token::Match(tok, "(|&&|%oror% %var% &&|%oror%|==|!=|<=|>=|<|>|-|%or% %var% )|&&|%oror%") &&
+                tok->strAt(1) == tok->strAt(3))
+            {
+                duplicateExpressionError(tok->next(), tok->tokAt(3), tok->strAt(2));
+            }
+            else if (Token::Match(tok, "(|&&|%oror% %var% . %var% &&|%oror%|==|!=|<=|>=|<|>|-|%or% %var% . %var% )|&&|%oror%") &&
+                     tok->strAt(1) == tok->strAt(5) && tok->strAt(3) == tok->strAt(7))
+            {
+                duplicateExpressionError(tok->next(), tok->tokAt(6), tok->strAt(4));
+            }
+        }
+    }
+}
+
+void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op)
+{
+    std::list<const Token *> toks;
+    toks.push_back(tok2);
+    toks.push_back(tok1);
+
+    reportError(toks, Severity::style, "duplicateExpression", "Same expression on both sides of \'" + op + "\'.\n"
+                "Finding the same expression on both sides of an operator is suspicious and might "
+                "indicate a cut and paste or logic error. Please examine this code carefully to "
+                "determine if it is correct.");
+}
+
 //-----------------------------------------------------------------------------
 
 void CheckOther::cstyleCastError(const Token *tok)

lib/checkother.h

@@ -65,6 +65,7 @@ public:
         checkOther.checkSelfAssignment();
         checkOther.checkDuplicateIf();
         checkOther.checkDuplicateBranch();
+        checkOther.checkDuplicateExpression();
 
         // information checks
         checkOther.checkVariableScope();
@@ -212,6 +213,9 @@ public:
     /** @brief %Check for suspicious code where if and else branch are the same (e.g "if (a) b = true; else b = true;") */
     void checkDuplicateBranch();
 
+    /** @brief %Check for suspicious code with the same expression on both sides of operator (e.g "if (a && a)") */
+    void checkDuplicateExpression();
+
     // Error messages..
     void cstyleCastError(const Token *tok);
     void dangerousUsageStrtolError(const Token *tok);
@@ -242,6 +246,7 @@ public:
     void comparisonOfBoolWithIntError(const Token *tok, const std::string &varname);
     void duplicateIfError(const Token *tok1, const Token *tok2);
     void duplicateBranchError(const Token *tok1, const Token *tok2);
+    void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -288,6 +293,7 @@ public:
         c.comparisonOfBoolWithIntError(0, "varname");
         c.duplicateIfError(0, 0);
         c.duplicateBranchError(0, 0);
+        c.duplicateExpressionError(0, 0, "&&");
     }
 
     std::string myName() const

test/testother.cpp

@@ -115,6 +115,7 @@ private:
 
         TEST_CASE(duplicateIf);
         TEST_CASE(duplicateBranch);
+        TEST_CASE(duplicateExpression);
     }
 
     void check(const char code[], const char *filename = NULL)
@@ -140,6 +141,7 @@ private:
         checkOther.clarifyCondition();
         checkOther.checkDuplicateIf();
         checkOther.checkDuplicateBranch();
+        checkOther.checkDuplicateExpression();
 
         // Simplify token list..
         tokenizer.simplifyTokenList();
@@ -2487,6 +2489,28 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void duplicateExpression()
+    {
+        check("voif foo() {\n"
+              "    if (a == a) { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
+
+        check("void fun() {\n"
+              "    return (a && a ||\n"
+              "            b == b &&\n"
+              "            c - c &&\n"
+              "            d > d &&\n"
+              "            e < e &&\n"
+              "            f);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n"
+                      "[test.cpp:3] -> [test.cpp:3]: (style) Same expression on both sides of '=='.\n"
+                      "[test.cpp:4] -> [test.cpp:4]: (style) Same expression on both sides of '-'.\n"
+                      "[test.cpp:5] -> [test.cpp:5]: (style) Same expression on both sides of '>'.\n"
+                      "[test.cpp:6] -> [test.cpp:6]: (style) Same expression on both sides of '<'.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)