diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 49ca594f4..16a7f7100 100644 --- a/lib/checkcondition.cpp +++ b/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 && - tok2->astOperand2() && + if (tok2->astOperand2() && !cond1->hasKnownIntValue() && - !tok2->astOperand2()->hasKnownIntValue() && - isOverlappingCond(cond1, tok2->astOperand2(), true)) - multiConditionError(tok2, cond1->linenr()); + !tok2->astOperand2()->hasKnownIntValue()) { + ErrorPath errorPath; + 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 diff --git a/lib/checkcondition.h b/lib/checkcondition.h index a8d14971f..354d4eff5 100644 --- a/lib/checkcondition.h +++ b/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); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 9ac8dfa42..c942a7327 100644 --- a/test/testcondition.cpp +++ b/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"