From b79619832eb428b8ab96cca7504076dafe1038c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Sun, 23 Dec 2018 12:42:18 +0100
Subject: [PATCH] Clarify warning

---
 lib/checktype.cpp | 17 ++++++++++-------
 test/testtype.cpp | 14 ++++++++++----
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/lib/checktype.cpp b/lib/checktype.cpp
index 21652f5c3..5d0e17911 100644
--- a/lib/checktype.cpp
+++ b/lib/checktype.cpp
@@ -243,14 +243,17 @@ void CheckType::checkSignConversion()
 
 void CheckType::signConversionError(const Token *tok, const bool constvalue)
 {
-    const std::string varname(tok ? tok->str() : "var");
+    const std::string expr(tok ? tok->expressionString() : "var");
 
-    reportError(tok,
-                Severity::warning,
-                "signConversion",
-                (constvalue) ?
-                "$symbol:" + varname + "\nSuspicious code: sign conversion of $symbol in calculation because '$symbol' has a negative value" :
-                "$symbol:" + varname + "\nSuspicious code: sign conversion of $symbol in calculation, even though $symbol can have a negative value", CWE195, false);
+    std::ostringstream msg;
+    if (tok && tok->isName())
+        msg << "$symbol:" << expr << "\n";
+    if (constvalue)
+        msg << "Suspicious code: sign conversion of '" << expr << "' in calculation because '" << expr << "' has a negative value";
+    else
+        msg << "Suspicious code: sign conversion of '" << expr << "' in calculation, even though '" << expr << "' can have a negative value";
+
+    reportError(tok, Severity::warning, "signConversion", msg.str(), CWE195, false);
 }
 
 
diff --git a/test/testtype.cpp b/test/testtype.cpp
index 0695ec433..7c14edd30 100644
--- a/test/testtype.cpp
+++ b/test/testtype.cpp
@@ -188,19 +188,25 @@ private:
 
     void signConversion() {
         check("x = -4 * (unsigned)y;");
-        ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of -4 in calculation because '-4' has a negative value\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of '-4' in calculation because '-4' has a negative value\n", errout.str());
+
+        check("unsigned int dostuff(int x) {\n" // x is signed
+              "  if (x==0) {}\n"
+              "  return (x-1)*sizeof(int);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious code: sign conversion of 'x-1' in calculation, even though 'x-1' can have a negative value\n", errout.str());
 
         check("unsigned int f1(signed int x, unsigned int y) {" // x is signed
               "  return x * y;\n"
               "}\n"
               "void f2() { f1(-4,4); }");
-        TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", "", errout.str());
+        TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of 'x' in calculation, even though x can have a negative value\n", "", errout.str());
 
         check("unsigned int f1(int x) {" // x has no signedness, but it can have the value -1 so assume it's signed
               "  return x * 5U;\n"
               "}\n"
               "void f2() { f1(-4); }");
-        TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", "", errout.str());
+        TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of 'x' in calculation, even though x can have a negative value\n", "", errout.str());
 
         check("unsigned int f1(int x) {" // #6168: FP for inner calculation
               "  return 5U * (1234 - x);\n" // <- signed subtraction, x is not sign converted
@@ -218,7 +224,7 @@ private:
         check("size_t foo(size_t x) {\n"
               " return -2 * x;\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious code: sign conversion of -2 in calculation because '-2' has a negative value\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious code: sign conversion of '-2' in calculation because '-2' has a negative value\n", errout.str());
     }
 
     void longCastAssign() {