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

Inspired by #110964

Author: HerrCai0907

clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp

@@ -12,20 +12,43 @@
 #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);
 }
 
+constexpr std::array<StringRef, 2> NameList{
+    "::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(
@@ -67,12 +90,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);
 }

clang-tools-extra/docs/ReleaseNotes.rst

@@ -182,6 +182,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.

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());
+}