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

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 -->

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;
 }

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

gui/codeeditstyledialog.cpp

@@ -235,7 +235,7 @@ void StyleEditDialog::updateStyle()
     mSampleEditor->setStyle(mStyleOutgoing);
 }
 
-CodeEditorStyle StyleEditDialog::getStyle()
+CodeEditorStyle StyleEditDialog::getStyle() const
 {
     return mStyleOutgoing;
 }

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();

gui/resultstree.cpp

@@ -1524,7 +1524,7 @@ void ResultsTree::setCheckDirectory(const QString &dir)
 }
 
 
-const QString& ResultsTree::getCheckDirectory()
+const QString& ResultsTree::getCheckDirectory() const
 {
     return mCheckPath;
 }

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.

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();
 }

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

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);
+    }
 }
 
 //---------------------------------------------------------------------------

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);

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

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;
 

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() {}
     };
 

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"