[clang-tidy] Fix false negative in readability-convert-member-functions-to-static

[njames93]

Summary

A nested class in a member function can erroneously confuse the the check into thinking that any CXXThisExpr found relate to the outer class, preventing any warnings.
Fix this by not traversing any nested classes.

Fixes https://github.com/llvm/llvm-project/issues/56765

[njames93]

Ping

[PiotrZSL]

Looks good, release notes need to be corrected (order of checks) (even that it apply without conflicts) and could land.
Test properly fail without fix.

clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp

AST_MATCHER(CXXMethodDecl, usesThis) {
   class FindUsageOfThis : public RecursiveASTVisitor<FindUsageOfThis> {
   public:
     bool Used = false;
 
     bool VisitCXXThisExpr(const CXXThisExpr *E) {
       Used = true;
       return false; // Stop traversal.
     }
+
+    // If we enter a class declaration, don't traverse into it as any usages of
+    // `this` will correspond to the nested class.
+    bool TraverseCXXRecordDecl(CXXRecordDecl *RD) { return true; }
+
   } UsageOfThis;
 
   // TraverseStmt does not modify its argument.
   UsageOfThis.TraverseStmt(const_cast<Stmt *>(Node.getBody()));
 
   return UsageOfThis.Used;
 }
 

clang-tools-extra/docs/ReleaseNotes.rst

 ^^^^^^^^^^
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Fixed a false negative in :doc:`readability-convert-member-functions-to-static
+  <clang-tidy/checks/readability/convert-member-functions-to-static>` when a 
+  nested class in a member function uses a ``this`` pointer.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp

int read_static() {
     return static_field;
   }
   void write_static() {
     // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'write_static' can be made static
     // CHECK-FIXES: {{^}}  static void write_static() {
     static_field = 1;
   }
 
+  void static_nested() {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'static_nested' can be made static
+    // CHECK-FIXES: {{^}}  static void static_nested() {
+    struct Nested {
+      int Foo;
+      int getFoo() { return Foo; }
+    };
+  }
+
+  void write_nested() {
+    struct Nested {
+      int Foo;
+      int getFoo() { return Foo; }
+    };
+    // Ensure we still detect usages of `this` once we leave the nested class definition.
+    field = 1;
+  }
+
   static int already_static() { return static_field; }
 
   int already_const() const { return field; }
 
   int already_const_convert_to_static() const {
     // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'already_const_convert_to_static' can be made static
     // CHECK-FIXES: {{^}}  static int already_const_convert_to_static() {
     return static_field;