From d939c6015aa489317e10f2e9f6f9810f7fe9f767 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sat, 21 Apr 2018 04:28:21 -0500
Subject: [PATCH] Report opposite expressions (#1182)

* Report opposite expressions

* Skip assignment operator
---
 lib/checkother.cpp | 17 ++++++++++++++++-
 lib/checkother.h   |  2 ++
 test/testother.cpp | 30 ++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 658be763d..ed5dfc553 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -1974,7 +1974,12 @@ void CheckOther::checkDuplicateExpression()
                             }
                             duplicateExpressionError(tok, tok, tok->str());
                         }
-                    }
+                    } 
+                } else if (styleEnabled && 
+                    isOppositeCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library, false) && 
+                    !Token::simpleMatch(tok, "=") &&
+                    isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand1())) {
+                    oppositeExpressionError(tok, tok, tok->str());
                 } else if (!Token::Match(tok, "[-/%]")) { // These operators are not associative
                     if (styleEnabled && tok->astOperand2() && tok->str() == tok->astOperand1()->str() && isSameExpression(_tokenizer->isCPP(), true, tok->astOperand2(), tok->astOperand1()->astOperand2(), _settings->library, true) && isWithoutSideEffects(_tokenizer->isCPP(), tok->astOperand2()))
                         duplicateExpressionError(tok->astOperand2(), tok->astOperand2(), tok->str());
@@ -2003,6 +2008,16 @@ void CheckOther::checkDuplicateExpression()
     }
 }
 
+void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op)
+{
+    const std::list<const Token *> toks = { tok2, tok1 };
+
+    reportError(toks, Severity::style, "oppositeExpression", "Opposite expression on both sides of \'" + op + "\'.\n"
+                "Finding the opposite 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.", CWE398, false);
+}
+
 void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op)
 {
     const std::list<const Token *> toks = { tok2, tok1 };
diff --git a/lib/checkother.h b/lib/checkother.h
index 967d148c2..13600a3d4 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -239,6 +239,7 @@ private:
     void misusedScopeObjectError(const Token *tok, const std::string &varname);
     void duplicateBranchError(const Token *tok1, const Token *tok2);
     void duplicateAssignExpressionError(const Token *tok1, const Token *tok2);
+    void oppositeExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
     void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
     void duplicateValueTernaryError(const Token *tok);
     void duplicateExpressionTernaryError(const Token *tok);
@@ -298,6 +299,7 @@ private:
         c.clarifyCalculationError(nullptr,  "+");
         c.clarifyStatementError(nullptr);
         c.duplicateBranchError(nullptr, nullptr);
+        c.oppositeExpressionError(nullptr, nullptr, "&&");
         c.duplicateExpressionError(nullptr, nullptr, "&&");
         c.duplicateValueTernaryError(nullptr);
         c.duplicateExpressionTernaryError(nullptr);
diff --git a/test/testother.cpp b/test/testother.cpp
index 19eae5f86..34e081073 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -131,6 +131,7 @@ private:
         TEST_CASE(duplicateValueTernary);
         TEST_CASE(duplicateExpressionTernary); // #6391
         TEST_CASE(duplicateExpressionTemplate); // #6930
+        TEST_CASE(oppositeExpression);
         TEST_CASE(duplicateVarExpression);
 
         TEST_CASE(checkSignOfUnsignedVariable);
@@ -3918,6 +3919,35 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void oppositeExpression() {
+        check("void f(bool a) { if(a && !a) {} }");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '&&'.\n", errout.str());
+
+        check("void f(bool a) { if(a != !a) {} }");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
+        
+        check("void f(bool a) { if( a == !(a) ) {}}");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
+        
+        check("void f(bool a) { if( a != !(a) ) {}}");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
+        
+        check("void f(bool a) { if( !(a) == a ) {}}");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
+        
+        check("void f(bool a) { if( !(a) != a ) {}}");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
+        
+        check("void f(bool a) { if( !(!a) == !(a) ) {}}");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '=='.\n", errout.str());
+        
+        check("void f(bool a) { if( !(!a) != !(a) ) {}}");
+        ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:1]: (style) Opposite expression on both sides of '!='.\n", errout.str());
+        
+        check("void f(bool a) { a = !a; }");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void duplicateVarExpression() {
         check("int f() __attribute__((pure));\n"
               "void test() {\n"