From 58d1de5814e9557394c5ab13f2f630ebcc88fa66 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Tue, 16 Oct 2018 23:57:33 -0500 Subject: [PATCH] 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 --- lib/checkother.cpp | 21 ++++++++++++--------- lib/checkother.h | 3 ++- test/testother.cpp | 22 +++++++++++----------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3de60d15e..4707126b5 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1984,11 +1984,9 @@ void CheckOther::checkDuplicateExpression() Token::Match(tok->astOperand2()->previous(), "%name% (") ) && 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->astOperand2(), nextAssign->astOperand2(), mSettings->library, true, false) && - tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString() && - !isUniqueExpression(tok->astOperand2())) { + tok->astOperand2()->expressionString() == nextAssign->astOperand2()->expressionString()) { bool assigned = false; const Scope * varScope = var1->scope() ? var1->scope() : &scope; for (const Token *assignTok = Token::findsimplematch(var2, ";"); assignTok && assignTok != varScope->bodyEnd; assignTok = assignTok->next()) { @@ -1999,8 +1997,10 @@ void CheckOther::checkDuplicateExpression() assigned = true; } } - if (!assigned) - duplicateAssignExpressionError(var1, var2); + if (!assigned && !isUniqueExpression(tok->astOperand2())) + 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); } -void CheckOther::duplicateAssignExpressionError(const Token *tok1, const Token *tok2) +void CheckOther::duplicateAssignExpressionError(const Token *tok1, const Token *tok2, bool inconclusive) { const std::list toks = { tok2, tok1 }; + const std::string& var1 = tok1 ? tok1->str() : "x"; + const std::string& var2 = tok2 ? tok2->str() : "x"; + reportError(toks, Severity::style, "duplicateAssignExpression", - "Same expression used in consecutive assignments of '" + tok1->str() + "' and '" + tok2->str() + "'.\n" - "Finding variables '" + tok1->str() + "' and '" + tok2->str() + "' that are assigned the same expression " + "Same expression used in consecutive assignments of '" + var1 + "' and '" + var2 + "'.\n" + "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 " - "determine if it is correct.", CWE398, false); + "determine if it is correct.", CWE398, inconclusive); } void CheckOther::duplicateExpressionTernaryError(const Token *tok, ErrorPath errors) diff --git a/lib/checkother.h b/lib/checkother.h index 9883359de..72a08f8bc 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -242,7 +242,7 @@ private: void selfAssignmentError(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 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 duplicateExpressionError(const Token *tok1, const Token *tok2, const Token *opTok, ErrorPath errors); void duplicateValueTernaryError(const Token *tok); @@ -307,6 +307,7 @@ private: c.clarifyCalculationError(nullptr, "+"); c.clarifyStatementError(nullptr); c.duplicateBranchError(nullptr, nullptr); + c.duplicateAssignExpressionError(nullptr, nullptr, true); c.oppositeExpressionError(nullptr, errorPath); c.duplicateExpressionError(nullptr, nullptr, nullptr, errorPath); c.duplicateValueTernaryError(nullptr); diff --git a/test/testother.cpp b/test/testother.cpp index 49c8e5a41..2755014fe 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -4457,7 +4457,7 @@ private: " int i = 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" "void test() {\n" @@ -4510,26 +4510,26 @@ private: " int start = 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" "void foo(SW* x, int i, int j) {\n" " int start = 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" "void test() {\n" " Foo f = Foo{};\n" " int i = 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" " int i = *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" "void test() {\n" @@ -4537,7 +4537,7 @@ private: " int i = 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" "void test() {\n" @@ -4545,7 +4545,7 @@ private: " int i = 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() { @@ -4557,7 +4557,7 @@ private: " use(i);\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; };" "void use(int);\n" @@ -4567,7 +4567,7 @@ private: " use(j);\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 check("struct P\n" @@ -4596,7 +4596,7 @@ private: " previous = current;\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() { @@ -4612,7 +4612,7 @@ private: " (void)a;\n" " (void)b;\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 check("void f() {\n"