Extend duplInheritedMember check to functions (danmar#5226)

chrchr-github · Jul 8, 2023 · 49b79b7 · 49b79b7
1 parent 218650d
commit 49b79b7
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 9 deletions.
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
@@ -2940,7 +2940,10 @@ static std::vector<DuplMemberFuncInfo> getDuplInheritedMemberFunctionsRecursive(
             if (classFuncIt.isImplicitlyVirtual())
                 continue;
             for (const Function& parentClassFuncIt : parentClassIt.type->classScope->functionList) {
-                if (classFuncIt.name() == parentClassFuncIt.name() && (parentClassFuncIt.access != AccessControl::Private || !skipPrivate))
+                if (classFuncIt.name() == parentClassFuncIt.name() &&
+                    (parentClassFuncIt.access != AccessControl::Private || !skipPrivate) &&
+                    !classFuncIt.isConstructor() && !classFuncIt.isDestructor() &&
+                    classFuncIt.argsMatch(parentClassIt.type->classScope, parentClassFuncIt.argDef, classFuncIt.argDef, emptyString, 0))
                     results.emplace_back(&classFuncIt, &parentClassFuncIt, &parentClassIt);
             }
         }
@@ -2954,27 +2957,36 @@ static std::vector<DuplMemberFuncInfo> getDuplInheritedMemberFunctionsRecursive(
 
 void CheckClass::checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase)
 {
-    const auto results = getDuplInheritedMembersRecursive(typeCurrent, typeBase);
-    for (const auto& r : results) {
+    const auto resultsVar = getDuplInheritedMembersRecursive(typeCurrent, typeBase);
+    for (const auto& r : resultsVar) {
         duplInheritedMembersError(r.classVar->nameToken(), r.parentClassVar->nameToken(),
                                   typeCurrent->name(), r.parentClass->type->name(), r.classVar->name(),
                                   typeCurrent->classScope->type == Scope::eStruct,
                                   r.parentClass->type->classScope->type == Scope::eStruct);
     }
+
+    const auto resultsFunc = getDuplInheritedMemberFunctionsRecursive(typeCurrent, typeBase);
+    for (const auto& r : resultsFunc) {
+        duplInheritedMembersError(r.classFunc->token, r.parentClassFunc->token,
+                                  typeCurrent->name(), r.parentClass->type->name(), r.classFunc->name(),
+                                  typeCurrent->classScope->type == Scope::eStruct,
+                                  r.parentClass->type->classScope->type == Scope::eStruct, /*isFunction*/ true);
+    }
 }
 
 void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2,
                                            const std::string &derivedName, const std::string &baseName,
-                                           const std::string &variableName, bool derivedIsStruct, bool baseIsStruct)
+                                           const std::string &memberName, bool derivedIsStruct, bool baseIsStruct, bool isFunction)
 {
     ErrorPath errorPath;
-    errorPath.emplace_back(tok2, "Parent variable '" + baseName + "::" + variableName + "'");
-    errorPath.emplace_back(tok1, "Derived variable '" + derivedName + "::" + variableName + "'");
+    const std::string member = isFunction ? "function" : "variable";
+    errorPath.emplace_back(tok2, "Parent " + member + " '" + baseName + "::" + memberName + "'");
+    errorPath.emplace_back(tok1, "Derived " + member + " '" + derivedName + "::" + memberName + "'");
 
-    const std::string symbols = "$symbol:" + derivedName + "\n$symbol:" + variableName + "\n$symbol:" + baseName;
+    const std::string symbols = "$symbol:" + derivedName + "\n$symbol:" + memberName + "\n$symbol:" + baseName;
 
     const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedName +
-                                "' defines member variable with name '" + variableName + "' also defined in its parent " +
+                                "' defines member " + member + " with name '" + memberName + "' also defined in its parent " +
                                 std::string(baseIsStruct ? "struct" : "class") + " '" + baseName + "'.";
     reportError(errorPath, Severity::warning, "duplInheritedMember", symbols + '\n' + message, CWE398, Certainty::normal);
 }

diff --git a/lib/checkclass.h b/lib/checkclass.h
@@ -228,7 +228,7 @@ class CPPCHECKLIB CheckClass : public Check {
     void selfInitializationError(const Token* tok, const std::string& varname);
     void pureVirtualFunctionCallInConstructorError(const Function * scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
     void virtualFunctionCallInConstructorError(const Function * scopeFunction, const std::list<const Token *> & tokStack, const std::string &funcname);
-    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 duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedName, const std::string &baseName, const std::string &memberName, bool derivedIsStruct, bool baseIsStruct, bool isFunction = false);
     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, bool isSameCode = false);

diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -663,6 +663,35 @@ class TestClass : public TestFixture {
                                   "struct B { bool a; };\n"
                                   "template<> struct A<1> : B {};\n");
         ASSERT_EQUALS("", errout.str());
+
+        checkDuplInheritedMembers("struct B {\n"
+                                  "    int g() const;\n"
+                                  "    virtual int f() const { return g(); }\n"
+                                  "};\n"
+                                  "struct D : B {\n"
+                                  "    int g() const;\n"
+                                  "    int f() const override { return g(); }\n"
+                                  "};\n");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (warning) The struct 'D' defines member function with name 'g' also defined in its parent struct 'B'.\n",
+                      errout.str());
+
+        checkDuplInheritedMembers("struct B {\n"
+                                  "    int g() const;\n"
+                                  "};\n"
+                                  "struct D : B {\n"
+                                  "    int g(int) const;\n"
+                                  "};\n");
+        ASSERT_EQUALS("", errout.str());
+
+        checkDuplInheritedMembers("struct S {\n"
+                                  "    struct T {\n"
+                                  "        T() {}\n"
+                                  "    };\n"
+                                  "};\n"
+                                  "struct T : S::T {\n"
+                                  "    T() : S::T() {}\n"
+                                  "};\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
 #define checkCopyConstructor(code) checkCopyConstructor_(code, __FILE__, __LINE__)