Fixed #7464 (warn about opposite if and else-if conditions)

lib/checkcondition.cpp

@@ -490,8 +490,12 @@ void CheckCondition::multiCondition()
             continue;
 
         const Token * const cond1 = scope.classDef->next()->astOperand2();
+        if (!cond1)
+            continue;
 
         const Token * tok2 = scope.classDef->next();
+
+        // Check each 'else if'
         for (;;) {
             tok2 = tok2->link();
             if (!Token::simpleMatch(tok2, ") {"))
@@ -501,17 +505,20 @@ void CheckCondition::multiCondition()
                 break;
             tok2 = tok2->tokAt(4);
 
-            if (cond1 &&
+            if (tok2->astOperand2() &&
-                tok2->astOperand2() &&
                 !cond1->hasKnownIntValue() &&
-                !tok2->astOperand2()->hasKnownIntValue() &&
+                !tok2->astOperand2()->hasKnownIntValue()) {
-                isOverlappingCond(cond1, tok2->astOperand2(), true))
+                ErrorPath errorPath;
-                multiConditionError(tok2, cond1->linenr());
+                if (isOverlappingCond(cond1, tok2->astOperand2(), true))
+                    overlappingElseIfConditionError(tok2, cond1->linenr());
+                else if (isOppositeCond(true, mTokenizer->isCPP(), cond1, tok2->astOperand2(), mSettings->library, true, true, &errorPath))
+                    oppositeElseIfConditionError(cond1, tok2, errorPath);
+            }
         }
     }
 }
 
-void CheckCondition::multiConditionError(const Token *tok, unsigned int line1)
+void CheckCondition::overlappingElseIfConditionError(const Token *tok, unsigned int line1)
 {
     std::ostringstream errmsg;
     errmsg << "Expression is always false because 'else if' condition matches previous condition at line "
@@ -520,6 +527,18 @@ void CheckCondition::multiConditionError(const Token *tok, unsigned int line1)
     reportError(tok, Severity::style, "multiCondition", errmsg.str(), CWE398, false);
 }
 
+void CheckCondition::oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath)
+{
+    std::ostringstream errmsg;
+    errmsg << "Expression is always true because 'else if' condition is opposite to previous condition at line "
+           << ifCond->linenr() << ".";
+
+    errorPath.emplace_back(ifCond, "first condition");
+    errorPath.emplace_back(elseIfCond, "else if condition is opposite to first condition");
+
+    reportError(errorPath, Severity::style, "multiCondition", errmsg.str(), CWE398, false);
+}
+
 //---------------------------------------------------------------------------
 // - Opposite inner conditions => always false
 // - (TODO) Same/Overlapping inner condition => always true

lib/checkcondition.h

@@ -134,7 +134,8 @@ private:
                          MathLib::bigint value2,
                          bool result);
     void duplicateConditionError(const Token *tok1, const Token *tok2, ErrorPath errorPath);
-    void multiConditionError(const Token *tok, unsigned int line1);
+    void overlappingElseIfConditionError(const Token *tok, unsigned int line1);
+    void oppositeElseIfConditionError(const Token *ifCond, const Token *elseIfCond, ErrorPath errorPath);
 
     void oppositeInnerConditionError(const Token *tok1, const Token* tok2, ErrorPath errorPath);
 
@@ -165,7 +166,7 @@ private:
         c.badBitmaskCheckError(nullptr);
         c.comparisonError(nullptr, "&", 6, "==", 1, false);
         c.duplicateConditionError(nullptr, nullptr, errorPath);
-        c.multiConditionError(nullptr,1);
+        c.overlappingElseIfConditionError(nullptr, 1);
         c.mismatchingBitAndError(nullptr, 0xf0, nullptr, 1);
         c.oppositeInnerConditionError(nullptr, nullptr, errorPath);
         c.identicalInnerConditionError(nullptr, nullptr, errorPath);

test/testcondition.cpp

@@ -57,7 +57,8 @@ private:
         TEST_CASE(mismatchingBitAnd);  // overlapping bitmasks
         TEST_CASE(comparison);         // CheckCondition::comparison test cases
         TEST_CASE(multicompare);       // mismatching comparisons
-        TEST_CASE(duplicateIf);        // duplicate conditions in if and else-if
+        TEST_CASE(overlappingElseIfCondition);  // overlapping conditions in if and else-if
+        TEST_CASE(oppositeElseIfCondition); // opposite conditions in if and else-if
 
         TEST_CASE(checkBadBitmaskCheck);
 
@@ -518,7 +519,7 @@ private:
         checkCondition.runChecks(&tokenizer, &settings1, this);
     }
 
-    void duplicateIf() {
+    void overlappingElseIfCondition() {
         check("void f(int a, int &b) {\n"
               "    if (a) { b = 1; }\n"
               "    else { if (a) { b = 2; } }\n"
@@ -692,6 +693,28 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void oppositeElseIfCondition() {
+        setMultiline();
+
+        check("void f(int x) {\n"
+              "    if (x) {}\n"
+              "    else if (!x) {}\n"
+              "}");
+        ASSERT_EQUALS("test.cpp:3:style:Expression is always true because 'else if' condition is opposite to previous condition at line 2.\n"
+                      "test.cpp:2:note:first condition\n"
+                      "test.cpp:3:note:else if condition is opposite to first condition\n", errout.str());
+
+        check("void f(int x) {\n"
+              "    int y = x;\n"
+              "    if (x) {}\n"
+              "    else if (!y) {}\n"
+              "}");
+        ASSERT_EQUALS("test.cpp:4:style:Expression is always true because 'else if' condition is opposite to previous condition at line 3.\n"
+                      "test.cpp:2:note:'y' is assigned value 'x' here.\n"
+                      "test.cpp:3:note:first condition\n"
+                      "test.cpp:4:note:else if condition is opposite to first condition\n", errout.str());
+    }
+
     void checkBadBitmaskCheck() {
         check("bool f(int x) {\n"
               "    bool b = x | 0x02;\n"