Skip to content

Commit eda5272

Browse files
danmardanmar
committed
Fixed cppcheck-opensource#6875 (Improve 'Redundant condition' error message)
1 parent 4bebb80 commit eda5272

2 files changed

Lines changed: 31 additions & 9 deletions

File tree

‎lib/checkcondition.cpp‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -642,11 +642,33 @@ void CheckCondition::checkIncorrectLogicOperator()
642642
}
643643

644644

645-
// 'A && (!A || B)' is equivalent with 'A || B'
646-
if (printStyle && (tok->str() == "||") && tok->astOperand1() && tok->astOperand2() && tok->astOperand2()->str() == "&&") {
645+
// 'A && (!A || B)' is equivalent with 'A && B'
646+
// 'A || (!A && B)' is equivalent with 'A || B'
647+
if (printStyle && tok->astOperand1() && tok->astOperand2() &&
648+
((tok->str() == "||" && tok->astOperand2()->str() == "&&") ||
649+
(tok->str() == "&&" && tok->astOperand2()->str() == "||"))) {
647650
const Token* tok2 = tok->astOperand2()->astOperand1();
648651
if (isOppositeCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok2, _settings->library.functionpure)) {
649-
redundantConditionError(tok, tok2->expressionString() + ". 'A && (!A || B)' is equivalent to 'A || B'");
652+
std::string expr1(tok->astOperand1()->expressionString());
653+
std::string expr2(tok->astOperand2()->astOperand1()->expressionString());
654+
std::string expr3(tok->astOperand2()->astOperand2()->expressionString());
655+
656+
if (expr1.length() + expr2.length() + expr3.length() > 50U) {
657+
if (expr1[0] == '!' && expr2[0] != '!') {
658+
expr1 = "!A";
659+
expr2 = "A";
660+
} else {
661+
expr1 = "!A";
662+
expr2 = "A";
663+
}
664+
665+
expr3 = "B";
666+
}
667+
668+
const std::string cond1 = expr1 + " " + tok->str() + " (" + expr2 + " " + tok->astOperand2()->str() + " " + expr3 + ")";
669+
const std::string cond2 = expr1 + " " + tok->str() + " " + expr3;
670+
671+
redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'");
650672
continue;
651673
}
652674
}

‎test/testcondition.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,25 +1364,25 @@ class TestCondition : public TestFixture {
13641364
" int y = rand(), z = rand();\n"
13651365
" if (y || (!y && z));\n"
13661366
"}");
1367-
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: !y. 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
1367+
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: !y. 'y || (!y && z)' is equivalent to 'y || z'\n", errout.str());
13681368

13691369
check("void f() {\n"
13701370
" int y = rand(), z = rand();\n"
13711371
" if (y || !y && z);\n"
13721372
"}");
1373-
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: !y. 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
1373+
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: !y. 'y || (!y && z)' is equivalent to 'y || z'\n", errout.str());
13741374

13751375
check("void f() {\n"
13761376
" if (!a || a && b) {}\n"
13771377
"}");
1378-
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: a. 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
1378+
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: a. '!a || (a && b)' is equivalent to '!a || b'\n", errout.str());
13791379

13801380

13811381
check("void f() {\n"
13821382
" if (!tok->next()->function() || \n"
13831383
" (tok->next()->function() && tok->next()->function()->isConstructor()));\n"
13841384
"}");
1385-
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: tok.next().function(). 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
1385+
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: tok.next().function(). '!A || (A && B)' is equivalent to '!A || B'\n", errout.str());
13861386

13871387
check("void f() {\n"
13881388
" if (!tok->next()->function() || \n"
@@ -1400,7 +1400,7 @@ class TestCondition : public TestFixture {
14001400
" if (!tok->next(1)->function(1) || \n"
14011401
" (tok->next(1)->function(1) && tok->next(1)->function(1)->isConstructor()));\n"
14021402
"}");
1403-
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: tok.next(1).function(1). 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
1403+
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: tok.next(1).function(1). '!A || (A && B)' is equivalent to '!A || B'\n", errout.str());
14041404

14051405
check("void f() {\n"
14061406
" if (!tok->next()->function(1) || \n"
@@ -1412,7 +1412,7 @@ class TestCondition : public TestFixture {
14121412
" int y = rand(), z = rand();\n"
14131413
" if (y==0 || y!=0 && z);\n"
14141414
"}");
1415-
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: y!=0. 'A && (!A || B)' is equivalent to 'A || B'\n", errout.str());
1415+
ASSERT_EQUALS("[test.cpp:3]: (style) Redundant condition: y!=0. 'y==0 || (y!=0 && z)' is equivalent to 'y==0 || z'\n", errout.str());
14161416

14171417
check("void f() {\n"
14181418
" if (x>0 || (x<0 && y)) {}\n"

0 commit comments

Comments
 (0)