From a7728fef488539215aca29082ac3e1fd6e93b8e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <danielm77@spray.se>
Date: Fri, 19 Aug 2011 00:14:15 +0200
Subject: [PATCH] New check: warn about such suspicious conditions: '(a & b ==
 c)'

---
 lib/checkother.cpp | 16 ++++++++++++----
 lib/checkother.h   |  4 ++--
 test/testother.cpp | 14 ++++++++++++--
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 9d0062dda..bf25276f7 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -137,13 +137,14 @@ void CheckOther::clarifyCalculationError(const Token *tok, const std::string &op
 
 
 // Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))'
+// Clarify condition '(a & b == c)' into '((a & b) == c)' or '(a & (b == c))'
 void CheckOther::clarifyCondition()
 {
     if (!_settings->isEnabled("style"))
         return;
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
-        if (Token::Match(tok, "( %var% ="))
+        if (Token::Match(tok, "( %var% [=&|^]"))
         {
             for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next())
             {
@@ -153,7 +154,7 @@ void CheckOther::clarifyCondition()
                     break;
                 else if (Token::Match(tok2, "<|<=|==|!=|>|>= %num% )"))
                 {
-                    clarifyConditionError(tok);
+                    clarifyConditionError(tok, tok->strAt(2) == "=");
                     break;
                 }
             }
@@ -161,12 +162,19 @@ void CheckOther::clarifyCondition()
     }
 }
 
-void CheckOther::clarifyConditionError(const Token *tok)
+void CheckOther::clarifyConditionError(const Token *tok, bool assign)
 {
+    std::string errmsg;
+    if (assign)
+        errmsg = "Suspicious condition (assignment+comparison), it can be clarified with parentheses";
+    else
+        errmsg = "Suspicious condition (bitwise operator + comparison), it can be clarified with parentheses\n"
+                 "Suspicious condition. Comparison operators have higher precedence than bitwise operators. Please clarify the condition with parentheses.";
+
     reportError(tok,
                 Severity::style,
                 "clarifyCondition",
-                "Suspicious condition (assignment+comparison), it can be clarified with parentheses");
+                errmsg);
 }
 
 
diff --git a/lib/checkother.h b/lib/checkother.h
index f3ddb09c0..8cf03417d 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -112,7 +112,7 @@ public:
 
     /** @brief Suspicious condition (assignment+comparison) */
     void clarifyCondition();
-    void clarifyConditionError(const Token *tok);
+    void clarifyConditionError(const Token *tok, bool assign);
 
     /** @brief Are there C-style pointer casts in a c++ file? */
     void warningOldStylePointerCast();
@@ -319,7 +319,7 @@ public:
         c.catchExceptionByValueError(0);
         c.memsetZeroBytesError(0, "varname");
         c.clarifyCalculationError(0, "+");
-        c.clarifyConditionError(0);
+        c.clarifyConditionError(0, true);
         c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
         c.incrementBooleanError(0);
         c.comparisonOfBoolWithIntError(0, "varname");
diff --git a/test/testother.cpp b/test/testother.cpp
index 6caa209e6..a9de95018 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -114,7 +114,8 @@ private:
 
         TEST_CASE(clarifyCalculation);
 
-        TEST_CASE(clarifyCondition);     // if (a = b() < 0)
+        TEST_CASE(clarifyCondition1);     // if (a = b() < 0)
+        TEST_CASE(clarifyCondition2);     // if (a & b == c)
 
         TEST_CASE(incorrectStringCompare);
 
@@ -2623,7 +2624,7 @@ private:
     }
 
     // clarify conditions with = and comparison
-    void clarifyCondition()
+    void clarifyCondition1()
     {
         check("void f() {\n"
               "    if (x = b() < 0) {}\n"
@@ -2631,6 +2632,15 @@ private:
         ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (assignment+comparison), it can be clarified with parentheses\n", errout.str());
     }
 
+    // clarify conditions with bitwise operator and comparison
+    void clarifyCondition2()
+    {
+        check("void f() {\n"
+              "    if (x & 2 == 2) {}\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison), it can be clarified with parentheses\n", errout.str());
+    }
+
     void incorrectStringCompare()
     {
         check("int f() {\n"