Ticket #695: Fixed VS 2010 issue, switched to "style" severity, changed error messages and updated unit tests
This commit is contained in:
parent
b7d92a4fc7
commit
e1e1dbfe97
|
@ -129,7 +129,7 @@ private:
|
||||||
|
|
||||||
/** disabled assignment operator and copy constructor */
|
/** disabled assignment operator and copy constructor */
|
||||||
void operator=(const Check &);
|
void operator=(const Check &);
|
||||||
Check(const Check &) = delete;
|
explicit Check(const Check &);
|
||||||
};
|
};
|
||||||
|
|
||||||
/// @}
|
/// @}
|
||||||
|
|
|
@ -219,7 +219,7 @@ void CheckClass::constructors()
|
||||||
|
|
||||||
void CheckClass::checkExplicitConstructors()
|
void CheckClass::checkExplicitConstructors()
|
||||||
{
|
{
|
||||||
if (!_settings->isEnabled("performance"))
|
if (!_settings->isEnabled("style"))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
const std::size_t classes = symbolDatabase->classAndStructScopes.size();
|
const std::size_t classes = symbolDatabase->classAndStructScopes.size();
|
||||||
|
@ -771,12 +771,16 @@ void CheckClass::noConstructorError(const Token *tok, const std::string &classna
|
||||||
|
|
||||||
void CheckClass::noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct)
|
void CheckClass::noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct)
|
||||||
{
|
{
|
||||||
reportError(tok, Severity::performance, "noExplicitConstructor", "The constructor of " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' should be marked as explicit. ");
|
const std::string message(std::string(isStruct ? "Struct" : "Class") + " '" + classname + "' has a constructor with 1 argument that is not explicit.");
|
||||||
|
const std::string verbose(message + " Such constructors should in general be explicit for type safety reasons. Using the explicit keyword in the constructor means some mistakes when using the class can be avoided.");
|
||||||
|
reportError(tok, Severity::style, "noExplicitConstructor", message + "\n" + verbose);
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckClass::noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct)
|
void CheckClass::noExplicitCopyMoveConstructorError(const Token *tok, const std::string &classname, bool isStruct)
|
||||||
{
|
{
|
||||||
reportError(tok, Severity::performance, "noExplicitCopyMoveConstructor", "The copy/move constructor of abstract " + std::string(isStruct ? "struct" : "class") + " '" + classname + "' should be marked as explicit. ");
|
const std::string message(std::string(isStruct ? "Abstract struct" : "Abstract class") + " '" + classname + "' has a copy/move constructor that is not explicit.");
|
||||||
|
const std::string verbose(message + " For abstract classes, even copy/move constructors may be declared explicit, as, by definition, abstract classes cannot be instantiated, and so objects of such type should never be passed by value.");
|
||||||
|
reportError(tok, Severity::style, "noExplicitCopyMoveConstructor", message + "\n" + verbose);
|
||||||
}
|
}
|
||||||
|
|
||||||
void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive)
|
void CheckClass::uninitVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive)
|
||||||
|
|
|
@ -208,7 +208,7 @@ private:
|
||||||
return "Check the code for each class.\n"
|
return "Check the code for each class.\n"
|
||||||
"- Missing constructors and copy constructors\n"
|
"- Missing constructors and copy constructors\n"
|
||||||
//"- Missing allocation of memory in copy constructor\n"
|
//"- Missing allocation of memory in copy constructor\n"
|
||||||
"- Constructors which should be explicit are explicit.\n"
|
"- Constructors which should be explicit are explicit\n"
|
||||||
"- Are all variables initialized by the constructors?\n"
|
"- Are all variables initialized by the constructors?\n"
|
||||||
"- Are all variables assigned by 'operator='?\n"
|
"- Are all variables assigned by 'operator='?\n"
|
||||||
"- Warn if memset, memcpy etc are used on a class\n"
|
"- Warn if memset, memcpy etc are used on a class\n"
|
||||||
|
|
|
@ -188,7 +188,7 @@ private:
|
||||||
// Clear the error log
|
// Clear the error log
|
||||||
errout.str("");
|
errout.str("");
|
||||||
Settings settings;
|
Settings settings;
|
||||||
settings.addEnabled("performance");
|
settings.addEnabled("style");
|
||||||
|
|
||||||
// Tokenize..
|
// Tokenize..
|
||||||
Tokenizer tokenizer(&settings, this);
|
Tokenizer tokenizer(&settings, this);
|
||||||
|
@ -235,21 +235,21 @@ private:
|
||||||
"{ \n"
|
"{ \n"
|
||||||
" Class(int i) { } \n"
|
" Class(int i) { } \n"
|
||||||
"};");
|
"};");
|
||||||
ASSERT_EQUALS("[test.cpp:3]: (performance) The constructor of class 'Class' should be marked as explicit. \n", errout.str());
|
ASSERT_EQUALS("[test.cpp:3]: (style) Class 'Class' has a constructor with 1 argument that is not explicit.\n", errout.str());
|
||||||
|
|
||||||
checkExplicitConstructors("class Class \n"
|
checkExplicitConstructors("class Class \n"
|
||||||
"{ \n"
|
"{ \n"
|
||||||
" Class(const Class& other) { } \n"
|
" Class(const Class& other) { } \n"
|
||||||
" virtual int i() = 0; \n"
|
" virtual int i() = 0; \n"
|
||||||
"};");
|
"};");
|
||||||
ASSERT_EQUALS("[test.cpp:3]: (performance) The copy/move constructor of abstract class 'Class' should be marked as explicit. \n", errout.str());
|
ASSERT_EQUALS("[test.cpp:3]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str());
|
||||||
|
|
||||||
checkExplicitConstructors("class Class \n"
|
checkExplicitConstructors("class Class \n"
|
||||||
"{ \n"
|
"{ \n"
|
||||||
" Class(Class&& other) { } \n"
|
" Class(Class&& other) { } \n"
|
||||||
" virtual int i() = 0; \n"
|
" virtual int i() = 0; \n"
|
||||||
"};");
|
"};");
|
||||||
ASSERT_EQUALS("[test.cpp:3]: (performance) The copy/move constructor of abstract class 'Class' should be marked as explicit. \n", errout.str());
|
ASSERT_EQUALS("[test.cpp:3]: (style) Abstract class 'Class' has a copy/move constructor that is not explicit.\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
void checkDuplInheritedMembers(const char code[]) {
|
void checkDuplInheritedMembers(const char code[]) {
|
||||||
|
|
Loading…
Reference in New Issue