Fix #12503 (usability: bogus suggestion in functionStatic message) (danmar#7978)

Author: danmar

lib/checkclass.cpp

@@ -2130,10 +2130,6 @@ void CheckClass::thisSubtractionError(const Token *tok)
 
 void CheckClass::checkConst()
 {
-    // This is an inconclusive check. False positives: #3322.
-    if (!mSettings->certainty.isEnabled(Certainty::inconclusive))
-        return;
-
     if (!mSettings->severity.isEnabled(Severity::style) &&
         !mSettings->isPremiumEnabled("functionConst") &&
         !mSettings->isPremiumEnabled("functionStatic"))
@@ -2220,6 +2216,9 @@ void CheckClass::checkConst()
             const bool suggestStatic = memberAccessed != MemberAccess::MEMBER && !func.isOperator();
             if ((returnsPtrOrRef || func.isConst() || func.hasLvalRefQualifier()) && !suggestStatic)
                 continue;
+            if (!suggestStatic && !mSettings->certainty.isEnabled(Certainty::inconclusive))
+                // functionConst is inconclusive. False positives: #3322.
+                continue;
             if (suggestStatic && func.isConst()) {
                 const auto overloads = func.getOverloadedFunctions();
                 if (overloads.size() > 1 && std::any_of(overloads.begin(), overloads.end(), [&](const Function* ovl) {
@@ -2717,16 +2716,10 @@ void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const st
     }
     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"
-                    + 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);
+                                "The member function '$symbol' can be static." :
+                                "Either there is a missing 'override', or the member function '$symbol' can be static.";
+        reportError(toks, Severity::style, "functionStatic",
+                    "$symbol:" + classname + "::" + funcname +"\n" + msg, CWE398, Certainty::normal);
     }
 }
 

man/checkers/functionConst.md

@@ -0,0 +1,101 @@
+# functionConst
+
+**Message**: Technically the member function 'x' can be const<br/>
+**Category**: Robustness<br/>
+**Severity**: Style (Inconclusive)<br/>
+**Language**: C++
+
+## Description
+
+This checker identifies member functions that do not modify any member variables and therefore could be declared as `const`. A const member function promises not to modify the object's state and enables the function to be called on const objects.
+
+The danger is that a const member function is allowed to have side effects outside the object. If you create a member function that formats the hard drive using
+system calls it might be technically possible to make the function const, but is it "correct"? Using a const object is not supposed to have side effects.
+
+For methods that has no side effects whatsoever; making them const is recommended.
+
+The checker analyzes member functions and detects when:
+- The function only reads member variables (never writes to them)
+- The function only calls other const member functions
+- The function only modifies parameters passed by reference or pointer (not member variables)
+- The function performs operations that don't change the object's logical state
+
+This warning is marked as **inconclusive** because while the function can technically be made const, it may not always be "correct".
+
+This warning helps improve code quality by:
+- Making the function's non-modifying nature explicit
+- Enabling the function to be called on const objects
+- Improving const-correctness throughout the codebase
+- Helping with compiler optimizations
+- Making code intentions clearer to other developers
+
+## Motivation
+
+The motivation of this checker is to improve robustness by making the code more const-correct.
+
+## How to fix
+
+Add the `const` keyword after the function signature to indicate that the function does not modify the object's state.
+
+Before:
+```cpp
+class Rectangle {
+    int width, height;
+public:
+    int getWidth() { return width; }
+    int getHeight() { return height; }
+    int getArea() { return width * height; }
+
+    void printInfo() {
+        std::cout << "Width: " << width << ", Height: " << height << std::endl;
+    }
+
+    bool isSquare() {
+        return width == height;
+    }
+
+    void copyDataTo(Rectangle& other) {
+        other.width = width;
+        other.height = height;
+    }
+};
+```
+
+After:
+```cpp
+class Rectangle {
+    int width, height;
+public:
+    int getWidth() const { return width; }
+    int getHeight() const { return height; }
+    int getArea() const { return width * height; }
+
+    void printInfo() const {
+        std::cout << "Width: " << width << ", Height: " << height << std::endl;
+    }
+
+    bool isSquare() const {
+        return width == height;
+    }
+
+    void copyDataTo(Rectangle& other) const {
+        other.width = width;
+        other.height = height;
+    }
+};
+```
+
+## Related checkers
+
+- `functionStatic` - for member functions that can be declared static
+- `constParameter` - for function parameters that can be const
+- `constParameterReference` - for reference parameters that can be const
+- `constParameterPointer` - for pointer parameters that can be const
+- `constVariable` - for local variables that can be const
+- `constVariableReference` - for local reference variables that can be const
+
+## Notes
+
+- This check is marked as **inconclusive** because the decision to make a function const should also consider the conceptual design
+- Virtual functions should be carefully considered before making them const, as this affects the entire inheritance hierarchy
+- Think about whether the function's purpose is to query state (should be const) or to perform an action (may not need to be const)

man/checkers/functionStatic.md

@@ -0,0 +1,64 @@
+# functionStatic
+
+**Message**: The member function 'x' can be static<br/>
+**Category**: Readability<br/>
+**Severity**: Style<br/>
+**Language**: C++
+
+## Description
+
+This checker identifies member functions that do not access any non-static member variables or call any non-static member functions. Such functions can be declared as `static` to indicate that they don't require an object instance to operate.
+
+This warning helps improve code quality by:
+- Making the function's independence from object state explicit
+- Enabling the function to be called without creating an object instance
+- Clarifying the function's scope and dependencies
+
+## Motivation
+
+The motivation of this checker is to improve readability.
+
+## How to fix
+
+Add the `static` keyword to the function declaration to indicate that it doesn't require an object instance.
+
+Before:
+```cpp
+class Calculator {
+public:
+    int add(int a, int b) {
+        return a + b;  // Only uses parameters
+    }
+
+    void printMessage() {
+        std::cout << "Hello World" << std::endl;  // Uses no instance data
+    }
+
+    bool isValidNumber(int num) {
+        return num > 0 && num < 1000;  // Pure function
+    }
+};
+```
+
+After:
+```cpp
+class Calculator {
+public:
+    static int add(int a, int b) {
+        return a + b;  // Can be called as Calculator::add(5, 3)
+    }
+
+    static void printMessage() {
+        std::cout << "Hello World" << std::endl;  // Can be called without instance
+    }
+
+    static bool isValidNumber(int num) {
+        return num > 0 && num < 1000;  // Clearly indicates no state dependency
+    }
+};
+```
+
+## Related checkers
+
+- `functionConst` - for member functions that can be declared const
+

test/cli/proj-inline-suppress-unusedFunction/B.hpp

@@ -2,5 +2,5 @@ class B {
 public:
     B();
 
-    void unusedFunctionTest();
+    static void unusedFunctionTest();
 };