Add checks for explicit constructors
Single-argument constructors should be explicit. Constructors with multiple arguments should not be marked explicit.
This commit is contained in:
parent
b88d61dcb4
commit
2ca932a3ae
|
@ -90,6 +90,14 @@ void CheckClass::constructors()
|
||||||
std::vector<Usage> usage(scope->varlist.size());
|
std::vector<Usage> usage(scope->varlist.size());
|
||||||
|
|
||||||
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
|
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
|
||||||
|
// check for explicit
|
||||||
|
if (func->type == Function::eConstructor) {
|
||||||
|
if (!func->isExplicit && func->argumentList.size() == 1)
|
||||||
|
explicitConstructorError(func->token, scope->className);
|
||||||
|
if (func->isExplicit && func->argumentList.size() > 1)
|
||||||
|
pointlessExplicitConstructorError(func->token, scope->className);
|
||||||
|
}
|
||||||
|
|
||||||
if (!func->hasBody || !(func->type == Function::eConstructor ||
|
if (!func->hasBody || !(func->type == Function::eConstructor ||
|
||||||
func->type == Function::eCopyConstructor ||
|
func->type == Function::eCopyConstructor ||
|
||||||
func->type == Function::eOperatorEqual))
|
func->type == Function::eOperatorEqual))
|
||||||
|
@ -515,6 +523,32 @@ void CheckClass::operatorEqVarError(const Token *tok, const std::string &classna
|
||||||
reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator=" + "'");
|
reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator=" + "'");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void CheckClass::explicitConstructorError(const Token *tok, const std::string &className)
|
||||||
|
{
|
||||||
|
reportError(tok, Severity::style,
|
||||||
|
"explicitConstructorError", "Constructor for '" +
|
||||||
|
className + "' should be explicit.\n"
|
||||||
|
"The single-argument constructor for '" + className + "' should "
|
||||||
|
"be explicit as it can be used for automatic conversion. This is "
|
||||||
|
"convenient but can also be a problem when automatic conversion "
|
||||||
|
"creates new objects when you were not expecting it. Adding the "
|
||||||
|
"explicit declaration to the constructor prevents it being called "
|
||||||
|
"for implicit conversions.");
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckClass::pointlessExplicitConstructorError(const Token *tok, const std::string &className)
|
||||||
|
{
|
||||||
|
reportError(tok, Severity::style,
|
||||||
|
"pointlessExplicitConstructorError", "Constructor for '" +
|
||||||
|
className + "' is marked explicit but"
|
||||||
|
" takes more than one argument.\n"
|
||||||
|
"The explicit keyword prevents constructor calls for implicit "
|
||||||
|
"conversions, but it is only needed for single-argument "
|
||||||
|
"constructors. The constructor for '" + className + "' takes "
|
||||||
|
"more than one argument so is not affected by the explicit "
|
||||||
|
"declaration.");
|
||||||
|
}
|
||||||
|
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
// ClassCheck: Unused private functions
|
// ClassCheck: Unused private functions
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
|
|
|
@ -128,6 +128,8 @@ private:
|
||||||
void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname);
|
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 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 initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname);
|
||||||
|
void explicitConstructorError(const Token *tok, const std::string &className);
|
||||||
|
void pointlessExplicitConstructorError(const Token *tok, const std::string &className);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
|
||||||
CheckClass c(0, settings, errorLogger);
|
CheckClass c(0, settings, errorLogger);
|
||||||
|
@ -143,6 +145,8 @@ private:
|
||||||
c.operatorEqToSelfError(0);
|
c.operatorEqToSelfError(0);
|
||||||
c.checkConstError(0, "class", "function");
|
c.checkConstError(0, "class", "function");
|
||||||
c.initializerListError(0, 0, "class", "variable");
|
c.initializerListError(0, 0, "class", "variable");
|
||||||
|
c.explicitConstructorError(0, "classname");
|
||||||
|
c.pointlessExplicitConstructorError(0, "classname");
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string myName() const {
|
std::string myName() const {
|
||||||
|
|
|
@ -81,6 +81,7 @@ private:
|
||||||
TEST_CASE(initvar_destructor); // No variables need to be initialized in a destructor
|
TEST_CASE(initvar_destructor); // No variables need to be initialized in a destructor
|
||||||
|
|
||||||
TEST_CASE(operatorEqSTL);
|
TEST_CASE(operatorEqSTL);
|
||||||
|
TEST_CASE(explicit_constructor);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -1043,6 +1044,22 @@ private:
|
||||||
"{ }", true);
|
"{ }", true);
|
||||||
ASSERT_EQUALS("[test.cpp:13]: (warning) Member variable 'Fred::ints' is not assigned a value in 'Fred::operator='\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:13]: (warning) Member variable 'Fred::ints' is not assigned a value in 'Fred::operator='\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void explicit_constructor()
|
||||||
|
{
|
||||||
|
check("class Fred\n"
|
||||||
|
"{\n"
|
||||||
|
" Fred(int i);\n"
|
||||||
|
"};\n"
|
||||||
|
"\n", true);
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Constructor for 'Fred' should be explicit.\n", errout.str());
|
||||||
|
check("class Fred\n"
|
||||||
|
"{\n"
|
||||||
|
" explicit Fred(int a, int b);\n"
|
||||||
|
"};\n"
|
||||||
|
"\n", true);
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (style) Constructor for 'Fred' is marked explicit but takes more than one argument.\n", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestConstructors)
|
REGISTER_TEST(TestConstructors)
|
||||||
|
|
Loading…
Reference in New Issue