Clarify warning message for 'Same expression on both sides of operator'

This commit is contained in:
Daniel Marjamäki 2018-08-05 10:48:02 +02:00
parent ed197f235a
commit 0e491b41a8
2 changed files with 38 additions and 36 deletions

View File

@ -2047,7 +2047,9 @@ void CheckOther::oppositeExpressionError(const Token *tok1, const Token *tok2, c
void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op) void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op)
{ {
const std::list<const Token *> toks = { tok2, tok1 }; std::list<const Token *> toks = { tok1 };
if (tok1 != tok2)
toks.push_front(tok2);
reportError(toks, Severity::style, "duplicateExpression", "Same expression on both sides of \'" + op + "\'.\n" reportError(toks, Severity::style, "duplicateExpression", "Same expression on both sides of \'" + op + "\'.\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 "

View File

@ -3488,7 +3488,7 @@ private:
check("void foo(int a) {\n" check("void foo(int a) {\n"
" if (a == a) { }\n" " if (a == a) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("void fun(int b) {\n" check("void fun(int b) {\n"
" return a && a ||\n" " return a && a ||\n"
@ -3497,45 +3497,45 @@ private:
" e < e &&\n" " e < e &&\n"
" f ;\n" " f ;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n" ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n"
"[test.cpp:3] -> [test.cpp:3]: (style) Same expression on both sides of '=='.\n" "[test.cpp:3]: (style) Same expression on both sides of '=='.\n"
"[test.cpp:4] -> [test.cpp:4]: (style) Same expression on both sides of '>'.\n" "[test.cpp:4]: (style) Same expression on both sides of '>'.\n"
"[test.cpp:5] -> [test.cpp:5]: (style) Same expression on both sides of '<'.\n", errout.str()); "[test.cpp:5]: (style) Same expression on both sides of '<'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" return a && a;\n" " return a && a;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" a = b && b;\n" " a = b && b;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("void foo(int b) {\n" check("void foo(int b) {\n"
" f(a,b == b);\n" " f(a,b == b);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("void foo(int b) {\n" check("void foo(int b) {\n"
" f(b == b, a);\n" " f(b == b, a);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (x!=2 || x!=2) {}\n" " if (x!=2 || x!=2) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\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] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\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] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\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"
@ -3550,7 +3550,7 @@ private:
check("void foo() {\n" check("void foo() {\n"
" if (a && b || a && b) {}\n" " if (a && b || a && b) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (a && b || b && c) {}\n" " if (a && b || b && c) {}\n"
@ -3560,22 +3560,22 @@ private:
check("void foo() {\n" check("void foo() {\n"
" if (a && b | b && c) {}\n" " if (a && b | b && c) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if ((a + b) | (a + b)) {}\n" " if ((a + b) | (a + b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if ((a | b) & (a | b)) {}\n" " if ((a | b) & (a | b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&'.\n", errout.str());
check("void foo(int a, int b) {\n" check("void foo(int a, int b) {\n"
" if ((a | b) == (a | b)) {}\n" " if ((a | b) == (a | b)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (a1[a2[c & 0xff] & 0xff]) {}\n" " if (a1[a2[c & 0xff] & 0xff]) {}\n"
@ -3599,12 +3599,12 @@ private:
check("void foo() {\n" check("void foo() {\n"
" if (a && b && b) {}\n" " if (a && b && b) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (a || b || b) {}\n" " if (a || b || b) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (a / 1000 / 1000) {}\n" " if (a / 1000 / 1000) {}\n"
@ -3614,7 +3614,7 @@ private:
check("int foo(int i) {\n" check("int foo(int i) {\n"
" return i/i;\n" " return i/i;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '/'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '/'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (a << 1 << 1) {}\n" " if (a << 1 << 1) {}\n"
@ -3653,13 +3653,13 @@ private:
" bool a = bar.isSet() && bar->isSet();\n" " bool a = bar.isSet() && bar->isSet();\n"
" bool b = bar.isSet() && bar.isSet();\n" " bool b = bar.isSet() && bar.isSet();\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) Same expression on both sides of '&&'.\n", errout.str());
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] -> [test.cpp:2]: (style) Same expression on both sides of '|'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '|'.\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"
@ -3674,13 +3674,13 @@ 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] -> [test.cpp:2]: (style) Same expression on both sides of '>'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '>'.\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"
" a++;\n" " a++;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("void f() {\n" check("void f() {\n"
" enum { Four = 4 };\n" " enum { Four = 4 };\n"
@ -3718,7 +3718,7 @@ private:
" if (sizeof(a) == sizeof(a)) { }\n" " if (sizeof(a) == sizeof(a)) { }\n"
" if (sizeof(a) == sizeof(b)) { }\n" " if (sizeof(a) == sizeof(b)) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
check("float bar(int) __attribute__((pure));\n" check("float bar(int) __attribute__((pure));\n"
"char foo(int) __attribute__((pure));\n" "char foo(int) __attribute__((pure));\n"
@ -3727,7 +3727,7 @@ private:
" if (unknown(a) == unknown(a)) { }\n" " if (unknown(a) == unknown(a)) { }\n"
" if (foo(a) == foo(a)) { }\n" " if (foo(a) == foo(a)) { }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:6]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (style) Same expression on both sides of '=='.\n", errout.str());
} }
void duplicateExpression2() { // check if float is NaN or Inf void duplicateExpression2() { // check if float is NaN or Inf
@ -3751,7 +3751,7 @@ private:
check("struct X { int i; };\n" check("struct X { int i; };\n"
"int f(struct X x) { return x.i == x.i; }"); "int f(struct X x) { return x.i == x.i; }");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '=='.\n", errout.str());
// #5284 - when type is unknown, assume it's float // #5284 - when type is unknown, assume it's float
check("int f() { return x==x; }"); check("int f() { return x==x; }");
@ -3784,7 +3784,7 @@ private:
"void A::foo() const {\n" "void A::foo() const {\n"
" if (bar() && bar()) {}\n" " if (bar() && bar()) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:6]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:6]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("struct A {\n" check("struct A {\n"
" void foo();\n" " void foo();\n"
@ -3808,7 +3808,7 @@ private:
" if (b.bar(1) && b.bar(1)) {}\n" " if (b.bar(1) && b.bar(1)) {}\n"
" if (a.bar(1) && a.bar(1)) {}\n" " if (a.bar(1) && a.bar(1)) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:11] -> [test.cpp:11]: (style) Same expression on both sides of '&&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:11]: (style) Same expression on both sides of '&&'.\n", errout.str());
check("class D { void strcmp(); };\n" check("class D { void strcmp(); };\n"
"void foo() {\n" "void foo() {\n"
@ -3820,7 +3820,7 @@ private:
check("void foo() {\n" check("void foo() {\n"
" if ((mystrcmp(a, b) == 0) || (mystrcmp(a, b) == 0)) {}\n" " if ((mystrcmp(a, b) == 0) || (mystrcmp(a, b) == 0)) {}\n"
"}", "test.cpp", false, false, true, &settings); "}", "test.cpp", false, false, true, &settings);
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void GetValue() { return rand(); }\n" check("void GetValue() { return rand(); }\n"
"void foo() {\n" "void foo() {\n"
@ -3832,19 +3832,19 @@ private:
"void foo() {\n" "void foo() {\n"
" if ((GetValue() == 0) || (GetValue() == 0)) { dostuff(); }\n" " if ((GetValue() == 0) || (GetValue() == 0)) { dostuff(); }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void GetValue() __attribute__((const));\n" check("void GetValue() __attribute__((const));\n"
"void GetValue() { return X; }\n" "void GetValue() { return X; }\n"
"void foo() {\n" "void foo() {\n"
" if ((GetValue() == 0) || (GetValue() == 0)) { dostuff(); }\n" " if ((GetValue() == 0) || (GetValue() == 0)) { dostuff(); }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:4]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (str == \"(\" || str == \"(\") {}\n" " if (str == \"(\" || str == \"(\") {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
check("void foo() {\n" check("void foo() {\n"
" if (bar(a) && !strcmp(a, b) && bar(a) && !strcmp(a, b)) {}\n" " if (bar(a) && !strcmp(a, b) && bar(a) && !strcmp(a, b)) {}\n"
@ -3861,7 +3861,7 @@ private:
check("void f(A *src) {\n" check("void f(A *src) {\n"
" if (dynamic_cast<B*>(src) || dynamic_cast<B*>(src)) {}\n" " if (dynamic_cast<B*>(src) || dynamic_cast<B*>(src)) {}\n"
"}\n", "test.cpp", false, false, false); // don't run simplifications "}\n", "test.cpp", false, false, false); // don't run simplifications
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '||'.\n", errout.str());
// #5819 // #5819
check("Vector func(Vector vec1) {\n" check("Vector func(Vector vec1) {\n"
@ -3872,7 +3872,7 @@ private:
check("Vector func(int vec1) {\n" check("Vector func(int vec1) {\n"
" return fabs(vec1 & vec1 & vec1);\n" " return fabs(vec1 & vec1 & vec1);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Same expression on both sides of '&'.\n", errout.str());
} }