walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Removed CheckClass::operatorEq: does not 'belong'

This commit is contained in:
Daniel Marjamäki 2020-06-17 20:35:30 +02:00
parent 97de6d2e5f
commit 3f8218af1b
3 changed files with 0 additions and 230 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
lib/checkclass.cpp
View File

@ -1328,58 +1328,6 @@ void CheckClass::memsetErrorFloat(const Token *tok, const std::string &type)
} }
//---------------------------------------------------------------------------
// ClassCheck: "void operator=(" and "const type & operator=("
//---------------------------------------------------------------------------
void CheckClass::operatorEq()
{
if (!mSettings->isEnabled(Settings::STYLE))
return;
for (const Scope * scope : mSymbolDatabase->classAndStructScopes) {
for (std::list<Function>::const_iterator func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
if (func->type == Function::eOperatorEqual && func->access == AccessControl::Public) {
// skip "deleted" functions - cannot be called anyway
if (func->isDelete())
continue;
// use definition for check so we don't have to deal with qualification
bool returnSelfRef = false;
if (func->retDef->str() == scope->className) {
if (Token::Match(func->retDef, "%type% &")) {
returnSelfRef = true;
} else {
// We might have "Self<template_parameters>&""
const Token * const tok = func->retDef->next();
if (tok && tok->str() == "<" && tok->link() && tok->link()->next() && tok->link()->next()->str() == "&")
returnSelfRef = true;
}
}
if (!returnSelfRef) {
// make sure we really have a copy assignment operator
const Token *paramTok = func->tokenDef->tokAt(2);
if (Token::Match(paramTok, "const| %name% &")) {
if (paramTok->str() == "const" &&
paramTok->strAt(1) == scope->className)
operatorEqReturnError(func->retDef, scope->className);
else if (paramTok->str() == scope->className)
operatorEqReturnError(func->retDef, scope->className);
}
}
}
}
}
}
void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className)
{
reportError(tok, Severity::style, "operatorEq",
"$symbol:" + className +"\n"
"'$symbol::operator=' should return '$symbol &'.\n"
"The $symbol::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '$symbol &$symbol::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers."
, CWE398, false);
}
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// ClassCheck: "C& operator=(const C&) { ... return *this; }" // ClassCheck: "C& operator=(const C&) { ... return *this; }"
// operator= should return a reference to *this // operator= should return a reference to *this

7
lib/checkclass.h
View File

@ -62,7 +62,6 @@ public:
// can't be a simplified check .. the 'sizeof' is used. // can't be a simplified check .. the 'sizeof' is used.
checkClass.checkMemset(); checkClass.checkMemset();
checkClass.constructors(); checkClass.constructors();
checkClass.operatorEq();
checkClass.privateFunctions(); checkClass.privateFunctions();
checkClass.operatorEqRetRefThis(); checkClass.operatorEqRetRefThis();
checkClass.thisSubtraction(); checkClass.thisSubtraction();
@ -104,9 +103,6 @@ public:
void checkMemset(); void checkMemset();
void checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation, std::set<const Scope *> parsedTypes); void checkMemsetType(const Scope *start, const Token *tok, const Scope *type, bool allocation, std::set<const Scope *> parsedTypes);
/** @brief 'operator=' should return something and it should not be const. */
void operatorEq();
/** @brief 'operator=' should return reference to *this */ /** @brief 'operator=' should return reference to *this */
void operatorEqRetRefThis(); // Warning upon no "return *this;" void operatorEqRetRefThis(); // Warning upon no "return *this;"
@ -170,7 +166,6 @@ private:
void memsetErrorFloat(const Token *tok, const std::string &type); void memsetErrorFloat(const Token *tok, const std::string &type);
void mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname); void mallocOnClassError(const Token* tok, const std::string &memfunc, const Token* classTok, const std::string &classname);
void mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok); void mallocOnClassWarning(const Token* tok, const std::string &memfunc, const Token* classTok);
void operatorEqReturnError(const Token *tok, const std::string &className);
void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive); void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived, bool inconclusive);
void thisSubtractionError(const Token *tok); void thisSubtractionError(const Token *tok);
void operatorEqRetRefThisError(const Token *tok); void operatorEqRetRefThisError(const Token *tok);
@ -208,7 +203,6 @@ private:
c.memsetErrorFloat(nullptr, "class"); c.memsetErrorFloat(nullptr, "class");
c.mallocOnClassWarning(nullptr, "malloc", nullptr); c.mallocOnClassWarning(nullptr, "malloc", nullptr);
c.mallocOnClassError(nullptr, "malloc", nullptr, "std::string"); c.mallocOnClassError(nullptr, "malloc", nullptr, "std::string");
c.operatorEqReturnError(nullptr, "class");
c.virtualDestructorError(nullptr, "Base", "Derived", false); c.virtualDestructorError(nullptr, "Base", "Derived", false);
c.thisSubtractionError(nullptr); c.thisSubtractionError(nullptr);
c.operatorEqRetRefThisError(nullptr); c.operatorEqRetRefThisError(nullptr);
@ -244,7 +238,6 @@ private:
"- Warn if memory for classes is allocated with malloc()\n" "- Warn if memory for classes is allocated with malloc()\n"
"- If it's a base class, check that the destructor is virtual\n" "- If it's a base class, check that the destructor is virtual\n"
"- Are there unused private functions?\n" "- Are there unused private functions?\n"
"- 'operator=' should return reference to self\n"
"- 'operator=' should check for assignment to self\n" "- 'operator=' should check for assignment to self\n"
"- Constness for member functions\n" "- Constness for member functions\n"
"- Order of initializations\n" "- Order of initializations\n"

171
test/testclass.cpp
View File

@ -74,11 +74,6 @@ private:
TEST_CASE(noOperatorEq); // class with memory management should have operator eq TEST_CASE(noOperatorEq); // class with memory management should have operator eq
TEST_CASE(noDestructor); // class with memory management should have destructor TEST_CASE(noDestructor); // class with memory management should have destructor
TEST_CASE(operatorEq1);
TEST_CASE(operatorEq2);
TEST_CASE(operatorEq3); // ticket #3051
TEST_CASE(operatorEq4); // ticket #3114
TEST_CASE(operatorEq5); // ticket #3296
TEST_CASE(operatorEqRetRefThis1); TEST_CASE(operatorEqRetRefThis1);
TEST_CASE(operatorEqRetRefThis2); // ticket #1323 TEST_CASE(operatorEqRetRefThis2); // ticket #1323
TEST_CASE(operatorEqRetRefThis3); // ticket #1405 TEST_CASE(operatorEqRetRefThis3); // ticket #1405
@ -966,172 +961,6 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
// Check the operator Equal
void checkOpertorEq(const char code[]) {
// Clear the error log
errout.str("");
settings0.inconclusive = true;
// Tokenize..
Tokenizer tokenizer(&settings0, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
// Check..
CheckClass checkClass(&tokenizer, &settings0, this);
checkClass.operatorEq();
}
void operatorEq1() {
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" void goo() {}"
" void operator=(const A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" void goo() {}"
" void operator=(const A&)=delete;\n"
"};");
ASSERT_EQUALS("", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" void goo() {}"
" void operator=(A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
checkOpertorEq("class A\n"
"{\n"
"private:\n"
" void operator=(const A&);\n"
"};");
ASSERT_EQUALS("", errout.str());
checkOpertorEq("class A\n"
"{\n"
"protected:\n"
" void operator=(const A&);\n"
"};");
ASSERT_EQUALS("", errout.str());
checkOpertorEq("class A\n"
"{\n"
"private:\n"
" void operator=(const A&)=delete;\n"
"};");
ASSERT_EQUALS("", errout.str());
checkOpertorEq("class A\n"
"{\n"
" void operator=(const A&);\n"
"};");
ASSERT_EQUALS("", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" void goo() {}\n"
"private:\n"
" void operator=(const A&);\n"
"};");
ASSERT_EQUALS("", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" void operator=(const A&);\n"
"};\n"
"class B\n"
"{\n"
"public:\n"
" void operator=(const B&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n"
"[test.cpp:9]: (style) 'B::operator=' should return 'B &'.\n", errout.str());
checkOpertorEq("struct A\n"
"{\n"
" void operator=(const A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:3]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
checkOpertorEq("struct A\n"
"{\n"
" void operator=(const A&)=delete;\n"
"};");
ASSERT_EQUALS("", errout.str());
// Ticket #7017
checkOpertorEq("template<class T> struct X {\n"
" inline X(const X& Rhs);\n"
" inline X<T>& operator =(const X& Rhs);\n"
"};");
ASSERT_EQUALS("", errout.str());
}
void operatorEq2() {
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" void * operator=(const A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" A * operator=(const A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" const A & operator=(const A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" B & operator=(const A&);\n"
"};");
ASSERT_EQUALS("[test.cpp:4]: (style) 'A::operator=' should return 'A &'.\n", errout.str());
}
void operatorEq3() { // ticket #3051
checkOpertorEq("class A\n"
"{\n"
"public:\n"
" A * operator=(const A*);\n"
"};");
ASSERT_EQUALS("", errout.str());
}
void operatorEq4() { // ticket #3114 (infinite loop)
checkOpertorEq("struct A {\n"
" A& operator=(A const& a) { return operator=(&a); }\n"
" A& operator=(const A*) { return *this; }\n"
"};");
ASSERT_EQUALS("", errout.str());
}
void operatorEq5() { // ticket #3296 (virtual operator)
checkOpertorEq(
"class A {\n"
" virtual A& operator=(const A &a) {return *this};\n"
"};");
ASSERT_EQUALS("", errout.str());
}
// Check that operator Equal returns reference to this // Check that operator Equal returns reference to this
void checkOpertorEqRetRefThis(const char code[]) { void checkOpertorEqRetRefThis(const char code[]) {
// Clear the error log // Clear the error log