[clang-tools-extra] [clang-tidy]detecting conversion directly by `make_unique` and `make_shared` in bugprone-optional-value-conversion (PR #119371)

[Congcong Cai via cfe-commits]

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

Inspired by #110964


>From 526e79fc6e15c7a72ed3867afbee3cbc747c7945 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 10 Dec 2024 20:44:12 +0800
Subject: [PATCH] [clang-tidy]detecting conversion directly by `make_unique`
 and `make_shared` in bugprone-optional-value-conversion

Inspired by #110964
---
 .../bugprone/OptionalValueConversionCheck.cpp | 37 ++++++++++---
 clang-tools-extra/docs/ReleaseNotes.rst       |  4 ++
 ...al-value-conversion-construct-from-std.cpp | 53 +++++++++++++++++++
 3 files changed, 86 insertions(+), 8 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
index f2ff27d85fb004..0ed8c8d00f4222 100644
--- a/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -12,20 +12,44 @@
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <array>
 
 using namespace clang::ast_matchers;
+using clang::ast_matchers::internal::Matcher;
 
 namespace clang::tidy::bugprone {
 
 namespace {
 
-AST_MATCHER_P(QualType, hasCleanType, ast_matchers::internal::Matcher<QualType>,
-              InnerMatcher) {
+AST_MATCHER_P(QualType, hasCleanType, Matcher<QualType>, InnerMatcher) {
   return InnerMatcher.matches(
       Node.getNonReferenceType().getUnqualifiedType().getCanonicalType(),
       Finder, Builder);
 }
 
+AST_MATCHER_P2(Expr, constructFrom, Matcher<QualType>, TypeMatcher,
+               Matcher<Expr>, ArgumentMatcher) {
+  std::array<StringRef, 2> NameList{
+      "::std::make_unique",
+      "::std::make_shared",
+  };
+  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())))))
+      .matches(Node, Finder, Builder);
+}
+
 } // namespace
 
 OptionalValueConversionCheck::OptionalValueConversionCheck(
@@ -67,12 +91,9 @@ void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
       callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
                hasArgument(0, ignoringImpCasts(OptionalDereferenceMatcher)));
   Finder->addMatcher(
-      cxxConstructExpr(
-          argumentCountIs(1U), hasType(BindOptionalType),
-          hasArgument(0U, ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
-                                                 StdMoveCallMatcher))),
-          unless(anyOf(hasAncestor(typeLoc()),
-                       hasAncestor(expr(matchers::hasUnevaluatedContext())))))
+      expr(constructFrom(BindOptionalType,
+                         ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
+                                                StdMoveCallMatcher))))
           .bind("expr"),
       this);
 }
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b2b66dca6ccf85..9c499159b73c7b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/forwarding-reference-overload>` check by fixing
   a crash when determining if an ``enable_if[_t]`` was found.
 
+- Improved :doc:`bugprone-optional-value-conversion
+  <clang-tidy/checks/bugprone/optional-value-conversion>` to support detecting
+  conversion directly by ``std::make_unique`` and ``std::make_shared``.
+
 - Improved :doc:`bugprone-posix-return
   <clang-tidy/checks/bugprone/posix-return>` check to support integer literals
   as LHS and posix call as RHS of comparison.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp
new file mode 100644
index 00000000000000..768ab1ce014cec
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion-construct-from-std.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t
+
+namespace std {
+template <typename T> struct optional {
+  constexpr optional() noexcept;
+  constexpr optional(T &&) noexcept;
+  constexpr optional(const T &) noexcept;
+  template <typename U> constexpr optional(U &&) noexcept;
+  const T &operator*() const;
+  T *operator->();
+  const T *operator->() const;
+  T &operator*();
+  const T &value() const;
+  T &value();
+  const T &get() const;
+  T &get();
+  T value_or(T) const;
+};
+
+template <class T> T &&move(T &x) { return static_cast<T &&>(x); }
+
+template <typename T> class default_delete {};
+
+template <typename type, typename Deleter = std::default_delete<type>>
+class unique_ptr {};
+
+template <typename type>
+class shared_ptr {};
+
+template <class T, class... Args> unique_ptr<T> make_unique(Args &&...args);
+template <class T, class... Args> shared_ptr<T> make_shared(Args &&...args);
+
+} // namespace std
+
+struct A {
+    explicit A (int);
+};
+std::optional<int> opt;
+
+void invalid() {
+  std::make_unique<std::optional<int>>(opt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  using A = std::optional<int>;
+  std::make_unique<A>(opt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  std::make_shared<std::optional<int>>(opt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+}
+
+void valid() {
+  std::make_unique<A>(opt.value());
+  std::make_shared<A>(opt.value());
+}