Support char literals in CheckCondition::checkIncorrectLogicOperator() (#5912)

lib/checkcondition.cpp

@@ -603,7 +603,7 @@ static inline T getvalue(const int test, const T value1, const T value2)
     return 0;
 }
 
-static bool parseComparison(const Token *comp, bool *not1, std::string *op, std::string *value, const Token **expr)
+static bool parseComparison(const Token *comp, bool *not1, std::string *op, std::string *value, const Token **expr, bool* inconclusive)
 {
     *not1 = false;
     while (comp && comp->str() == "!") {
@@ -636,8 +636,10 @@ static bool parseComparison(const Token *comp, bool *not1, std::string *op, std:
         *expr = comp;
     }
 
+    *inconclusive = *inconclusive || ((*value)[0] == '\'' && !(*op == "!=" || *op == "=="));
+
     // Only float and int values are currently handled
-    if (!MathLib::isInt(*value) && !MathLib::isFloat(*value))
+    if (!MathLib::isInt(*value) && !MathLib::isFloat(*value) && (*value)[0] != '\'')
         return false;
 
     return true;
@@ -679,7 +681,7 @@ void CheckCondition::checkIncorrectLogicOperator()
                 isOppositeCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
 
                 const bool alwaysTrue(tok->str() == "||");
-                incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue);
+                incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue, false);
                 continue;
             }
 
@@ -710,7 +712,7 @@ void CheckCondition::checkIncorrectLogicOperator()
                     const std::string cond1 = expr1 + " " + tok->str() + " (" + expr2 + " " + tok->astOperand2()->str() + " " + expr3 + ")";
                     const std::string cond2 = expr1 + " " + tok->str() + " " + expr3;
 
-                    redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'");
+                    redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'", false);
                     continue;
                 }
             }
@@ -723,18 +725,23 @@ void CheckCondition::checkIncorrectLogicOperator()
             // Comparison #2 (RHS)
             const Token *comp2 = tok->astOperand2();
 
+            bool inconclusive = false;
+
             // Parse LHS
             bool not1;
             std::string op1, value1;
             const Token *expr1;
-            if (!parseComparison(comp1, &not1, &op1, &value1, &expr1))
+            if (!parseComparison(comp1, &not1, &op1, &value1, &expr1, &inconclusive))
                 continue;
 
             // Parse RHS
             bool not2;
             std::string op2, value2;
             const Token *expr2;
-            if (!parseComparison(comp2, &not2, &op2, &value2, &expr2))
+            if (!parseComparison(comp2, &not2, &op2, &value2, &expr2, &inconclusive))
+                continue;
+
+            if (inconclusive && !_settings->inconclusive)
                 continue;
 
             if (isSameExpression(_tokenizer->isCPP(), true, comp1, comp2, _settings->library.functionpure))
@@ -799,40 +806,40 @@ void CheckCondition::checkIncorrectLogicOperator()
             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);
+                incorrectLogicOperatorError(tok, text, alwaysTrue, inconclusive);
             } else if (printStyle && secondTrue) {
                 const std::string text = "If '" + cond1str + "', the comparison '" + cond2str +
                                          "' is always " + (secondTrue ? "true" : "false") + ".";
-                redundantConditionError(tok, text);
+                redundantConditionError(tok, text, inconclusive);
             } else if (printStyle && firstTrue) {
                 //const std::string text = "The comparison " + cond1str + " is always " +
                 //                         (firstTrue ? "true" : "false") + " when " +
                 //                         cond2str + ".";
                 const std::string text = "If '" + cond2str + "', the comparison '" + cond1str +
                                          "' is always " + (firstTrue ? "true" : "false") + ".";
-                redundantConditionError(tok, text);
+                redundantConditionError(tok, text, inconclusive);
             }
         }
     }
 }
 
-void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always)
+void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive)
 {
     if (always)
         reportError(tok, Severity::warning, "incorrectLogicOperator",
                     "Logical disjunction always evaluates to true: " + condition + ".\n"
                     "Logical disjunction always evaluates to true: " + condition + ". "
-                    "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, false);
+                    "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, inconclusive);
     else
         reportError(tok, Severity::warning, "incorrectLogicOperator",
                     "Logical conjunction always evaluates to false: " + condition + ".\n"
                     "Logical conjunction always evaluates to false: " + condition + ". "
-                    "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, false);
+                    "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, inconclusive);
 }
 
-void CheckCondition::redundantConditionError(const Token *tok, const std::string &text)
+void CheckCondition::redundantConditionError(const Token *tok, const std::string &text, bool inconclusive)
 {
-    reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text, CWE398, false);
+    reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text, CWE398, inconclusive);
 }
 
 //-----------------------------------------------------------------------------

lib/checkcondition.h

@@ -117,8 +117,8 @@ private:
 
     void oppositeInnerConditionError(const Token *tok1, const Token* tok2);
 
-    void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always);
+    void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive);
-    void redundantConditionError(const Token *tok, const std::string &text);
+    void redundantConditionError(const Token *tok, const std::string &text, bool inconclusive);
 
     void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
 
@@ -137,8 +137,8 @@ private:
         c.multiConditionError(nullptr,1);
         c.mismatchingBitAndError(nullptr, 0xf0, 0, 1);
         c.oppositeInnerConditionError(nullptr, 0);
-        c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true);
+        c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false);
-        c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.");
+        c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false);
         c.moduloAlwaysTrueFalseError(nullptr, "1");
         c.clarifyConditionError(nullptr, true, false);
         c.alwaysTrueFalseError(nullptr, true);

test/testcondition.cpp

@@ -84,10 +84,12 @@ private:
         TEST_CASE(checkInvalidTestForOverflow);
     }
 
-    void check(const char code[], const char* filename = "test.cpp") {
+    void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) {
         // Clear the error buffer..
         errout.str("");
 
+        settings0.inconclusive = inconclusive;
+
         CheckCondition checkCondition;
 
         // Tokenize..
@@ -946,13 +948,28 @@ private:
     void incorrectLogicOperator6() { // char literals
         check("void f(char x) {\n"
               "  if (x == '1' || x == '2') {}\n"
-              "}");
+              "}", "test.cpp", true);
         ASSERT_EQUALS("", errout.str());
 
         check("void f(char x) {\n"
               "  if (x == '1' && x == '2') {}\n"
-              "}");
+              "}", "test.cpp", true);
-        TODO_ASSERT_EQUALS("error", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == '1' && x == '2'.\n", errout.str());
+
+        check("int f(char c) {\n"
+              "  return (c >= 'a' && c <= 'z');\n"
+              "}", "test.cpp", true);
+        ASSERT_EQUALS("", errout.str());
+
+        check("int f(char c) {\n"
+              "  return (c <= 'a' && c >= 'z');\n"
+              "}", "test.cpp", true);
+        ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Logical conjunction always evaluates to false: c <= 'a' && c >= 'z'.\n", errout.str());
+
+        check("int f(char c) {\n"
+              "  return (c <= 'a' && c >= 'z');\n"
+              "}", "test.cpp", false);
+        ASSERT_EQUALS("", errout.str());
     }
 
     void incorrectLogicOperator7() { // opposite expressions