From ea02bd905acda6e200ddd7da7713270ee187f091 Mon Sep 17 00:00:00 2001
From: PKEuS <philipp.kloke@web.de>
Date: Tue, 18 Oct 2011 21:37:03 +0200
Subject: [PATCH] Fixed #3225 (Boolean comparison with string literals)

---
 lib/checkother.cpp | 17 +++++++++++++++++
 lib/checkother.h   |  3 +++
 test/testother.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 5b0c8dea5..037276951 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -1963,6 +1963,18 @@ void CheckOther::checkIncorrectStringCompare()
                 incorrectStringCompareError(tok->next(), "substr", tok->str(), tok->tokAt(8)->str());
             }
         }
+        if (Token::Match(tok, "&&|%oror% %str% &&|%oror%|)")) {
+            // assert(condition && "debug message") would be considered a fp.
+            if (tok->str() == "&&" && tok->tokAt(2)->str() == ")" && tok->tokAt(2)->link()->previous()->str() == "assert")
+                continue;
+            incorrectStringBooleanError(tok->tokAt(1), tok->tokAt(1)->str());
+        }
+        if (Token::Match(tok, "if|while|assert ( %str% &&|%oror%|)")) {
+            // assert("debug message" && condition) would be considered a fp.
+            if (tok->tokAt(3)->str() == "&&" && tok->str() == "assert")
+                continue;
+            incorrectStringBooleanError(tok->tokAt(2), tok->tokAt(2)->str());
+        }
     }
 }
 
@@ -1971,6 +1983,11 @@ void CheckOther::incorrectStringCompareError(const Token *tok, const std::string
     reportError(tok, Severity::warning, "incorrectStringCompare", "String literal " + string + " doesn't match length argument for " + func + "(" + len + ").");
 }
 
+void CheckOther::incorrectStringBooleanError(const Token *tok, const std::string& string)
+{
+    reportError(tok, Severity::warning, "incorrectStringBooleanError", "A boolean comparison with the string literal " + string + " is always true.");
+}
+
 //-----------------------------------------------------------------------------
 // check for duplicate expressions in if statements
 // if (a) { } else if (a) { }
diff --git a/lib/checkother.h b/lib/checkother.h
index e09776410..2269473d0 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -261,6 +261,7 @@ public:
     void sizeofForArrayParameterError(const Token *tok);
     void sizeofForNumericParameterError(const Token *tok);
     void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len);
+    void incorrectStringBooleanError(const Token *tok, const std::string& string);
     void incrementBooleanError(const Token *tok);
     void comparisonOfBoolWithIntError(const Token *tok, const std::string &expression);
     void duplicateIfError(const Token *tok1, const Token *tok2);
@@ -312,6 +313,7 @@ public:
         c.clarifyCalculationError(0, "+");
         c.clarifyConditionError(0, true, false);
         c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
+        c.incorrectStringBooleanError(0, "\"Hello World\"");
         c.incrementBooleanError(0);
         c.comparisonOfBoolWithIntError(0, "varname");
         c.duplicateIfError(0, 0);
@@ -369,6 +371,7 @@ public:
                "* comparison of a boolean expression with an integer other than 0 or 1\n"
                "* suspicious condition (assignment+comparison)\n"
                "* suspicious condition (runtime comparison of string literals)\n"
+               "* suspicious condition (string literals as boolean)\n"
                "* duplicate break statement\n"
                "* testing if unsigned variable is negative\n"
                "* testing is unsigned variable is positive\n"
diff --git a/test/testother.cpp b/test/testother.cpp
index 24d3c75a8..8de67f356 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -3063,6 +3063,51 @@ private:
               " return \"Hello\" == test.substr( 0 , 5 ) ? : 0 : 1 ;\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("int f() {\n"
+              " if (\"Hello\") { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) A boolean comparison with the string literal \"Hello\" is always true.\n", errout.str());
+
+        check("int f() {\n"
+              " if (\"Hello\" && 1) { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) A boolean comparison with the string literal \"Hello\" is always true.\n", errout.str());
+
+        check("int f() {\n"
+              " if (1 && \"Hello\") { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) A boolean comparison with the string literal \"Hello\" is always true.\n", errout.str());
+
+        check("int f() {\n"
+              " while (\"Hello\") { }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) A boolean comparison with the string literal \"Hello\" is always true.\n", errout.str());
+
+        check("int f() {\n"
+              " assert (test || \"Hello\");\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) A boolean comparison with the string literal \"Hello\" is always true.\n", errout.str());
+
+        check("int f() {\n"
+              " assert (test && \"Hello\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int f() {\n"
+              " assert (\"Hello\" || test);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) A boolean comparison with the string literal \"Hello\" is always true.\n", errout.str());
+
+        check("int f() {\n"
+              " assert (\"Hello\" && test);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int f() {\n"
+              " return f2(\"Hello\");\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
     }