[clang-tools-extra] [clang-tidy] Add C++ member function support to custom bugprone-unsafe-functions matches (PR #117165)

[via cfe-commits]

https://github.com/Discookie updated https://github.com/llvm/llvm-project/pull/117165

>From e90ab99dde0945d103959fa73ea2d31852f753e7 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Thu, 21 Nov 2024 14:33:24 +0000
Subject: [PATCH] [clang-tidy] Add C++ member function support to user-defined
 bugprone-unsafe-functions matches

---
 .../bugprone/UnsafeFunctionsCheck.cpp         | 38 ++++++++++++++-----
 clang-tools-extra/docs/ReleaseNotes.rst       |  2 +-
 .../unsafe-functions-custom-regex.cpp         | 25 +++++++++++-
 3 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index 604a7cac0e4903..a45949314a4ca8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -256,13 +256,32 @@ void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) {
                                           .bind(CustomFunctionNamesId)))
                            .bind(DeclRefId),
                        this);
+    // C++ member calls do not contain a DeclRefExpr to the function decl.
+    // Instead, they contain a MemberExpr that refers to the decl.
+    Finder->addMatcher(memberExpr(member(functionDecl(CustomFunctionsMatcher)
+                                             .bind(CustomFunctionNamesId)))
+                           .bind(DeclRefId),
+                       this);
   }
 }
 
 void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId);
-  const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
-  assert(DeclRef && FuncDecl && "No valid matched node in check()");
+  const Expr *SourceExpr;
+  const FunctionDecl *FuncDecl;
+
+  if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId)) {
+    SourceExpr = DeclRef;
+    FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
+  } else if (const auto *Member =
+                 Result.Nodes.getNodeAs<MemberExpr>(DeclRefId)) {
+    SourceExpr = Member;
+    FuncDecl = cast<FunctionDecl>(Member->getMemberDecl());
+  } else {
+    llvm_unreachable("No valid matched node in check()");
+    return;
+  }
+
+  assert(SourceExpr && FuncDecl && "No valid matched node in check()");
 
   // Only one of these are matched at a time.
   const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>(
@@ -286,14 +305,15 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
             Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str();
 
         if (Entry.Replacement.empty()) {
-          diag(DeclRef->getExprLoc(), "function %0 %1; it should not be used")
+          diag(SourceExpr->getExprLoc(),
+               "function %0 %1; it should not be used")
               << FuncDecl << Reason << Entry.Replacement
-              << DeclRef->getSourceRange();
+              << SourceExpr->getSourceRange();
         } else {
-          diag(DeclRef->getExprLoc(),
+          diag(SourceExpr->getExprLoc(),
                "function %0 %1; '%2' should be used instead")
               << FuncDecl << Reason << Entry.Replacement
-              << DeclRef->getSourceRange();
+              << SourceExpr->getSourceRange();
         }
 
         return;
@@ -323,9 +343,9 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
   if (!ReplacementFunctionName)
     return;
 
-  diag(DeclRef->getExprLoc(), "function %0 %1; '%2' should be used instead")
+  diag(SourceExpr->getExprLoc(), "function %0 %1; '%2' should be used instead")
       << FuncDecl << getRationaleFor(FunctionName)
-      << ReplacementFunctionName.value() << DeclRef->getSourceRange();
+      << ReplacementFunctionName.value() << SourceExpr->getSourceRange();
 }
 
 void UnsafeFunctionsCheck::registerPPCallbacks(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f967dfabd1c940..94f70c51f00a81 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,7 +192,7 @@ Changes in existing checks
 
 - Improved :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
-  additional functions to match.
+  additional functions to match, including C++ member functions.
 
 - Improved :doc:`bugprone-use-after-move
   <clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp
index fc97d1bc93bc54..b707e471ef7cc4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp
@@ -1,11 +1,16 @@
 // RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX         %s bugprone-unsafe-functions %t --\
-// RUN:   -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix'}}"
+// RUN:   -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix;^::S::member_match_,,is matched on a C++ class member'}}"
 // RUN: %check_clang_tidy -check-suffix=STRICT-REGEX         %s bugprone-unsafe-functions %t --\
-// RUN:   -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match'}}"
+// RUN:   -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match;^::S::member_match_1$,,is matched on a C++ class member'}}"
 
 void name_match();
 void prefix_match();
 
+struct S {
+  static void member_match_1() {}
+  void member_match_2() {}
+};
+
 namespace regex_test {
 void name_match();
 void prefix_match();
@@ -42,3 +47,19 @@ void f1() {
   // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used
   // no-warning STRICT-REGEX
 }
+
+void f2() {
+  S s;
+
+  S::member_match_1();
+  // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+  // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+
+  s.member_match_1();
+  // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+  // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+
+  s.member_match_2();
+  // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_2' is matched on a C++ class member; it should not be used
+  // no-warning STRICT-REGEX
+}