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

Expand the duplicate variable assignment warnings when the inconclusive flag is used (#1433) 

* Warn for more duplicate var expressions when inconclusive is set

* Fix issue with missing function name
This commit is contained in:
Paul Fultz II 2018-10-16 23:57:33 -05:00 committed by Daniel Marjamäki
parent 937da6bd46
commit 58d1de5814
3 changed files with 25 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
lib/checkother.cpp
View File

@ -1984,11 +1984,9 @@ void CheckOther::checkDuplicateExpression()
Token::Match(tok->astOperand2()->previous(), "%name% (") Token::Match(tok->astOperand2()->previous(), "%name% (")
) && ) &&
tok->next()->tokType() != Token::eType && tok->next()->tokType() != Token::eType &&
tok->next()->tokType() != Token::eName &&
isSameExpression(mTokenizer->isCPP(), true, tok->next(), nextAssign->next(), mSettings->library, true, false) && isSameExpression(mTokenizer->isCPP(), true, tok->next(), nextAssign->next(), mSettings->library, true, false) &&
isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) && isSameExpression(mTokenizer->isCPP(), true, tok->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) &&
tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString() && tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString()) {
!isUniqueExpression(tok->astOperand2())) {
bool assigned = false; bool assigned = false;
const Scope * varScope = var1->scope() ? var1->scope() : &scope; const Scope * varScope = var1->scope() ? var1->scope() : &scope;
for (const Token *assignTok = Token::findsimplematch(var2, ";"); assignTok && assignTok != varScope->bodyEnd; assignTok = assignTok->next()) { for (const Token *assignTok = Token::findsimplematch(var2, ";"); assignTok && assignTok != varScope->bodyEnd; assignTok = assignTok->next()) {
@ -1999,8 +1997,10 @@ void CheckOther::checkDuplicateExpression()
assigned = true; assigned = true;
} }
} }
if (!assigned) if (!assigned && !isUniqueExpression(tok->astOperand2()))
duplicateAssignExpressionError(var1, var2); duplicateAssignExpressionError(var1, var2, false);
else if(mSettings->inconclusive)
duplicateAssignExpressionError(var1, var2, true);
} }
} }
} }
@ -2098,15 +2098,18 @@ void CheckOther::duplicateExpressionError(const Token *tok1, const Token *tok2,
"determine if it is correct.", CWE398, false); "determine if it is correct.", CWE398, false);
} }
void CheckOther::duplicateAssignExpressionError(const Token *tok1, const Token *tok2) void CheckOther::duplicateAssignExpressionError(const Token *tok1, const Token *tok2, bool inconclusive)
{ {
const std::list<const Token *> toks = { tok2, tok1 }; const std::list<const Token *> toks = { tok2, tok1 };
const std::string& var1 = tok1 ? tok1->str() : "x";
const std::string& var2 = tok2 ? tok2->str() : "x";
reportError(toks, Severity::style, "duplicateAssignExpression", reportError(toks, Severity::style, "duplicateAssignExpression",
"Same expression used in consecutive assignments of '" + tok1->str() + "' and '" + tok2->str() + "'.\n" "Same expression used in consecutive assignments of '" + var1 + "' and '" + var2 + "'.\n"
"Finding variables '" + tok1->str() + "' and '" + tok2->str() + "' that are assigned the same expression " "Finding variables '" + var1 + "' and '" + var2 + "' that are assigned the same expression "
"is suspicious and might indicate a cut and paste or logic error. Please examine this code carefully to " "is suspicious and might 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, inconclusive);
} }
void CheckOther::duplicateExpressionTernaryError(const Token *tok, ErrorPath errors) void CheckOther::duplicateExpressionTernaryError(const Token *tok, ErrorPath errors)

3
lib/checkother.h
View File

@ -242,7 +242,7 @@ private:
void selfAssignmentError(const Token *tok, const std::string &varname); void selfAssignmentError(const Token *tok, const std::string &varname);
void misusedScopeObjectError(const Token *tok, const std::string &varname); void misusedScopeObjectError(const Token *tok, const std::string &varname);
void duplicateBranchError(const Token *tok1, const Token *tok2); void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateAssignExpressionError(const Token *tok1, const Token *tok2); void duplicateAssignExpressionError(const Token *tok1, const Token *tok2, bool inconclusive);
void oppositeExpressionError(const Token *opTok, ErrorPath errors); void oppositeExpressionError(const Token *opTok, ErrorPath errors);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors); void duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors);
void duplicateValueTernaryError(const Token *tok); void duplicateValueTernaryError(const Token *tok);
@ -307,6 +307,7 @@ private:
c.clarifyCalculationError(nullptr, "+"); c.clarifyCalculationError(nullptr, "+");
c.clarifyStatementError(nullptr); c.clarifyStatementError(nullptr);
c.duplicateBranchError(nullptr, nullptr); c.duplicateBranchError(nullptr, nullptr);
c.duplicateAssignExpressionError(nullptr, nullptr, true);
c.oppositeExpressionError(nullptr, errorPath); c.oppositeExpressionError(nullptr, errorPath);
c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath); c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath);
c.duplicateValueTernaryError(nullptr); c.duplicateValueTernaryError(nullptr);

22
test/testother.cpp
View File

@ -4457,7 +4457,7 @@ private:
" int i = f.f();\n" " int i = f.f();\n"
" int j = f.f();\n" " int j = f.f();\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("struct Foo { int f(); int g(); };\n" check("struct Foo { int f(); int g(); };\n"
"void test() {\n" "void test() {\n"
@ -4510,26 +4510,26 @@ private:
" int start = x->first;\n" " int start = x->first;\n"
" int end = x->first;\n" " int end = x->first;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'start' and 'end'.\n", errout.str());
check("struct SW { int first; };\n" check("struct SW { int first; };\n"
"void foo(SW* x, int i, int j) {\n" "void foo(SW* x, int i, int j) {\n"
" int start = x->first;\n" " int start = x->first;\n"
" int end = x->first;\n" " int end = x->first;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'start' and 'end'.\n", errout.str());
check("struct Foo { int f() const; };\n" check("struct Foo { int f() const; };\n"
"void test() {\n" "void test() {\n"
" Foo f = Foo{};\n" " Foo f = Foo{};\n"
" int i = f.f();\n" " int i = f.f();\n"
" int j = f.f();\n" " int j = f.f();\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("void test(int * p) {\n" check("void test(int * p) {\n"
" int i = *p;\n" " int i = *p;\n"
" int j = *p;\n" " int j = *p;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("struct Foo { int f() const; int g(int) const; };\n" check("struct Foo { int f() const; int g(int) const; };\n"
"void test() {\n" "void test() {\n"
@ -4537,7 +4537,7 @@ private:
" int i = f.f();\n" " int i = f.f();\n"
" int j = f.f();\n" " int j = f.f();\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("struct Foo { int f() const; };\n" check("struct Foo { int f() const; };\n"
"void test() {\n" "void test() {\n"
@ -4545,7 +4545,7 @@ private:
" int i = f.f();\n" " int i = f.f();\n"
" int j = f.f();\n" " int j = f.f();\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:4]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
} }
void duplicateVarExpressionAssign() { void duplicateVarExpressionAssign() {
@ -4557,7 +4557,7 @@ private:
" use(i);\n" " use(i);\n"
" i = j;\n" " i = j;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
check("struct A { int x; int y; };" check("struct A { int x; int y; };"
"void use(int);\n" "void use(int);\n"
@ -4567,7 +4567,7 @@ private:
" use(j);\n" " use(j);\n"
" j = i;\n" " j = i;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (style, inconclusive) Same expression used in consecutive assignments of 'i' and 'j'.\n", errout.str());
// Issue #8612 // Issue #8612
check("struct P\n" check("struct P\n"
@ -4596,7 +4596,7 @@ private:
" previous = current;\n" " previous = current;\n"
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:16] -> [test.cpp:15]: (style, inconclusive) Same expression used in consecutive assignments of 'current' and 'previous'.\n", errout.str());
} }
void duplicateVarExpressionCrash() { void duplicateVarExpressionCrash() {
@ -4612,7 +4612,7 @@ private:
" (void)a;\n" " (void)a;\n"
" (void)b;\n" " (void)b;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:7]: (style, inconclusive) Same expression used in consecutive assignments of 'a' and 'b'.\n", errout.str());
// Issue #8712 // Issue #8712
check("void f() {\n" check("void f() {\n"