Fixed false positive redundantAssignment when calling function in assignment (#4513)

lib/checkother.cpp

@@ -663,7 +663,7 @@ void CheckOther::sizeofForPointerError(const Token *tok, const std::string &varn
 
 static bool nonLocal(const Variable* var)
 {
-    return (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference();
+    return !var || (!var->isLocal() && !var->isArgument()) || var->isStatic() || var->isReference();
 }
 
 static void eraseNotLocalArg(std::map<unsigned int, const Token*>& container, const SymbolDatabase* symbolDatabase)
@@ -717,6 +717,12 @@ void CheckOther::checkRedundantAssignment()
                                 break;
                             else if (tok2->varId() == tok->varId())
                                 error = false;
+                            else if (Token::Match(tok2, "%var% (") && nonLocal(tok->variable())) { // Called function might use the variable
+                                const Function* func = symbolDatabase->findFunction(tok2);
+                                const Variable* var = tok->variable();
+                                if (!var || var->isGlobal() || var->isReference() || ((!func || func->functionScope->functionOf) && tok2->strAt(-1) != ".")) // Global variable, or member function
+                                    error = false;
+                            }
                         }
                         if (error) {
                             if (scope->type == Scope::eSwitch && Token::findmatch(it->second, "default|case", tok))

test/testother.cpp

@@ -6822,7 +6822,7 @@ private:
         check("void f() {\n"
               "    Foo& bar = foo();\n"
               "    bar = x;\n"
-              "    bar = y();\n"
+              "    bar = y;\n"
               "}");
         ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (performance, inconclusive) Variable 'bar' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
 
@@ -6889,6 +6889,57 @@ private:
               "    i = 2;\n"
               "}");
         ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (performance) Variable 'i' is reassigned a value before the old one has been used.\n", errout.str());
+
+        // #4513
+        check("int x;\n"
+              "int g() {\n"
+              "    return x*x;\n"
+              "}\n"
+              "void f() {\n"
+              "    x = 2;\n"
+              "    x = g();\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int g() {\n"
+              "    return x*x;\n"
+              "}\n"
+              "void f(int x) {\n"
+              "    x = 2;\n"
+              "    x = g();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6]: (performance) Variable 'x' is reassigned a value before the old one has been used.\n", errout.str());
+
+        check("void f() {\n"
+              "    Foo& bar = foo();\n"
+              "    bar = x;\n"
+              "    bar = y();\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("class C {\n"
+              "    int x;\n"
+              "    void g() { return x*x; }\n"
+              "    void f();\n"
+              "};\n"
+              "\n"
+              "void C::f() {\n"
+              "    x = 2;\n"
+              "    x = g();\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("class C {\n"
+              "    int x;\n"
+              "    void g() { return x*x; }\n"
+              "    void f();\n"
+              "};\n"
+              "\n"
+              "void C::f(Foo z) {\n"
+              "    x = 2;\n"
+              "    x = z.g();\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:9]: (performance, inconclusive) Variable 'x' is reassigned a value before the old one has been used if variable is no semaphore variable.\n", errout.str());
     }
 
     void redundantMemWrite() {