From a596a7a8fe656314ae3686d81894cfd26419ba14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <danielm77@spray.se>
Date: Mon, 24 Jan 2011 21:40:49 +0100
Subject: [PATCH] Fixed #2494 (New check: clarify calculation when using ?:
 operator)

---
 lib/checkother.cpp | 54 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/checkother.h   |  8 +++++++
 test/testother.cpp | 14 ++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index d8e065610..76af4e0a5 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -35,6 +35,60 @@ CheckOther instance;
 //---------------------------------------------------------------------------
 
 
+void CheckOther::clarifyCalculation()
+{
+    if (!_settings->_checkCodingStyle)
+        return;
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        if (tok->str() == "?")
+        {
+            // condition
+            const Token *cond = tok->previous();
+            if (cond->isName() || cond->isNumber())
+                cond = cond->previous();
+            else if (cond->str() == ")")
+                cond = cond->link()->previous();
+            else
+                continue;
+
+            // multiplication
+            if (cond->str() == "*")
+                cond = cond->previous();
+            else
+                continue;
+
+            // skip previous multiplications..
+            while (cond && cond->strAt(-1) == "*" && (cond->isName() || cond->isNumber()))
+                cond = cond->tokAt(-2);
+
+            if (!cond)
+                continue;
+
+            // first multiplication operand
+            if (cond->str() == ")")
+            {
+                clarifyCalculationError(cond);
+            }
+            else if (cond->isName() || cond->isNumber())
+            {
+                if (Token::Match(cond->previous(),"return|+|-|,|("))
+                    clarifyCalculationError(cond);
+            }
+        }
+    }
+}
+
+void CheckOther::clarifyCalculationError(const Token *tok)
+{
+    reportError(tok,
+                Severity::information,
+                "clarifyCalculation",
+                "Please clarify precedence: 'a*b?..'\n"
+                "Found a suspicious multiplication of condition. Please use parantheses to clarify the code. "
+                "The code 'a*b?1:2' should be written as either '(a*b)?1:2' or 'a*(b?1:2)'.");
+}
+
 
 void CheckOther::warningOldStylePointerCast()
 {
diff --git a/lib/checkother.h b/lib/checkother.h
index 0d0798dd7..bace5a82e 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -70,6 +70,8 @@ public:
     {
         CheckOther checkOther(tokenizer, settings, errorLogger);
 
+        checkOther.clarifyCalculation();
+
         // Coding style checks
         checkOther.checkConstantFunctionParameter();
         checkOther.checkIncompleteStatement();
@@ -87,6 +89,10 @@ public:
         checkOther.checkMemsetZeroBytes();
     }
 
+    /** @brief Clarify calculation for ".. a * b ? .." */
+    void clarifyCalculation();
+    void clarifyCalculationError(const Token *tok);
+
     /** @brief Are there C-style pointer casts in a c++ file? */
     void warningOldStylePointerCast();
 
@@ -236,6 +242,7 @@ public:
         c.unassignedVariableError(0, "varname");
         c.catchExceptionByValueError(0);
         c.memsetZeroBytesError(0, "varname");
+        c.clarifyCalculationError(0);
     }
 
     std::string name() const
@@ -274,6 +281,7 @@ public:
                "* assignment of a variable to itself\n"
                "* mutual exclusion over || always evaluating to true\n"
                "* exception caught by value instead of by reference\n"
+               "* Clarify calculation with parantheses\n"
 
                // optimisations
                "* optimisation: detect post increment/decrement\n";
diff --git a/test/testother.cpp b/test/testother.cpp
index f71d7eb97..8546c5d83 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -99,6 +99,8 @@ private:
         TEST_CASE(memsetZeroBytes);
 
         TEST_CASE(sizeofForArrayParameter);
+
+        TEST_CASE(clarifyCalculation);
     }
 
     void check(const char code[], const char *filename = NULL)
@@ -134,6 +136,7 @@ private:
         checkOther.checkIncorrectLogicOperator();
         checkOther.checkCatchExceptionByValue();
         checkOther.checkMemsetZeroBytes();
+        checkOther.clarifyCalculation();
     }
 
 
@@ -1748,8 +1751,19 @@ private:
               "}\n"
              );
         ASSERT_EQUALS("", errout.str());
+    }
 
+    void clarifyCalculation()
+    {
+        check("int f(char c) {\n"
+              "    return 10 * (c == 0) ? 1 : 2;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (information) Please clarify precedence: 'a*b?..'\n", errout.str());
 
+        check("void f(char c) {\n"
+              "    printf(\"%i\", 10 * (c == 0) ? 1 : 2);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (information) Please clarify precedence: 'a*b?..'\n", errout.str());
     }
 
 };