From bcafb30d0d5f94183b323368885a77956f980c8c Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Sat, 2 Jan 2010 17:29:55 +0100 Subject: [PATCH] Fixed #1211 (add struct support for constructor checks) --- lib/checkclass.cpp | 49 +++--- lib/checkclass.h | 10 +- lib/tokenize.cpp | 4 +- lib/tokenize.h | 5 +- test/testconstructors.cpp | 324 ++++++++++++++++++++++++++++++++++++++ test/testtokenize.cpp | 26 ++- 6 files changed, 384 insertions(+), 34 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index fb1522e33..5b1684723 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -41,12 +41,12 @@ CheckClass instance; //--------------------------------------------------------------------------- -CheckClass::Var *CheckClass::getVarList(const Token *tok1, bool withClasses) +CheckClass::Var *CheckClass::getVarList(const Token *tok1, bool withClasses, bool isStruct) { // Get variable list.. Var *varlist = NULL; unsigned int indentlevel = 0; - bool priv = true; + bool priv = !isStruct; for (const Token *tok = tok1; tok; tok = tok->next()) { if (!tok->next()) @@ -189,7 +189,7 @@ void CheckClass::initVar(Var *varlist, const char varname[]) } //--------------------------------------------------------------------------- -void CheckClass::initializeVarList(const Token *tok1, const Token *ftok, Var *varlist, const char classname[], std::list &callstack) +void CheckClass::initializeVarList(const Token *tok1, const Token *ftok, Var *varlist, const char classname[], std::list &callstack, bool isStruct) { bool Assign = false; unsigned int indentlevel = 0; @@ -285,10 +285,10 @@ void CheckClass::initializeVarList(const Token *tok1, const Token *ftok, Var *va { callstack.push_back(ftok->str()); int i = 0; - const Token *ftok2 = Tokenizer::findClassFunction(tok1, classname, ftok->strAt(0), i); + const Token *ftok2 = Tokenizer::findClassFunction(tok1, classname, ftok->strAt(0), i, isStruct); if (ftok2) { - initializeVarList(tok1, ftok2, varlist, classname, callstack); + initializeVarList(tok1, ftok2, varlist, classname, callstack, isStruct); } else // there is a called member function, but it is not defined where we can find it, so we assume it initializes everything { @@ -340,22 +340,22 @@ void CheckClass::initializeVarList(const Token *tok1, const Token *ftok, Var *va void CheckClass::constructors() { - const char pattern_class[] = "class %var% [{:]"; + const char pattern_class[] = "class|struct %var% [{:]"; // Locate class const Token *tok1 = Token::findmatch(_tokenizer->tokens(), pattern_class); while (tok1) { - const char *className[2]; - className[0] = tok1->strAt(1); - className[1] = 0; + const char *className; + className = tok1->strAt(1); const Token *classNameToken = tok1->tokAt(1); + bool isStruct = tok1->str() == "struct"; /** @todo handling of private constructors should be improved */ bool hasPrivateConstructor = false; { int indentlevel = 0; - bool isPrivate = true; + bool isPrivate = !isStruct; for (const Token *tok = tok1; tok; tok = tok->next()) { // Indentation @@ -409,14 +409,14 @@ void CheckClass::constructors() if (_settings->_checkCodingStyle) { // Get class variables... - Var *varlist = getVarList(tok1, false); + Var *varlist = getVarList(tok1, false, isStruct); // If there is a private variable, there should be a constructor.. for (const Var *var = varlist; var; var = var->next) { if (var->priv) { - noConstructorError(tok1, classNameToken->str()); + noConstructorError(tok1, classNameToken->str(), isStruct); break; } } @@ -435,27 +435,27 @@ void CheckClass::constructors() } // Check constructors - checkConstructors(tok1, className[0], hasPrivateConstructor); + checkConstructors(tok1, className, hasPrivateConstructor, isStruct); // Check assignment operators - checkConstructors(tok1, "operator =", hasPrivateConstructor); + checkConstructors(tok1, "operator =", hasPrivateConstructor, isStruct); tok1 = Token::findmatch(tok1->next(), pattern_class); } } -void CheckClass::checkConstructors(const Token *tok1, const char funcname[], bool hasPrivateConstructor) +void CheckClass::checkConstructors(const Token *tok1, const char funcname[], bool hasPrivateConstructor, bool isStruct) { const char * const className = tok1->strAt(1); // Check that all member variables are initialized.. bool withClasses = bool(_settings->_showAll && std::string(funcname) == "operator ="); - Var *varlist = getVarList(tok1, withClasses); + Var *varlist = getVarList(tok1, withClasses, isStruct); int indentlevel = 0; - const Token *constructor_token = Tokenizer::findClassFunction(tok1, className, funcname, indentlevel); + const Token *constructor_token = Tokenizer::findClassFunction(tok1, className, funcname, indentlevel, isStruct); std::list callstack; - initializeVarList(tok1, constructor_token, varlist, className, callstack); + initializeVarList(tok1, constructor_token, varlist, className, callstack, isStruct); while (constructor_token) { // Check if any variables are uninitialized @@ -500,9 +500,9 @@ void CheckClass::checkConstructors(const Token *tok1, const char funcname[], boo for (Var *var = varlist; var; var = var->next) var->init = false; - constructor_token = Tokenizer::findClassFunction(constructor_token->next(), className, funcname, indentlevel); + constructor_token = Tokenizer::findClassFunction(constructor_token->next(), className, funcname, indentlevel, isStruct); callstack.clear(); - initializeVarList(tok1, constructor_token, varlist, className, callstack); + initializeVarList(tok1, constructor_token, varlist, className, callstack, isStruct); } // Delete the varlist.. @@ -522,7 +522,7 @@ void CheckClass::checkConstructors(const Token *tok1, const char funcname[], boo void CheckClass::privateFunctions() { // Locate some class - for (const Token *tok1 = Token::findmatch(_tokenizer->tokens(), "class %var% {"); tok1; tok1 = Token::findmatch(tok1->next(), "class %var% {")) + for (const Token *tok1 = Token::findmatch(_tokenizer->tokens(), "class|struct %var% {"); tok1; tok1 = Token::findmatch(tok1->next(), "class|struct %var% {")) { /** @todo check that the whole class implementation is seen */ // until the todo above is fixed we only check classes that are @@ -535,7 +535,8 @@ void CheckClass::privateFunctions() // Get private functions.. std::list FuncList; FuncList.clear(); - bool priv = false; + bool isStruct = tok1->str() == "struct"; + bool priv = !isStruct; unsigned int indent_level = 0; for (const Token *tok = tok1; tok; tok = tok->next()) { @@ -1214,9 +1215,9 @@ void CheckClass::thisSubtraction() } } -void CheckClass::noConstructorError(const Token *tok, const std::string &classname) +void CheckClass::noConstructorError(const Token *tok, const std::string &classname, bool isStruct) { - reportError(tok, Severity::style, "noConstructor", "The class '" + classname + "' has no constructor. Member variables not initialized."); + reportError(tok, Severity::style, "noConstructor", "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' has no constructor. Member variables not initialized."); } void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool hasPrivateConstructor) diff --git a/lib/checkclass.h b/lib/checkclass.h index 6037cc160..49e9e2aaa 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -110,15 +110,15 @@ private: Var *next; }; - void initializeVarList(const Token *tok1, const Token *ftok, Var *varlist, const char classname[], std::list &callstack); + void initializeVarList(const Token *tok1, const Token *ftok, Var *varlist, const char classname[], std::list &callstack, bool isStruct); void initVar(Var *varlist, const char varname[]); - Var *getVarList(const Token *tok1, bool withClasses); + Var *getVarList(const Token *tok1, bool withClasses, bool isStruct); // Check constructors for a specified class - void checkConstructors(const Token *tok1, const char funcname[], bool hasPrivateConstructor); + void checkConstructors(const Token *tok1, const char funcname[], bool hasPrivateConstructor, bool isStruct); // Reporting errors.. - void noConstructorError(const Token *tok, const std::string &classname); + void noConstructorError(const Token *tok, const std::string &classname, bool isStruct); void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool hasPrivateConstructor); void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname); void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname); @@ -132,7 +132,7 @@ private: void getErrorMessages() { - noConstructorError(0, "classname"); + noConstructorError(0, "classname", false); uninitVarError(0, "classname", "varname", false); operatorEqVarError(0, "classname", ""); unusedPrivateFunctionError(0, "classname", "funcname"); diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 425c7ef52..b4a92ef26 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -4548,12 +4548,12 @@ std::string Tokenizer::file(const Token *tok) const //--------------------------------------------------------------------------- -const Token * Tokenizer::findClassFunction(const Token *tok, const char classname[], const char funcname[], int &indentlevel) +const Token * Tokenizer::findClassFunction(const Token *tok, const char classname[], const char funcname[], int &indentlevel, bool isStruct) { if (indentlevel < 0 || tok == NULL) return NULL; - const std::string classPattern(std::string("class ") + classname + " :|{"); + const std::string classPattern(std::string(isStruct ? "struct " : "class ") + classname + " :|{"); const std::string internalPattern(std::string("!!~ ") + funcname + " ("); const std::string externalPattern(std::string(classname) + " :: " + funcname + " ("); diff --git a/lib/tokenize.h b/lib/tokenize.h index f39354a92..d1d7da6cf 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -106,14 +106,15 @@ public: std::string file(const Token *tok) const; /** - * Find a class member function + * Find a class or struct member function * @param tok where to begin the search * @param classname name of class * @param funcname name of function ("~ Fred" => destructor for fred, "%var%" => any function) * @param indentlevel Just an integer that you initialize to 0 before the first call. + * @param is it a struct * @return First matching token or NULL. */ - static const Token *findClassFunction(const Token *tok, const char classname[], const char funcname[], int &indentlevel); + static const Token *findClassFunction(const Token *tok, const char classname[], const char funcname[], int &indentleveal, bool isStruct = false); /** * get error messages diff --git a/test/testconstructors.cpp b/test/testconstructors.cpp index 5d36bec94..eec6e4b34 100644 --- a/test/testconstructors.cpp +++ b/test/testconstructors.cpp @@ -81,17 +81,53 @@ private: void simple1() { + check("class Fred\n" + "{\n" + "public:\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + check("class Fred\n" "{\n" "private:\n" " int i;\n" "};\n"); ASSERT_EQUALS("[test.cpp:1]: (style) The class 'Fred' has no constructor. Member variables not initialized.\n", errout.str()); + + check("struct Fred\n" + "{\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + "private:\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:1]: (style) The struct 'Fred' has no constructor. Member variables not initialized.\n", errout.str()); } void simple2() { + check("class Fred\n" + "{\n" + "public:\n" + " Fred() : i(0) { }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("class Fred\n" + "{\n" + "public:\n" + " Fred() { i = 0; }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + check("class Fred\n" "{\n" "public:\n" @@ -99,11 +135,52 @@ private: " int i;\n" "};\n"); ASSERT_EQUALS("[test.cpp:4]: (style) Member variable not initialized in the constructor 'Fred::i'\n", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred() : i(0) { }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred() { i = 0; }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred() { }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Member variable not initialized in the constructor 'Fred::i'\n", errout.str()); } void simple3() { + check("class Fred\n" + "{\n" + "public:\n" + " Fred();\n" + " int i;\n" + "};\n" + "Fred::Fred() :i(0)\n" + "{ }\n"); + ASSERT_EQUALS("", errout.str()); + + check("class Fred\n" + "{\n" + "public:\n" + " Fred();\n" + " int i;\n" + "};\n" + "Fred::Fred()\n" + "{ i = 0; }\n"); + ASSERT_EQUALS("", errout.str()); + check("class Fred\n" "{\n" "public:\n" @@ -113,6 +190,33 @@ private: "Fred::Fred()\n" "{ }\n"); ASSERT_EQUALS("[test.cpp:7]: (style) Member variable not initialized in the constructor 'Fred::i'\n", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred();\n" + " int i;\n" + "};\n" + "Fred::Fred() :i(0)\n" + "{ }\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred();\n" + " int i;\n" + "};\n" + "Fred::Fred()\n" + "{ i = 0; }\n"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred();\n" + " int i;\n" + "};\n" + "Fred::Fred()\n" + "{ }\n"); + ASSERT_EQUALS("[test.cpp:6]: (style) Member variable not initialized in the constructor 'Fred::i'\n", errout.str()); } @@ -132,6 +236,20 @@ private: " i = _i;\n" "}\n"); ASSERT_EQUALS("[test.cpp:8]: (style) Member variable not initialized in the constructor 'Fred::i'\n", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred();\n" + " Fred(int _i);\n" + " int i;\n" + "};\n" + "Fred::Fred()\n" + "{ }\n" + "Fred::Fred(int _i)\n" + "{\n" + " i = _i;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (style) Member variable not initialized in the constructor 'Fred::i'\n", errout.str()); } @@ -145,6 +263,14 @@ private: " int i;\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred()\n" + " { this->i = 0; }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void initvar_if() @@ -162,6 +288,20 @@ private: " int i;\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + "public:\n" + " Fred()\n" + " {\n" + " if (true)\n" + " i = 0;\n" + " else\n" + " i = 1;\n" + " }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void initvar_operator_eq1() @@ -204,6 +344,40 @@ private: "int main() {}\n"); ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " int i;\n" + "\n" + " Fred()\n" + " { i = 0; }\n" + "\n" + " Fred(const Fred &fred)\n" + " { *this = fred; }\n" + "\n" + " const Fred & operator=(const Fred &fred)\n" + " { i = fred.i; return *this; }\n" + "};\n"); + + ASSERT_EQUALS("", errout.str()); + + check("struct A\n" + "{\n" + " A() : i(0), j(0) {}\n" + "\n" + " A &operator=(const int &value)\n" + " {\n" + " i = value;\n" + " return (*this);\n" + " }\n" + "\n" + " int i;\n" + " int j;\n" + "};\n" + "\n" + "int main() {}\n"); + + ASSERT_EQUALS("", errout.str()); } @@ -217,6 +391,14 @@ private: " int i;\n" "};\n"); ASSERT_EQUALS("[test.cpp:5]: (possible style) Member variable 'Fred::i' is not assigned a value in 'Fred::operator='\n", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred() { i = 0; }\n" + " void operator=(const Fred &fred) { }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:4]: (possible style) Member variable 'Fred::i' is not assigned a value in 'Fred::operator='\n", errout.str()); } void initvar_operator_eq3() @@ -231,6 +413,16 @@ private: " int i;\n" "};\n"); ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " Fred() { Init(); }\n" + " void operator=(const Fred &fred) { Init(); }\n" + "private:\n" + " void Init() { i = 0; }\n" + " int i;\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); } void initvar_same_classname() @@ -256,6 +448,46 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + + check("void func1()\n" + "{\n" + " struct Fred\n" + " {\n" + " int a;\n" + " Fred() { a = 0; }\n" + " };\n" + "}\n" + "\n" + "void func2()\n" + "{\n" + " class Fred\n" + " {\n" + " int b;\n" + " Fred() { b = 0; }\n" + " };\n" + "}\n"); + + ASSERT_EQUALS("", errout.str()); + + check("void func1()\n" + "{\n" + " struct Fred\n" + " {\n" + " int a;\n" + " Fred() { a = 0; }\n" + " };\n" + "}\n" + "\n" + "void func2()\n" + "{\n" + " struct Fred\n" + " {\n" + " int b;\n" + " Fred() { b = 0; }\n" + " };\n" + "}\n"); + + ASSERT_EQUALS("", errout.str()); } void initvar_chained_assign() @@ -276,6 +508,21 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + + check("struct c\n" + "{\n" + " c();\n" + "\n" + " int m_iMyInt1;\n" + " int m_iMyInt2;\n" + "}\n" + "\n" + "c::c()\n" + "{\n" + " m_iMyInt1 = m_iMyInt2 = 0;\n" + "}\n"); + + ASSERT_EQUALS("", errout.str()); } @@ -310,11 +557,61 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); + + check("struct c\n" + "{\n" + " c();\n" + " c(bool b);" + "\n" + " void InitInt();\n" + "\n" + " int m_iMyInt;\n" + " int m_bMyBool;\n" + "}\n" + "\n" + "c::c()\n" + "{\n" + " m_bMyBool = false;\n" + " InitInt();" + "}\n" + "\n" + "c::c(bool b)\n" + "{\n" + " m_bMyBool = b;\n" + " InitInt();\n" + "}\n" + "\n" + "void c::InitInt()\n" + "{\n" + " m_iMyInt = 0;\n" + "}\n"); + + ASSERT_EQUALS("", errout.str()); } void initvar_constvar() { + check("class Fred\n" + "{\n" + "public:\n" + " const char *s;\n" + " Fred();\n" + "};\n" + "Fred::Fred() : s(NULL)\n" + "{ }"); + ASSERT_EQUALS("", errout.str()); + + check("class Fred\n" + "{\n" + "public:\n" + " const char *s;\n" + " Fred();\n" + "};\n" + "Fred::Fred()\n" + "{ s = NULL; }"); + ASSERT_EQUALS("", errout.str()); + check("class Fred\n" "{\n" "public:\n" @@ -324,6 +621,33 @@ private: "Fred::Fred()\n" "{ }"); ASSERT_EQUALS("[test.cpp:7]: (style) Member variable not initialized in the constructor 'Fred::s'\n", errout.str()); + + check("struct Fred\n" + "{\n" + " const char *s;\n" + " Fred();\n" + "};\n" + "Fred::Fred() : s(NULL)\n" + "{ }"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " const char *s;\n" + " Fred();\n" + "};\n" + "Fred::Fred()\n" + "{ s = NULL; }"); + ASSERT_EQUALS("", errout.str()); + + check("struct Fred\n" + "{\n" + " const char *s;\n" + " Fred();\n" + "};\n" + "Fred::Fred()\n" + "{ }"); + ASSERT_EQUALS("[test.cpp:6]: (style) Member variable not initialized in the constructor 'Fred::s'\n", errout.str()); } diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 469ec3e77..003ca427c 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -142,6 +142,7 @@ private: TEST_CASE(simplify_constants2); TEST_CASE(findClassFunction1); + TEST_CASE(findClassFunction2); TEST_CASE(vardecl1); TEST_CASE(vardecl2); @@ -2254,7 +2255,7 @@ private: const char code[] = "class Fred" "{\n" -// "public:\n" + "public:\n" " Fred()\n" " { }\n" "};\n"; @@ -2273,6 +2274,29 @@ private: ASSERT_EQUALS(0, tok ? 1 : 0); } + void findClassFunction2() + { + const char code[] = + "struct Fred" + "{\n" + " Fred()\n" + " { }\n" + "};\n"; + + // tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + int i; + + i = 0; + const Token *tok = Tokenizer::findClassFunction(tokenizer.tokens(), "Fred", "%var%", i, true); + ASSERT_EQUALS(true, Token::simpleMatch(tok, "Fred ( ) {")); + tok = Tokenizer::findClassFunction(tok->next(), "Fred", "%var%", i, false); + ASSERT_EQUALS(0, tok ? 1 : 0); + } + void vardecl1() { const char code[] = "unsigned int a, b;";