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 = ¶m; + 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 = ¶m; + 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