fix #3008 (New check: Order of initialisation list)

lib/checkclass.cpp

@@ -1784,3 +1784,104 @@ void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const st
                 "sense conceptually. Think about your design and task of the function first - is "
                 "it a function that must not change object internal state?");
 }
+
+//---------------------------------------------------------------------------
+// ClassCheck: Check that initializer list is in declared order.
+//---------------------------------------------------------------------------
+
+struct VarInfo
+{
+    VarInfo(const Variable *_var, const Token *_tok)
+        : var(_var), tok(_tok) { }
+
+    const Variable *var;
+    const Token *tok;
+};
+
+void CheckClass::initializerList()
+{
+    if (!_settings->isEnabled("style"))
+        return;
+
+    // This check is not inconclusive.  However it only determines if the initialization
+    // order is incorrect.  It does not determine if being out of order causes
+    // a real error.  Out of order is not necessarily an error but you can never
+    // have an error if the list is in order so this enforces defensive programming.
+    if (!_settings->inconclusive)
+        return;
+
+    createSymbolDatabase();
+
+    std::list<Scope>::const_iterator info;
+
+    // iterate through all scopes looking for classes and structures
+    for (info = symbolDatabase->scopeList.begin(); info != symbolDatabase->scopeList.end(); ++info)
+    {
+        if (!info->isClassOrStruct())
+            continue;
+
+        std::list<Function>::const_iterator func;
+
+        // iterate through all member functions looking for constructors
+        for (func = info->functionList.begin(); func != info->functionList.end(); ++func)
+        {
+            if (func->type == Function::eConstructor && func->hasBody)
+            {
+                // check for initializer list
+                const Token *tok = func->arg->link()->next();
+
+                if (tok->str() == ":")
+                {
+                    std::vector<VarInfo> vars;
+                    tok = tok->next();
+
+                    // find all variable initializations in list
+                    while (tok && tok->str() != "{")
+                    {
+                        if (Token::Match(tok, "%var% ("))
+                        {
+                            const Variable *var = info->getVariable(tok->str());
+
+                            if (var)
+                                vars.push_back(VarInfo(var, tok));
+
+                            if (Token::Match(tok->tokAt(2), "%var% ="))
+                            {
+                                var = info->getVariable(tok->strAt(2));
+
+                                if (var)
+                                    vars.push_back(VarInfo(var, tok->tokAt(2)));
+                            }
+                        }
+                        tok = tok->next();
+                    }
+
+                    // need at least 2 members to have out of order initialization
+                    if (vars.size() > 1)
+                    {
+                        for (unsigned int i = 1; i < vars.size(); i++)
+                        {
+                            // check for out of order initialization
+                            if (vars[i].var->index() < vars[i - 1].var->index())
+                                initializerListError(vars[i].tok,vars[i].var->nameToken(), info->className, vars[i].var->name());
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+void CheckClass::initializerListError(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &varname)
+{
+    std::list<const Token *> toks;
+    toks.push_back(tok1);
+    toks.push_back(tok2);
+    reportError(toks, Severity::style, "initializerList",
+                "Member variable '" + classname + "::" +
+                varname + "' is in the wrong order in the initializer list.\n"
+                "Members are initialized in the order they are declared, not the "
+                "order they are in the initializer list.  Keeping the initializer list "
+                "in the same order that the members were declared prevents order dependent "
+                "initialization errors.");
+}

lib/checkclass.h

@@ -63,6 +63,7 @@ public:
         checkClass.operatorEqRetRefThis();
         checkClass.thisSubtraction();
         checkClass.operatorEqToSelf();
+        checkClass.initializerList();
 
         checkClass.virtualDestructor();
         checkClass.checkConst();
@@ -104,6 +105,9 @@ public:
     /** @brief can member function be const? */
     void checkConst();
 
+    /** @brief Check initializer list order */
+    void initializerList();
+
 private:
     /**
      * @brief Create symbol database. For performance reasons, only call
@@ -126,6 +130,7 @@ private:
     void operatorEqToSelfError(const Token *tok);
     void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname);
     void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname);
+    void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -141,6 +146,7 @@ private:
         c.operatorEqRetRefThisError(0);
         c.operatorEqToSelfError(0);
         c.checkConstError(0, "class", "function");
+        c.initializerListError(0, 0, "class", "variable");
     }
 
     std::string myName() const

test/testclass.cpp

@@ -204,6 +204,8 @@ private:
         TEST_CASE(constIfCfg);  // ticket #1881 - fp when there are #if
         TEST_CASE(constFriend); // ticket #1921 - fp for friend function
         TEST_CASE(constUnion);	// ticket #2111 - fp when there are union
+
+        TEST_CASE(initializerList);
     }
 
     // Check the operator Equal
@@ -6441,6 +6443,37 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void checkInitializerList(const char code[])
+    {
+        // Clear the error log
+        errout.str("");
+
+        // Check..
+        Settings settings;
+        settings.addEnabled("style");
+        settings.inconclusive = true;
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.simplifyTokenList();
+
+        CheckClass checkClass(&tokenizer, &settings, this);
+        checkClass.initializerList();
+    }
+
+    void initializerList()
+    {
+        checkInitializerList("class Fred {\n"
+                             "    int a, b, c;\n"
+                             "public:\n"
+                             "    Fred() : c(0), b(0), a(0) { }\n"
+                             "};");
+        ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (style) Member variable 'Fred::b' is in the wrong order in the initializer list.\n"
+                      "[test.cpp:4] -> [test.cpp:2]: (style) Member variable 'Fred::a' is in the wrong order in the initializer list.\n", errout.str());
+    }
+
 };
 
 REGISTER_TEST(TestClass)