From 855b01cd5a20d527b2d86da14daf26d6773d04ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <danielm77@spray.se>
Date: Sat, 2 Apr 2011 11:43:20 +0200
Subject: [PATCH] Clarify precedence 'a*b?c:d' : warn for addition, subtraction
 and division also. tried to clarify the message more

---
 lib/checkother.cpp | 30 ++++++++++++++++++++----------
 lib/checkother.h   |  4 ++--
 test/testother.cpp |  9 +++++++--
 3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 01ea707cc..41a1a6774 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -85,12 +85,13 @@ void CheckOther::clarifyCalculation()
             else
                 continue;
 
-            // multiplication
-            if (cond && cond->str() == "*")
-                cond = cond->previous();
-            else
+            // calculation
+            if (!Token::Match(cond, "[+-*/]"))
                 continue;
 
+            const char op = cond->str()[0];
+            cond = cond->previous();
+
             // skip previous multiplications..
             while (cond && cond->strAt(-1) == "*" && (cond->isName() || cond->isNumber()))
                 cond = cond->tokAt(-2);
@@ -101,25 +102,34 @@ void CheckOther::clarifyCalculation()
             // first multiplication operand
             if (cond->str() == ")")
             {
-                clarifyCalculationError(cond);
+                clarifyCalculationError(cond, op);
             }
             else if (cond->isName() || cond->isNumber())
             {
                 if (Token::Match(cond->previous(),"return|+|-|,|("))
-                    clarifyCalculationError(cond);
+                    clarifyCalculationError(cond, op);
             }
         }
     }
 }
 
-void CheckOther::clarifyCalculationError(const Token *tok)
+void CheckOther::clarifyCalculationError(const Token *tok, char op)
 {
+    // suspicious calculation
+    const std::string calc(std::string("'a") + op + "b?c:d'");
+
+    // recommended calculation #1
+    const std::string s1(std::string("'(a") + op + "b)?c:d'");
+
+    // recommended calculation #2
+    const std::string s2(std::string("'a") + op + "(b?c:d)'");
+
     reportError(tok,
                 Severity::style,
                 "clarifyCalculation",
-                "Please clarify precedence: 'a*b?..'\n"
-                "Found a suspicious multiplication of condition. Please use parentheses to clarify the code. "
-                "The code 'a*b?1:2' should be written as either '(a*b)?1:2' or 'a*(b?1:2)'.");
+                "Suspicious calculation, " + calc + " can be clarified as either " + s1 + " or " + s2 + "\n" +
+                "Suspicious calculation. Please use parentheses to clarify the code. "
+                "The code " + calc + " should be written as either " + s1 + " or " + s2 + ".");
 }
 
 
diff --git a/lib/checkother.h b/lib/checkother.h
index 1e65b7e6e..ad2b8c5ee 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -97,7 +97,7 @@ public:
 
     /** @brief Clarify calculation for ".. a * b ? .." */
     void clarifyCalculation();
-    void clarifyCalculationError(const Token *tok);
+    void clarifyCalculationError(const Token *tok, char op);
 
     /** @brief Suspicious condition (assignment+comparison) */
     void clarifyCondition();
@@ -269,7 +269,7 @@ public:
         c.unassignedVariableError(0, "varname");
         c.catchExceptionByValueError(0);
         c.memsetZeroBytesError(0, "varname");
-        c.clarifyCalculationError(0);
+        c.clarifyCalculationError(0, '+');
         c.clarifyConditionError(0);
         c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
         c.incrementBooleanError(0);
diff --git a/test/testother.cpp b/test/testother.cpp
index 123632126..8126ad0a4 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -2274,18 +2274,23 @@ private:
         check("int f(char c) {\n"
               "    return 10 * (c == 0) ? 1 : 2;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious calculation, 'a*b?c:d' can be clarified as either '(a*b)?c:d' or 'a*(b?c:d)'\n", errout.str());
 
         check("void f(char c) {\n"
               "    printf(\"%i\", 10 * (c == 0) ? 1 : 2);\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (style) Please clarify precedence: 'a*b?..'\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious calculation, 'a*b?c:d' can be clarified as either '(a*b)?c:d' or 'a*(b?c:d)'\n", errout.str());
 
         // Ticket #2585 - segmentation fault for invalid code
         check("abcdef?" "?<"
               "123456?" "?>"
               "+?" "?=");
         ASSERT_EQUALS("", errout.str());
+
+        check("void f(char c) {\n"
+              "    printf(\"%i\", 1 + 1 ? 1 : 2);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious calculation, 'a+b?c:d' can be clarified as either '(a+b)?c:d' or 'a+(b?c:d)'\n", errout.str());
     }
 
     // clarify conditions with = and comparison