Detect code duplication in overrider

chrchr-github · Jun 30, 2023 · cacd636 · cacd636
1 parent a40e581
commit cacd636
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 4 deletions.
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
@@ -3036,7 +3036,7 @@ void CheckClass::overrideError(const Function *funcInBase, const Function *funcI
                 Certainty::normal);
 }
 
-void CheckClass::uselessOverrideError(const Function *funcInBase, const Function *funcInDerived)
+void CheckClass::uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode)
 {
     const std::string functionName = funcInDerived ? ((funcInDerived->isDestructor() ? "~" : "") + funcInDerived->name()) : "";
     const std::string funcType = (funcInDerived && funcInDerived->isDestructor()) ? "destructor" : "function";
@@ -3047,9 +3047,15 @@ void CheckClass::uselessOverrideError(const Function *funcInBase, const Function
         errorPath.emplace_back(funcInDerived->tokenDef, char(std::toupper(funcType[0])) + funcType.substr(1) + " in derived class");
     }
 
+    std::string errStr = "\nThe " + funcType + " '$symbol' overrides a " + funcType + " in a base class but ";
+    if (isSameCode) {
+        errStr += "is identical to the overridden function";
+    }
+    else
+        errStr += "just delegates back to the base class.";
     reportError(errorPath, Severity::style, "uselessOverride",
-                "$symbol:" + functionName + "\n"
-                "The " + funcType + " '$symbol' overrides a " + funcType + " in a base class but just delegates back to the base class.",
+                "$symbol:" + functionName +
+                errStr,
                 CWE(0U) /* Unknown CWE! */,
                 Certainty::normal);
 }
@@ -3085,6 +3091,35 @@ void CheckClass::checkUselessOverride()
             const Function* baseFunc = func.getOverriddenFunction();
             if (!baseFunc || baseFunc->isPure())
                 continue;
+            if (baseFunc->functionScope) {
+                bool isSameCode = true;
+                const Token* baseTok = baseFunc->argDef;
+                const Token* funcTok = func.argDef;
+                while (baseTok != baseFunc->argDef->link() && funcTok != func.argDef->link()) {
+                    if (baseTok->str() != funcTok->str()) {
+                        isSameCode = false;
+                        break;
+                    }
+                    baseTok = baseTok->next();
+                    funcTok = funcTok->next();
+                }
+                if (isSameCode) {
+                    baseTok = baseFunc->functionScope->bodyStart;
+                    funcTok = func.functionScope->bodyStart;
+                    while (baseTok != baseFunc->functionScope->bodyEnd && funcTok != func.functionScope->bodyEnd) {
+                        if (baseTok->str() != funcTok->str()) {
+                            isSameCode = false;
+                            break;
+                        }
+                        baseTok = baseTok->next();
+                        funcTok = funcTok->next();
+                    }
+                    if (isSameCode) {
+                        uselessOverrideError(baseFunc, &func, true);
+                        continue;
+                    }
+                }
+            }
             if (const Token* const call = getSingleFunctionCall(func.functionScope)) {
                 if (call->function() != baseFunc)
                     continue;

diff --git a/lib/checkclass.h b/lib/checkclass.h
@@ -231,7 +231,7 @@ class CPPCHECKLIB CheckClass : public Check {
     void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedName, const std::string &baseName, const std::string &variableName, bool derivedIsStruct, bool baseIsStruct);
     void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
     void overrideError(const Function *funcInBase, const Function *funcInDerived);
-    void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived);
+    void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode = false);
     void thisUseAfterFree(const Token *self, const Token *free, const Token *use);
     void unsafeClassRefMemberError(const Token *tok, const std::string &varname);
     void checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase);

diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -8403,6 +8403,48 @@ class TestClass : public TestFixture {
                              "    int f() override { return B::f(); }\n"
                              "};");
         ASSERT_EQUALS("", errout.str());
+
+        checkUselessOverride("struct S { virtual void f(); };\n"
+                             "struct D : S {\n"
+                             "    void f() final { S::f(); }\n"
+                             "};");
+        ASSERT_EQUALS("", errout.str());
+
+        checkUselessOverride("struct S {\n"
+                             "protected:\n"
+                             "    virtual void f();\n"
+                             "};\n"
+                             "struct D : S {\n"
+                             "public:\n"
+                             "    void f() override { S::f(); }\n"
+                             "};");
+        ASSERT_EQUALS("", errout.str());
+
+        checkUselessOverride("struct B { virtual void f(int, int, int) const; };\n" // #11799
+                             "struct D : B {\n"
+                             "    int m = 42;\n"
+                             "    void f(int a, int b, int c) const override;\n"
+                             "};\n"
+                             "void D::f(int a, int b, int c) const {\n"
+                             "    B::f(a, b, m);\n"
+                             "};");
+        ASSERT_EQUALS("", errout.str());
+
+        checkUselessOverride("struct B {\n"
+                             "    virtual int f() { return 1; }\n"
+                             "    virtual int g() { return 7; }\n"
+                             "    virtual int h(int i, int j) { return i + j; }\n"
+                             "    virtual int j(int i, int j) { return i + j; }\n"
+                             "};\n"
+                             "struct D : B {\n"
+                             "    int f() override { return 2; }\n"
+                             "    int g() override { return 7; }\n"
+                             "    int h(int j, int i) override { return i + j; }\n"
+                             "    int j(int i, int j) override { return i + j; }\n"
+                             "};");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:9]: (style) The function 'g' overrides a function in a base class but is identical to the overridden function\n"
+                      "[test.cpp:5] -> [test.cpp:11]: (style) The function 'j' overrides a function in a base class but is identical to the overridden function\n",
+                      errout.str());
     }
 
 #define checkUnsafeClassRefMember(code) checkUnsafeClassRefMember_(code, __FILE__, __LINE__)