walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Improve message for same expressions (#1349) 

* Improve message for same expressions

* Update message
This commit is contained in:
Paul Fultz II 2018-08-25 07:25:31 -05:00 committed by Daniel Marjamäki
parent 18343d1122
commit a5f4c5d0eb
2 changed files with 27 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lib/checkother.cpp
View File

@ -2057,12 +2057,19 @@ void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2,
const std::string& expr1 = tok1 ? tok1->expressionString() : "x"; const std::string& expr1 = tok1 ? tok1->expressionString() : "x";
const std::string& expr2 = tok2 ? tok2->expressionString() : "x"; const std::string& expr2 = tok2 ? tok2->expressionString() : "x";
std::string endingPhrase = expr1 == expr2 ? "." :
" because the value of '" + expr1 + "' and '" + expr2 + "' are the same.";
const std::string& op = opTok ? opTok->str() : "&&"; const std::string& op = opTok ? opTok->str() : "&&";
std::string msg = "Same expression on both sides of \'" + op + "\'";
if (expr1 != expr2) {
std::string exprMsg = "The expression \'" + expr1 + " " + op + " " + expr2 + "\' is always ";
if(Token::Match(opTok, "==|>=|<="))
msg = exprMsg + "true";
else if(Token::Match(opTok, "!=|>|<"))
msg = exprMsg + "false";
msg += " because '" + expr1 + "' and '" + expr2 + "' represent the same value";
}
reportError(errors, Severity::style, "duplicateExpression", "Same expression on both sides of \'" + op + "\'" + endingPhrase + "\n"
reportError(errors, Severity::style, "duplicateExpression", msg + ".\n"
"Finding the same expression on both sides of an operator is suspicious and might " "Finding the same expression on both sides of an operator is suspicious and might "
"indicate a cut and paste or logic error. Please examine this code carefully to " "indicate a cut and paste or logic error. Please examine this code carefully to "
"determine if it is correct.", CWE398, false); "determine if it is correct.", CWE398, false);

32
test/testother.cpp
View File

@ -3533,12 +3533,12 @@ private:
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((a < b) && (b > a)) { }\n" " if ((a < b) && (b > a)) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&' because the value of 'a<b' and 'b>a' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&' because 'a<b' and 'b>a' represent the same value.\n", errout.str());
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((a <= b) && (b >= a)) { }\n" " if ((a <= b) && (b >= a)) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&' because the value of 'a<=b' and 'b>=a' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&' because 'a<=b' and 'b>=a' represent the same value.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (x!=2 || y!=3 || x!=2) {}\n" " if (x!=2 || y!=3 || x!=2) {}\n"
@ -3662,7 +3662,7 @@ private:
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((b + a) | (a + b)) {}\n" " if ((b + a) | (a + b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|' because the value of 'b+a' and 'a+b' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|' because 'b+a' and 'a+b' represent the same value.\n", errout.str());
check("void foo(const std::string& a, const std::string& b) {\n" check("void foo(const std::string& a, const std::string& b) {\n"
" return a.find(b+\"&\") || a.find(\"&\"+b);\n" " return a.find(b+\"&\") || a.find(\"&\"+b);\n"
@ -3677,7 +3677,7 @@ private:
check("void foo(double a, double b) {\n" check("void foo(double a, double b) {\n"
" if ((b + a) > (a + b)) {}\n" " if ((b + a) > (a + b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '>' because the value of 'b+a' and 'a+b' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) The expression 'b+a > a+b' is always false because 'b+a' and 'a+b' represent the same value.\n", errout.str());
check("void f(int x) {\n" check("void f(int x) {\n"
" if ((x == 1) && (x == 0x00000001))\n" " if ((x == 1) && (x == 0x00000001))\n"
@ -3916,13 +3916,13 @@ private:
" const int i = sizeof(int);\n" " const int i = sizeof(int);\n"
" if ( i != sizeof (int)){}\n" " if ( i != sizeof (int)){}\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'i' and 'sizeof(int)' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'i != sizeof(int)' is always false because 'i' and 'sizeof(int)' represent the same value.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" const int i = sizeof(int);\n" " const int i = sizeof(int);\n"
" if ( sizeof (int) != i){}\n" " if ( sizeof (int) != i){}\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'sizeof(int)' and 'i' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'sizeof(int) != i' is always false because 'sizeof(int)' and 'i' represent the same value.\n", errout.str());
check("void f(int a = 1) { if ( a != 1){}}\n"); check("void f(int a = 1) { if ( a != 1){}}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -3931,21 +3931,21 @@ private:
" int a = 1;\n" " int a = 1;\n"
" if ( a != 1){} \n" " if ( a != 1){} \n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" int a = 1;\n" " int a = 1;\n"
" int b = 1;\n" " int b = 1;\n"
" if ( a != b){} \n" " if ( a != b){} \n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:4]: (style) Same expression on both sides of '!=' because the value of 'a' and 'b' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != b' is always false because 'a' and 'b' represent the same value.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" int a = 1;\n" " int a = 1;\n"
" int b = a;\n" " int b = a;\n"
" if ( a != b){} \n" " if ( a != b){} \n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Same expression on both sides of '!=' because the value of 'a' and 'b' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != b' is always false because 'a' and 'b' represent the same value.\n", errout.str());
check("void use(int);\n" check("void use(int);\n"
"void f() {\n" "void f() {\n"
@ -3954,7 +3954,7 @@ private:
" use(b);\n" " use(b);\n"
" if ( a != 1){} \n" " if ( a != 1){} \n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
check("void use(int);\n" check("void use(int);\n"
"void f() {\n" "void f() {\n"
@ -3978,7 +3978,7 @@ private:
" void f() {\n" " void f() {\n"
" if ( a != 1){} \n" " if ( a != 1){} \n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
check("int a = 1;\n" check("int a = 1;\n"
" void f() {\n" " void f() {\n"
@ -3990,7 +3990,7 @@ private:
" static const int a = 1;\n" " static const int a = 1;\n"
" if ( a != 1){} \n" " if ( a != 1){} \n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" static int a = 1;\n" " static int a = 1;\n"
@ -4003,7 +4003,7 @@ private:
" if ( a != 1){\n" " if ( a != 1){\n"
" a++;\n" " a++;\n"
" }}\n"); " }}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
check("void f(int b) {\n" check("void f(int b) {\n"
" int a = 1;\n" " int a = 1;\n"
@ -4082,7 +4082,7 @@ private:
" int a = 1;\n" " int a = 1;\n"
" while ( a != 1){}\n" " while ( a != 1){}\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
check("void f() { int a = 1; while ( a != 1){ a++; }}\n"); check("void f() { int a = 1; while ( a != 1){ a++; }}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -4098,7 +4098,7 @@ private:
" if( i != 0 ) {}\n" " if( i != 0 ) {}\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Same expression on both sides of '!=' because the value of 'i' and '0' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) The expression 'i != 0' is always false because 'i' and '0' represent the same value.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" for(int i = 0; i < 10;) {\n" " for(int i = 0; i < 10;) {\n"
@ -4139,7 +4139,7 @@ private:
" b++;\n" " b++;\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Same expression on both sides of '!=' because the value of 'a' and '1' are the same.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) The expression 'a != 1' is always false because 'a' and '1' represent the same value.\n", errout.str());
} }
void duplicateExpressionTernary() { // #6391 void duplicateExpressionTernary() { // #6391