From 3925a27182a05e81daf07f15803638a05db452ea Mon Sep 17 00:00:00 2001
From: chrchr-github <78114321+chrchr-github@users.noreply.github.com>
Date: Tue, 30 Aug 2022 18:08:24 +0200
Subject: [PATCH] Fix FP knowConditionTrueFalse (#4419)

* Fix #10426 FN (style) Condition 's.empty()' is always false

* Fix test

* Fix FP knowConditionTrueFalse

* Check for casts

* Fix FP for user-defined functions

* Adjust condition detection

* Tweaks
---
 lib/checkcondition.cpp | 15 ++++++++++-----
 test/cfg/qt.cpp        |  2 +-
 test/testcondition.cpp | 27 +++++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
index eaf0ac338..6dbba4d7d 100644
--- a/lib/checkcondition.cpp
+++ b/lib/checkcondition.cpp
@@ -1512,15 +1512,20 @@ void CheckCondition::alwaysTrueFalse()
                     continue;
             }
 
+            const Token* astTop = tok->astTop();
             const bool constIfWhileExpression =
-                tok->astParent() && Token::Match(tok->astTop()->astOperand1(), "if|while") && !tok->astTop()->astOperand1()->isConstexpr() &&
+                tok->astParent() && Token::Match(astTop->astOperand1(), "if|while") && !astTop->astOperand1()->isConstexpr() &&
                 (Token::Match(tok->astParent(), "%oror%|&&") || Token::Match(tok->astParent()->astOperand1(), "if|while"));
             const bool constValExpr = tok->isNumber() && Token::Match(tok->astParent(),"%oror%|&&|?"); // just one number in boolean expression
-            const bool compExpr = Token::Match(tok, "%comp%|!|("); // a compare expression
+            const bool compExpr = Token::Match(tok, "%comp%|!"); // a compare expression
             const bool ternaryExpression = Token::simpleMatch(tok->astParent(), "?");
-            const bool returnExpression = Token::simpleMatch(tok->astTop(), "return") && (tok->isComparisonOp() || Token::Match(tok, "&&|%oror%"));
-
-            if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression || returnExpression))
+            const bool isReturn = Token::simpleMatch(astTop, "return");
+            const bool returnExpression = isReturn && (tok->isComparisonOp() || Token::Match(tok, "&&|%oror%"));
+            const bool callExpression = Token::simpleMatch(tok, "(");
+            if (!(constIfWhileExpression || constValExpr || compExpr || ternaryExpression || returnExpression || callExpression))
+                continue;
+            // only warn for functions returning bool
+            if (isReturn && callExpression && !(scope->function && scope->function->retDef && scope->function->retDef->str() == "bool"))
                 continue;
 
             const Token* expr1 = tok->astOperand1(), *expr2 = tok->astOperand2();
diff --git a/test/cfg/qt.cpp b/test/cfg/qt.cpp
index c793e48fc..3493dfe49 100644
--- a/test/cfg/qt.cpp
+++ b/test/cfg/qt.cpp
@@ -29,7 +29,7 @@ void QString1(QString s)
     }
 }
 
-int QString2()
+bool QString2()
 {
     QString s;
     // cppcheck-suppress knownConditionTrueFalse
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index f85fdebab..086aa3517 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -4307,15 +4307,38 @@ private:
         ASSERT_EQUALS("", errout.str());
 
         // #10426
-        check("void f() {\n"
+        check("int f() {\n"
               "    std::string s;\n"
               "    for (; !s.empty();) {}\n"
               "    for (; s.empty();) {}\n"
               "    if (s.empty()) {}\n"
+              "    if ((bool)0) {}\n"
+              "    return s.empty();"
               "}\n");
         ASSERT_EQUALS("[test.cpp:3]: (style) Condition '!s.empty()' is always false\n"
                       "[test.cpp:4]: (style) Condition 's.empty()' is always true\n"
-                      "[test.cpp:5]: (style) Condition 's.empty()' is always true\n",
+                      "[test.cpp:5]: (style) Condition 's.empty()' is always true\n"
+                      "[test.cpp:6]: (style) Condition '(bool)0' is always false\n",
+                      errout.str());
+
+        check("int f(bool b) {\n"
+              "    if (b) return static_cast<int>(1);\n"
+              "    return (int)0;\n"
+              "}\n"
+              "bool g(bool b) {\n"
+              "    if (b) return static_cast<int>(1);\n"
+              "    return (int)0;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:6]: (style) Condition 'static_cast<int>(1)' is always true\n"
+                      "[test.cpp:7]: (style) Condition '(int)0' is always false\n",
+                      errout.str());
+
+        check("int f() { return 3; }\n"
+              "int g() { return f(); }\n"
+              "int h() { if (f()) {} }\n"
+              "int i() { return f() == 3; }\n");
+        ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'f()' is always true\n"
+                      "[test.cpp:4]: (style) Condition 'f()==3' is always true\n",
                       errout.str());
     }