diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 6b6d8e997..7c234cf55 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -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) diff --git a/lib/checkother.h b/lib/checkother.h index 45aed569e..7764aeeaa 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -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); diff --git a/test/testother.cpp b/test/testother.cpp index 31f7f0f89..58f0b926e 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -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"