From a9fda3f488a4ae0699f8db638360fc8efab83205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 4 Sep 2019 10:55:41 +0200 Subject: [PATCH] Clarify redundantVarAssignment warnings --- lib/checkother.cpp | 14 ++++----- test/testother.cpp | 73 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 62 insertions(+), 25 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 98b34708c..a03233c8e 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -536,22 +536,22 @@ void CheckOther::redundantCopyInSwitchError(const Token *tok1, const Token* tok2 void CheckOther::redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive) { - const std::list callstack = { tok1, tok2 }; + const ErrorPath errorPath = { ErrorPathItem(tok1, var + " is assigned"), ErrorPathItem(tok2, var + " is overwritten") }; if (inconclusive) - reportError(callstack, Severity::style, "redundantAssignment", + reportError(errorPath, Severity::style, "redundantAssignment", "$symbol:" + var + "\n" "Variable '$symbol' is reassigned a value before the old one has been used if variable is no semaphore variable.\n" "Variable '$symbol' is reassigned a value before the old one has been used. Make sure that this variable is not used like a semaphore in a threading environment before simplifying this code.", CWE563, true); else - reportError(callstack, Severity::style, "redundantAssignment", + reportError(errorPath, Severity::style, "redundantAssignment", "$symbol:" + var + "\n" "Variable '$symbol' is reassigned a value before the old one has been used.", CWE563, false); } void CheckOther::redundantInitializationError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive) { - const std::list callstack = { tok2, tok1 }; - reportError(callstack, Severity::style, "redundantInitialization", + const ErrorPath errorPath = { ErrorPathItem(tok1, var + " is initialized"), ErrorPathItem(tok2, var + " is overwritten") }; + reportError(errorPath, Severity::style, "redundantInitialization", "$symbol:" + var + "\nRedundant initialization for '$symbol'. The initialized value is overwritten before it is read.", CWE563, inconclusive); @@ -559,8 +559,8 @@ void CheckOther::redundantInitializationError(const Token *tok1, const Token* to void CheckOther::redundantAssignmentInSwitchError(const Token *tok1, const Token* tok2, const std::string &var) { - const std::list callstack = { tok1, tok2 }; - reportError(callstack, Severity::warning, "redundantAssignInSwitch", + const ErrorPath errorPath = { ErrorPathItem(tok1, "$symbol is assigned"), ErrorPathItem(tok2, "$symbol is overwritten") }; + reportError(errorPath, Severity::warning, "redundantAssignInSwitch", "$symbol:" + var + "\n" "Variable '$symbol' is reassigned a value before the old one has been used. 'break;' missing?", CWE563, false); } diff --git a/test/testother.cpp b/test/testother.cpp index 0c2ec457e..030ca96c9 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -5985,13 +5985,16 @@ private: } void redundantVarAssignment() { + setMultiline(); + // Simple tests check("void f(int i) {\n" " i = 1;\n" " i = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str()); - + ASSERT_EQUALS("test.cpp:3:style:Variable 'i' is reassigned a value before the old one has been used.\n" + "test.cpp:2:note:i is assigned\n" + "test.cpp:3:note:i is overwritten\n", errout.str()); // non-local variable => only show warning when inconclusive is used check("int i;\n" @@ -5999,14 +6002,18 @@ private: " i = 1;\n" " i = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'i' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:i is assigned\n" + "test.cpp:4:note:i is overwritten\n", errout.str()); check("void f() {\n" " int i;\n" " i = 1;\n" " i = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'i' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:i is assigned\n" + "test.cpp:4:note:i is overwritten\n", errout.str()); check("void f() {\n" " static int i;\n" @@ -6020,7 +6027,9 @@ private: " i[2] = 1;\n" " i[2] = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i[2]' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'i[2]' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:i[2] is assigned\n" + "test.cpp:4:note:i[2] is overwritten\n", errout.str()); check("void f(int x) {\n" " int i[10];\n" @@ -6035,7 +6044,9 @@ private: " i[x] = 1;\n" " i[x] = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'i[x]' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'i[x]' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:i[x] is assigned\n" + "test.cpp:4:note:i[x] is overwritten\n", errout.str()); // Testing different types check("void f() {\n" @@ -6065,7 +6076,9 @@ private: " bar();\n" " i = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'i' is reassigned a value before the old one has been used.\n" + "test.cpp:2:note:i is assigned\n" + "test.cpp:4:note:i is overwritten\n", errout.str()); check("int i;\n" "void f() {\n" @@ -6089,7 +6102,9 @@ private: " bar();\n" " i = 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:5:style:Variable 'i' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:i is assigned\n" + "test.cpp:5:note:i is overwritten\n", errout.str()); check("void bar(int i) {}\n" "void f(int i) {\n" @@ -6120,7 +6135,9 @@ private: " i = 1;\n" " i = 2;\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (style) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:5:style:Variable 'i' is reassigned a value before the old one has been used.\n" + "test.cpp:4:note:i is assigned\n" + "test.cpp:5:note:i is overwritten\n", errout.str()); // #4513 check("int x;\n" @@ -6140,7 +6157,9 @@ private: " x = 2;\n" " x = g();\n" "}"); - ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6]: (style) Variable 'x' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:6:style:Variable 'x' is reassigned a value before the old one has been used.\n" + "test.cpp:5:note:x is assigned\n" + "test.cpp:6:note:x is overwritten\n", errout.str()); check("void f() {\n" " Foo& bar = foo();\n" @@ -6188,7 +6207,9 @@ private: " x = 1;\n" " return x + 1;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'x' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'x' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:x is assigned\n" + "test.cpp:4:note:x is overwritten\n", errout.str()); // from #3103 (avoid a false positive) check("int foo(){\n" @@ -6237,7 +6258,9 @@ private: " ab.a = 2;\n" " return ab.a;\n" "}"); - ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6]: (style) Variable 'ab.a' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:6:style:Variable 'ab.a' is reassigned a value before the old one has been used.\n" + "test.cpp:5:note:ab.a is assigned\n" + "test.cpp:6:note:ab.a is overwritten\n", errout.str()); check("struct AB { int a; int b; };\n" "\n" @@ -6349,7 +6372,7 @@ private: " barney(x);\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) The scope of the variable 'p' can be reduced.\n", + ASSERT_EQUALS("test.cpp:2:style:The scope of the variable 'p' can be reduced.\n", errout.str()); check("void foo() {\n" @@ -6374,7 +6397,9 @@ private: " if (memptr)\n" " memptr = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'memptr' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'memptr' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:memptr is assigned\n" + "test.cpp:4:note:memptr is overwritten\n", errout.str()); // Pointer function argument (#3857) check("void f(float * var)\n" @@ -6382,14 +6407,18 @@ private: " var[0] = 0.2f;\n" " var[0] = 0.2f;\n" // <-- is initialized twice "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable 'var[0]' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable 'var[0]' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:var[0] is assigned\n" + "test.cpp:4:note:var[0] is overwritten\n", errout.str()); check("void f(float * var)\n" "{\n" " *var = 0.2f;\n" " *var = 0.2f;\n" // <-- is initialized twice "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Variable '*var' is reassigned a value before the old one has been used.\n", errout.str()); + ASSERT_EQUALS("test.cpp:4:style:Variable '*var' is reassigned a value before the old one has been used.\n" + "test.cpp:3:note:*var is assigned\n" + "test.cpp:4:note:*var is overwritten\n", errout.str()); } void redundantVarAssignment_struct() { @@ -6550,17 +6579,25 @@ private: } void redundantInitialization() { + setMultiline(); + check("void f() {\n" " int err = -ENOMEM;\n" " err = dostuff();\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Redundant initialization for 'err'. The initialized value is overwritten before it is read.\n", errout.str()); + ASSERT_EQUALS("test.cpp:3:style:Redundant initialization for 'err'. The initialized value is overwritten before it is read.\n" + "test.cpp:2:note:err is initialized\n" + "test.cpp:3:note:err is overwritten\n", + errout.str()); check("void f() {\n" " struct S s = {1,2,3};\n" " s = dostuff();\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Redundant initialization for 's'. The initialized value is overwritten before it is read.\n", errout.str()); + ASSERT_EQUALS("test.cpp:3:style:Redundant initialization for 's'. The initialized value is overwritten before it is read.\n" + "test.cpp:2:note:s is initialized\n" + "test.cpp:3:note:s is overwritten\n", + errout.str()); check("void f() {\n" " int *p = NULL;\n"