Fix #13282 FN functionConst with unknown base class (danmar#6990)

chrchr-github · web-flow · commit 0003ad2951e9 · 2024-12-05T20:48:41.000+01:00
diff --git a/cfg/qt.cfg b/cfg/qt.cfg
@@ -5359,7 +5359,7 @@
   <define name="Q_LIKELY(expr)" value="expr"/>
   <define name="Q_NAMESPACE" value=""/>
   <define name="Q_NULLPTR" value="NULL"/>
-  <define name="Q_OBJECT" value="static void qt_static_metacall(QObject*,QMetaObject::Call,int,void**);const MetaObject* metaObject() const;void* qt_metacast(const char*);int qt_metacall(QMetaObject::Call,int,void**);"/>
+  <define name="Q_OBJECT" value="static void qt_static_metacall(QObject*,QMetaObject::Call,int,void**);virtual const MetaObject* metaObject() const;virtual void* qt_metacast(const char*);virtual int qt_metacall(QMetaObject::Call,int,void**);"/>
   <define name="Q_PRIVATE_SLOT(d, signature)" value=""/>
   <define name="Q_SLOTS" value=""/>
   <!-- Treat as variadic macro to avoid preprocessorErrorDirective -->
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -135,7 +135,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
@@ -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
@@ -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
@@ -2172,7 +2172,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;
@@ -2200,9 +2201,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);
         }
     }
 }
@@ -2277,16 +2278,21 @@ 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) {
                 if (isMemberVar(derivedFrom->classScope, tok))
                     return true;
             }
         }
+        if (!foundAllBaseClasses)
+            return true;
     }
 
     return false;
@@ -2609,35 +2615,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<const Token *> 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
@@ -194,8 +194,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
@@ -4576,13 +4576,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
@@ -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<const Function*> getOverloadedFunctions() const;
 
diff --git 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
@@ -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"

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ void SelectColorButton::setColor(const QColor& color)`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`// cppcheck-suppress unusedFunction`
`68`		`-const QColor& SelectColorButton::getColor()`
	`68`	`+const QColor& SelectColorButton::getColor() const`
`69`	`69`	`{`
`70`	`70`	`return mColor;`
`71`	`71`	`}`
`@@ -122,7 +122,7 @@ void SelectFontWeightCombo::setWeight(QFont::Weight weight)`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`// cppcheck-suppress unusedFunction`
`125`		`-const QFont::Weight& SelectFontWeightCombo::getWeight()`
	`125`	`+const QFont::Weight& SelectFontWeightCombo::getWeight() const`
`126`	`126`	`{`
`127`	`127`	`return mWeight;`
`128`	`128`	`}`
Original file line number	Diff line number	Diff line change
`@@ -235,7 +235,7 @@ void StyleEditDialog::updateStyle()`
`235`	`235`	`mSampleEditor->setStyle(mStyleOutgoing);`
`236`	`236`	`}`
`237`	`237`
`238`		`-CodeEditorStyle StyleEditDialog::getStyle()`
	`238`	`+CodeEditorStyle StyleEditDialog::getStyle() const`
`239`	`239`	`{`
`240`	`240`	`return mStyleOutgoing;`
`241`	`241`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1524,7 +1524,7 @@ void ResultsTree::setCheckDirectory(const QString &dir)`
`1524`	`1524`	`}`
`1525`	`1525`
`1526`	`1526`
`1527`		`-const QString& ResultsTree::getCheckDirectory()`
	`1527`	`+const QString& ResultsTree::getCheckDirectory() const`
`1528`	`1528`	`{`
`1529`	`1529`	`return mCheckPath;`
`1530`	`1530`	`}`
Original file line number	Diff line number	Diff line change
`@@ -262,7 +262,7 @@ void ResultsView::printPreview()`
`262`	`262`	`dialog.exec();`
`263`	`263`	`}`
`264`	`264`
`265`		`-void ResultsView::print(QPrinter* printer)`
	`265`	`+void ResultsView::print(QPrinter* printer) const`
`266`	`266`	`{`
`267`	`267`	`if (!hasResults()) {`
`268`	`268`	`QMessageBox msgBox;`
`@@ -300,7 +300,7 @@ void ResultsView::setCheckDirectory(const QString &dir)`
`300`	`300`	`mUI->mTree->setCheckDirectory(dir);`
`301`	`301`	`}`
`302`	`302`
`303`		`-QString ResultsView::getCheckDirectory()`
	`303`	`+QString ResultsView::getCheckDirectory() const`
`304`	`304`	`{`
`305`	`305`	`return mUI->mTree->getCheckDirectory();`
`306`	`306`	`}`