From 006b8055e5ab180fe1ff2372b67891f1e25516e5 Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Tue, 20 Aug 2013 06:29:19 +0200 Subject: [PATCH] Fixed #4676 (Duplicated inherited member check) --- lib/checkclass.cpp | 49 ++++++++++++++++++++++++++++++++ lib/checkclass.h | 10 ++++++- test/testclass.cpp | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 723d87989..e8cbf47cd 100644 --- a/lib/checkclass.cpp +++ b/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::const_iterator classIt = symbolDatabase->typeList.begin(); + classIt != symbolDatabase->typeList.end(); + ++classIt) { + // Iterate over the parent classes + for (std::vector::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::const_iterator classVarIt = classIt->classScope->varlist.begin(); + classVarIt != classIt->classScope->varlist.end(); + ++classVarIt) { + for (std::list::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 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 & CheckClass::callsPureVirtualFunction(const Function & function, std::map > & callsPureVirtualFunctionMap) { diff --git a/lib/checkclass.h b/lib/checkclass.h index 2b20d5e67..4257d9d0c 100644 --- a/lib/checkclass.h +++ b/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 & 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 diff --git a/test/testclass.cpp b/test/testclass.cpp index 3922f21e1..bc9f4af2f 100644 --- a/test/testclass.cpp +++ b/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[]) {