[clang-tools-extra] [clang-tidy][NFC]refactor matcher for bugprone-optional-value-conversion (PR #130415)

[Congcong Cai via cfe-commits]

https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/130415

The old `constructFrom` has hidden requirement which TypeMatcher must be used before ArgumentMatcher because there are bind inside.
Inlining this function to make it more intuitive.

>From ca39210f6a28569116759f9e62b6120ddd746968 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 8 Mar 2025 21:22:47 +0800
Subject: [PATCH] [clang-tidy][NFC]refactor matcher for
 bugprone-optional-value-conversion

The old `constructFrom` has hidden requirement which TypeMatcher must be used before ArgumentMatcher because there are bind inside.
Inlining this function to make it more intuitive.
---
 .../bugprone/OptionalValueConversionCheck.cpp | 42 +++++++++----------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
index 55ca4809f058a..33e823ac07490 100644
--- a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -27,28 +27,11 @@ AST_MATCHER_P(QualType, hasCleanType, Matcher<QualType>, InnerMatcher) {
       Finder, Builder);
 }
 
-constexpr std::array<StringRef, 2> NameList{
+constexpr std::array<StringRef, 2> MakeSmartPtrList{
     "::std::make_unique",
     "::std::make_shared",
 };
 
-Matcher<Expr> constructFrom(Matcher<QualType> TypeMatcher,
-                            Matcher<Expr> ArgumentMatcher) {
-  return expr(
-      anyOf(
-          // construct optional
-          cxxConstructExpr(argumentCountIs(1U), hasType(TypeMatcher),
-                           hasArgument(0U, ArgumentMatcher)),
-          // known template methods in std
-          callExpr(argumentCountIs(1),
-                   callee(functionDecl(
-                       matchers::matchesAnyListedName(NameList),
-                       hasTemplateArgument(0, refersToType(TypeMatcher)))),
-                   hasArgument(0, ArgumentMatcher))),
-      unless(anyOf(hasAncestor(typeLoc()),
-                   hasAncestor(expr(matchers::hasUnevaluatedContext())))));
-}
-
 } // namespace
 
 OptionalValueConversionCheck::OptionalValueConversionCheck(
@@ -74,7 +57,7 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
   auto EqualsBoundOptionalType =
       qualType(hasCleanType(equalsBoundNode("optional-type")));
 
-  auto OptionalDereferenceMatcher = callExpr(
+  auto OptionalDerefMatcherImpl = callExpr(
       anyOf(
           cxxOperatorCallExpr(hasOverloadedOperatorName("*"),
                               hasUnaryOperand(hasType(EqualsBoundOptionalType)))
@@ -88,11 +71,24 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
 
   auto StdMoveCallMatcher =
       callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
-               hasArgument(0, ignoringImpCasts(OptionalDereferenceMatcher)));
+               hasArgument(0, ignoringImpCasts(OptionalDerefMatcherImpl)));
+  auto OptionalDerefMatcher =
+      ignoringImpCasts(anyOf(OptionalDerefMatcherImpl, StdMoveCallMatcher));
+
   Finder->addMatcher(
-      expr(constructFrom(BindOptionalType,
-                         ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
-                                                StdMoveCallMatcher))))
+      expr(anyOf(
+               // construct optional
+               cxxConstructExpr(argumentCountIs(1), hasType(BindOptionalType),
+                                hasArgument(0, OptionalDerefMatcher)),
+               // known template methods in std
+               callExpr(
+                   argumentCountIs(1),
+                   callee(functionDecl(
+                       matchers::matchesAnyListedName(MakeSmartPtrList),
+                       hasTemplateArgument(0, refersToType(BindOptionalType)))),
+                   hasArgument(0, OptionalDerefMatcher))),
+           unless(anyOf(hasAncestor(typeLoc()),
+                        hasAncestor(expr(matchers::hasUnevaluatedContext())))))
           .bind("expr"),
       this);
 }