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

uselessOverride: Detect code duplication in overriding function (#5219)

This commit is contained in:
chrchr-github 2023-07-05 22:58:01 +02:00 committed by GitHub
parent ee5cf0f141
commit dde45455bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 55 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
lib/checkclass.cpp
View File

@ -3036,7 +3036,7 @@ void CheckClass::overrideError(const Function *funcInBase, const Function *funcI
Certainty::normal);
}
void CheckClass::uselessOverrideError(const Function *funcInBase, const Function *funcInDerived)
void CheckClass::uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode)
{
const std::string functionName = funcInDerived ? ((funcInDerived->isDestructor() ? "~" : "") + funcInDerived->name()) : "";
const std::string funcType = (funcInDerived && funcInDerived->isDestructor()) ? "destructor" : "function";
@ -3047,9 +3047,15 @@ void CheckClass::uselessOverrideError(const Function *funcInBase, const Function
errorPath.emplace_back(funcInDerived->tokenDef, char(std::toupper(funcType[0])) + funcType.substr(1) + " in derived class");
}
std::string errStr = "\nThe " + funcType + " '$symbol' overrides a " + funcType + " in a base class but ";
if (isSameCode) {
errStr += "is identical to the overridden function";
}
else
errStr += "just delegates back to the base class.";
reportError(errorPath, Severity::style, "uselessOverride",
"$symbol:" + functionName + "\n"
"The " + funcType + " '$symbol' overrides a " + funcType + " in a base class but just delegates back to the base class.",
"$symbol:" + functionName +
errStr,
CWE(0U) /* Unknown CWE! */,
Certainty::normal);
}
@ -3071,6 +3077,23 @@ static const Token* getSingleFunctionCall(const Scope* scope) {
return nullptr;
}
static bool compareTokenRanges(const Token* start1, const Token* end1, const Token* start2, const Token* end2) {
const Token* tok1 = start1;
const Token* tok2 = start2;
bool isEqual = false;
while (tok1 && tok2) {
if (tok1->str() != tok2->str())
break;
if (tok1 == end1 && tok2 == end2) {
isEqual = true;
break;
}
tok1 = tok1->next();
tok2 = tok2->next();
}
return isEqual;
}
void CheckClass::checkUselessOverride()
{
if (!mSettings->severity.isEnabled(Severity::style))
@ -3093,6 +3116,18 @@ void CheckClass::checkUselessOverride()
return f.name() == func.name();
}))
continue;
if (baseFunc->functionScope) {
bool isSameCode = compareTokenRanges(baseFunc->argDef, baseFunc->argDef->link(), func.argDef, func.argDef->link()); // function arguments
if (isSameCode) {
isSameCode = compareTokenRanges(baseFunc->functionScope->bodyStart, baseFunc->functionScope->bodyEnd, // function body
func.functionScope->bodyStart, func.functionScope->bodyEnd);
if (isSameCode) {
uselessOverrideError(baseFunc, &func, true);
continue;
}
}
}
if (const Token* const call = getSingleFunctionCall(func.functionScope)) {
if (call->function() != baseFunc)
continue;

2
lib/checkclass.h
View File

@ -231,7 +231,7 @@ private:
void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedName, const std::string &baseName, const std::string &variableName, bool derivedIsStruct, bool baseIsStruct);
void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
void overrideError(const Function *funcInBase, const Function *funcInDerived);
void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived);
void uselessOverrideError(const Function *funcInBase, const Function *funcInDerived, bool isSameCode = false);
void thisUseAfterFree(const Token *self, const Token *free, const Token *use);
void unsafeClassRefMemberError(const Token *tok, const std::string &varname);
void checkDuplInheritedMembersRecursive(const Type* typeCurrent, const Type* typeBase);

4
lib/valueflow.cpp
View File

@ -7239,10 +7239,6 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer {
return false;
}
bool isGlobal() const override {
return false;
}
bool lowerToPossible() override {
for (auto&& p:values) {
if (p.second.isImpossible())

16
test/testclass.cpp
View File

@ -8449,6 +8449,22 @@ private:
" }\n"
"};");
ASSERT_EQUALS("", errout.str());
checkUselessOverride("struct B {\n"
" virtual int f() { return 1; }\n"
" virtual int g() { return 7; }\n"
" virtual int h(int i, int j) { return i + j; }\n"
" virtual int j(int i, int j) { return i + j; }\n"
"};\n"
"struct D : B {\n"
" int f() override { return 2; }\n"
" int g() override { return 7; }\n"
" int h(int j, int i) override { return i + j; }\n"
" int j(int i, int j) override { return i + j; }\n"
"};");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:9]: (style) The function 'g' overrides a function in a base class but is identical to the overridden function\n"
"[test.cpp:5] -> [test.cpp:11]: (style) The function 'j' overrides a function in a base class but is identical to the overridden function\n",
errout.str());
}
#define checkUnsafeClassRefMember(code) checkUnsafeClassRefMember_(code, __FILE__, __LINE__)