From f7115208923482d022de63182f9c279d376ca629 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Tue, 21 Jul 2015 22:26:22 +0200
Subject: [PATCH] Fixed #6852 (false negative: logical conjunction 'x == 0')

---
 lib/checkcondition.cpp | 35 ++++++++++++++++++++++++++++++-----
 test/testcondition.cpp | 10 ++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp
index 21d791079..ddca80846 100644
--- a/lib/checkcondition.cpp
+++ b/lib/checkcondition.cpp
@@ -621,19 +621,29 @@ static bool parseComparison(const Token *comp, bool *not1, std::string *op, std:
         comp = comp->astOperand1();
     }
 
-    if (!comp || !comp->isComparisonOp() || !comp->astOperand1() || !comp->astOperand2())
+    if (!comp)
         return false;
 
-    if (comp->astOperand1()->isLiteral()) {
+    if (!comp->isComparisonOp() || !comp->astOperand1() || !comp->astOperand2()) {
+        *op = "!=";
+        *value = "0";
+        *expr = comp;
+    } else if (comp->astOperand1()->isLiteral()) {
+        if (comp->astOperand1()->isExpandedMacro())
+            return false;
         *op = invertOperatorForOperandSwap(comp->str());
         *value = comp->astOperand1()->str();
         *expr = comp->astOperand2();
     } else if (comp->astOperand2()->isLiteral()) {
+        if (comp->astOperand2()->isExpandedMacro())
+            return false;
         *op = comp->str();
         *value = comp->astOperand2()->str();
         *expr = comp->astOperand1();
     } else {
-        return false;
+        *op = "!=";
+        *value = "0";
+        *expr = comp;
     }
 
     // Only float and int values are currently handled
@@ -643,6 +653,21 @@ static bool parseComparison(const Token *comp, bool *not1, std::string *op, std:
     return true;
 }
 
+static std::string conditionString(bool not1, const Token *expr1, const std::string &op, const std::string &value1)
+{
+    if (expr1->astParent()->isComparisonOp())
+        return std::string(not1 ? "!(" : "") +
+               (expr1->isName() ? expr1->str() : std::string("EXPR")) +
+               " " +
+               op +
+               " " +
+               value1 +
+               (not1 ? ")" : "");
+
+    return std::string(not1 ? "!" : "") +
+           (expr1->isName() ? expr1->str() : std::string("EXPR"));
+}
+
 void CheckCondition::checkIncorrectLogicOperator()
 {
     const bool printStyle = _settings->isEnabled("style");
@@ -758,8 +783,8 @@ void CheckCondition::checkIncorrectLogicOperator()
                 secondTrue &= !(result1 && !result2);
             }
 
-            const std::string cond1str = std::string(not1?"!(":"") + (expr1->isName() ? expr1->str() : "EXPR") + " " + op1 + " " + value1 + std::string(not1?")":"");
-            const std::string cond2str = std::string(not2?"!(":"") + (expr2->isName() ? expr2->str() : "EXPR") + " " + op2 + " " + value2 + std::string(not2?")":"");
+            const std::string cond1str = conditionString(not1, expr1, op1, value1);
+            const std::string cond2str = conditionString(not2, expr2, op2, value2);
             if (printWarning && (alwaysTrue || alwaysFalse)) {
                 const std::string text = cond1str + " " + tok->str() + " " + cond2str;
                 incorrectLogicOperatorError(tok, text, alwaysTrue);
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index ed6e41388..9074bba01 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -574,6 +574,16 @@ private:
              );
         ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
 
+        check("void f(int x) {\n"
+              "  if (x<0 && !x) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 0 && !x.\n", errout.str());
+
+        check("void f(int x) {\n"
+              "  if (x==0 && x) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 0 && x.\n", errout.str());
+
         check("void f(int x) {\n" // ast..
               "    if (y == 1 && x == 1 && x == 7) { }\n"
               "}\n");