Improved CheckOther::checkIncorrectLogicOperator:

- Added a lot of additional pattern
- Rewrote error messages to make them more understandable and better fitting to the situation. (Fixed #3664)
- Cleanup in unit tests
Improved message of static string comparision check
This commit is contained in:
PKEuS 2012-03-15 20:38:28 +01:00
parent fc84f55f80
commit b6057a1148
3 changed files with 164 additions and 214 deletions

View File

@ -1024,22 +1024,21 @@ void CheckOther::checkIncorrectLogicOperator()
if (nextTok) {
// Find the common variable and the two different-valued constants
unsigned int variableTested = 0;
std::string firstConstant, secondConstant;
bool varFirst1, varFirst2;
unsigned int varId;
const Token *varTok = NULL;
const Token *var1Tok = NULL, *var2Tok = NULL;
if (Token::Match(term1Tok, "%var% %any% %num%")) {
varTok = term1Tok;
varId = varTok->varId();
var1Tok = term1Tok;
varId = var1Tok->varId();
if (!varId) {
continue;
}
varFirst1 = true;
firstConstant = term1Tok->strAt(2);
} else if (Token::Match(term1Tok, "%num% %any% %var%")) {
varTok = term1Tok->tokAt(2);
varId = varTok->varId();
var1Tok = term1Tok->tokAt(2);
varId = var1Tok->varId();
if (!varId) {
continue;
}
@ -1050,26 +1049,18 @@ void CheckOther::checkIncorrectLogicOperator()
}
if (Token::Match(term2Tok, "%var% %any% %num%")) {
const unsigned int varId2 = term2Tok->varId();
if (!varId2 || varId != varId2) {
continue;
}
var2Tok = term2Tok;
varFirst2 = true;
secondConstant = term2Tok->strAt(2);
variableTested = varId;
} else if (Token::Match(term2Tok, "%num% %any% %var%")) {
const unsigned int varId2 = term2Tok->tokAt(2)->varId();
if (!varId2 || varId != varId2) {
continue;
}
var2Tok = term2Tok->tokAt(2);
varFirst2 = false;
secondConstant = term2Tok->str();
variableTested = varId;
} else {
continue;
}
if (variableTested == 0 || firstConstant.empty() || secondConstant.empty()) {
if (varId != var2Tok->varId() || firstConstant.empty() || secondConstant.empty()) {
continue;
}
@ -1084,22 +1075,34 @@ void CheckOther::checkIncorrectLogicOperator()
Relation relation;
LogicError error;
} conditions[] = {
{ "!!&&", { NA, "!=" }, "%oror%", { NA, "!=" }, "!!&&", NotEqual, AlwaysTrue }, // (x != 1) || (x != 3) <- always true
{ 0, { NA, "==" }, "&&", { NA, "==" }, 0, NotEqual, AlwaysFalse }, // (x == 1) && (x == 3) <- always false
{ 0, { First, "<" }, "&&", { First, ">" }, 0, LessEqual, AlwaysFalse }, // (x < 1) && (x > 3) <- always false
{ 0, { First, "<=" }, "&&", { First, ">|>=" }, 0, LessEqual, AlwaysFalse }, // (x <= 1) && (x > 3) <- always false
{ 0, { First, "<" }, "&&", { First, ">=" }, 0, LessEqual, AlwaysFalse }, // (x < 1) && (x >= 3) <- always false
{ "!!&&", { First , ">" }, "%oror%", { First, "<" }, "!!&&", Less, AlwaysTrue }, // (x > 3) || (x < 10) <- always true
{ "!!&&", { First , ">=" }, "%oror%", { First, "<|<=" }, "!!&&", LessEqual, AlwaysTrue }, // (x >= 3) || (x < 10) <- always true
{ "!!&&", { First , ">" }, "%oror%", { First, "<=" }, "!!&&", LessEqual, AlwaysTrue }, // (x > 3) || (x <= 10) <- always true
{ 0, { First, ">" }, "&&", { NA, "!=" }, 0, MoreEqual, SecondTrue }, // (x > 5) && (x != 1) <- second expression always true
{ 0, { First, "<" }, "&&", { NA, "!=" }, 0, LessEqual, SecondTrue }, // (x < 1) && (x != 3) <- second expression always true
{ 0, { First, ">=" }, "&&", { NA, "!=" }, 0, More, SecondTrue }, // (x >= 5) && (x != 1) <- second expression always true
{ 0, { First, "<=" }, "&&", { NA, "!=" }, 0, Less, SecondTrue }, // (x <= 1) && (x != 3) <- second expression always true
{ 0, { First, ">" }, "&&", { NA, "==" }, 0, MoreEqual, SecondFalse }, // (x > 5) && (x == 1) <- second expression always false
{ 0, { First, "<" }, "&&", { NA, "==" }, 0, LessEqual, SecondFalse }, // (x < 1) && (x == 3) <- second expression always false
{ 0, { First, ">=" }, "&&", { NA, "==" }, 0, More, SecondFalse }, // (x >= 5) && (x == 1) <- second expression always false
{ 0, { First, "<=" }, "&&", { NA, "==" }, 0, Less, SecondFalse }, // (x <= 1) && (x == 3) <- second expression always false
{ "!!&&", { NA, "!=" }, "%oror%", { NA, "!=" }, "!!&&", NotEqual, AlwaysTrue }, // (x != 1) || (x != 3) <- always true
{ 0, { NA, "==" }, "&&", { NA, "==" }, 0, NotEqual, AlwaysFalse }, // (x == 1) && (x == 3) <- always false
{ "!!&&", { First, ">" }, "%oror%", { First, "<" }, "!!&&", Less, AlwaysTrue }, // (x > 3) || (x < 10) <- always true
{ "!!&&", { First, ">=" }, "%oror%", { First, "<|<=" }, "!!&&", LessEqual, AlwaysTrue }, // (x >= 3) || (x < 10) <- always true
{ "!!&&", { First, ">" }, "%oror%", { First, "<=" }, "!!&&", LessEqual, AlwaysTrue }, // (x > 3) || (x <= 10) <- always true
{ 0, { First, "<" }, "&&", { First, ">" }, 0, LessEqual, AlwaysFalse }, // (x < 1) && (x > 3) <- always false
{ 0, { First, "<=" }, "&&", { First, ">|>=" }, 0, Less, AlwaysFalse }, // (x <= 1) && (x > 3) <- always false
{ 0, { First, "<" }, "&&", { First, ">=" }, 0, Less, AlwaysFalse }, // (x < 1) && (x >= 3) <- always false
{ "!!&&", { First, ">" }, "%oror%", { NA, "==" }, "!!&&", LessEqual, AlwaysTrue }, // (x > 3) || (x == 4) <- always true
{ "!!&&", { First, "<" }, "%oror%", { NA, "==" }, "!!&&", MoreEqual, AlwaysTrue }, // (x < 5) || (x == 4) <- always true
{ "!!&&", { First, ">=" }, "%oror%", { NA, "==" }, "!!&&", Less, AlwaysTrue }, // (x >= 3) || (x == 4) <- always true
{ "!!&&", { First, "<=" }, "%oror%", { NA, "==" }, "!!&&", More, AlwaysTrue }, // (x <= 5) || (x == 4) <- always true
{ 0, { First, ">" }, "&&", { NA, "==" }, 0, MoreEqual, AlwaysFalse }, // (x > 5) && (x == 1) <- always false
{ 0, { First, "<" }, "&&", { NA, "==" }, 0, LessEqual, AlwaysFalse }, // (x < 1) && (x == 3) <- always false
{ 0, { First, ">=" }, "&&", { NA, "==" }, 0, More, AlwaysFalse }, // (x >= 5) && (x == 1) <- always false
{ 0, { First, "<=" }, "&&", { NA, "==" }, 0, Less, AlwaysFalse }, // (x <= 1) && (x == 3) <- always false
{ "!!&&", { First, ">" }, "%oror%", { NA, "!=" }, "!!&&", MoreEqual, SecondTrue }, // (x > 5) || (x != 1) <- second expression always true
{ "!!&&", { First, "<" }, "%oror%", { NA, "!=" }, "!!&&", LessEqual, SecondTrue }, // (x < 1) || (x != 3) <- second expression always true
{ "!!&&", { First, ">=" }, "%oror%", { NA, "!=" }, "!!&&", More, SecondTrue }, // (x >= 5) || (x != 1) <- second expression always true
{ "!!&&", { First, "<=" }, "%oror%", { NA, "!=" }, "!!&&", Less, SecondTrue }, // (x <= 1) || (x != 3) <- second expression always true
{ 0, { First, ">" }, "&&", { NA, "!=" }, 0, MoreEqual, SecondTrue }, // (x > 5) && (x != 1) <- second expression always true
{ 0, { First, "<" }, "&&", { NA, "!=" }, 0, LessEqual, SecondTrue }, // (x < 1) && (x != 3) <- second expression always true
{ 0, { First, ">=" }, "&&", { NA, "!=" }, 0, More, SecondTrue }, // (x >= 5) && (x != 1) <- second expression always true
{ 0, { First, "<=" }, "&&", { NA, "!=" }, 0, Less, SecondTrue }, // (x <= 1) && (x != 3) <- second expression always true
{ "!!&&", { First, ">|>=" }, "%oror%", { First, ">|>=" }, "!!&&", LessEqual, SecondTrue }, // (x > 4) || (x > 5) <- second expression always true
{ "!!&&", { First, "<|<=" }, "%oror%", { First, "<|<=" }, "!!&&", MoreEqual, SecondTrue }, // (x < 5) || (x < 4) <- second expression always true
{ 0, { First, ">|>=" }, "&&", { First, ">|>=" }, 0, LessEqual, SecondTrue }, // (x > 4) && (x > 5) <- second expression always true
{ 0, { First, "<|<=" }, "&&", { First, "<|<=" }, 0, MoreEqual, SecondTrue }, // (x < 5) && (x < 4) <- second expression always true
};
for (unsigned int i = 0; i < (sizeof(conditions) / sizeof(conditions[0])); i++) {
@ -1112,6 +1115,8 @@ void CheckOther::checkIncorrectLogicOperator()
if (conditions[i].after != 0 && !Token::Match(nextTok, conditions[i].after))
continue;
std::string cond1str = var1Tok->str() + " " + (varFirst1?op1Tok->str():invertOperatorForOperandSwap(op1Tok->str())) + " " + firstConstant;
std::string cond2str = var2Tok->str() + " " + (varFirst2?op3Tok->str():invertOperatorForOperandSwap(op3Tok->str())) + " " + secondConstant;
// cond1 op cond2
bool error = analyzeLogicOperatorCondition(conditions[i].c1, conditions[i].c2, false, false,
varFirst1, varFirst2, firstConstant, secondConstant,
@ -1135,6 +1140,8 @@ void CheckOther::checkIncorrectLogicOperator()
!varFirst1, !varFirst2, firstConstant, secondConstant,
op1Tok, op3Tok,
conditions[i].relation);
if (!error)
std::swap(cond1str, cond2str);
// cond2 op cond1 // swap conditions
if (!error)
error = analyzeLogicOperatorCondition(conditions[i].c1, conditions[i].c2, false, false,
@ -1161,13 +1168,13 @@ void CheckOther::checkIncorrectLogicOperator()
conditions[i].relation);
if (error) {
if (conditions[i].error == AlwaysFalse || conditions[i].error == AlwaysTrue)
incorrectLogicOperatorError(term1Tok, conditions[i].error == AlwaysTrue);
else {
std::string text("When " + varTok->str() + " is greater than " + firstConstant + ", the comparison " +
varTok->str() + " " + conditions[i].c2.opTokStr + " " + secondConstant +
" is always " + (conditions[i].error == SecondTrue ? "true." : "false."));
secondAlwaysTrueFalseWhenFirstTrueError(term1Tok, text);
if (conditions[i].error == AlwaysFalse || conditions[i].error == AlwaysTrue) {
const std::string text = cond1str + " " + op2Tok->str() + " " + cond2str;
incorrectLogicOperatorError(term1Tok, text, conditions[i].error == AlwaysTrue);
} else {
const std::string text = "If " + cond1str + ", the comparison " + cond2str +
" is always " + ((conditions[i].error == SecondTrue || conditions[i].error == AlwaysTrue) ? "true" : "false") + ".\n";
redundantConditionError(term1Tok, text);
}
break;
}
@ -1176,19 +1183,21 @@ void CheckOther::checkIncorrectLogicOperator()
}
}
void CheckOther::incorrectLogicOperatorError(const Token *tok, bool always)
void CheckOther::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always)
{
if (always)
reportError(tok, Severity::warning,
"incorrectLogicOperator", "Mutual exclusion over || always evaluates to true. Did you intend to use && instead?");
reportError(tok, Severity::warning, "incorrectLogicOperator",
"Logical disjunction always evaluates to true: " + condition + ".\n"
"Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?");
else
reportError(tok, Severity::warning,
"incorrectLogicOperator", "Expression always evaluates to false. Did you intend to use || instead?");
reportError(tok, Severity::warning, "incorrectLogicOperator",
"Logical conjunction always evaluates to false: " + condition + ".\n"
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?");
}
void CheckOther::secondAlwaysTrueFalseWhenFirstTrueError(const Token *tok, const std::string &truefalse)
void CheckOther::redundantConditionError(const Token *tok, const std::string &text)
{
reportError(tok, Severity::style, "secondAlwaysTrueFalseWhenFirstTrue", truefalse);
reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text);
}
//---------------------------------------------------------------------------
@ -3015,7 +3024,7 @@ void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2,
//---------------------------------------------------------------------------
void CheckOther::checkAlwaysTrueOrFalseStringCompare()
{
if (!_settings->isEnabled("style") && !_settings->isEnabled("performance"))
if (!_settings->isEnabled("style"))
return;
const char pattern1[] = "strncmp|strcmp|stricmp|strcmpi|strcasecmp|wcscmp ( %str% , %str% ";
@ -3026,7 +3035,7 @@ void CheckOther::checkAlwaysTrueOrFalseStringCompare()
while (tok && (tok = Token::findmatch(tok, pattern1)) != NULL) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(4);
alwaysTrueFalseStringCompareError(tok, str1, str2, str1==str2);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(5);
}
@ -3034,7 +3043,7 @@ void CheckOther::checkAlwaysTrueOrFalseStringCompare()
while (tok && (tok = Token::findmatch(tok, pattern2)) != NULL) {
const std::string &str1 = tok->strAt(4);
const std::string &str2 = tok->strAt(6);
alwaysTrueFalseStringCompareError(tok, str1, str2, str1==str2);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(7);
}
@ -3051,29 +3060,21 @@ void CheckOther::checkAlwaysTrueOrFalseStringCompare()
while (tok && (tok = Token::findmatch(tok, "!!+ %str% ==|!= %str% !!+")) != NULL) {
const std::string &str1 = tok->strAt(1);
const std::string &str2 = tok->strAt(3);
alwaysTrueFalseStringCompareError(tok, str1, str2, str1==str2);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(5);
}
}
void CheckOther::alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2, bool warning)
void CheckOther::alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2)
{
const std::size_t stringLen = 10;
const std::string string1 = (str1.size() < stringLen) ? str1 : (str1.substr(0, stringLen-2) + "..");
const std::string string2 = (str2.size() < stringLen) ? str2 : (str2.substr(0, stringLen-2) + "..");
if (warning) {
reportError(tok, Severity::warning, "staticStringCompare",
"Comparison of always identical static strings.\n"
"The compared strings, '" + string1 + "' and '" + string2 + "', are always identical. "
"If the purpose is to compare these two strings, the comparison is unnecessary. "
"If the strings are supposed to be different, then there is a bug somewhere.");
} else if (_settings->isEnabled("performance")) {
reportError(tok, Severity::performance, "staticStringCompare",
"Unnecessary comparison of static strings.\n"
"The compared strings, '" + string1 + "' and '" + string2 + "', are static and always different. "
"If the purpose is to compare these two strings, the comparison is unnecessary.");
}
reportError(tok, Severity::warning, "staticStringCompare",
"Unnecessary comparision of static strings.\n"
"The compared strings, '" + string1 + "' and '" + string2 + "', are always " + (str1==str2?"identical":"unequal") + ". "
"Therefore the comparision is unnecessary and looks suspicious.");
}
void CheckOther::alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2)

View File

@ -288,8 +288,8 @@ private:
void switchCaseFallThrough(const Token *tok);
void selfAssignmentError(const Token *tok, const std::string &varname);
void assignmentInAssertError(const Token *tok, const std::string &varname);
void incorrectLogicOperatorError(const Token *tok, bool always);
void secondAlwaysTrueFalseWhenFirstTrueError(const Token *tok, const std::string &truefalse);
void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always);
void redundantConditionError(const Token *tok, const std::string &text);
void misusedScopeObjectError(const Token *tok, const std::string &varname);
void memsetZeroBytesError(const Token *tok, const std::string &varname);
void sizeofForArrayParameterError(const Token *tok);
@ -302,7 +302,7 @@ private:
void duplicateIfError(const Token *tok1, const Token *tok2);
void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2, bool warning);
void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void duplicateBreakError(const Token *tok);
void unreachableCodeError(const Token* tok);
@ -349,8 +349,8 @@ private:
c.selfAssignmentError(0, "varname");
c.assignmentInAssertError(0, "varname");
c.invalidScanfError(0);
c.incorrectLogicOperatorError(0, true);
c.secondAlwaysTrueFalseWhenFirstTrueError(0, "when first comparison is true, the 2nd comparison is always true");
c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true);
c.redundantConditionError(0, "when first comparison is true, the 2nd comparison is always true");
c.memsetZeroBytesError(0, "varname");
c.clarifyCalculationError(0, "+");
c.clarifyConditionError(0, true, false);
@ -361,7 +361,7 @@ private:
c.duplicateIfError(0, 0);
c.duplicateBranchError(0, 0);
c.duplicateExpressionError(0, 0, "&&");
c.alwaysTrueFalseStringCompareError(0, "str1", "str2", true);
c.alwaysTrueFalseStringCompareError(0, "str1", "str2");
c.alwaysTrueStringVariableCompareError(0, "varname1", "varname2");
c.duplicateBreakError(0);
c.unreachableCodeError(0);

View File

@ -113,6 +113,7 @@ private:
TEST_CASE(incorrectLogicOperator1);
TEST_CASE(incorrectLogicOperator2);
TEST_CASE(incorrectLogicOperator3);
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
TEST_CASE(incorrectLogicOp_condSwapping);
@ -2615,21 +2616,14 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
check("void f(int x) {\n"
" if (x != 1 || x != 3)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
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 (1 != x || 3 != x)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
check("void f(int x, int y) {\n"
" if (x != 1 || y != 1)\n"
@ -2695,7 +2689,7 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:4]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (warning) Logical disjunction always evaluates to true: x != 5 || x != 6.\n", errout.str());
check("void f(int x, int y) {\n"
" const int ERR1 = 5;\n"
@ -2717,20 +2711,6 @@ private:
}
void incorrectLogicOperator2() {
check("void f(int x) {\n"
" if ((x == 1) && (x == 3))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
check("void f(int x) {\n"
" if ((x == 1.0) && (x == 3.0))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
check("void f(float x) {\n"
" if ((x == 1) && (x == 1.0))\n"
" a++;\n"
@ -2750,14 +2730,14 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 3.\n", errout.str());
check("void f(int x) {\n"
" if (x == 1.0 && x == 3.0)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1.0 && x == 3.0.\n", errout.str());
check("void f(float x) {\n"
" if (x == 1 && x == 1.0)\n"
@ -2771,49 +2751,42 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 1.\n", errout.str());
check("void f(int x) {\n"
" if (x < 1.0 && x > 1.0)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1.0 && x > 1.0.\n", errout.str());
check("void f(int x) {\n"
" if (x < 1 && x > 1.0)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 1.0.\n", errout.str());
check("void f(int x) {\n"
" if (x < 1 && x > 3)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(float x) {\n"
" if (x < 1.0 && x > 3.0)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1.0 && x > 3.0.\n", errout.str());
check("void f(int x) {\n"
" if (1 > x && 3 < x)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
check("void f(int x) {\n"
" if (x < 1 && x > 1)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (x < 3 && x > 1)\n"
@ -2827,28 +2800,28 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x < 10.\n", errout.str());
check("void f(int x) {\n"
" if (x >= 3 || x <= 10)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x <= 10.\n", errout.str());
check("void f(int x) {\n"
" if (x >= 3 || x < 10)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x < 10.\n", errout.str());
check("void f(int x) {\n"
" if (x > 3 || x <= 10)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x <= 10.\n", errout.str());
check("void f(int x) {\n"
" if (x > 3 || x < 3)\n"
@ -2862,21 +2835,21 @@ private:
" a++;\n"
"}"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x <= 3.\n", errout.str());
check("void f(int x) {\n"
" if (x >= 3 || x < 3)\n"
" a++;\n"
"}"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x < 3.\n", errout.str());
check("void f(int x) {\n"
" if (x > 3 || x <= 3)\n"
" a++;\n"
"}"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Mutual exclusion over || always evaluates to true. Did you intend to use && instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x <= 3.\n", errout.str());
check("void f(int x) {\n"
" if (x > 10 || x < 3)\n"
@ -2885,6 +2858,20 @@ private:
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (x > 5 && x == 1)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 5 && x == 1.\n", errout.str());
check("void f(int x) {\n"
" if (x > 5 && x == 6)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
// #3419
check("void f() {\n"
" if ( &q != &a && &q != &b ) { }\n"
@ -2892,13 +2879,37 @@ private:
ASSERT_EQUALS("", errout.str());
}
void incorrectLogicOperator3() {
check("void f(int x, bool& b) {\n"
" b = x > 3 || x == 4;\n"
" b = x < 5 || x == 4;\n"
" b = x >= 3 || x == 4;\n"
" b = x <= 5 || x == 4;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x == 4.\n"
"[test.cpp:3]: (warning) Logical disjunction always evaluates to true: x < 5 || x == 4.\n"
"[test.cpp:4]: (warning) Logical disjunction always evaluates to true: x >= 3 || x == 4.\n"
"[test.cpp:5]: (warning) Logical disjunction always evaluates to true: x <= 5 || x == 4.\n", errout.str());
check("void f(int x, bool& b) {\n"
" b = x > 5 && x == 1;\n"
" b = x < 1 && x == 3;\n"
" b = x >= 5 && x == 1;\n"
" b = x <= 1 && x == 3;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 5 && x == 1.\n"
"[test.cpp:3]: (warning) Logical conjunction always evaluates to false: x < 1 && x == 3.\n"
"[test.cpp:4]: (warning) Logical conjunction always evaluates to false: x >= 5 && x == 1.\n"
"[test.cpp:5]: (warning) Logical conjunction always evaluates to false: x <= 1 && x == 3.\n", errout.str());
}
void secondAlwaysTrueFalseWhenFirstTrueError() {
check("void f(int x) {\n"
" if (x > 5 && x != 1)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x != 1 is always true.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n", errout.str());
check("void f(int x) {\n"
" if (x > 5 && x != 6)\n"
@ -2912,7 +2923,7 @@ private:
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x != 1 is always true.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n", errout.str());
check("void f(int x) {\n"
" if ((x > 5) && (x != 6))\n"
@ -2921,89 +2932,27 @@ private:
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (5 < x && x != 1)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x != 1 is always true.\n", errout.str());
check("void f(int x, bool& b) {\n"
" b = x > 5 || x != 1;\n"
" b = x < 1 || x != 3;\n"
" b = x >= 5 || x != 1;\n"
" b = x <= 1 || x != 3;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n"
"[test.cpp:3]: (style) Redundant condition: If x < 1, the comparison x != 3 is always true.\n"
"[test.cpp:4]: (style) Redundant condition: If x >= 5, the comparison x != 1 is always true.\n"
"[test.cpp:5]: (style) Redundant condition: If x <= 1, the comparison x != 3 is always true.\n", errout.str());
check("void f(int x) {\n"
" if (5 < x && x != 6)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if ((5 < x) && (x != 1))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x != 1 is always true.\n", errout.str());
check("void f(int x) {\n"
" if ((5 < x) && (x != 6))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (x > 5 && x == 1)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x == 1 is always false.\n", errout.str());
check("void f(int x) {\n"
" if (x > 5 && x == 6)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if ((x > 5) && (x == 1))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x == 1 is always false.\n", errout.str());
check("void f(int x) {\n"
" if ((x > 5) && (x == 6))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if (5 < x && x == 1)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x == 1 is always false.\n", errout.str());
check("void f(int x) {\n"
" if (5 < x && x == 6)\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x) {\n"
" if ((5 < x) && (x == 1))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:2]: (style) When x is greater than 5, the comparison x == 1 is always false.\n", errout.str());
check("void f(int x) {\n"
" if ((5 < x) && (x == 6))\n"
" a++;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
check("void f(int x, bool& b) {\n"
" b = x > 6 && x > 5;\n"
" b = x > 5 || x > 6;\n"
" b = x < 6 && x < 5;\n"
" b = x < 5 || x < 6;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x > 6 is always true.\n"
"[test.cpp:3]: (style) Redundant condition: If x > 5, the comparison x > 6 is always true.\n"
"[test.cpp:4]: (style) Redundant condition: If x < 6, the comparison x < 5 is always true.\n"
"[test.cpp:5]: (style) Redundant condition: If x < 6, the comparison x < 5 is always true.\n", errout.str());
}
void incorrectLogicOp_condSwapping() {
@ -3011,49 +2960,49 @@ private:
" if (x < 1 && x > 3)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (1 > x && x > 3)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (x < 1 && 3 < x)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (1 > x && 3 < x)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (x > 3 && x < 1)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (3 < x && x < 1)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (x > 3 && 1 > x)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
check("void f(int x) {\n"
" if (3 < x && 1 > x)\n"
" a++;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Expression always evaluates to false. Did you intend to use || instead?\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
}
void comparisonOfBoolExpressionWithInt() {
@ -4138,7 +4087,7 @@ private:
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Comparison of always identical static strings.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (warning) Unnecessary comparision of static strings.\n", errout.str());
check_preprocess_suppress(
"int main()\n"
@ -4148,7 +4097,7 @@ private:
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (performance) Unnecessary comparison of static strings.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (warning) Unnecessary comparision of static strings.\n", errout.str());
check_preprocess_suppress(
"#define MACRO \"Hotdog\"\n"
@ -4159,7 +4108,7 @@ private:
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:4]: (performance) Unnecessary comparison of static strings.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (warning) Unnecessary comparision of static strings.\n", errout.str());
check_preprocess_suppress(
"int main()\n"
@ -4179,7 +4128,7 @@ private:
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of always identical static strings.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (warning) Unnecessary comparision of static strings.\n", errout.str());
check(
"int foo(const char *buf)\n"
@ -4197,7 +4146,7 @@ private:
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of always identical static strings.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Unnecessary comparision of static strings.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
@ -4205,7 +4154,7 @@ private:
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of always identical static strings.\n", errout.str());
ASSERT_EQUALS("[test.cpp:2]: (warning) Unnecessary comparision of static strings.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"