From a89141f733cef817c586bb6da0ea69a5a323874e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Sat, 6 Jan 2024 00:14:08 +0100 Subject: [PATCH] [clang-tidy] Add check `readability-avoid-return-with-void-value` (#76249) --- .../AvoidReturnWithVoidValueCheck.cpp | 57 +++++++++++++++ .../AvoidReturnWithVoidValueCheck.h | 44 ++++++++++++ .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../avoid-return-with-void-value.rst | 51 ++++++++++++++ .../avoid-return-with-void-value.cpp | 69 +++++++++++++++++++ 8 files changed, 232 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp new file mode 100644 index 0000000000000..e3400f614fa56 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.cpp @@ -0,0 +1,57 @@ +//===--- AvoidReturnWithVoidValueCheck.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 "AvoidReturnWithVoidValueCheck.h" +#include "clang/AST/Stmt.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +static constexpr auto IgnoreMacrosName = "IgnoreMacros"; +static constexpr auto IgnoreMacrosDefault = true; + +static constexpr auto StrictModeName = "StrictMode"; +static constexpr auto StrictModeDefault = true; + +AvoidReturnWithVoidValueCheck::AvoidReturnWithVoidValueCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreMacros( + Options.getLocalOrGlobal(IgnoreMacrosName, IgnoreMacrosDefault)), + StrictMode(Options.getLocalOrGlobal(StrictModeName, StrictModeDefault)) {} + +void AvoidReturnWithVoidValueCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + returnStmt( + hasReturnValue(allOf(hasType(voidType()), unless(initListExpr()))), + optionally(hasParent(compoundStmt().bind("compound_parent")))) + .bind("void_return"), + this); +} + +void AvoidReturnWithVoidValueCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *VoidReturn = Result.Nodes.getNodeAs("void_return"); + if (IgnoreMacros && VoidReturn->getBeginLoc().isMacroID()) + return; + if (!StrictMode && !Result.Nodes.getNodeAs("compound_parent")) + return; + diag(VoidReturn->getBeginLoc(), "return statement within a void function " + "should not have a specified return value"); +} + +void AvoidReturnWithVoidValueCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, IgnoreMacrosName, IgnoreMacros); + Options.store(Opts, StrictModeName, StrictMode); +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h new file mode 100644 index 0000000000000..f8148db43cd95 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/AvoidReturnWithVoidValueCheck.h @@ -0,0 +1,44 @@ +//===--- AvoidReturnWithVoidValueCheck.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_READABILITY_AVOIDRETURNWITHVOIDVALUECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDRETURNWITHVOIDVALUECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Finds return statements with `void` values used within functions with `void` +/// result types. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/avoid-return-with-void-value.html +class AvoidReturnWithVoidValueCheck : public ClangTidyCheck { +public: + AvoidReturnWithVoidValueCheck(StringRef Name, ClangTidyContext *Context); + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + bool IgnoreMacros; + bool StrictMode; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_AVOIDRETURNWITHVOIDVALUECHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 5452c2d48a461..408c822b861c5 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyReadabilityModule AvoidConstParamsInDecls.cpp + AvoidReturnWithVoidValueCheck.cpp AvoidUnconditionalPreprocessorIfCheck.cpp BracesAroundStatementsCheck.cpp ConstReturnTypeCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index b8e6e64143206..0b0aad7c0dcb3 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "AvoidConstParamsInDecls.h" +#include "AvoidReturnWithVoidValueCheck.h" #include "AvoidUnconditionalPreprocessorIfCheck.h" #include "BracesAroundStatementsCheck.h" #include "ConstReturnTypeCheck.h" @@ -63,6 +64,8 @@ class ReadabilityModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "readability-avoid-const-params-in-decls"); + CheckFactories.registerCheck( + "readability-avoid-return-with-void-value"); CheckFactories.registerCheck( "readability-avoid-unconditional-preprocessor-if"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4d25e2ebe85f5..08ade306b5a07 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -230,6 +230,12 @@ New checks Detects C++ code where a reference variable is used to extend the lifetime of a temporary object that has just been constructed. +- New :doc:`readability-avoid-return-with-void-value + ` check. + + Finds return statements with ``void`` values used within functions with + ``void`` result types. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index b36bf7d497b9d..2f86121ad8729 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -337,6 +337,7 @@ Clang-Tidy Checks :doc:`portability-simd-intrinsics `, :doc:`portability-std-allocator-const `, :doc:`readability-avoid-const-params-in-decls `, "Yes" + :doc:`readability-avoid-return-with-void-value `, :doc:`readability-avoid-unconditional-preprocessor-if `, :doc:`readability-braces-around-statements `, "Yes" :doc:`readability-const-return-type `, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst new file mode 100644 index 0000000000000..d802f9be829c4 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-return-with-void-value.rst @@ -0,0 +1,51 @@ +.. title:: clang-tidy - readability-avoid-return-with-void-value + +readability-avoid-return-with-void-value +======================================== + +Finds return statements with ``void`` values used within functions with +``void`` result types. + +A function with a ``void`` return type is intended to perform a task without +producing a return value. Return statements with expressions could lead +to confusion and may miscommunicate the function's intended behavior. + +Example: + +.. code-block:: + + void g(); + void f() { + // ... + return g(); + } + +In a long function body, the ``return`` statement suggests that the function +returns a value. However, ``return g();`` is a combination of two statements +that should be written as + +.. code-block:: + + g(); + return; + +to make clear that ``g()`` is called and immediately afterwards the function +returns (nothing). + +In C, the same issue is detected by the compiler if the ``-Wpedantic`` mode +is enabled. + +Options +------- + +.. option:: IgnoreMacros + + The value `false` specifies that return statements expanded + from macros are not checked. The default value is `true`. + +.. option:: StrictMode + + The value `false` specifies that a direct return statement shall + be excluded from the analysis if it is the only statement not + contained in a block like ``if (cond) return g();``. The default + value is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp new file mode 100644 index 0000000000000..f00407c99ce57 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-return-with-void-value.cpp @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s readability-avoid-return-with-void-value %t +// RUN: %check_clang_tidy -check-suffixes=,INCLUDE-MACROS %s readability-avoid-return-with-void-value %t \ +// RUN: -- -config="{CheckOptions: [{key: readability-avoid-return-with-void-value.IgnoreMacros, value: false}]}" \ +// RUN: -- +// RUN: %check_clang_tidy -check-suffixes=LENIENT %s readability-avoid-return-with-void-value %t \ +// RUN: -- -config="{CheckOptions: [{key: readability-avoid-return-with-void-value.StrictMode, value: false}]}" \ +// RUN: -- + +void f1(); + +void f2() { + return f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} + +void f3(bool b) { + if (b) return f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + return f2(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} + +template +T f4() {} + +void f5() { + return f4(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} + +void f6() { return; } + +int f7() { return 1; } + +int f8() { return f7(); } + +void f9() { + return (void)f7(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} + +#define RETURN_VOID return (void)1 + +void f10() { + RETURN_VOID; + // CHECK-MESSAGES-INCLUDE-MACROS: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +} + +template +struct C { + C(A) {} +}; + +template +C f11() { return {}; } + +using VOID = void; + +VOID f12(); + +VOID f13() { + return f12(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] + // CHECK-MESSAGES-LENIENT: :[[@LINE-2]]:5: warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value] +}