diff --git a/cfg/qt.cfg b/cfg/qt.cfg index 15acfb0c6af..83f35d617a5 100644 --- a/cfg/qt.cfg +++ b/cfg/qt.cfg @@ -5359,7 +5359,7 @@ - + diff --git a/gui/codeeditstylecontrols.cpp b/gui/codeeditstylecontrols.cpp index 99121c10afd..1623d75a68a 100644 --- a/gui/codeeditstylecontrols.cpp +++ b/gui/codeeditstylecontrols.cpp @@ -65,7 +65,7 @@ void SelectColorButton::setColor(const QColor& color) } // cppcheck-suppress unusedFunction -const QColor& SelectColorButton::getColor() +const QColor& SelectColorButton::getColor() const { return mColor; } @@ -122,7 +122,7 @@ void SelectFontWeightCombo::setWeight(QFont::Weight weight) } // cppcheck-suppress unusedFunction -const QFont::Weight& SelectFontWeightCombo::getWeight() +const QFont::Weight& SelectFontWeightCombo::getWeight() const { return mWeight; } diff --git a/gui/codeeditstylecontrols.h b/gui/codeeditstylecontrols.h index dbc7aedeed1..e40746a77a3 100644 --- a/gui/codeeditstylecontrols.h +++ b/gui/codeeditstylecontrols.h @@ -36,7 +36,7 @@ class SelectColorButton : public QPushButton { explicit SelectColorButton(QWidget* parent); void setColor(const QColor& color); - const QColor& getColor(); + const QColor& getColor() const; signals: // NOLINTNEXTLINE(readability-inconsistent-declaration-parameter-name) - caused by generated MOC code @@ -57,7 +57,7 @@ class SelectFontWeightCombo : public QComboBox { explicit SelectFontWeightCombo(QWidget* parent); void setWeight(QFont::Weight weight); - const QFont::Weight& getWeight(); + const QFont::Weight& getWeight() const; signals: // NOLINTNEXTLINE(readability-inconsistent-declaration-parameter-name) - caused by generated MOC code diff --git a/gui/codeeditstyledialog.cpp b/gui/codeeditstyledialog.cpp index b7b782ffc39..33bd9ff7e1a 100644 --- a/gui/codeeditstyledialog.cpp +++ b/gui/codeeditstyledialog.cpp @@ -235,7 +235,7 @@ void StyleEditDialog::updateStyle() mSampleEditor->setStyle(mStyleOutgoing); } -CodeEditorStyle StyleEditDialog::getStyle() +CodeEditorStyle StyleEditDialog::getStyle() const { return mStyleOutgoing; } diff --git a/gui/codeeditstyledialog.h b/gui/codeeditstyledialog.h index bd7a70ac2a5..92b31efd46a 100644 --- a/gui/codeeditstyledialog.h +++ b/gui/codeeditstyledialog.h @@ -43,7 +43,7 @@ class StyleEditDialog : public QDialog { explicit StyleEditDialog(const CodeEditorStyle& newStyle, QWidget *parent = nullptr); - CodeEditorStyle getStyle(); + CodeEditorStyle getStyle() const; private: void updateControls(); diff --git a/gui/resultstree.cpp b/gui/resultstree.cpp index c0e07969333..4e3001a6877 100644 --- a/gui/resultstree.cpp +++ b/gui/resultstree.cpp @@ -1524,7 +1524,7 @@ void ResultsTree::setCheckDirectory(const QString &dir) } -const QString& ResultsTree::getCheckDirectory() +const QString& ResultsTree::getCheckDirectory() const { return mCheckPath; } diff --git a/gui/resultstree.h b/gui/resultstree.h index ff5af1a9d09..370a2c0f256 100644 --- a/gui/resultstree.h +++ b/gui/resultstree.h @@ -137,7 +137,7 @@ class ResultsTree : public QTreeView { * @return Directory containing source files */ - const QString& getCheckDirectory(); + const QString& getCheckDirectory() const; /** * @brief Check if there are any visible results in view. diff --git a/gui/resultsview.cpp b/gui/resultsview.cpp index 709dc22b8d2..e76a427a5ee 100644 --- a/gui/resultsview.cpp +++ b/gui/resultsview.cpp @@ -262,7 +262,7 @@ void ResultsView::printPreview() dialog.exec(); } -void ResultsView::print(QPrinter* printer) +void ResultsView::print(QPrinter* printer) const { if (!hasResults()) { QMessageBox msgBox; @@ -300,7 +300,7 @@ void ResultsView::setCheckDirectory(const QString &dir) mUI->mTree->setCheckDirectory(dir); } -QString ResultsView::getCheckDirectory() +QString ResultsView::getCheckDirectory() const { return mUI->mTree->getCheckDirectory(); } diff --git a/gui/resultsview.h b/gui/resultsview.h index 8dc37b24591..771f04488ff 100644 --- a/gui/resultsview.h +++ b/gui/resultsview.h @@ -139,7 +139,7 @@ class ResultsView : public QWidget { * @return Directory containing source files */ - QString getCheckDirectory(); + QString getCheckDirectory() const; /** * Set settings used in checking @@ -331,7 +331,7 @@ public slots: * @brief Slot printing the current report to the printer. * @param printer The printer used for printing the report. */ - void print(QPrinter* printer); + void print(QPrinter* printer) const; /** * @brief Slot opening a print preview dialog diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 4b308a5fe64..cc68bcccbd5 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2176,7 +2176,8 @@ void CheckClass::checkConst() } // check if base class function is virtual - if (!scope->definedType->derivedFrom.empty() && func.isImplicitlyVirtual(true)) + bool foundAllBaseClasses = true; + if (!scope->definedType->derivedFrom.empty() && func.isImplicitlyVirtual(true, &foundAllBaseClasses) && foundAllBaseClasses) continue; MemberAccess memberAccessed = MemberAccess::NONE; @@ -2204,9 +2205,9 @@ void CheckClass::checkConst() functionName += "]"; if (func.isInline()) - checkConstError(func.token, classname, functionName, suggestStatic); + checkConstError(func.token, classname, functionName, suggestStatic, foundAllBaseClasses); else // not inline - checkConstError2(func.token, func.tokenDef, classname, functionName, suggestStatic); + checkConstError2(func.token, func.tokenDef, classname, functionName, suggestStatic, foundAllBaseClasses); } } } @@ -2281,9 +2282,12 @@ bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) const // not found in this class if (!scope->definedType->derivedFrom.empty()) { // check each base class + bool foundAllBaseClasses = true; for (const Type::BaseInfo & i : scope->definedType->derivedFrom) { // find the base class const Type *derivedFrom = i.type; + if (!derivedFrom) + foundAllBaseClasses = false; // find the function in the base class if (derivedFrom && derivedFrom->classScope && derivedFrom->classScope != scope) { @@ -2291,6 +2295,8 @@ bool CheckClass::isMemberVar(const Scope *scope, const Token *tok) const return true; } } + if (!foundAllBaseClasses) + return true; } return false; @@ -2613,35 +2619,41 @@ bool CheckClass::checkConstFunc(const Scope *scope, const Function *func, Member return true; } -void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic) +void CheckClass::checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic, bool foundAllBaseClasses) { - checkConstError2(tok, nullptr, classname, funcname, suggestStatic); + checkConstError2(tok, nullptr, classname, funcname, suggestStatic, foundAllBaseClasses); } -void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic) +void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic, bool foundAllBaseClasses) { std::list toks{ tok1 }; if (tok2) toks.push_back(tok2); - if (!suggestStatic) + if (!suggestStatic) { + const std::string msg = foundAllBaseClasses ? + "Technically the member function '$symbol' can be const.\nThe member function '$symbol' can be made a const " : + "Either there is a missing 'override', or the member function '$symbol' can be const.\nUnless it overrides a base class member, the member function '$symbol' can be made a const "; reportError(toks, Severity::style, "functionConst", "$symbol:" + classname + "::" + funcname +"\n" - "Technically the member function '$symbol' can be const.\n" - "The member function '$symbol' can be made a const " + + msg + "function. Making this function 'const' should not cause compiler errors. " "Even though the function can be made const function technically it may not make " "sense conceptually. Think about your design and the task of the function first - is " "it a function that must not change object internal state?", CWE398, Certainty::inconclusive); - else + } + else { + const std::string msg = foundAllBaseClasses ? + "Technically the member function '$symbol' can be static (but you may consider moving to unnamed namespace).\nThe member function '$symbol' can be made a static " : + "Either there is a missing 'override', or the member function '$symbol' can be static.\nUnless it overrides a base class member, the member function '$symbol' can be made a static "; reportError(toks, Severity::performance, "functionStatic", "$symbol:" + classname + "::" + funcname +"\n" - "Technically the member function '$symbol' can be static (but you may consider moving to unnamed namespace).\n" - "The member function '$symbol' can be made a static " + + msg + "function. Making a function static can bring a performance benefit since no 'this' instance is " "passed to the function. This change should not cause compiler errors but it does not " "necessarily make sense conceptually. Think about your design and the task of the function first - " "is it a function that must not access members of class instances? And maybe it is more appropriate " "to move this function to an unnamed namespace.", CWE398, Certainty::inconclusive); + } } //--------------------------------------------------------------------------- diff --git a/lib/checkclass.h b/lib/checkclass.h index 6cf0fb32f8b..2ec7fc1abe4 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -202,8 +202,8 @@ class CPPCHECKLIB CheckClass : public Check { void operatorEqShouldBeLeftUnimplementedError(const Token *tok); void operatorEqMissingReturnStatementError(const Token *tok, bool error); void operatorEqToSelfError(const Token *tok); - void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic); - void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic); + void checkConstError(const Token *tok, const std::string &classname, const std::string &funcname, bool suggestStatic, bool foundAllBaseClasses = true); + void checkConstError2(const Token *tok1, const Token *tok2, const std::string &classname, const std::string &funcname, bool suggestStatic, bool foundAllBaseClasses = true); void initializerListError(const Token *tok1,const Token *tok2, const std::string & classname, const std::string &varname, const std::string& argname = {}); void suggestInitializationList(const Token *tok, const std::string& varname); void selfInitializationError(const Token* tok, const std::string& varname); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index dde0b7ddf7e..fa1f286c8e9 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -4574,13 +4574,15 @@ void Function::addArguments(const SymbolDatabase *symbolDatabase, const Scope *s } } -bool Function::isImplicitlyVirtual(bool defaultVal) const +bool Function::isImplicitlyVirtual(bool defaultVal, bool* pFoundAllBaseClasses) const { if (hasVirtualSpecifier() || hasOverrideSpecifier() || hasFinalSpecifier()) return true; bool foundAllBaseClasses = true; if (getOverriddenFunction(&foundAllBaseClasses)) //If it overrides a base class's method then it's virtual return true; + if (pFoundAllBaseClasses) + *pFoundAllBaseClasses = foundAllBaseClasses; if (foundAllBaseClasses) //If we've seen all the base classes and none of the above were true then it must not be virtual return false; return defaultVal; //If we can't see all the bases classes then we can't say conclusively diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index bba22b78b3f..64c53ed15ff 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -778,7 +778,7 @@ class CPPCHECKLIB Function { void addArguments(const SymbolDatabase *symbolDatabase, const Scope *scope); /** @brief check if this function is virtual in the base classes */ - bool isImplicitlyVirtual(bool defaultVal = false) const; + bool isImplicitlyVirtual(bool defaultVal = false, bool* pFoundAllBaseClasses = nullptr) const; std::vector getOverloadedFunctions() const; diff --git a/test/cfg/qt.cpp b/test/cfg/qt.cpp index f00c15c7696..e77036a4636 100644 --- a/test/cfg/qt.cpp +++ b/test/cfg/qt.cpp @@ -665,6 +665,7 @@ namespace { class Fred : public QObject { Q_OBJECT private slots: + // cppcheck-suppress functionStatic void foo(); }; void Fred::foo() {} @@ -726,6 +727,7 @@ namespace { ~MyObject1() {} public slots: signals: + // cppcheck-suppress functionStatic void test() {} }; diff --git a/test/testclass.cpp b/test/testclass.cpp index 5fd45925183..bb57f47452b 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -187,6 +187,7 @@ class TestClass : public TestFixture { TEST_CASE(const93); TEST_CASE(const94); TEST_CASE(const95); // #13320 - do not warn about r-value ref method + TEST_CASE(const96); TEST_CASE(const_handleDefaultParameters); TEST_CASE(const_passThisToMemberOfOtherClass); @@ -5041,7 +5042,7 @@ class TestClass : public TestFixture { "public:\n" " void f(){}\n" "};"); - ASSERT_EQUALS("", errout_str()); + ASSERT_EQUALS("[test.cpp:3]: (performance, inconclusive) Either there is a missing 'override', or the member function 'derived::f' can be static.\n", errout_str()); } void const34() { // ticket #1964 @@ -5827,7 +5828,8 @@ class TestClass : public TestFixture { " inherited::set(inherited::Key(key));\n" " }\n" "};\n", nullptr, false); - ASSERT_EQUALS("", errout_str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (performance, inconclusive) Either there is a missing 'override', or the member function 'MixerParticipant::GetAudioFrame' can be static.\n", + errout_str()); } void const62() { @@ -6674,8 +6676,6 @@ class TestClass : public TestFixture { "struct S<0> {};\n" "struct D : S<150> {};\n"); // don't hang - // we are not interested in the output - just consume it - ignore_errout(); } void const93() { // #12162 @@ -6714,6 +6714,15 @@ class TestClass : public TestFixture { ASSERT_EQUALS("", errout_str()); } + void const96() { // #13282 + checkConst("struct S : B {\n" + " bool f() { return b; }\n" + " bool g() override { return b; }\n" + " bool b;\n" + "};\n"); + ASSERT_EQUALS("[test.cpp:2]: (style, inconclusive) Either there is a missing 'override', or the member function 'S::f' can be const.\n", errout_str()); + } + void const_handleDefaultParameters() { checkConst("struct Foo {\n" " void foo1(int i, int j = 0) {\n"