From 66de41b313e6da1a4067fccb070b8f4eb2261d7c Mon Sep 17 00:00:00 2001
From: Robert Reif <reif@earthlink.net>
Date: Sat, 9 Apr 2011 17:05:27 -0400
Subject: [PATCH] partial fix for #2700 (common logic or cut and paste errors)

---
 lib/checkother.cpp | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/checkother.h   |  6 +++++
 test/testother.cpp | 24 +++++++++++++++++++
 3 files changed, 88 insertions(+)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 31c076398..5449b5335 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -3157,6 +3157,64 @@ void CheckOther::duplicateIfError(const Token *tok1, const Token *tok2)
                 "if it is correct.");
 }
 
+//-----------------------------------------------------------------------------
+// check for duplicate code in if and else branches
+// if (a) { b = true; } else { b = true; }
+//-----------------------------------------------------------------------------
+
+void CheckOther::checkDuplicateBranch()
+{
+    if (!_settings->_checkCodingStyle)
+        return;
+
+    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;
+
+        // check all the code in the function for if (..) else
+        for (const Token *tok = scope->classStart; tok && tok != scope->classStart->link(); tok = tok->next())
+        {
+            if (Token::Match(tok, "if (") && tok->strAt(-1) != "else" &&
+                Token::Match(tok->next()->link(), ") {") &&
+                Token::Match(tok->next()->link()->next()->link(), "} else {"))
+            {
+                // save if branch code
+                std::string branch1 = stringifyTokens(tok->next()->link()->tokAt(2), tok->next()->link()->next()->link()->previous());
+
+                // find else branch
+                const Token *tok1 = tok->next()->link()->next()->link();
+
+                // save else branch code
+                std::string branch2 = stringifyTokens(tok1->tokAt(3), tok1->tokAt(2)->link()->previous());
+
+                // check for duplicates
+                if (branch1 == branch2)
+                    duplicateBranchError(tok, tok1->tokAt(2));
+
+                tok = tok->next()->link()->next();
+            }
+        }
+    }
+}
+
+void CheckOther::duplicateBranchError(const Token *tok1, const Token *tok2)
+{
+    std::list<const Token *> toks;
+    toks.push_back(tok2);
+    toks.push_back(tok1);
+
+    reportError(toks, Severity::style, "duplicateBranch", "Found duplicate branches for if and else.\n"
+                "Finding the same code for an if branch and an else branch 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)
diff --git a/lib/checkother.h b/lib/checkother.h
index 002e6cbec..31c12d8d0 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -64,6 +64,7 @@ public:
         checkOther.checkSizeofForArrayParameter();
         checkOther.checkSelfAssignment();
         checkOther.checkDuplicateIf();
+        checkOther.checkDuplicateBranch();
 
         // information checks
         checkOther.checkVariableScope();
@@ -208,6 +209,9 @@ public:
     /** @brief %Check for suspicious code where multiple if have the same expression (e.g "if (a) { } else if (a) { }") */
     void checkDuplicateIf();
 
+    /** @brief %Check for suspicious code where if and else branch are the same (e.g "if (a) b = true; else b = true;") */
+    void checkDuplicateBranch();
+
     // Error messages..
     void cstyleCastError(const Token *tok);
     void dangerousUsageStrtolError(const Token *tok);
@@ -237,6 +241,7 @@ public:
     void incrementBooleanError(const Token *tok);
     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 getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -282,6 +287,7 @@ public:
         c.incrementBooleanError(0);
         c.comparisonOfBoolWithIntError(0, "varname");
         c.duplicateIfError(0, 0);
+        c.duplicateBranchError(0, 0);
     }
 
     std::string myName() const
diff --git a/test/testother.cpp b/test/testother.cpp
index 49ec12cbe..b42097501 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -114,6 +114,7 @@ private:
         TEST_CASE(comparisonOfBoolWithInt);
 
         TEST_CASE(duplicateIf);
+        TEST_CASE(duplicateBranch);
     }
 
     void check(const char code[], const char *filename = NULL)
@@ -138,6 +139,7 @@ private:
         checkOther.checkSizeofForArrayParameter();
         checkOther.clarifyCondition();
         checkOther.checkDuplicateIf();
+        checkOther.checkDuplicateBranch();
 
         // Simplify token list..
         tokenizer.simplifyTokenList();
@@ -2453,6 +2455,28 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void duplicateBranch()
+    {
+        check("void f(int a, int &b) {\n"
+              "    if (a)\n"
+              "        b = 1;\n"
+              "    else\n"
+              "        b = 1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (style) Found duplicate branches for if and else.\n", errout.str());
+
+        check("void f(int a, int &b) {\n"
+              "    if (a) {\n"
+              "        if (a == 1)\n"
+              "            b = 2;\n"
+              "        else\n"
+              "            b = 2;\n"
+              "    } else\n"
+              "        b = 1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:3]: (style) Found duplicate branches for if and else.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)