CheckClass: Better checking of what operator= returns

lib/checkclass.cpp

@@ -1180,6 +1180,8 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
 {
     bool foundReturn = false;
 
+    const Token* const startTok = tok;
+
     for (; tok && tok != last; tok = tok->next()) {
         // check for return of reference to this
         if (tok->str() == "return") {
@@ -1228,8 +1230,26 @@ void CheckClass::checkReturnPtrThis(const Scope *scope, const Function *func, co
                 operatorEqRetRefThisError(func->token);
         }
     }
-    if (!foundReturn)
+    if (foundReturn) {
-        operatorEqRetRefThisError(func->token);
+        return;
+    }
+    if (startTok->next() == last) {
+        if (Token::Match(func->argDef, std::string("( const " + scope->className + " &").c_str())) {
+            // Typical wrong way to suppress default assignment operator by declaring it and leaving empty
+            operatorEqMissingReturnStatementError(func->token, func->access == Public);
+        } else {
+            operatorEqMissingReturnStatementError(func->token, true);
+        }
+        return;
+    }
+    if (_settings->library.isScopeNoReturn(last, 0)) {
+        // Typical wrong way to prohibit default assignment operator
+        // by always throwing an exception or calling a noreturn function
+        operatorEqShouldBeLeftUnimplementedError(func->token);
+        return;
+    }
+
+    operatorEqMissingReturnStatementError(func->token, func->access == Public);
 }
 
 void CheckClass::operatorEqRetRefThisError(const Token *tok)
@@ -1237,6 +1257,20 @@ void CheckClass::operatorEqRetRefThisError(const Token *tok)
     reportError(tok, Severity::style, "operatorEqRetRefThis", "'operator=' should return reference to 'this' instance.");
 }
 
+void CheckClass::operatorEqShouldBeLeftUnimplementedError(const Token *tok)
+{
+    reportError(tok, Severity::style, "operatorEqShouldBeLeftUnimplemented", "'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.");
+}
+
+void CheckClass::operatorEqMissingReturnStatementError(const Token *tok, bool error)
+{
+    if (error) {
+        reportError(tok, Severity::error, "operatorEqMissingReturnStatement", "No 'return' statement in non-void function causes undefined behavior.");
+    } else {
+        operatorEqRetRefThisError(tok);
+    }
+}
+
 //---------------------------------------------------------------------------
 // ClassCheck: "C& operator=(const C& rhs) { if (this == &rhs) ... }"
 // operator= should check for assignment to self

lib/checkclass.h

@@ -151,6 +151,8 @@ private:
     void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive);
     void thisSubtractionError(const Token *tok);
     void operatorEqRetRefThisError(const Token *tok);
+    void operatorEqShouldBeLeftUnimplementedError(const Token *tok);
+    void operatorEqMissingReturnStatementError(const Token *tok, bool error);
     void operatorEqToSelfError(const Token *tok);
     void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic);
     void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic);
@@ -178,6 +180,8 @@ private:
         c.virtualDestructorError(0, "Base", "Derived", false);
         c.thisSubtractionError(0);
         c.operatorEqRetRefThisError(0);
+        c.operatorEqMissingReturnStatementError(0, true);
+        c.operatorEqShouldBeLeftUnimplementedError(0);
         c.operatorEqToSelfError(0);
         c.checkConstError(0, "class", "function", false);
         c.checkConstError(0, "class", "function", true);

test/testclass.cpp

@@ -812,7 +812,7 @@ private:
             "{\n"
             "  szp &operator =(int *other) {};\n"
             "};");
-        ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
 
         checkOpertorEqRetRefThis(
             "class szp\n"
@@ -820,7 +820,7 @@ private:
             "  szp &operator =(int *other);\n"
             "};\n"
             "szp &szp::operator =(int *other) {}");
-        ASSERT_EQUALS("[test.cpp:5]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
     }
 
     void operatorEqRetRefThis3() {
@@ -889,15 +889,49 @@ private:
             "public:\n"
             "    A & operator=(const A &a) { }\n"
             "};");
+        ASSERT_EQUALS("[test.cpp:3]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
+
+        checkOpertorEqRetRefThis(
+            "class A {\n"
+            "protected:\n"
+            "    A & operator=(const A &a) {}\n"
+            "};");
         ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
 
+        checkOpertorEqRetRefThis(
+            "class A {\n"
+            "private:\n"
+            "    A & operator=(const A &a) {}\n"
+            "};");
+        ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
+
+        checkOpertorEqRetRefThis(
+            "class A {\n"
+            "public:\n"
+            "    A & operator=(const A &a) {\n"
+            "        rand();\n"
+            "        throw std::exception();\n"
+            "    }\n"
+            "};");
+        ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.\n", errout.str());
+
+        checkOpertorEqRetRefThis(
+            "class A {\n"
+            "public:\n"
+            "    A & operator=(const A &a) {\n"
+            "        rand();\n"
+            "        abort();\n"
+            "    }\n"
+            "};");
+        ASSERT_EQUALS("[test.cpp:3]: (style) 'operator=' should either return reference to 'this' instance or be declared private and left unimplemented.\n", errout.str());
+
         checkOpertorEqRetRefThis(
             "class A {\n"
             "public:\n"
             "    A & operator=(const A &a);\n"
             "};\n"
             "A & A :: operator=(const A &a) { }");
-        ASSERT_EQUALS("[test.cpp:5]: (style) 'operator=' should return reference to 'this' instance.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:5]: (error) No 'return' statement in non-void function causes undefined behavior.\n", errout.str());
     }
 
     void operatorEqRetRefThis6() { // ticket #2478 (segmentation fault)