PiotrZSL updated this revision to Diff 515475.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.

Added support for ->, added more tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147357/new/

https://reviews.llvm.org/D147357

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/optional-value-conversion.cpp
@@ -0,0 +1,213 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-optional-value-conversion %t -- --fix-notes
+// RUN: %check_clang_tidy -check-suffix=CUSTOM -std=c++17-or-later %s bugprone-optional-value-conversion %t -- \
+// RUN: -config="{CheckOptions: [{key: 'bugprone-optional-value-conversion.OptionalTypes', value: 'CustomOptional'}, \
+// RUN:                          {key: 'bugprone-optional-value-conversion.ValueMethods', value: '::Read$;::Ooo$'}]}" --fix-notes
+
+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);
+  }
+}
+
+namespace boost {
+  template<typename T>
+  struct optional {
+    constexpr optional() noexcept;
+    constexpr optional(const T&) noexcept;
+    const T& operator*() const;
+    const T& get() const;
+  };
+}
+
+namespace absl {
+  template<typename T>
+  struct optional {
+    constexpr optional() noexcept;
+    constexpr optional(const T&) noexcept;
+    const T& operator*() const;
+    const T& value() const;
+  };
+}
+
+template<typename T>
+struct CustomOptional {
+  CustomOptional();
+  CustomOptional(const T&);
+  const T& Read() const;
+  T& operator*();
+  T& Ooo();
+};
+
+void takeOptionalValue(std::optional<int>);
+void takeOptionalRef(const std::optional<int>&);
+void takeOptionalRRef(std::optional<int>&&);
+void takeOtherOptional(std::optional<long>);
+void takeBOptionalValue(boost::optional<int>);
+void takeAOptionalValue(absl::optional<int>);
+
+void incorrect(std::optional<int> param)
+{
+  std::optional<int>* ptr = &param;
+  takeOptionalValue(**ptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalValue(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(param);
+  takeOptionalValue(ptr->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*ptr);
+  takeOptionalRef(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(param.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(ptr->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(*ptr);
+  takeOptionalRef(param.operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(param);
+  takeOptionalRef(ptr->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(*ptr);
+  std::optional<int> p = *param;
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: std::optional<int> p = param;
+
+  takeOptionalValue(std::move(**ptr));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(*ptr));
+  takeOptionalValue(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalValue(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalValue(std::move(ptr->value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(*ptr));
+  takeOptionalValue(std::move(param.operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(std::move(param));
+  takeOptionalRef(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(param));
+  takeOptionalRef(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(param));
+  takeOptionalRef(std::move(ptr->value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(*ptr));
+  takeOptionalRef(std::move(param.operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRef(std::move(param));
+  takeOptionalRRef(std::move(*param));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(param));
+  takeOptionalRRef(std::move(param.value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(param));
+  takeOptionalRRef(std::move(ptr->value()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(*ptr));
+  takeOptionalRRef(std::move(param.operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(param));
+  takeOptionalRRef(std::move(ptr->operator*()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalRRef(std::move(*ptr));
+  std::optional<int> p2 = std::move(*param);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: std::optional<int> p2 = std::move(param);
+
+  std::optional<std::optional<int>> opt;
+  takeOptionalValue(opt->value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*opt);
+  takeOptionalValue(opt->operator*());
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: conversion from 'std::optional<int>' into 'int' and back into 'std::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeOptionalValue(*opt);
+
+  boost::optional<int> bopt;
+  takeBOptionalValue(*bopt);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'boost::optional<int>' into 'int' and back into 'boost::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeBOptionalValue(bopt);
+  takeBOptionalValue(bopt.get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'boost::optional<int>' into 'int' and back into 'boost::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeBOptionalValue(bopt);
+
+  absl::optional<int> aopt;
+  takeAOptionalValue(*aopt);
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'absl::optional<int>' into 'int' and back into 'absl::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeAOptionalValue(aopt);
+  takeAOptionalValue(aopt.value());
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: conversion from 'absl::optional<int>' into 'int' and back into 'absl::optional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES: takeAOptionalValue(aopt);
+}
+
+void takeCustom(const CustomOptional<int>&);
+
+void testCustom(CustomOptional<int> param) {
+  takeCustom(*param);
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE-1]]:14: warning: conversion from 'CustomOptional<int>' into 'int' and back into 'CustomOptional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES-CUSTOM: takeCustom(param);
+  takeCustom(param.Read());
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE-1]]:14: warning: conversion from 'CustomOptional<int>' into 'int' and back into 'CustomOptional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES-CUSTOM: takeCustom(param);
+  takeCustom(param.Ooo());
+  // CHECK-MESSAGES-CUSTOM: :[[@LINE-1]]:14: warning: conversion from 'CustomOptional<int>' into 'int' and back into 'CustomOptional<int>', remove potentially error-prone optional dereference [bugprone-optional-value-conversion]
+  // CHECK-FIXES-CUSTOM: takeCustom(param);
+}
+
+void correct(std::optional<int> param)
+{
+  takeOtherOptional(*param);
+  takeOtherOptional(param.value());
+  takeOtherOptional(param.value_or(5U));
+  takeOtherOptional(param.operator*());
+
+  std::optional<long> p = *param;
+  takeOptionalValue(param.value_or(5U));
+  takeOptionalRef(param.value_or(5U));
+
+  std::optional<int>* ptr = &param;
+  takeOtherOptional(**ptr);
+  takeOtherOptional(ptr->value());
+  takeOtherOptional(ptr->value_or(5U));
+  takeOtherOptional(ptr->operator*());
+
+  std::optional<long>* p2 = &p;
+  takeOptionalValue(p2->value_or(5U));
+  takeOptionalRef(p2->value_or(5U));
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -106,6 +106,7 @@
    `bugprone-no-escape <bugprone/no-escape.html>`_,
    `bugprone-non-zero-enum-to-bool-conversion <bugprone/non-zero-enum-to-bool-conversion.html>`_,
    `bugprone-not-null-terminated-result <bugprone/not-null-terminated-result.html>`_, "Yes"
+   `bugprone-optional-value-conversion <bugprone/optional-value-conversion.html>`_,
    `bugprone-parent-virtual-call <bugprone/parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone/posix-return.html>`_, "Yes"
    `bugprone-redundant-branch-condition <bugprone/redundant-branch-condition.html>`_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/optional-value-conversion.rst
@@ -0,0 +1,50 @@
+.. title:: clang-tidy - bugprone-optional-value-conversion
+
+bugprone-optional-value-conversion
+==================================
+
+Detects potentially unintentional and redundant conversions where a value is
+extracted from an optional-like type and then used to create a new instance of
+the same optional-like type.
+
+These conversions might be the result of developer oversight, leftovers from
+code refactoring, or other situations that could lead to unintended exceptions
+or cases where the resulting optional is always initialized, which might be
+unexpected behavior.
+
+.. code-block:: c++
+
+    #include <optional>
+
+    void print(std::optional<int>);
+
+    int main()
+    {
+      std::optional<int> opt;
+      // ...
+
+      // Unintentional conversion from std::optional<int> to int and back to
+      // std::optional<int>:
+      print(opt.value());
+
+      // ...
+    }
+
+Value extraction using ``operator *`` is matched by default.
+The support for non-standard optional types such as ``boost::optional`` or
+``absl::optional`` may be limited.
+
+Options:
+--------
+
+.. option:: OptionalTypes
+
+    Semicolon-separated list of (fully qualified) optional type names or regular
+    expressions that match the optional types.
+    Default value is `::std::optional;::absl::optional;::boost::optional`.
+
+.. option:: ValueMethods
+
+    Semicolon-separated list of (fully qualified) method names or regular
+    expressions that match the methods.
+    Default value is `::value$;::get$`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -112,6 +112,13 @@
   Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum`` type
   doesn't have a zero-value enumerator.
 
+- New :doc:`bugprone-optional-value-conversion
+  <clang-tidy/checks/bugprone/optional-value-conversion>` check.
+
+  Detects potentially unintentional and redundant conversions where a value is
+  extracted from an optional-like type and then used to create a new instance
+  of the same optional-like type.
+
 - New :doc:`bugprone-unsafe-functions
   <clang-tidy/checks/bugprone/unsafe-functions>` check.
 
Index: clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.h
@@ -0,0 +1,38 @@
+//===--- OptionalValueConversionCheck.h - clang-tidy ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include <vector>
+
+namespace clang::tidy::bugprone {
+
+/// Detects potentially unintentional and redundant conversions where a value is
+/// extracted from an optional-like type and then used to create a new instance
+/// of the same optional-like type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/optional-value-conversion.html
+class OptionalValueConversionCheck : public ClangTidyCheck {
+public:
+  OptionalValueConversionCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override;
+
+private:
+  std::vector<StringRef> OptionalTypes;
+  std::vector<StringRef> ValueMethods;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_OPTIONALVALUECONVERSIONCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/OptionalValueConversionCheck.cpp
@@ -0,0 +1,125 @@
+//===--- OptionalValueConversionCheck.cpp - clang-tidy --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "OptionalValueConversionCheck.h"
+#include "../utils/LexerUtils.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(QualType, hasCleanType, ast_matchers::internal::Matcher<QualType>,
+              InnerMatcher) {
+  return InnerMatcher.matches(
+      Node.getNonReferenceType().getUnqualifiedType().getCanonicalType(),
+      Finder, Builder);
+}
+
+} // namespace
+
+OptionalValueConversionCheck::OptionalValueConversionCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      OptionalTypes(utils::options::parseStringList(
+          Options.get("OptionalTypes",
+                      "::std::optional;::absl::optional;::boost::optional"))),
+      ValueMethods(utils::options::parseStringList(
+          Options.get("ValueMethods", "::value$;::get$"))) {}
+
+std::optional<TraversalKind>
+OptionalValueConversionCheck::getCheckTraversalKind() const {
+  return TK_AsIs;
+}
+
+void OptionalValueConversionCheck::registerMatchers(MatchFinder *Finder) {
+  auto ConstructTypeMatcher =
+      qualType(hasCleanType(qualType().bind("optional-type")));
+
+  auto CallTypeMatcher =
+      qualType(hasCleanType(equalsBoundNode("optional-type")));
+
+  auto OptionalDereferenceMatcher = callExpr(
+      anyOf(
+          cxxOperatorCallExpr(hasOverloadedOperatorName("*"),
+                              hasUnaryOperand(hasType(CallTypeMatcher)))
+              .bind("op"),
+          cxxMemberCallExpr(thisPointerType(CallTypeMatcher),
+                            callee(cxxMethodDecl(anyOf(
+                                hasOverloadedOperatorName("*"),
+                                matchers::matchesAnyListedName(ValueMethods)))))
+              .bind("fun")),
+      hasType(qualType().bind("value-type")));
+
+  auto StdMoveCallMatcher =
+      callExpr(argumentCountIs(1), callee(functionDecl(hasName("::std::move"))),
+               hasArgument(0, ignoringImpCasts(OptionalDereferenceMatcher)));
+  Finder->addMatcher(
+      cxxConstructExpr(
+          argumentCountIs(1U),
+          hasDeclaration(cxxConstructorDecl(
+              ofClass(matchers::matchesAnyListedName(OptionalTypes)))),
+          hasType(ConstructTypeMatcher),
+          hasArgument(0U, ignoringImpCasts(anyOf(OptionalDereferenceMatcher,
+                                                 StdMoveCallMatcher))))
+          .bind("expr"),
+      this);
+}
+
+void OptionalValueConversionCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "OptionalTypes",
+                utils::options::serializeStringList(OptionalTypes));
+  Options.store(Opts, "ValueMethods",
+                utils::options::serializeStringList(ValueMethods));
+}
+
+void OptionalValueConversionCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedExpr = Result.Nodes.getNodeAs<Expr>("expr");
+  const auto *OptionalType = Result.Nodes.getNodeAs<QualType>("optional-type");
+  const auto *ValueType = Result.Nodes.getNodeAs<QualType>("value-type");
+
+  diag(MatchedExpr->getExprLoc(),
+       "conversion from %0 into %1 and back into %0, remove potentially "
+       "error-prone optional dereference")
+      << *OptionalType << ValueType->getUnqualifiedType();
+
+  if (const auto *OperatorExpr =
+          Result.Nodes.getNodeAs<CXXOperatorCallExpr>("op")) {
+    diag(OperatorExpr->getExprLoc(), "remove '*' to silence this warning",
+         DiagnosticIDs::Note)
+        << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+               OperatorExpr->getBeginLoc(), OperatorExpr->getExprLoc()));
+    return;
+  }
+  if (const auto *CallExpr = Result.Nodes.getNodeAs<CXXMemberCallExpr>("fun")) {
+    const SourceLocation Begin =
+        utils::lexer::getPreviousToken(CallExpr->getExprLoc(),
+                                       *Result.SourceManager, getLangOpts())
+            .getLocation();
+    auto Diag =
+        diag(CallExpr->getExprLoc(),
+             "remove call to %0 to silence this warning", DiagnosticIDs::Note);
+    Diag << CallExpr->getMethodDecl()
+         << FixItHint::CreateRemoval(
+                CharSourceRange::getTokenRange(Begin, CallExpr->getEndLoc()));
+    if (const auto *Member =
+            llvm::dyn_cast<MemberExpr>(CallExpr->getCallee()->IgnoreImplicit());
+        Member && Member->isArrow())
+      Diag << FixItHint::CreateInsertion(CallExpr->getBeginLoc(), "*");
+    return;
+  }
+}
+
+} // namespace clang::tidy::bugprone
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -35,6 +35,7 @@
   NoEscapeCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NotNullTerminatedResultCheck.cpp
+  OptionalValueConversionCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -40,6 +40,7 @@
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
 #include "NotNullTerminatedResultCheck.h"
+#include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
@@ -135,6 +136,8 @@
         "bugprone-move-forwarding-reference");
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(
         "bugprone-multiple-statement-macro");
+    CheckFactories.registerCheck<OptionalValueConversionCheck>(
+        "bugprone-optional-value-conversion");
     CheckFactories.registerCheck<RedundantBranchConditionCheck>(
         "bugprone-redundant-branch-condition");
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to