Symbol database: fixed false positive. ticket: #1895

lib/checkclass.cpp

@@ -1461,7 +1461,7 @@ void CheckClass::noMemset()
 
 
 //---------------------------------------------------------------------------
-// ClassCheck: "void operator=("
+// ClassCheck: "void operator=(" and "const type & operator=("
 //---------------------------------------------------------------------------
 
 void CheckClass::operatorEq()
@@ -1483,6 +1483,8 @@ void CheckClass::operatorEq()
             {
                 if (it->token->strAt(-2) == "void")
                     operatorEqReturnError(it->token->tokAt(-2));
+                else if (Token::Match(it->token->tokAt(-4), "const %type% &"))
+                    operatorEqReturnConstError(it->token->tokAt(-4));
             }
         }
     }
@@ -1493,6 +1495,56 @@ void CheckClass::operatorEq()
 // operator= should return a reference to *this
 //---------------------------------------------------------------------------
 
+void CheckClass::checkReturnPtrThis(const SpaceInfo *info, const Func *func, const Token *tok, const Token *last)
+{
+    bool foundReturn = false;
+
+    for (; tok && tok != last; tok = tok->next())
+    {
+        // check for return of reference to this
+        if (tok->str() == "return")
+        {
+            foundReturn = true;
+            std::string cast("( " + info->className + " & )");
+            if (Token::Match(tok->next(), cast.c_str()))
+                tok = tok->tokAt(4);
+
+            // check if a function is called
+            if (Token::Match(tok->tokAt(1), "%any% (") &&
+                tok->tokAt(2)->link()->next()->str() == ";")
+            {
+                std::list<Func>::const_iterator it;
+
+                // check if it is a member function
+                for (it = info->functionList.begin(); it != info->functionList.end(); ++it)
+                {
+                    // check for a regular function with the same name and a bofy
+                    if (it->type == Func::Function && it->hasBody &&
+                        it->token->str() == tok->next()->str())
+                    {
+                        // check for the proper return type
+                        if (it->tokenDef->previous()->str() == "&" &&
+                            it->tokenDef->strAt(-2) == info->className)
+                        {
+                            // make sure it's not a const function
+                            if (it->arg->link()->next()->str() != "const")
+                                checkReturnPtrThis(info, &*it, it->arg->link()->next(), it->arg->link()->next()->link());
+                        }
+                    }
+                }
+            }
+
+            // check of *this is returned
+            else if (!(Token::Match(tok->tokAt(1), "(| * this ;|=") ||
+                       Token::Match(tok->tokAt(1), "(| * this +=") ||
+                       Token::Match(tok->tokAt(1), "operator = (")))
+                operatorEqRetRefThisError(func->token);
+        }
+    }
+    if (!foundReturn)
+        operatorEqRetRefThisError(func->token);
+}
+
 void CheckClass::operatorEqRetRefThis()
 {
     if (!_settings->_checkCodingStyle)
@@ -1518,26 +1570,7 @@ void CheckClass::operatorEqRetRefThis()
                     // find the ')'
                     const Token *tok = it->token->next()->link();
 
-                    bool foundReturn = false;
+                    checkReturnPtrThis(info, &*it, tok->tokAt(2), tok->next()->link());
-                    const Token *last = tok->next()->link();
-                    for (tok = tok->tokAt(2); tok && tok != last; tok = tok->next())
-                    {
-                        // check for return of reference to this
-                        if (tok->str() == "return")
-                        {
-                            foundReturn = true;
-                            std::string cast("( " + info->className + " & )");
-                            if (Token::Match(tok->next(), cast.c_str()))
-                                tok = tok->tokAt(4);
-
-                            if (!(Token::Match(tok->tokAt(1), "(| * this ;|=") ||
-                                  Token::Match(tok->tokAt(1), "(| * this +=") ||
-                                  Token::Match(tok->tokAt(1), "operator = (")))
-                                operatorEqRetRefThisError(it->token);
-                        }
-                    }
-                    if (!foundReturn)
-                        operatorEqRetRefThisError(it->token);
                 }
             }
         }
@@ -2196,6 +2229,11 @@ void CheckClass::operatorEqReturnError(const Token *tok)
     reportError(tok, Severity::style, "operatorEq", "'operator=' should return something");
 }
 
+void CheckClass::operatorEqReturnConstError(const Token *tok)
+{
+    reportError(tok, Severity::style, "operatorEqReturnConst", "'operator=' should not return a const reference");
+}
+
 void CheckClass::virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived)
 {
     reportError(tok, Severity::error, "virtualDestructor", "Class " + Base + " which is inherited by class " + Derived + " does not have a virtual destructor");

lib/checkclass.h

@@ -87,7 +87,7 @@ public:
      */
     void noMemset();
 
-    /** @brief 'operator=' should return something. */
+    /** @brief 'operator=' should return something and it should not be const. */
     void operatorEq();
 
     /** @brief 'operator=' should return reference to *this */
@@ -318,6 +318,7 @@ private:
     void memsetClassError(const Token *tok, const std::string &memfunc);
     void memsetStructError(const Token *tok, const std::string &memfunc, const std::string &classname);
     void operatorEqReturnError(const Token *tok);
+    void operatorEqReturnConstError(const Token *tok);
     void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived);
     void thisSubtractionError(const Token *tok);
     void operatorEqRetRefThisError(const Token *tok);
@@ -335,6 +336,7 @@ private:
         memsetClassError(0, "memfunc");
         memsetStructError(0, "memfunc", "classname");
         operatorEqReturnError(0);
+        operatorEqReturnConstError(0);
         virtualDestructorError(0, "Base", "Derived");
         thisSubtractionError(0);
         operatorEqRetRefThisError(0);
@@ -359,6 +361,9 @@ private:
                "* 'operator=' should check for assignment to self\n"
                "* Constness for member functions\n";
     }
+
+private:
+    void checkReturnPtrThis(const SpaceInfo *info, const Func *func, const Token *tok, const Token *last);
 };
 /// @}
 //---------------------------------------------------------------------------

test/testclass.cpp

@@ -88,6 +88,7 @@ private:
         TEST_CASE(noConstructor5);
 
         TEST_CASE(operatorEq1);
+        TEST_CASE(operatorEq2);
         TEST_CASE(operatorEqRetRefThis1);
         TEST_CASE(operatorEqRetRefThis2); // ticket #1323
         TEST_CASE(operatorEqRetRefThis3); // ticket #1405
@@ -99,6 +100,7 @@ private:
         TEST_CASE(operatorEqToSelf4);   // nested class with multiple inheritance
         TEST_CASE(operatorEqToSelf5);   // ticket # 1233
         TEST_CASE(operatorEqToSelf6);   // ticket # 1550
+        TEST_CASE(operatorEqToSelf7);
         TEST_CASE(memsetOnStruct);
         TEST_CASE(memsetVector);
         TEST_CASE(memsetOnClass);
@@ -230,6 +232,16 @@ private:
         ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return something\n", errout.str());
     }
 
+    void operatorEq2()
+    {
+        checkOpertorEq("class A\n"
+                       "{\n"
+                       "public:\n"
+                       "    const A& operator=(const A&);\n"
+                       "};\n");
+        ASSERT_EQUALS("[test.cpp:4]: (style) 'operator=' should not return a const reference\n", errout.str());
+    }
+
     // Check that operator Equal returns reference to this
     void checkOpertorEqRetRefThis(const char code[])
     {
@@ -1205,6 +1217,24 @@ private:
         ASSERT_EQUALS("[test.cpp:8]: (style) 'operator=' should check for assignment to self\n", errout.str());
     }
 
+    void operatorEqToSelf7()
+    {
+        checkOpertorEqToSelf(
+            "class A\n"
+            "{\n"
+            "public:\n"
+            "    A & assign(const A & a)\n"
+            "    {\n"
+            "        return *this;\n"
+            "    }\n"
+            "    A & operator=(const A &a)\n"
+            "    {\n"
+            "        return assign(a);\n"
+            "    }\n"
+            "};");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     // Check that base classes have virtual destructors
     void checkVirtualDestructor(const char code[])
     {