Fixed #4676 (Duplicated inherited member check)

lib/checkclass.cpp

@@ -1931,6 +1931,55 @@ void CheckClass::checkPureVirtualFunctionCall()
     }
 }
 
+void CheckClass::checkDuplInheritedMembers()
+{
+    if (!_settings->isEnabled("warning"))
+        return;
+
+    // Iterate over all classes
+    for (std::list<Type>::const_iterator classIt = symbolDatabase->typeList.begin();
+         classIt != symbolDatabase->typeList.end();
+         ++classIt) {
+        // Iterate over the parent classes
+        for (std::vector<Type::BaseInfo>::const_iterator parentClassIt = classIt->derivedFrom.begin();
+             parentClassIt != classIt->derivedFrom.end();
+             ++parentClassIt) {
+            // Check if there is info about the 'Base' class
+            if (!parentClassIt->type || !parentClassIt->type->classScope)
+                continue;
+            // Check if they have a member variable in common
+            for (std::list<Variable>::const_iterator classVarIt = classIt->classScope->varlist.begin();
+                 classVarIt != classIt->classScope->varlist.end();
+                 ++classVarIt) {
+                for (std::list<Variable>::const_iterator parentClassVarIt = parentClassIt->type->classScope->varlist.begin();
+                     parentClassVarIt != parentClassIt->type->classScope->varlist.end();
+                     ++parentClassVarIt) {
+                    if (classVarIt->name() == parentClassVarIt->name()) { // Check if the class and its parent have a common variable
+                        duplInheritedMembersError(classVarIt->nameToken(), parentClassVarIt->nameToken(),
+                                                  classIt->name(), parentClassIt->type->name(), classVarIt->name(),
+                                                  classIt->classScope->type == Scope::eStruct,
+                                                  parentClassIt->type->classScope->type == Scope::eStruct);
+                    }
+                }
+            }
+        }
+    }
+}
+
+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)
+{
+    std::list<const Token *> toks;
+    toks.push_back(tok1);
+    toks.push_back(tok2);
+
+    const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedname +
+                                "' defines member variable with name '" + variablename + "' also defined in its parent " +
+                                std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'.";
+    reportError(toks, Severity::warning, "duplInheritedMember", message);
+}
+
 const std::list<const Token *> & CheckClass::callsPureVirtualFunction(const Function & function,
         std::map<const Function *, std::list<const Token *> > & callsPureVirtualFunctionMap)
 {

lib/checkclass.h

@@ -74,6 +74,8 @@ public:
         checkClass.checkConst();
         checkClass.copyconstructors();
         checkClass.checkPureVirtualFunctionCall();
+
+        checkClass.checkDuplInheritedMembers();
     }
 
 
@@ -122,6 +124,9 @@ public:
     /** @brief call of pure virtual funcion */
     void checkPureVirtualFunctionCall();
 
+    /** @brief Check duplicated inherited members */
+    void checkDuplInheritedMembers();
+
 private:
     const SymbolDatabase *symbolDatabase;
 
@@ -146,6 +151,7 @@ private:
     void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
     void suggestInitializationList(const Token *tok, const std::string& varname);
     void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
+    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 getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckClass c(0, settings, errorLogger);
@@ -168,6 +174,7 @@ private:
         c.checkConstError(0, "class", "function", true);
         c.initializerListError(0, 0, "class", "variable");
         c.suggestInitializationList(0, "variable");
+        c.duplInheritedMembersError(0, 0, "class", "class", "variable", false, false);
     }
 
     static std::string myName() {
@@ -190,7 +197,8 @@ private:
                "* Order of initializations\n"
                "* Suggest usage of initialization list\n"
                "* Suspicious subtraction from 'this'\n"
-               "* Call of pure virtual function in constructor/desctructor\n";
+               "* Call of pure virtual function in constructor/desctructor\n"
+               "* Duplicated inherited data members\n";
     }
 
     // operatorEqRetRefThis helper function

test/testclass.cpp

@@ -176,6 +176,77 @@ private:
         TEST_CASE(pureVirtualFunctionCall);
         TEST_CASE(pureVirtualFunctionCallOtherClass);
         TEST_CASE(pureVirtualFunctionCallWithBody);
+
+        TEST_CASE(duplInheritedMembers);
+    }
+
+    void checkDuplInheritedMembers(const char code[]) {
+        // Clear the error log
+        errout.str("");
+        Settings settings;
+        settings.addEnabled("warning");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.simplifyTokenList();
+
+        // Check..
+        CheckClass checkClass(&tokenizer, &settings, this);
+        checkClass.checkDuplInheritedMembers();
+    }
+
+    void duplInheritedMembers() {
+        checkDuplInheritedMembers("class Base {\n"
+                                  "   int x;\n"
+                                  "};\n"
+                                  "struct Derived : Base {\n"
+                                  "   int x;\n"
+                                  "};");
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:2]: (warning) The struct 'Derived' defines member variable with name 'x' also defined in its parent class 'Base'.\n", errout.str());
+
+        checkDuplInheritedMembers("class Base0 {\n"
+                                  "   int x;\n"
+                                  "};\n"
+                                  "class Base1 {\n"
+                                  "   int x;\n"
+                                  "};\n"
+                                  "struct Derived : Base0, Base1 {\n"
+                                  "   int x;\n"
+                                  "};");
+        ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:2]: (warning) The struct 'Derived' defines member variable with name 'x' also defined in its parent class 'Base0'.\n"
+                      "[test.cpp:8] -> [test.cpp:5]: (warning) The struct 'Derived' defines member variable with name 'x' also defined in its parent class 'Base1'.\n", errout.str());
+
+        checkDuplInheritedMembers("class Base {\n"
+                                  "   int x;\n"
+                                  "};\n"
+                                  "struct Derived : Base {\n"
+                                  "   int y;\n"
+                                  "};");
+        ASSERT_EQUALS("", errout.str());
+
+        checkDuplInheritedMembers("class A {\n"
+                                  "   int x;\n"
+                                  "};\n"
+                                  "struct B {\n"
+                                  "   int x;\n"
+                                  "};");
+        ASSERT_EQUALS("", errout.str());
+
+        // Unknown 'Base' class
+        checkDuplInheritedMembers("class Derived : public UnknownBase {\n"
+                                  "  int x;\n"
+                                  "};");
+        ASSERT_EQUALS("", errout.str());
+
+        checkDuplInheritedMembers("class Base {\n"
+                                  "   int x;\n"
+                                  "};\n"
+                                  "class Derived : public Base {\n"
+                                  "};");
+        ASSERT_EQUALS("", errout.str());
+
     }
 
     void checkCopyConstructor(const char code[]) {